From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36478) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkD04-00043M-La for qemu-devel@nongnu.org; Tue, 13 May 2014 09:44:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkCzx-0000Ku-6t for qemu-devel@nongnu.org; Tue, 13 May 2014 09:44:08 -0400 Received: from cantor2.suse.de ([195.135.220.15]:52567 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkCzw-0000Ke-Ti for qemu-devel@nongnu.org; Tue, 13 May 2014 09:44:01 -0400 Message-ID: <5372219E.6070503@suse.de> Date: Tue, 13 May 2014 15:43:58 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1399485959-15579-1-git-send-email-mjrosato@linux.vnet.ibm.com> <1399485959-15579-4-git-send-email-mjrosato@linux.vnet.ibm.com> <53707BB3.1060904@de.ibm.com> <53721B27.1090507@linux.vnet.ibm.com> In-Reply-To: <53721B27.1090507@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Matthew Rosato , Christian Borntraeger , qemu-devel@nongnu.org Cc: Cornelia Huck , imammedo@redhat.com, rth@twiddle.net, aliguori@amazon.com, pbonzini@redhat.com 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 >>> --- >>> 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