Patchwork [RFC,v3,08/19] pc: calculate dimm physical addresses and adjust memory map

login
register
mail settings
Submitter Vasilis Liaskovitis
Date Sept. 24, 2012, 3:27 p.m.
Message ID <20120924152720.GD4457@dhcp-192-168-178-175.profitbricks.localdomain>
Download mbox | patch
Permalink /patch/186464/
State New
Headers show

Comments

Vasilis Liaskovitis - Sept. 24, 2012, 3:27 p.m.
On Sat, Sep 22, 2012 at 02:15:28PM +0000, Blue Swirl wrote:
> > +
> > +/* Function to configure memory offsets of hotpluggable dimms */
> > +
> > +target_phys_addr_t pc_set_hp_memory_offset(uint64_t size)
> > +{
> > +    target_phys_addr_t ret;
> > +
> > +    /* on first call, initialize ram_hp_offset */
> > +    if (!ram_hp_offset) {
> > +        if (ram_size >= PCI_HOLE_START ) {
> > +            ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START);
> > +        } else {
> > +            ram_hp_offset = ram_size;
> > +        }
> > +    }
> > +
> > +    if (ram_hp_offset >= 0x100000000LL) {
> > +        ret = ram_hp_offset;
> > +        above_4g_hp_mem_size += size;
> > +        ram_hp_offset += size;
> > +    }
> > +    /* if dimm fits before pci hole, append it normally */
> > +    else if (ram_hp_offset + size <= PCI_HOLE_START) {
> 
> } else if ...
> 
> > +        ret = ram_hp_offset;
> > +        below_4g_hp_mem_size += size;
> > +        ram_hp_offset += size;
> > +    }
> > +    /* otherwise place it above 4GB */
> > +    else {
> 
> } else {
> 
> > +        ret = 0x100000000LL;
> > +        above_4g_hp_mem_size += size;
> > +        ram_hp_offset = 0x100000000LL + size;
> > +    }
> > +
> > +    return ret;
> > +}
> 
> But the function and use of lots of global variables is ugly. The dimm
> devices should be just created in piix_pci.c (i440fx) directly with
> correct offsets and sizes, so all  below_4g_mem_size etc. calculations
> should be moved there. That would implement the PMC part of i440fx.
> 
> For ISA PC, probably the board should create the DIMMs since there may
> not be a memory controller. The >4G logic does not make sense there
> anyway.

What about moving the implementation to pc_piix.c?
Initial RAM and pci windows are already calculated in pc_init1, and then passed
to i440fx_init. The memory bus could be attached to i440fx for pci-enabled pc
and to isabus-bridge for isa-pc (isa-pc not tested yet).

Something like the following:

---
 hw/pc.h      |    1 +
 hw/pc_piix.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 52 insertions(+), 6 deletions(-)
Blue Swirl - Sept. 29, 2012, 11:27 a.m.
On Mon, Sep 24, 2012 at 3:27 PM, Vasilis Liaskovitis
<vasilis.liaskovitis@profitbricks.com> wrote:
> On Sat, Sep 22, 2012 at 02:15:28PM +0000, Blue Swirl wrote:
>> > +
>> > +/* Function to configure memory offsets of hotpluggable dimms */
>> > +
>> > +target_phys_addr_t pc_set_hp_memory_offset(uint64_t size)
>> > +{
>> > +    target_phys_addr_t ret;
>> > +
>> > +    /* on first call, initialize ram_hp_offset */
>> > +    if (!ram_hp_offset) {
>> > +        if (ram_size >= PCI_HOLE_START ) {
>> > +            ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START);
>> > +        } else {
>> > +            ram_hp_offset = ram_size;
>> > +        }
>> > +    }
>> > +
>> > +    if (ram_hp_offset >= 0x100000000LL) {
>> > +        ret = ram_hp_offset;
>> > +        above_4g_hp_mem_size += size;
>> > +        ram_hp_offset += size;
>> > +    }
>> > +    /* if dimm fits before pci hole, append it normally */
>> > +    else if (ram_hp_offset + size <= PCI_HOLE_START) {
>>
>> } else if ...
>>
>> > +        ret = ram_hp_offset;
>> > +        below_4g_hp_mem_size += size;
>> > +        ram_hp_offset += size;
>> > +    }
>> > +    /* otherwise place it above 4GB */
>> > +    else {
>>
>> } else {
>>
>> > +        ret = 0x100000000LL;
>> > +        above_4g_hp_mem_size += size;
>> > +        ram_hp_offset = 0x100000000LL + size;
>> > +    }
>> > +
>> > +    return ret;
>> > +}
>>
>> But the function and use of lots of global variables is ugly. The dimm
>> devices should be just created in piix_pci.c (i440fx) directly with
>> correct offsets and sizes, so all  below_4g_mem_size etc. calculations
>> should be moved there. That would implement the PMC part of i440fx.
>>
>> For ISA PC, probably the board should create the DIMMs since there may
>> not be a memory controller. The >4G logic does not make sense there
>> anyway.
>
> What about moving the implementation to pc_piix.c?
> Initial RAM and pci windows are already calculated in pc_init1, and then passed
> to i440fx_init. The memory bus could be attached to i440fx for pci-enabled pc
> and to isabus-bridge for isa-pc (isa-pc not tested yet).

I'd move the calculations also to i440fx, it (PMC) determines the
memory configuration on real HW too.

>
> Something like the following:
>
> ---
>  hw/pc.h      |    1 +
>  hw/pc_piix.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/hw/pc.h b/hw/pc.h
> index e4db071..d6cc43b 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -10,6 +10,7 @@
>  #include "memory.h"
>  #include "ioapic.h"
>
> +#define PCI_HOLE_START 0xe0000000
>  /* PC-style peripherals (also used by other machines).  */
>
>  /* serial.c */
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 88ff041..17db95a 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -43,6 +43,7 @@
>  #include "xen.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "dimm.h"
>  #ifdef CONFIG_XEN
>  #  include <xen/hvm/hvm_info_table.h>
>  #endif
> @@ -52,6 +53,8 @@
>  static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>  static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>  static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
> +static ram_addr_t below_4g_hp_mem_size = 0;
> +static ram_addr_t above_4g_hp_mem_size = 0;
>
>  static void kvm_piix3_setup_irq_routing(bool pci_enabled)
>  {
> @@ -117,6 +120,41 @@ static void ioapic_init(GSIState *gsi_state)
>      }
>  }
>
> +static target_phys_addr_t pc_set_hp_memory_offset(uint64_t size)
> +{
> +    target_phys_addr_t ret;
> +    static ram_addr_t ram_hp_offset = 0;
> +
> +    /* on first call, initialize ram_hp_offset */
> +    if (!ram_hp_offset) {
> +        if (ram_size >= PCI_HOLE_START ) {
> +            ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START);
> +        } else {
> +            ram_hp_offset = ram_size;
> +        }
> +    }
> +
> +    if (ram_hp_offset >= 0x100000000LL) {
> +        ret = ram_hp_offset;
> +        above_4g_hp_mem_size += size;
> +        ram_hp_offset += size;
> +    }
> +    /* if dimm fits before pci hole, append it normally */
> +    else if (ram_hp_offset + size <= PCI_HOLE_START) {
> +        ret = ram_hp_offset;
> +        below_4g_hp_mem_size += size;
> +        ram_hp_offset += size;
> +    }
> +    /* otherwise place it above 4GB */
> +    else {
> +        ret = 0x100000000LL;
> +        above_4g_hp_mem_size += size;
> +        ram_hp_offset = 0x100000000LL + size;
> +    }
> +
> +    return ret;
> +}
> +
>  /* PC hardware initialisation */
>  static void pc_init1(MemoryRegion *system_memory,
>                       MemoryRegion *system_io,
> @@ -155,9 +193,9 @@ static void pc_init1(MemoryRegion *system_memory,
>          kvmclock_create();
>      }
>
> -    if (ram_size >= 0xe0000000 ) {
> -        above_4g_mem_size = ram_size - 0xe0000000;
> -        below_4g_mem_size = 0xe0000000;
> +    if (ram_size >= PCI_HOLE_START ) {
> +        above_4g_mem_size = ram_size - PCI_HOLE_START;
> +        below_4g_mem_size = PCI_HOLE_START;
>      } else {
>          above_4g_mem_size = 0;
>          below_4g_mem_size = ram_size;
> @@ -172,6 +210,9 @@ static void pc_init1(MemoryRegion *system_memory,
>          rom_memory = system_memory;
>      }
>
> +    /* adjust memory map for hotplug dimms */
> +    dimm_calc_offsets(pc_set_hp_memory_offset);
> +
>      /* allocate ram and load rom/bios */
>      if (!xen_enabled()) {
>          fw_cfg = pc_memory_init(system_memory,
> @@ -192,18 +233,22 @@ static void pc_init1(MemoryRegion *system_memory,
>      if (pci_enabled) {
>          pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
>                                system_memory, system_io, ram_size,
> -                              below_4g_mem_size,
> -                              0x100000000ULL - below_4g_mem_size,
> -                              0x100000000ULL + above_4g_mem_size,
> +                              below_4g_mem_size + below_4g_hp_mem_size,
> +                              0x100000000ULL - below_4g_mem_size
> +                                - below_4g_hp_mem_size,
> +                              0x100000000ULL + above_4g_mem_size
> +                                + above_4g_hp_mem_size,
>                                (sizeof(target_phys_addr_t) == 4
>                                 ? 0
>                                 : ((uint64_t)1 << 62)),
>                                pci_memory, ram_memory);
> +        main_memory_bus_create(object_resolve_path("i440fx", NULL));
>      } else {
>          pci_bus = NULL;
>          i440fx_state = NULL;
>          isa_bus = isa_bus_new(NULL, system_io);
>          no_hpet = 1;
> +        main_memory_bus_create(object_resolve_path("isabus-bridge", NULL));
>      }
>      isa_bus_irqs(isa_bus, gsi);
>
> --
> 1.7.9
>

Patch

diff --git a/hw/pc.h b/hw/pc.h
index e4db071..d6cc43b 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -10,6 +10,7 @@ 
 #include "memory.h"
 #include "ioapic.h"
 
+#define PCI_HOLE_START 0xe0000000
 /* PC-style peripherals (also used by other machines).  */
 
 /* serial.c */
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 88ff041..17db95a 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -43,6 +43,7 @@ 
 #include "xen.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "dimm.h"
 #ifdef CONFIG_XEN
 #  include <xen/hvm/hvm_info_table.h>
 #endif
@@ -52,6 +53,8 @@ 
 static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
+static ram_addr_t below_4g_hp_mem_size = 0;
+static ram_addr_t above_4g_hp_mem_size = 0;
 
 static void kvm_piix3_setup_irq_routing(bool pci_enabled)
 {
@@ -117,6 +120,41 @@  static void ioapic_init(GSIState *gsi_state)
     }
 }
 
+static target_phys_addr_t pc_set_hp_memory_offset(uint64_t size)
+{
+    target_phys_addr_t ret;
+    static ram_addr_t ram_hp_offset = 0;
+
+    /* on first call, initialize ram_hp_offset */
+    if (!ram_hp_offset) {
+        if (ram_size >= PCI_HOLE_START ) {
+            ram_hp_offset = 0x100000000LL + (ram_size - PCI_HOLE_START);
+        } else {
+            ram_hp_offset = ram_size;
+        }
+    }
+
+    if (ram_hp_offset >= 0x100000000LL) {
+        ret = ram_hp_offset;
+        above_4g_hp_mem_size += size;
+        ram_hp_offset += size;
+    }
+    /* if dimm fits before pci hole, append it normally */
+    else if (ram_hp_offset + size <= PCI_HOLE_START) {
+        ret = ram_hp_offset;
+        below_4g_hp_mem_size += size;
+        ram_hp_offset += size;
+    }
+    /* otherwise place it above 4GB */
+    else {
+        ret = 0x100000000LL;
+        above_4g_hp_mem_size += size;
+        ram_hp_offset = 0x100000000LL + size;
+    }
+
+    return ret;
+}
+
 /* PC hardware initialisation */
 static void pc_init1(MemoryRegion *system_memory,
                      MemoryRegion *system_io,
@@ -155,9 +193,9 @@  static void pc_init1(MemoryRegion *system_memory,
         kvmclock_create();
     }
 
-    if (ram_size >= 0xe0000000 ) {
-        above_4g_mem_size = ram_size - 0xe0000000;
-        below_4g_mem_size = 0xe0000000;
+    if (ram_size >= PCI_HOLE_START ) {
+        above_4g_mem_size = ram_size - PCI_HOLE_START;
+        below_4g_mem_size = PCI_HOLE_START;
     } else {
         above_4g_mem_size = 0;
         below_4g_mem_size = ram_size;
@@ -172,6 +210,9 @@  static void pc_init1(MemoryRegion *system_memory,
         rom_memory = system_memory;
     }
 
+    /* adjust memory map for hotplug dimms */
+    dimm_calc_offsets(pc_set_hp_memory_offset);
+
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
         fw_cfg = pc_memory_init(system_memory,
@@ -192,18 +233,22 @@  static void pc_init1(MemoryRegion *system_memory,
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
                               system_memory, system_io, ram_size,
-                              below_4g_mem_size,
-                              0x100000000ULL - below_4g_mem_size,
-                              0x100000000ULL + above_4g_mem_size,
+                              below_4g_mem_size + below_4g_hp_mem_size,
+                              0x100000000ULL - below_4g_mem_size
+                                - below_4g_hp_mem_size,
+                              0x100000000ULL + above_4g_mem_size
+                                + above_4g_hp_mem_size,
                               (sizeof(target_phys_addr_t) == 4
                                ? 0
                                : ((uint64_t)1 << 62)),
                               pci_memory, ram_memory);
+        main_memory_bus_create(object_resolve_path("i440fx", NULL));
     } else {
         pci_bus = NULL;
         i440fx_state = NULL;
         isa_bus = isa_bus_new(NULL, system_io);
         no_hpet = 1;
+        main_memory_bus_create(object_resolve_path("isabus-bridge", NULL));
     }
     isa_bus_irqs(isa_bus, gsi);