Re: [Qemu-devel] [PATCH v5 2/3] pc & q35: Add new machine opt max-ram-below-4g

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jun 17, 2014 at 02:44:41PM -0400, Don Slutz wrote:
> On 06/17/14 14:22, 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.
> >I'm not sure I get it.
> >
> >All this only has effect if you have >4G RAM, right?
> 
> Nope.  When it takes effect includes RAM > max-ram-below-4g. I.E.
> 
>    -machine pc,max-ram-below-4g=2G -m 3G
> 
> Will have 2G in 32bit space (below 4G) and 1G in 64bit space (above 4G).
> 
> >Presumably you then have a 64 bit guest so why does it
> >care about memory/MMIO being below 4G?
> 
> It is not the guest that matters, it is all the PCI devices in use. There are
> ones (all old hardware) that only support 32bit addresses.

Emulated devices? Let's just teach them to support 64 bit BARs.
Looks like a nicer solution than asking user to make
this decision.

>  When using
> these you may need more room.
> 
> Also pci-passthru of real hardware that is 32bit only may require this.
>    -Don Slutz

Guest and host BARs are unrelated so it seems we could allow 64 bit BARs
even when they are 32 bit on host.

Though PCI Express spec requires 64 bit BARs since version 1.0,
are there really many devices like this out there?
Care giving examples?
They won't well on physical systems either.

> 
> >
> >
> >>Signed-off-by: Don Slutz <dslutz@verizon.com>
> >>---
> >>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);
> >>+        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) {
> >>+        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 {
> >>@@ -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