Re: [PATCH RFC 3/5] s390x: prepare device memory address space

[David Hildenbrand]

On 09.07.20 12:59, Cornelia Huck wrote:
> On Wed,  8 Jul 2020 20:51:33 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> Let's allocate the device memory information and setup the device
>> memory address space. Expose the maximum ramsize via SCLP and the actual
>> initial ramsize via diag260.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c         | 43 ++++++++++++++++++++++++++++++
>>  hw/s390x/sclp.c                    | 12 +++++++--
>>  include/hw/s390x/s390-virtio-ccw.h |  3 +++
>>  target/s390x/diag.c                |  4 +--
>>  4 files changed, 58 insertions(+), 4 deletions(-)
> 
> (...)
> 
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index c3b1e24b2c..6b33eb0efc 100644
>> --- a/target/s390x/diag.c
>> +++ b/target/s390x/diag.c
>> @@ -32,8 +32,8 @@ void handle_diag_260(CPUS390XState *env, uint64_t r1, 
>> uint64_t r3, uintptr_t ra)
>>      ram_addr_t addr, length;
>>      uint64_t tmp;
>>  
>> -    /* TODO: Unlock with new QEMU machine. */
>> -    if (false) {
>> +    /* Support for diag260 is glued to support for memory devices. */
> 
> I'm wondering why you need to do this... sure, the availability of a
> new diagnose could be perceived as a guest-visible change, but does the
> information presented change anything? Without memory devices, it will
> just duplicate the information already reported via SCLP, IIUC?

Yes, it's essentially providing redundant information without memory
devices.

One could sense diag260 in the guest and assume it will work on
successive invocations. E.g., issue subcode 0xc while checking for
exceptions, then issue subcode 0x10 without checking for exceptions. If
we migrate in between, we could be in trouble.

Yes, it's somewhat unlikely, I don't have a strong opinion here. Gluing
it to some migration-safe mechanism (here, the machine) felt like the
right thing to do.

-- 
Thanks,

David / dhildenb