Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] s390x/sclp: fix maxram calculation

[David Hildenbrand <david@redhat.com>]

On 30.07.2018 17:47, Cornelia Huck wrote:
> On Mon, 30 Jul 2018 17:43:42 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 30.07.2018 16:09, Christian Borntraeger wrote:
>>> We clamp down ram_size to match the sclp increment size. We do
>>> not do the same for maxram_size, which means for large guests
>>> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
>>> This can break other code (e.g. CMMA migration) which uses maxram_size
>>> to calculate the number of pages and then throws some errors.
>>>
>>> Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> CC: qemu-stable@nongnu.org
>>> CC: David Hildenbrand <david@redhat.com>
>>> ---
>>>  hw/s390x/sclp.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>>> index bd2a024..4510a80 100644
>>> --- a/hw/s390x/sclp.c
>>> +++ b/hw/s390x/sclp.c
>>> @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>>      initial_mem = initial_mem >> increment_size << increment_size;
>>>  
>>>      machine->ram_size = initial_mem;
>>> +    machine->maxram_size = initial_mem;
>>>      /* let's propagate the changed ram size into the global variable. */
>>>      ram_size = initial_mem;
>>>  }
>>>   
>>
>> BTW, I handle it in may private patch like this
>>
>>  static inline SCLPDevice *get_sclp_device(void)
>>  {
>> @@ -319,9 +321,12 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>       * down to align with the nearest increment boundary. */
>>      initial_mem = initial_mem >> increment_size << increment_size;
>>
>> -    machine->ram_size = initial_mem;
>> -    /* let's propagate the changed ram size into the global variable. */
>> -    ram_size = initial_mem;
>> +    /* propagate the changed ram size into the different places */
>> +    if (initial_mem != machine->ram_size) {
>> +        machine->maxram_size -= machine->ram_size - initial_mem;
>> +        machine->ram_size = initial_mem;
>> +        ram_size = initial_mem;
>> +    }
>>  }
>>
>> You would right now overwrite any maxmem setting (which might be ok as
>> we don't support it yet).
>>
> 
> So, will you (for whatever value of 'you') submit more patches for 3.1?
> 

Once we have memory device support for s390x that will be needed. But
the person that implemented cmma migration should fix the handling and
only try to migrate memory that is actually there. This was broken
before my patch, just never happened as Linux always onlines all memory
it sees via SCLP (thereby creating the memory regions).

-- 

Thanks,

David / dhildenb