qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.9 10/10] memhp: move DIMM devices into ded


From: Igor Mammedov
Subject: Re: [Qemu-devel] [PATCH for-2.9 10/10] memhp: move DIMM devices into dedicated scope with related common methods
Date: Thu, 22 Dec 2016 14:31:24 +0100

On Thu, 22 Dec 2016 14:31:19 +0200
Marcel Apfelbaum <address@hidden> wrote:

> On 12/06/2016 01:32 AM, Igor Mammedov wrote:
> > Move DIMM devices from global _SB scope to a new \_SB.MHPC
> > container along with common methods used by DIMMs:
> >   MCRS, MRST, MPXM, MOST, MEJ00, MSCN, MTFY
> >
> > this reduces AML size on 12 * #slots bytes,
> > i.e. up to 3072 bytes for 265 slots.
> >  
> 
> Can you please explain how the bytes number are reduced?
> Is it because the devices path is now relative to the new container?
yes

> If so, can you give an example of before/after?
as example:

-        Device (MP02)
-        {
-            Name (_UID, "0x02")  // _UID: Unique ID
-            Name (_HID, EisaId ("PNP0C80") /* Memory Device */)  // _HID: 
Hardware ID
-            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
-            {
-                Return (\_SB.PCI0.MHPD.MCRS (_UID))
-            }
-
-            Method (_STA, 0, NotSerialized)  // _STA: Status
-            {
-                Return (\_SB.PCI0.MHPD.MRST (_UID))
-            }
-
-            Method (_PXM, 0, NotSerialized)  // _PXM: Device Proximity
-            {
-                Return (\_SB.PCI0.MHPD.MPXM (_UID))
-            }
-
-            Method (_OST, 3, NotSerialized)  // _OST: OSPM Status Indication
-            {
-                Return (\_SB.PCI0.MHPD.MOST (_UID, Arg0, Arg1, Arg2))
-            }
-
-            Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device
-            {
-                Return (\_SB.PCI0.MHPD.MEJ0 (_UID, Arg0))
-            }
-        }
----
+        Device (MP02)
+        {
+            Name (_UID, "0x02")  // _UID: Unique ID
+            Name (_HID, EisaId ("PNP0C80") /* Memory Device */)  // _HID: 
Hardware ID
+            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
+            {
+                Return (MCRS (_UID))
+            }
+
+            Method (_STA, 0, NotSerialized)  // _STA: Status
+            {
+                Return (MRST (_UID))
+            }
+
+            Method (_PXM, 0, NotSerialized)  // _PXM: Device Proximity
+            {
+                Return (MPXM (_UID))
+            }
+
+            Method (_OST, 3, NotSerialized)  // _OST: OSPM Status Indication
+            {
+                Return (MOST (_UID, Arg0, Arg1, Arg2))
+            }
+
+            Method (_EJ0, 1, NotSerialized)  // _EJx: Eject Device
+            {
+                Return (MEJ0 (_UID, Arg0))
+            }
+        }

> 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/acpi/memory_hotplug.c | 190 
> > ++++++++++++++++++++++++-----------------------
> >  1 file changed, 97 insertions(+), 93 deletions(-)
> >
> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> > index fb04d24..210073d 100644
> > --- a/hw/acpi/memory_hotplug.c
> > +++ b/hw/acpi/memory_hotplug.c
> > @@ -31,6 +31,7 @@
> >  #define MEMORY_SLOT_SCAN_METHOD      "MSCN"
> >  #define MEMORY_HOTPLUG_DEVICE        "MHPD"
> >  #define MEMORY_HOTPLUG_IO_LEN         24
> > +#define MEMORY_DEVICES_CONTAINER     "\\_SB.MHPC"
> >
> >  static uint16_t memhp_io_base;
> >
> > @@ -343,9 +344,8 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >      int i;
> >      Aml *ifctx;
> >      Aml *method;
> > -    Aml *sb_scope;
> > +    Aml *dev_container;
> >      Aml *mem_ctrl_dev;
> > -    char *scan_path;
> >      char *mhp_res_path;
> >
> >      if (!memhp_io_base) {
> > @@ -356,24 +356,11 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >      mem_ctrl_dev = aml_device("%s", mhp_res_path);
> >      {
> >          Aml *crs;
> > -        Aml *field;
> > -        Aml *one = aml_int(1);
> > -        Aml *zero = aml_int(0);
> > -        Aml *ret_val = aml_local(0);
> > -        Aml *slot_arg0 = aml_arg(0);
> > -        Aml *slots_nr = aml_name(MEMORY_SLOTS_NUMBER);
> > -        Aml *ctrl_lock = aml_name(MEMORY_SLOT_LOCK);
> > -        Aml *slot_selector = aml_name(MEMORY_SLOT_SLECTOR);
> >
> >          aml_append(mem_ctrl_dev, aml_name_decl("_HID", 
> > aml_string("PNP0A06")));  
> 
> Do we need the above declaration twice? Here is the first occurrence.
> 
> >          aml_append(mem_ctrl_dev,
> >              aml_name_decl("_UID", aml_string("Memory hotplug resources")));
[...]

> > +
> > +    dev_container = aml_device(MEMORY_DEVICES_CONTAINER);
> > +    {
[...]
> > +        aml_append(dev_container, aml_name_decl("_HID", 
> > aml_string("PNP0A06")));  
> 
> And here is the second occurrence.
It's different containers, the first one is for MMIO registers
for x86 it's placed \_SB.PCI0.MHPD consumes resources from parent's PCI0._CRS
and the second is for memory devices \_SB.MHPC which consumes resources
outside of PCI0._CRS scope.

 
> 
> Thanks,
> Marcel
> 
> > +        aml_append(dev_container,
> > +            aml_name_decl("_UID", aml_string("DIMM devices")));
> > +
> > +        assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
> > +        aml_append(dev_container,
> > +            aml_name_decl(MEMORY_SLOTS_NUMBER, aml_int(nr_mem))
> > +        );
> > +
> > +        field = aml_field(mmio_path, AML_DWORD_ACC,
> >                            AML_NOLOCK, AML_PRESERVE);
> >          aml_append(field, /* read only */
> >              aml_named_field(MEMORY_SLOT_ADDR_LOW, 32));
> > @@ -398,9 +410,9 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_named_field(MEMORY_SLOT_SIZE_HIGH, 32));
> >          aml_append(field, /* read only */
> >              aml_named_field(MEMORY_SLOT_PROXIMITY, 32));
> > -        aml_append(mem_ctrl_dev, field);
> > +        aml_append(dev_container, field);
> >
> > -        field = aml_field(MEMORY_HOTPLUG_IO_REGION, AML_BYTE_ACC,
> > +        field = aml_field(mmio_path, AML_BYTE_ACC,
> >                            AML_NOLOCK, AML_WRITE_AS_ZEROS);
> >          aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
> >          aml_append(field, /* 1 if enabled, read only */
> > @@ -414,9 +426,9 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >          aml_append(field,
> >              /* initiates device eject, write only */
> >              aml_named_field(MEMORY_SLOT_EJECT, 1));
> > -        aml_append(mem_ctrl_dev, field);
> > +        aml_append(dev_container, field);
> >
> > -        field = aml_field(MEMORY_HOTPLUG_IO_REGION, AML_DWORD_ACC,
> > +        field = aml_field(mmio_path, AML_DWORD_ACC,
> >                            AML_NOLOCK, AML_PRESERVE);
> >          aml_append(field, /* DIMM selector, write only */
> >              aml_named_field(MEMORY_SLOT_SLECTOR, 32));
> > @@ -424,7 +436,8 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_named_field(MEMORY_SLOT_OST_EVENT, 32));
> >          aml_append(field, /* _OST status code, write only */
> >              aml_named_field(MEMORY_SLOT_OST_STATUS, 32));
> > -        aml_append(mem_ctrl_dev, field);
> > +        aml_append(dev_container, field);
> > +        g_free(mmio_path);
> >
> >          method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> >          ifctx = aml_if(aml_equal(slots_nr, zero));
> > @@ -434,9 +447,9 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >          aml_append(method, ifctx);
> >          /* present, functioning, decoding, not shown in UI */
> >          aml_append(method, aml_return(aml_int(0xB)));
> > -        aml_append(mem_ctrl_dev, method);
> > +        aml_append(dev_container, method);
> >
> > -        aml_append(mem_ctrl_dev, aml_mutex(MEMORY_SLOT_LOCK, 0));
> > +        aml_append(dev_container, aml_mutex(MEMORY_SLOT_LOCK, 0));
> >
> >          method = aml_method(MEMORY_SLOT_SCAN_METHOD, 0, AML_NOTSERIALIZED);
> >          {
> > @@ -492,7 +505,7 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_append(method, aml_release(ctrl_lock));
> >              aml_append(method, aml_return(one));
> >          }
> > -        aml_append(mem_ctrl_dev, method);
> > +        aml_append(dev_container, method);
> >
> >          method = aml_method(MEMORY_SLOT_STATUS_METHOD, 1, 
> > AML_NOTSERIALIZED);
> >          {
> > @@ -512,7 +525,7 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_append(method, aml_release(ctrl_lock));
> >              aml_append(method, aml_return(ret_val));
> >          }
> > -        aml_append(mem_ctrl_dev, method);
> > +        aml_append(dev_container, method);
> >
> >          method = aml_method(MEMORY_SLOT_CRS_METHOD, 1, AML_SERIALIZED);
> >          {
> > @@ -603,7 +616,7 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_append(method, aml_release(ctrl_lock));
> >              aml_append(method, aml_return(mr64));
> >          }
> > -        aml_append(mem_ctrl_dev, method);
> > +        aml_append(dev_container, method);
> >
> >          method = aml_method(MEMORY_SLOT_PROXIMITY_METHOD, 1,
> >                              AML_NOTSERIALIZED);
> > @@ -617,7 +630,7 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_append(method, aml_release(ctrl_lock));
> >              aml_append(method, aml_return(ret_val));
> >          }
> > -        aml_append(mem_ctrl_dev, method);
> > +        aml_append(dev_container, method);
> >
> >          method = aml_method(MEMORY_SLOT_OST_METHOD, 4, AML_NOTSERIALIZED);
> >          {
> > @@ -631,7 +644,7 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_append(method, aml_store(aml_arg(2), ost_status));
> >              aml_append(method, aml_release(ctrl_lock));
> >          }
> > -        aml_append(mem_ctrl_dev, method);
> > +        aml_append(dev_container, method);
> >
> >          method = aml_method(MEMORY_SLOT_EJECT_METHOD, 2, 
> > AML_NOTSERIALIZED);
> >          {
> > @@ -643,75 +656,66 @@ void build_memory_hotplug_aml(Aml *table, uint32_t 
> > nr_mem,
> >              aml_append(method, aml_store(one, eject));
> >              aml_append(method, aml_release(ctrl_lock));
> >          }
> > -        aml_append(mem_ctrl_dev, method);
> > -    }
> > -    aml_append(table, mem_ctrl_dev);
> > -
> > -    sb_scope = aml_scope("_SB");
> > -    /* build memory devices */
> > -    for (i = 0; i < nr_mem; i++) {
> > -        Aml *dev;
> > -        char *s;
> > -
> > -        dev = aml_device("MP%02X", i);
> > -        aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
> > -        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> > -
> > -        method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
> > -        s = g_strdup_printf("%s.%s", mhp_res_path, MEMORY_SLOT_CRS_METHOD);
> > -        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > -        g_free(s);
> > -        aml_append(dev, method);
> > -
> > -        method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> > -        s = g_strdup_printf("%s.%s", mhp_res_path, 
> > MEMORY_SLOT_STATUS_METHOD);
> > -        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > -        g_free(s);
> > -        aml_append(dev, method);
> > -
> > -        method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
> > -        s = g_strdup_printf("%s.%s", mhp_res_path,
> > -                            MEMORY_SLOT_PROXIMITY_METHOD);
> > -        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > -        g_free(s);
> > -        aml_append(dev, method);
> > -
> > -        method = aml_method("_OST", 3, AML_NOTSERIALIZED);
> > -        s = g_strdup_printf("%s.%s", mhp_res_path, MEMORY_SLOT_OST_METHOD);
> > -        aml_append(method, aml_return(aml_call4(
> > -            s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> > -        )));
> > -        g_free(s);
> > -        aml_append(dev, method);
> > -
> > -        method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> > -        s = g_strdup_printf("%s.%s", mhp_res_path, 
> > MEMORY_SLOT_EJECT_METHOD);
> > -        aml_append(method, aml_return(aml_call2(
> > -                   s, aml_name("_UID"), aml_arg(0))));
> > -        g_free(s);
> > -        aml_append(dev, method);
> > -
> > -        aml_append(sb_scope, dev);
> > -    }
> > +        aml_append(dev_container, method);
> > +
> > +        /* build memory devices */
> > +        for (i = 0; i < nr_mem; i++) {
> > +            Aml *dev;
> > +            const char *s;
> > +
> > +            dev = aml_device("MP%02X", i);
> > +            aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", 
> > i)));
> > +            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> > +
> > +            method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
> > +            s = MEMORY_SLOT_CRS_METHOD;
> > +            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > +            aml_append(dev, method);
> > +
> > +            method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> > +            s = MEMORY_SLOT_STATUS_METHOD;
> > +            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > +            aml_append(dev, method);
> > +
> > +            method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
> > +            s = MEMORY_SLOT_PROXIMITY_METHOD;
> > +            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > +            aml_append(dev, method);
> > +
> > +            method = aml_method("_OST", 3, AML_NOTSERIALIZED);
> > +            s = MEMORY_SLOT_OST_METHOD;
> > +            aml_append(method, aml_return(aml_call4(
> > +                s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> > +            )));
> > +            aml_append(dev, method);
> > +
> > +            method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> > +            s = MEMORY_SLOT_EJECT_METHOD;
> > +            aml_append(method, aml_return(aml_call2(
> > +                       s, aml_name("_UID"), aml_arg(0))));
> > +            aml_append(dev, method);
> > +
> > +            aml_append(dev_container, dev);
> > +        }
> >
> > -    /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> > -     *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> > -     */
> > -    method = aml_method(MEMORY_SLOT_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
> > -    for (i = 0; i < nr_mem; i++) {
> > -        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> > -        aml_append(ifctx,
> > -            aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> > -        );
> > -        aml_append(method, ifctx);
> > +        /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> > +         *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> > +         */
> > +        method = aml_method(MEMORY_SLOT_NOTIFY_METHOD, 2, 
> > AML_NOTSERIALIZED);
> > +        for (i = 0; i < nr_mem; i++) {
> > +            ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> > +            aml_append(ifctx,
> > +                aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> > +            );
> > +            aml_append(method, ifctx);
> > +        }
> > +        aml_append(dev_container, method);
> >      }
> > -    aml_append(sb_scope, method);
> > -    aml_append(table, sb_scope);
> > +    aml_append(table, dev_container);
> >
> >      method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
> > -    scan_path = g_strdup_printf("%s.%s", mhp_res_path, 
> > MEMORY_SLOT_SCAN_METHOD);
> > -    aml_append(method, aml_call0(scan_path));
> > -    g_free(scan_path);
> > +    aml_append(method,
> > +        aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD));
> >      aml_append(table, method);
> >
> >      g_free(mhp_res_path);
> >  
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]