[Qemu-devel] [PATCH 0/3] some numa & memory related fixes

[Qemu-devel] [PATCH 0/3] some numa & memory related fixes

[Hu Tao]

See each patch for the detail.

Hu Tao (3):
  hw:i386: typo fix: MEMORY_HOPTLUG_DEVICE -> MEMORY_HOTPLUG_DEVICE
  pc-dimm: check if node property exceeds available numa nodes
  numa: show hex number in error message for consistency and prefix them
    with 0x

 hw/i386/acpi-dsdt.dsl           |  4 ++--
 hw/i386/acpi-dsdt.hex.generated |  8 ++++----
 hw/i386/q35-acpi-dsdt.dsl       |  4 ++--
 hw/i386/ssdt-mem.dsl            | 16 ++++++++--------
 hw/i386/ssdt-misc.dsl           |  2 +-
 hw/mem/pc-dimm.c                |  5 +++++
 include/hw/acpi/pc-hotplug.h    |  2 +-
 numa.c                          |  4 ++--
 8 files changed, 25 insertions(+), 20 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/3] hw:i386: typo fix: MEMORY_HOPTLUG_DEVICE -> MEMORY_HOTPLUG_DEVICE

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/acpi-dsdt.dsl           |  4 ++--
 hw/i386/acpi-dsdt.hex.generated |  8 ++++----
 hw/i386/q35-acpi-dsdt.dsl       |  4 ++--
 hw/i386/ssdt-mem.dsl            | 16 ++++++++--------
 hw/i386/ssdt-misc.dsl           |  2 +-
 include/hw/acpi/pc-hotplug.h    |  2 +-
 6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 6ba0170..559f4b6 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -302,7 +302,7 @@ DefinitionBlock (
 /****************************************************************
  * General purpose events
  ****************************************************************/
-    External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD, MethodObj)
+    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD, MethodObj)
 
     Scope(\_GPE) {
         Name(_HID, "ACPI0006")
@@ -321,7 +321,7 @@ DefinitionBlock (
         }
         Method(_E03) {
             // Memory hotplug event
-            \_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
+            \_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
         }
         Method(_L04) {
         }
diff --git a/hw/i386/acpi-dsdt.hex.generated b/hw/i386/acpi-dsdt.hex.generated
index 6c8a1fc..a21bf41 100644
--- a/hw/i386/acpi-dsdt.hex.generated
+++ b/hw/i386/acpi-dsdt.hex.generated
@@ -8,7 +8,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x0,
 0x0,
 0x1,
-0x2e,
+0x1f,
 0x42,
 0x58,
 0x50,
@@ -31,9 +31,9 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x4e,
 0x54,
 0x4c,
-0x13,
-0x9,
-0x12,
+0x28,
+0x5,
+0x10,
 0x20,
 0x10,
 0x49,
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index 8c3eae7..054b035 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -410,7 +410,7 @@ DefinitionBlock (
 /****************************************************************
  * General purpose events
  ****************************************************************/
-    External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD, MethodObj)
+    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD, MethodObj)
 
     Scope(\_GPE) {
         Name(_HID, "ACPI0006")
@@ -425,7 +425,7 @@ DefinitionBlock (
         }
         Method(_E03) {
             // Memory hotplug event
-            \_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
+            \_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
         }
         Method(_L04) {
         }
diff --git a/hw/i386/ssdt-mem.dsl b/hw/i386/ssdt-mem.dsl
index 8e17bd1..22ff5dd 100644
--- a/hw/i386/ssdt-mem.dsl
+++ b/hw/i386/ssdt-mem.dsl
@@ -39,10 +39,10 @@ ACPI_EXTRACT_ALL_CODE ssdm_mem_aml
 DefinitionBlock ("ssdt-mem.aml", "SSDT", 0x02, "BXPC", "CSSDT", 0x1)
 {
 
-    External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_CRS_METHOD, MethodObj)
-    External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD, MethodObj)
-    External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_OST_METHOD, MethodObj)
-    External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD, MethodObj)
+    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_CRS_METHOD, MethodObj)
+    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD, MethodObj)
+    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD, MethodObj)
+    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD, MethodObj)
 
     Scope(\_SB) {
 /*  v------------------ DO NOT EDIT ------------------v */
@@ -58,19 +58,19 @@ DefinitionBlock ("ssdt-mem.aml", "SSDT", 0x02, "BXPC", "CSSDT", 0x1)
             Name(_HID, EISAID("PNP0C80"))
 
             Method(_CRS, 0) {
-                Return(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_CRS_METHOD(_UID))
+                Return(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_CRS_METHOD(_UID))
             }
 
             Method(_STA, 0) {
-                Return(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD(_UID))
+                Return(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD(_UID))
             }
 
             Method(_PXM, 0) {
-                Return(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD(_UID))
+                Return(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD(_UID))
             }
 
             Method(_OST, 3) {
-                \_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_OST_METHOD(_UID, Arg0, Arg1, Arg2)
+                \_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD(_UID, Arg0, Arg1, Arg2)
             }
         }
     }
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index d329b8b..0fd4480 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -120,7 +120,7 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
 
     External(MEMORY_SLOT_NOTIFY_METHOD, MethodObj)
     Scope(\_SB.PCI0) {
-        Device(MEMORY_HOPTLUG_DEVICE) {
+        Device(MEMORY_HOTPLUG_DEVICE) {
             Name(_HID, "PNP0A06")
             Name(_UID, "Memory hotplug resources")
 
diff --git a/include/hw/acpi/pc-hotplug.h b/include/hw/acpi/pc-hotplug.h
index bf5157d..b9db295 100644
--- a/include/hw/acpi/pc-hotplug.h
+++ b/include/hw/acpi/pc-hotplug.h
@@ -32,7 +32,7 @@
 #define ACPI_MEMORY_HOTPLUG_IO_LEN 24
 #define ACPI_MEMORY_HOTPLUG_BASE 0x0a00
 
-#define MEMORY_HOPTLUG_DEVICE        MHPD
+#define MEMORY_HOTPLUG_DEVICE        MHPD
 #define MEMORY_SLOTS_NUMBER          MDNR
 #define MEMORY_HOTPLUG_IO_REGION     HPMR
 #define MEMORY_SLOT_ADDR_LOW         MRBL
-- 
1.9.3

[Qemu-devel] [PATCH 2/3] pc-dimm: check if the value of node property

[Hu Tao]

If user specifies a node number that exceeds the available numa nodes in
emulated system for pc-dimm device, the device will reports an invalid _PXM
to OSPM. Fix it by checking the node value.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/mem/pc-dimm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 08f49ed..92e276f 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -252,6 +252,11 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
         return;
     }
+    if (dimm->node >= nb_numa_nodes) {
+        error_setg(errp, "'" PC_DIMM_NODE_PROP
+                   "' exceeds numa node number: %" PRId32, nb_numa_nodes);
+        return;
+    }
 }
 
 static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
-- 
1.9.3

[Qemu-devel] [PATCH 3/3] numa: show hex number in error message for consistency and prefix them with 0x

[Hu Tao]

The error messages before and after patch are:

before:
qemu-system-x86_64: total memory for NUMA nodes (134217728) should equal RAM size (20000000)

after:
qemu-system-x86_64: total memory for NUMA nodes (0x8000000) should equal RAM size (0x20000000)

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 numa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/numa.c b/numa.c
index 7bf7834..c78cec9 100644
--- a/numa.c
+++ b/numa.c
@@ -210,8 +210,8 @@ void set_numa_nodes(void)
             numa_total += numa_info[i].node_mem;
         }
         if (numa_total != ram_size) {
-            error_report("total memory for NUMA nodes (%" PRIu64 ")"
-                         " should equal RAM size (" RAM_ADDR_FMT ")",
+            error_report("total memory for NUMA nodes (0x%" PRIx64 ")"
+                         " should equal RAM size (0x" RAM_ADDR_FMT ")",
                          numa_total, ram_size);
             exit(1);
         }
-- 
1.9.3

Re: [Qemu-devel] [PATCH 2/3] pc-dimm: check if the value of node property

[Michael S. Tsirkin]

On Mon, Aug 04, 2014 at 04:16:08PM +0800, Hu Tao wrote:
> If user specifies a node number that exceeds the available numa nodes in
> emulated system for pc-dimm device, the device will reports an invalid _PXM
> to OSPM. Fix it by checking the node value.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/mem/pc-dimm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 08f49ed..92e276f 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -252,6 +252,11 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
>          error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
>          return;
>      }
> +    if (dimm->node >= nb_numa_nodes) {
> +        error_setg(errp, "'" PC_DIMM_NODE_PROP
> +                   "' exceeds numa node number: %" PRId32, nb_numa_nodes);

PRId32 is wrong here, this variable is int, use %d.
Also, this message isn't very clear, I fixed it up
with a patch on top.

> +        return;
> +    }
>  }
>  
>  static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
> -- 
> 1.9.3

Re: [Qemu-devel] [PATCH 0/3] some numa & memory related fixes

[Michael S. Tsirkin]

On Mon, Aug 04, 2014 at 04:16:06PM +0800, Hu Tao wrote:
> See each patch for the detail.
> 
> Hu Tao (3):
>   hw:i386: typo fix: MEMORY_HOPTLUG_DEVICE -> MEMORY_HOTPLUG_DEVICE
>   pc-dimm: check if node property exceeds available numa nodes
>   numa: show hex number in error message for consistency and prefix them
>     with 0x


Thanks, applied.

>  hw/i386/acpi-dsdt.dsl           |  4 ++--
>  hw/i386/acpi-dsdt.hex.generated |  8 ++++----
>  hw/i386/q35-acpi-dsdt.dsl       |  4 ++--
>  hw/i386/ssdt-mem.dsl            | 16 ++++++++--------
>  hw/i386/ssdt-misc.dsl           |  2 +-
>  hw/mem/pc-dimm.c                |  5 +++++
>  include/hw/acpi/pc-hotplug.h    |  2 +-
>  numa.c                          |  4 ++--
>  8 files changed, 25 insertions(+), 20 deletions(-)
> 
> -- 
> 1.9.3

Re: [Qemu-devel] [PATCH 2/3] pc-dimm: check if the value of node property

[Hu Tao]

On Mon, Aug 04, 2014 at 02:22:48PM +0200, Michael S. Tsirkin wrote:
> On Mon, Aug 04, 2014 at 04:16:08PM +0800, Hu Tao wrote:
> > If user specifies a node number that exceeds the available numa nodes in
> > emulated system for pc-dimm device, the device will reports an invalid _PXM
> > to OSPM. Fix it by checking the node value.
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/mem/pc-dimm.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > index 08f49ed..92e276f 100644
> > --- a/hw/mem/pc-dimm.c
> > +++ b/hw/mem/pc-dimm.c
> > @@ -252,6 +252,11 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
> >          error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
> >          return;
> >      }
> > +    if (dimm->node >= nb_numa_nodes) {
> > +        error_setg(errp, "'" PC_DIMM_NODE_PROP
> > +                   "' exceeds numa node number: %" PRId32, nb_numa_nodes);
> 
> PRId32 is wrong here, this variable is int, use %d.
> Also, this message isn't very clear, I fixed it up
> with a patch on top.

Thanks!

> 
> > +        return;
> > +    }
> >  }
> >  
> >  static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
> > -- 
> > 1.9.3