Re: [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment

[Alexander Graf <agraf@suse.de>]

On 13.05.14 15:16, Matthew Rosato wrote:
> On 05/12/2014 03:43 AM, Christian Borntraeger wrote:
>> On 07/05/14 20:05, Matthew Rosato wrote:
>>> When determining the memory increment size, use the maxmem size if
>>> it was specified.
>>>
>>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>>> ---
>>>   hw/s390x/s390-virtio-ccw.c |   44 ++++++++++++++++++++++++++++++++++++--------
>>>   target-s390x/cpu.h         |    3 +++
>>>   2 files changed, 39 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 0d4f6ae..a8be0f7 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -17,6 +17,7 @@
>>>   #include "ioinst.h"
>>>   #include "css.h"
>>>   #include "virtio-ccw.h"
>>> +#include "qemu/config-file.h"
>>>
>>>   void io_subsystem_reset(void)
>>>   {
>>> @@ -84,17 +85,33 @@ static void ccw_init(QEMUMachineInitArgs *args)
>>>       ram_addr_t my_ram_size = args->ram_size;
>>>       MemoryRegion *sysmem = get_system_memory();
>>>       MemoryRegion *ram = g_new(MemoryRegion, 1);
>>> -    int shift = 0;
>>> +    sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>>       uint8_t *storage_keys;
>>>       int ret;
>>>       VirtualCssBus *css_bus;
>>> -
>>> -    /* s390x ram size detection needs a 16bit multiplier + an increment. So
>>> -       guests > 64GB can be specified in 2MB steps etc. */
>>> -    while ((my_ram_size >> (20 + shift)) > 65535) {
>>> -        shift++;
>>> +    QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
>>> +    ram_addr_t pad_size = 0;
>>> +    ram_addr_t maxmem = qemu_opt_get_size(opts, "maxmem", 0);
>>> +    ram_addr_t standby_mem_size = maxmem - my_ram_size;
>>> +
>>> +    /* The storage increment size is a multiple of 1M and is a power of 2.
>>> +     * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
>>> +     * The variable 'mhd->increment_size' is an exponent of 2 that can be
>>> +     * used to calculate the size (in bytes) of an increment. */
>>> +    mhd->increment_size = 20;
>>> +    while ((my_ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>>> +        mhd->increment_size++;
>>> +    }
>>> +    while ((standby_mem_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>>> +        mhd->increment_size++;
>>>       }
>> Looking back into the mail thread, Alex requested to make the code for standy/non-standby identical.
>> Now: The limit of 1020 (MAX_STORAGE_INCREMENTS) is only given if standby memory exists. Without standby memory, we could still used 64k as a divider.(zVM also does only impose this limit with standby memory).
>> What does that mean: With this patch the memory size granularity gets bigger. e.g. a guest can have
>> 1019MB, 1020MB, 1022MB, 1024MB and so on (1021MB is no longer possible, but it was before).
> Hmm, this is a good point.  I didn't think about it when I made the
> change per Alex.  If nobody has a strong opinion here, I think I'd
> rather go back to special casing this in the next version, to keep the
> 'normal case' (without standby memory) more robust.

Wouldn't it be more confusing if the guest configuration suddenly 
changes when you add standby memory?


Alex