From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44500) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wu0S8-0004EQ-Cz for qemu-devel@nongnu.org; Mon, 09 Jun 2014 10:21:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wu0S2-0000ZM-V1 for qemu-devel@nongnu.org; Mon, 09 Jun 2014 10:21:36 -0400 Received: from omzsmtpe04.verizonbusiness.com ([199.249.25.207]:14638) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wu0S2-0000Yv-JM for qemu-devel@nongnu.org; Mon, 09 Jun 2014 10:21:30 -0400 From: Don Slutz Message-ID: <5395C2C9.7070200@terremark.com> Date: Mon, 09 Jun 2014 10:20:57 -0400 MIME-Version: 1.0 References: <1402077126-17799-1-git-send-email-dslutz@verizon.com> <1402077126-17799-3-git-send-email-dslutz@verizon.com> <20140608154014.GE8464@redhat.com> In-Reply-To: <20140608154014.GE8464@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 2/3] pc & q35: Add new machine opt max-ram-below-4g List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Don Slutz Cc: xen-devel@lists.xensource.com, Stefano Stabellini , qemu-devel@nongnu.org, Anthony Liguori , Igor Mammedov , =?ISO-8859-1?Q?Andreas_F=E4rber?= On 06/08/14 11:40, Michael S. Tsirkin wrote: > On Fri, Jun 06, 2014 at 01:52:05PM -0400, Don Slutz wrote: >> This is a pc & q35 only machine opt. One use is to allow for more >> ram in a 32bit guest for example: >> >> -machine pc,max-ram-below-4g=3.75G >> >> If you add enough PCI devices then all mmio for them will not fit >> below 4G which may not be the layout the user wanted. This allows >> you to increase the below 4G address space that PCI devices can use >> (aka decrease ram below 4G) and therefore in more cases not have any >> mmio that is above 4G. >> >> For example using "-machine pc,max-ram-below-4g=2G" on the command >> line will limit the amount of ram that is below 4G to 2G. >> >> Signed-off-by: Don Slutz >> --- >> v5: >> Re-work based on: >> >> https://github.com/imammedo/qemu/commits/memory-hotplug-v11 >> >> >> hw/i386/pc.c | 38 ++++++++++++++++++++++++++++++++++++++ >> hw/i386/pc_piix.c | 15 ++++++++++++--- >> hw/i386/pc_q35.c | 15 ++++++++++++--- >> include/hw/i386/pc.h | 3 +++ >> vl.c | 4 ++++ >> 5 files changed, 69 insertions(+), 6 deletions(-) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 7cdba10..bccb746 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -1644,11 +1644,49 @@ pc_machine_get_hotplug_memory_region_size(Object *obj, Visitor *v, void *opaque, >> visit_type_int(v, &value, name, errp); >> } >> >> +static void pc_machine_get_max_ram_below_4g(Object *obj, Visitor *v, >> + void *opaque, const char *name, >> + Error **errp) >> +{ >> + PCMachineState *pcms = PC_MACHINE(obj); >> + uint64_t value = pcms->max_ram_below_4g; >> + >> + visit_type_size(v, &value, name, errp); >> +} >> + >> +static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v, >> + void *opaque, const char *name, >> + Error **errp) >> +{ >> + PCMachineState *pcms = PC_MACHINE(obj); >> + Error *error = NULL; >> + uint64_t value; >> + >> + visit_type_size(v, &value, name, &error); >> + if (error) { >> + error_propagate(errp, error); >> + return; >> + } >> + if (value > (1ULL << 32)) { >> + error_set(&error, ERROR_CLASS_GENERIC_ERROR, >> + "Machine option 'max-ram-below-4g=%"PRIu64 >> + "' expects size less then or equal to 4G", value); > less than But the test is greater then. So "not greater then" is "less then or equal". Or did you want the test changed? >> + error_propagate(errp, error); >> + return; >> + } >> + >> + pcms->max_ram_below_4g = value; >> +} >> + >> static void pc_machine_initfn(Object *obj) >> { >> object_property_add(obj, PC_MACHINE_MEMHP_REGION_SIZE, "int", >> pc_machine_get_hotplug_memory_region_size, >> NULL, NULL, NULL, NULL); >> + object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size", >> + pc_machine_get_max_ram_below_4g, >> + pc_machine_set_max_ram_below_4g, >> + NULL, NULL, NULL); >> } >> >> static void pc_machine_class_init(ObjectClass *oc, void *data) >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 40f6eaf..25f4727 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -98,6 +98,13 @@ static void pc_init1(MachineState *machine, >> DeviceState *icc_bridge; >> FWCfgState *fw_cfg = NULL; >> PcGuestInfo *guest_info; >> + Object *mo = qdev_get_machine(); >> + PCMachineState *pcms = PC_MACHINE(mo); >> + ram_addr_t lowmem = 0xe0000000; >> + >> + if (pcms && pcms->max_ram_below_4g) { > Is pcms ever NULL? If yes why? Not that I know of. I would be happy to convert this to an assert(pcms). >> + lowmem = pcms->max_ram_below_4g; >> + } >> >> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory). >> * If it doesn't, we need to split it in chunks below and above 4G. >> @@ -106,8 +113,10 @@ static void pc_init1(MachineState *machine, >> * For old machine types, use whatever split we used historically to avoid >> * breaking migration. >> */ >> - if (machine->ram_size >= 0xe0000000) { >> - ram_addr_t lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000; >> + if (machine->ram_size >= lowmem) { >> + if (!(pcms && pcms->max_ram_below_4g) && gigabyte_align) { >> + lowmem = 0xc0000000; >> + } >> above_4g_mem_size = machine->ram_size - lowmem; >> below_4g_mem_size = lowmem; >> } else { > > So why do we need gigabyte_align anymore? Because of qemu 2.0 and the user is not required to specify this option. > Can't we set property to 0xc0000000 by default, and > override for old machine types? There is a strange compatibility part here. Since this code includes ram_size (see: http://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05146.html ) and xen has a different default. > Also, a value that isn't a multiple of 1G will lead to bad > performance for large machines which do have above_4g_mem_size. > Let's detect and print a warning. Will Do. -Don Slutz > > >> @@ -122,7 +131,7 @@ static void pc_init1(MachineState *machine, >> } >> >> icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> + object_property_add_child(mo, "icc-bridge", >> OBJECT(icc_bridge), NULL); >> >> pc_cpus_init(machine->cpu_model, icc_bridge); >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index e28ce40..155cdf1 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -85,6 +85,13 @@ static void pc_q35_init(MachineState *machine) >> PCIDevice *ahci; >> DeviceState *icc_bridge; >> PcGuestInfo *guest_info; >> + Object *mo = qdev_get_machine(); >> + PCMachineState *pcms = PC_MACHINE(mo); >> + ram_addr_t lowmem = 0xb0000000; >> + >> + if (pcms && pcms->max_ram_below_4g) { >> + lowmem = pcms->max_ram_below_4g; >> + } >> >> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory >> * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping >> @@ -95,8 +102,10 @@ static void pc_q35_init(MachineState *machine) >> * For old machine types, use whatever split we used historically to avoid >> * breaking migration. >> */ >> - if (machine->ram_size >= 0xb0000000) { >> - ram_addr_t lowmem = gigabyte_align ? 0x80000000 : 0xb0000000; >> + if (machine->ram_size >= lowmem) { >> + if (!(pcms && pcms->max_ram_below_4g) && gigabyte_align) { >> + lowmem = 0x800000000; >> + } >> above_4g_mem_size = machine->ram_size - lowmem; >> below_4g_mem_size = lowmem; >> } else { >> @@ -111,7 +120,7 @@ static void pc_q35_init(MachineState *machine) >> } >> >> icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> + object_property_add_child(mo, "icc-bridge", >> OBJECT(icc_bridge), NULL); >> >> pc_cpus_init(machine->cpu_model, icc_bridge); >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 19530bd..2d8b562 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -32,10 +32,13 @@ struct PCMachineState { >> MemoryRegion hotplug_memory; >> >> HotplugHandler *acpi_dev; >> + >> + uint64_t max_ram_below_4g; >> }; >> >> #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" >> #define PC_MACHINE_MEMHP_REGION_SIZE "hotplug-memory-region-size" >> +#define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g" >> >> /** >> * PCMachineClass: >> diff --git a/vl.c b/vl.c >> index 5e77a27..cffb9c5 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -382,6 +382,10 @@ static QemuOptsList qemu_machine_opts = { >> .name = "kvm-type", >> .type = QEMU_OPT_STRING, >> .help = "Specifies the KVM virtualization mode (HV, PR)", >> + },{ >> + .name = PC_MACHINE_MAX_RAM_BELOW_4G, >> + .type = QEMU_OPT_SIZE, >> + .help = "maximum ram below the 4G boundary (32bit boundary)", >> }, >> { /* End of list */ } >> }, >> -- >> 1.8.4 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v5 2/3] pc & q35: Add new machine opt max-ram-below-4g Date: Mon, 09 Jun 2014 10:20:57 -0400 Message-ID: <5395C2C9.7070200@terremark.com> References: <1402077126-17799-1-git-send-email-dslutz@verizon.com> <1402077126-17799-3-git-send-email-dslutz@verizon.com> <20140608154014.GE8464@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140608154014.GE8464@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org To: "Michael S. Tsirkin" , Don Slutz Cc: xen-devel@lists.xensource.com, Stefano Stabellini , qemu-devel@nongnu.org, Anthony Liguori , Igor Mammedov , =?ISO-8859-1?Q?Andreas_F=E4rber?= List-Id: xen-devel@lists.xenproject.org On 06/08/14 11:40, Michael S. Tsirkin wrote: > On Fri, Jun 06, 2014 at 01:52:05PM -0400, Don Slutz wrote: >> This is a pc & q35 only machine opt. One use is to allow for more >> ram in a 32bit guest for example: >> >> -machine pc,max-ram-below-4g=3.75G >> >> If you add enough PCI devices then all mmio for them will not fit >> below 4G which may not be the layout the user wanted. This allows >> you to increase the below 4G address space that PCI devices can use >> (aka decrease ram below 4G) and therefore in more cases not have any >> mmio that is above 4G. >> >> For example using "-machine pc,max-ram-below-4g=2G" on the command >> line will limit the amount of ram that is below 4G to 2G. >> >> Signed-off-by: Don Slutz >> --- >> v5: >> Re-work based on: >> >> https://github.com/imammedo/qemu/commits/memory-hotplug-v11 >> >> >> hw/i386/pc.c | 38 ++++++++++++++++++++++++++++++++++++++ >> hw/i386/pc_piix.c | 15 ++++++++++++--- >> hw/i386/pc_q35.c | 15 ++++++++++++--- >> include/hw/i386/pc.h | 3 +++ >> vl.c | 4 ++++ >> 5 files changed, 69 insertions(+), 6 deletions(-) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 7cdba10..bccb746 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -1644,11 +1644,49 @@ pc_machine_get_hotplug_memory_region_size(Object *obj, Visitor *v, void *opaque, >> visit_type_int(v, &value, name, errp); >> } >> >> +static void pc_machine_get_max_ram_below_4g(Object *obj, Visitor *v, >> + void *opaque, const char *name, >> + Error **errp) >> +{ >> + PCMachineState *pcms = PC_MACHINE(obj); >> + uint64_t value = pcms->max_ram_below_4g; >> + >> + visit_type_size(v, &value, name, errp); >> +} >> + >> +static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v, >> + void *opaque, const char *name, >> + Error **errp) >> +{ >> + PCMachineState *pcms = PC_MACHINE(obj); >> + Error *error = NULL; >> + uint64_t value; >> + >> + visit_type_size(v, &value, name, &error); >> + if (error) { >> + error_propagate(errp, error); >> + return; >> + } >> + if (value > (1ULL << 32)) { >> + error_set(&error, ERROR_CLASS_GENERIC_ERROR, >> + "Machine option 'max-ram-below-4g=%"PRIu64 >> + "' expects size less then or equal to 4G", value); > less than But the test is greater then. So "not greater then" is "less then or equal". Or did you want the test changed? >> + error_propagate(errp, error); >> + return; >> + } >> + >> + pcms->max_ram_below_4g = value; >> +} >> + >> static void pc_machine_initfn(Object *obj) >> { >> object_property_add(obj, PC_MACHINE_MEMHP_REGION_SIZE, "int", >> pc_machine_get_hotplug_memory_region_size, >> NULL, NULL, NULL, NULL); >> + object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size", >> + pc_machine_get_max_ram_below_4g, >> + pc_machine_set_max_ram_below_4g, >> + NULL, NULL, NULL); >> } >> >> static void pc_machine_class_init(ObjectClass *oc, void *data) >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 40f6eaf..25f4727 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -98,6 +98,13 @@ static void pc_init1(MachineState *machine, >> DeviceState *icc_bridge; >> FWCfgState *fw_cfg = NULL; >> PcGuestInfo *guest_info; >> + Object *mo = qdev_get_machine(); >> + PCMachineState *pcms = PC_MACHINE(mo); >> + ram_addr_t lowmem = 0xe0000000; >> + >> + if (pcms && pcms->max_ram_below_4g) { > Is pcms ever NULL? If yes why? Not that I know of. I would be happy to convert this to an assert(pcms). >> + lowmem = pcms->max_ram_below_4g; >> + } >> >> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory). >> * If it doesn't, we need to split it in chunks below and above 4G. >> @@ -106,8 +113,10 @@ static void pc_init1(MachineState *machine, >> * For old machine types, use whatever split we used historically to avoid >> * breaking migration. >> */ >> - if (machine->ram_size >= 0xe0000000) { >> - ram_addr_t lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000; >> + if (machine->ram_size >= lowmem) { >> + if (!(pcms && pcms->max_ram_below_4g) && gigabyte_align) { >> + lowmem = 0xc0000000; >> + } >> above_4g_mem_size = machine->ram_size - lowmem; >> below_4g_mem_size = lowmem; >> } else { > > So why do we need gigabyte_align anymore? Because of qemu 2.0 and the user is not required to specify this option. > Can't we set property to 0xc0000000 by default, and > override for old machine types? There is a strange compatibility part here. Since this code includes ram_size (see: http://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05146.html ) and xen has a different default. > Also, a value that isn't a multiple of 1G will lead to bad > performance for large machines which do have above_4g_mem_size. > Let's detect and print a warning. Will Do. -Don Slutz > > >> @@ -122,7 +131,7 @@ static void pc_init1(MachineState *machine, >> } >> >> icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> + object_property_add_child(mo, "icc-bridge", >> OBJECT(icc_bridge), NULL); >> >> pc_cpus_init(machine->cpu_model, icc_bridge); >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index e28ce40..155cdf1 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -85,6 +85,13 @@ static void pc_q35_init(MachineState *machine) >> PCIDevice *ahci; >> DeviceState *icc_bridge; >> PcGuestInfo *guest_info; >> + Object *mo = qdev_get_machine(); >> + PCMachineState *pcms = PC_MACHINE(mo); >> + ram_addr_t lowmem = 0xb0000000; >> + >> + if (pcms && pcms->max_ram_below_4g) { >> + lowmem = pcms->max_ram_below_4g; >> + } >> >> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory >> * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping >> @@ -95,8 +102,10 @@ static void pc_q35_init(MachineState *machine) >> * For old machine types, use whatever split we used historically to avoid >> * breaking migration. >> */ >> - if (machine->ram_size >= 0xb0000000) { >> - ram_addr_t lowmem = gigabyte_align ? 0x80000000 : 0xb0000000; >> + if (machine->ram_size >= lowmem) { >> + if (!(pcms && pcms->max_ram_below_4g) && gigabyte_align) { >> + lowmem = 0x800000000; >> + } >> above_4g_mem_size = machine->ram_size - lowmem; >> below_4g_mem_size = lowmem; >> } else { >> @@ -111,7 +120,7 @@ static void pc_q35_init(MachineState *machine) >> } >> >> icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> + object_property_add_child(mo, "icc-bridge", >> OBJECT(icc_bridge), NULL); >> >> pc_cpus_init(machine->cpu_model, icc_bridge); >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 19530bd..2d8b562 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -32,10 +32,13 @@ struct PCMachineState { >> MemoryRegion hotplug_memory; >> >> HotplugHandler *acpi_dev; >> + >> + uint64_t max_ram_below_4g; >> }; >> >> #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" >> #define PC_MACHINE_MEMHP_REGION_SIZE "hotplug-memory-region-size" >> +#define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g" >> >> /** >> * PCMachineClass: >> diff --git a/vl.c b/vl.c >> index 5e77a27..cffb9c5 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -382,6 +382,10 @@ static QemuOptsList qemu_machine_opts = { >> .name = "kvm-type", >> .type = QEMU_OPT_STRING, >> .help = "Specifies the KVM virtualization mode (HV, PR)", >> + },{ >> + .name = PC_MACHINE_MAX_RAM_BELOW_4G, >> + .type = QEMU_OPT_SIZE, >> + .help = "maximum ram below the 4G boundary (32bit boundary)", >> }, >> { /* End of list */ } >> }, >> -- >> 1.8.4