Re: [qemu-s390x] [PATCH RFCv2] s390x/sclp: remove memory hotplug support

[Christian Borntraeger]

On 02/20/2018 12:18 PM, Christian Borntraeger wrote:
> Given that we have trouble modeling this properly I somewhat like the idea.
> We should have never folded this into the common code memory hotplug 
> infrastructure as it is different. Instead we really should have provided
> a different interface and we should have build it with migration support. 
> 
> If we need it again in the future, we can certainly add it back via an 
> s390 specific interface.
> 
> 
> On 02/19/2018 06:42 PM, David Hildenbrand wrote:
>> From an architecture point of view, nothing can be mapped into the address
>> space on s390x. All there is is memory. Therefore there is also not really
>> an interface to communicate such information to the guest. All we can do is
>> specify the maximum ram address and guests can probe in that range if
>> memory is available and usable (TPROT).
>>
>> Also memory hotplug is strange. The guest can decide at some point in
>> time to add / remove memory in some range. And nobody can really hinder it
>> from doing so. So if we specify right now e.g.
>>     -m 2G,slots=2,maxmem=20G
>> An ordinary fedora guest will happily online (hotplug) all memory,
>> resulting in a guest consuming 20G. So it really behaves rather like
>>     -m 22G
>> There is no way to hotplug memory from the outside like on other
>> architectures. This is of course bad for upper management layers.
>>
>> As the guest can create/delete memory regions while it is running, of
>> course migration support is not available and tricky to implement.
>>
>> With virtualization, it is different. We might want to map something
>> into guest address space (e.g. fake DAX devices) and not detect it
>> automatically as memory. So we really want to use the maxmem and slots
>> parameter just like on all other architectures. Such devices will have
>> to expose the applicable memory range themselves. To finally be able to
>> provide memory hotplug to guests, we will need a new paravirtualized
>> interface to do that (e.g. something into the direction of virtio-mem).
>>
>> This implies, that maxmem cannot be used for s390x memory hotplug
>> anymore and has to go. This simplifies the code quite a bit.
>>
>> As migration support is not working, this change cannot really break
>> migration as guests without slots and maxmem don't see the SCLP
>> features. Also, the ram size calcualtion does not change.
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
> [...]
> 
> looks mostly sane. Give me some more time to review. 
> 
>> @@ -540,9 +302,6 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>  {
>>      MachineState *machine = MACHINE(qdev_get_machine());
>>      ram_addr_t initial_mem = machine->ram_size;
>> -    ram_addr_t max_mem = machine->maxram_size;
>> -    ram_addr_t standby_mem = max_mem - initial_mem;
>> -    ram_addr_t pad_mem = 0;
>>      int increment_size = 20;
>>
>>      /* The storage increment size is a multiple of 1M and is a power of 2.
>> @@ -552,34 +311,14 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>      while ((initial_mem >> increment_size) > MAX_STORAGE_INCREMENTS) {
>>          increment_size++;
>>      }
> 
> 
> In theory we could now be more fine grained (as we no longer expose the 
> increments)
> but this would break migration so better try now to be clever. (e.g. with
> compat machines)
>                                       try NOT of course.