qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields"


From: Zhu Guihua
Subject: Re: [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register
Date: Tue, 28 Apr 2015 13:49:43 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0


On 04/28/2015 03:09 AM, Michael S. Tsirkin wrote:
On Mon, Apr 27, 2015 at 04:47:20PM +0800, Zhu Guihua wrote:
0 bit in Memory device control fields must be cleared before writing to
register. But now this field isn't cleared when other fields are written.

To solve this bug, This patch fixes UpdateRule to WriteAsZeros in "Memory
device control fields" register.

Reviewed-by: Igor Mammedov <address@hidden>
Signed-off-by: Zhu Guihua <address@hidden>
Applied, thanks!
Please check pci branch in my tree as I had to
resolve some trivial conflicts.

What about other un-applied patches in this series ?
Should I rebase them on your pci branch and resend then again ?

Thanks,
Zhu


---
  docs/specs/acpi_mem_hotplug.txt | 5 ++++-
  hw/i386/acpi-build.c            | 2 +-
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index dcc4808..68daa14 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -31,7 +31,10 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
access):
        [0xc-0x13] reserved, writes into it are ignored
        [0x14] Memory device control fields
            bits:
-              0: reserved, OSPM must clear it before writing to register
+              0: reserved, OSPM must clear it before writing to register.
+                 Due to BUG in versions prior 2.4 that field isn't cleared
+                 when other fields are written. Keep it reserved and don't
+                 try to reuse it.
                1: if set to 1 clears device insert event, set by OSPM
                   after it has emitted device check event for the
                   selected memory device
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index dc1b8e8..eb66fef 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -924,7 +924,7 @@ build_ssdt(GArray *table_data, GArray *linker,
          aml_append(scope, field);
field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc,
-                          aml_preserve);
+                          aml_write_as_zeros);
          aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
          aml_append(field, /* 1 if enabled, read only */
              aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
--
1.9.3
On Mon, Apr 27, 2015 at 04:47:21PM +0800, Zhu Guihua wrote:
- implements QEMU hardware part of memory hot unplug protocol
   described at "docs/spec/acpi_mem_hotplug.txt"
- handles memory remove notification event
- handles device eject notification

Reviewed-by: Igor Mammedov <address@hidden>
Signed-off-by: Zhu Guihua <address@hidden>
---
  docs/specs/acpi_mem_hotplug.txt   | 11 +++++++++--
  hw/acpi/memory_hotplug.c          | 21 ++++++++++++++++++++-
  hw/core/qdev.c                    |  2 +-
  hw/i386/acpi-build.c              | 12 ++++++++++++
  hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 ++++++++++++-
  include/hw/acpi/pc-hotplug.h      |  3 +++
  include/hw/qdev-core.h            |  1 +
  trace-events                      |  3 +++
  8 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 68daa14..3df3620 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -19,7 +19,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
access):
                1: Device insert event, used to distinguish device for which
                   no device check event to OSPM was issued.
                   It's valid only when bit 1 is set.
-              2-7: reserved and should be ignored by OSPM
+              2: Device remove event, used to distinguish device for which
+                 no device eject request to OSPM was issued.
+              3-7: reserved and should be ignored by OSPM
        [0x15-0x17] reserved
write access:
@@ -38,7 +40,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
access):
                1: if set to 1 clears device insert event, set by OSPM
                   after it has emitted device check event for the
                   selected memory device
-              2-7: reserved, OSPM must clear them before writing to register
+              2: if set to 1 clears device remove event, set by OSPM
+                 after it has emitted device eject request for the
+                 selected memory device
+              3: if set to 1 initiates device eject, set by OSPM when it
+                 triggers memory device removal and calls _EJ0 method
+              4-7: reserved, OSPM must clear them before writing to register
Selecting memory device slot beyond present range has no effect on platform:
     - write accesses to memory hot-plug registers not documented above are
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 07e281f..35bbfeb 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -2,6 +2,7 @@
  #include "hw/acpi/pc-hotplug.h"
  #include "hw/mem/pc-dimm.h"
  #include "hw/boards.h"
+#include "hw/qdev-core.h"
  #include "trace.h"
  #include "qapi-event.h"
@@ -91,6 +92,8 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
      MemHotplugState *mem_st = opaque;
      MemStatus *mdev;
      ACPIOSTInfo *info;
+    DeviceState *dev = NULL;
+    HotplugHandler *hotplug_ctrl = NULL;
if (!mem_st->dev_count) {
          return;
@@ -128,13 +131,29 @@ static void acpi_memory_hotplug_write(void *opaque, 
hwaddr addr, uint64_t data,
          qapi_event_send_acpi_device_ost(info, &error_abort);
          qapi_free_ACPIOSTInfo(info);
          break;
-    case 0x14:
+    case 0x14: /* set is_* fields  */
          mdev = &mem_st->devs[mem_st->selector];
          if (data & 2) { /* clear insert event */
              mdev->is_inserting  = false;
              trace_mhp_acpi_clear_insert_evt(mem_st->selector);
+        } else if (data & 4) {
+            mdev->is_removing = false;
+            trace_mhp_acpi_clear_remove_evt(mem_st->selector);
+        } else if (data & 8) {
+            if (!mdev->is_enabled) {
+                trace_mhp_acpi_ejecting_invalid_slot(mem_st->selector);
+                break;
+            }
+
+            dev = DEVICE(mdev->dimm);
+            hotplug_ctrl = qdev_get_hotplug_handler(dev);
+            /* call pc-dimm unplug cb */
+            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
+            trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
          }
          break;
+    default:
+        break;
      }
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6e6a65d..b0f0f84 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -273,7 +273,7 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int 
alias_id,
      dev->alias_required_for_version = required_for_version;
  }
-static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
  {
      HotplugHandler *hotplug_ctrl = NULL;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index eb66fef..96c1db3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -931,6 +931,12 @@ build_ssdt(GArray *table_data, GArray *linker,
          aml_append(field,
              /*(read) 1 if has a insert event. (write) 1 to clear event */
              aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
+        aml_append(field,
+            /* (read) 1 if has a remove event. (write) 1 to clear event */
+            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
+        aml_append(field,
+            /* initiates device eject, write only */
+            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
          aml_append(scope, field);
field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
@@ -975,6 +981,12 @@ build_ssdt(GArray *table_data, GArray *linker,
              )));
              aml_append(dev, method);
+ method = aml_method("_EJ0", 1);
+            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
+            aml_append(method, aml_return(aml_call2(
+                       s, aml_name("_UID"), aml_arg(0))));
+            aml_append(dev, method);
+
              aml_append(sb_scope, dev);
          }
diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
index 1e9ec39..c2bb6a1 100644
--- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
@@ -29,6 +29,8 @@
              External(MEMORY_SLOT_PROXIMITY, FieldUnitObj) // read only
              External(MEMORY_SLOT_ENABLED, FieldUnitObj) // 1 if enabled, read 
only
              External(MEMORY_SLOT_INSERT_EVENT, FieldUnitObj) // (read) 1 if 
has a insert event. (write) 1 to clear event
+            External(MEMORY_SLOT_REMOVE_EVENT, FieldUnitObj) // (read) 1 if 
has a remove event. (write) 1 to clear event
+            External(MEMORY_SLOT_EJECT, FieldUnitObj) // initiates device 
eject, write only
              External(MEMORY_SLOT_SLECTOR, FieldUnitObj) // DIMM selector, 
write only
              External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event code, 
write only
              External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST status 
code, write only
@@ -55,8 +57,10 @@
                      If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // Memory 
device needs check
                          MEMORY_SLOT_NOTIFY_METHOD(Local0, 1)
                          Store(1, MEMORY_SLOT_INSERT_EVENT)
+                    } Elseif (LEqual(MEMORY_SLOT_REMOVE_EVENT, One)) { // 
Ejection request
+                        MEMORY_SLOT_NOTIFY_METHOD(Local0, 3)
+                        Store(1, MEMORY_SLOT_REMOVE_EVENT)
                      }
-                    // TODO: handle memory eject request
                      Add(Local0, One, Local0) // goto next DIMM
                  }
                  Release(MEMORY_SLOT_LOCK)
@@ -156,5 +160,12 @@
                  Store(Arg2, MEMORY_SLOT_OST_STATUS)
                  Release(MEMORY_SLOT_LOCK)
              }
+
+            Method(MEMORY_SLOT_EJECT_METHOD, 2) {
+                Acquire(MEMORY_SLOT_LOCK, 0xFFFF)
+                Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select DIMM
+                Store(1, MEMORY_SLOT_EJECT)
+                Release(MEMORY_SLOT_LOCK)
+            }
          } // Device()
      } // Scope()
diff --git a/include/hw/acpi/pc-hotplug.h b/include/hw/acpi/pc-hotplug.h
index efa6ed7..0513c1c 100644
--- a/include/hw/acpi/pc-hotplug.h
+++ b/include/hw/acpi/pc-hotplug.h
@@ -43,6 +43,8 @@
  #define MEMORY_SLOT_PROXIMITY        MPX
  #define MEMORY_SLOT_ENABLED          MES
  #define MEMORY_SLOT_INSERT_EVENT     MINS
+#define MEMORY_SLOT_REMOVE_EVENT     MRMV
+#define MEMORY_SLOT_EJECT            MEJ
  #define MEMORY_SLOT_SLECTOR          MSEL
  #define MEMORY_SLOT_OST_EVENT        MOEV
  #define MEMORY_SLOT_OST_STATUS       MOSC
@@ -51,6 +53,7 @@
  #define MEMORY_SLOT_CRS_METHOD       MCRS
  #define MEMORY_SLOT_OST_METHOD       MOST
  #define MEMORY_SLOT_PROXIMITY_METHOD MPXM
+#define MEMORY_SLOT_EJECT_METHOD     MEJ0
  #define MEMORY_SLOT_NOTIFY_METHOD    MTFY
  #define MEMORY_SLOT_SCAN_METHOD      MSCN
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 4e673f9..5b7acf1 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -266,6 +266,7 @@ int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
  void qdev_init_nofail(DeviceState *dev);
  void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                   int required_for_version);
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
  void qdev_unplug(DeviceState *dev, Error **errp);
  void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
                                    DeviceState *dev, Error **errp);
diff --git a/trace-events b/trace-events
index 30eba92..46f6ef0 100644
--- a/trace-events
+++ b/trace-events
@@ -1562,6 +1562,7 @@ vfio_put_base_device(int fd) "close vdev->fd=%d"
#hw/acpi/memory_hotplug.c
  mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
+mhp_acpi_ejecting_invalid_slot(uint32_t slot) "0x%"PRIx32
  mhp_acpi_read_addr_lo(uint32_t slot, uint32_t addr) "slot[0x%"PRIx32"] addr lo: 
0x%"PRIx32
  mhp_acpi_read_addr_hi(uint32_t slot, uint32_t addr) "slot[0x%"PRIx32"] addr hi: 
0x%"PRIx32
  mhp_acpi_read_size_lo(uint32_t slot, uint32_t size) "slot[0x%"PRIx32"] size lo: 
0x%"PRIx32
@@ -1572,6 +1573,8 @@ mhp_acpi_write_slot(uint32_t slot) "set active slot: 
0x%"PRIx32
  mhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "slot[0x%"PRIx32"] OST EVENT: 
0x%"PRIx32
  mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST 
STATUS: 0x%"PRIx32
  mhp_acpi_clear_insert_evt(uint32_t slot) "slot[0x%"PRIx32"] clear insert 
event"
+mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove event"
+mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
# hw/i386/pc.c
  mhp_pc_dimm_assigned_slot(int slot) "0x%d"
--
1.9.3
On Mon, Apr 27, 2015 at 04:47:22PM +0800, Zhu Guihua wrote:
When memory hot unplug fails, this patch adds support to send
QMP event to notify mgmt about this failure.

Reviewed-by: Igor Mammedov <address@hidden>
Signed-off-by: Zhu Guihua <address@hidden>
---
  docs/qmp/qmp-events.txt  | 17 +++++++++++++++++
  hw/acpi/memory_hotplug.c | 10 +++++++++-
  qapi/event.json          | 14 ++++++++++++++
  trace-events             |  1 +
  4 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d759d19..3be468f 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -226,6 +226,23 @@ Example:
  { "event": "GUEST_PANICKED",
       "data": { "action": "pause" } }
+MEM_HOT_UNPLUG_ERROR
+--------------------
+Emitted when memory hot unplug error occurs.
+
+Data:
+
+- "device": device name (json-string)
+- "msg": Informative message (e.g., reason for the error) (json-string)
+
+Example:
+
+{ "event": "MEM_HOT_UNPLUG_ERROR"
+  "data": { "device": "dimm1",
+            "msg": "acpi: device unplug for unsupported device"
+  },
+  "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
+
  NIC_RX_FILTER_CHANGED
  ---------------------
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 35bbfeb..34cef1e 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -94,6 +94,7 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr 
addr, uint64_t data,
      ACPIOSTInfo *info;
      DeviceState *dev = NULL;
      HotplugHandler *hotplug_ctrl = NULL;
+    Error *local_err = NULL;
if (!mem_st->dev_count) {
          return;
@@ -148,7 +149,14 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr 
addr, uint64_t data,
              dev = DEVICE(mdev->dimm);
              hotplug_ctrl = qdev_get_hotplug_handler(dev);
              /* call pc-dimm unplug cb */
-            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
+            hotplug_handler_unplug(hotplug_ctrl, dev, &local_err);
+            if (local_err) {
+                trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
+                qapi_event_send_mem_unplug_error(dev->id,
+                                                 error_get_pretty(local_err),
+                                                 &error_abort);
+                break;
+            }
              trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
          }
          break;
diff --git a/qapi/event.json b/qapi/event.json
index c51dc49..378dda5 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -330,3 +330,17 @@
  ##
  { 'event': 'VSERPORT_CHANGE',
    'data': { 'id': 'str', 'open': 'bool' } }
+
+##
+# @MEM_UNPLUG_ERROR
+#
+# Emitted when memory hot unplug error occurs.
+#
+# @device: device name
+#
+# @msg: Informative message
+#
+# Since: 2.4
+##
+{ 'event': 'MEM_UNPLUG_ERROR',
+  'data': { 'device': 'str', 'msg': 'str' } }
diff --git a/trace-events b/trace-events
index 46f6ef0..11387c3 100644
--- a/trace-events
+++ b/trace-events
@@ -1575,6 +1575,7 @@ mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) 
"slot[0x%"PRIx32"] OST STA
  mhp_acpi_clear_insert_evt(uint32_t slot) "slot[0x%"PRIx32"] clear insert 
event"
  mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove 
event"
  mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
+mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm delete 
failed"
# hw/i386/pc.c
  mhp_pc_dimm_assigned_slot(int slot) "0x%d"
--
1.9.3

.





reply via email to

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