Re: [Qemu-devel] [RESEND PATCH v1 12/13] acpi: Add hardware implementation for memory hot unplug.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:19 +0800
Tang Chen <address@hidden> wrote:

> This patch adds a new bit to memory hotplug IO port indicating that
> ej0 has been evaluated by guest OS. And call pc-dimm unplug cb to do
> the real removal.
> 
> 
> Signed-off-by: Hu Tao <address@hidden>
> Signed-off-by: Tang Chen <address@hidden>
> ---
>  docs/specs/acpi_mem_hotplug.txt |  8 ++++++--
>  hw/acpi/memory_hotplug.c        | 23 ++++++++++++++++++++---
>  hw/core/qdev.c                  |  2 +-
>  include/hw/qdev-core.h          |  1 +
>  4 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index 1290994..28a1ffa 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 indicate that device is being
> +              removed.
bit has the same semantic as bit 1,
Please update description to match it.

> +              3-7: reserved and should be ignored by OSPM
>        [0x15-0x17] reserved
>  
>    write access:
> @@ -35,7 +37,9 @@ 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: set by hardware after it has emitted device eject event for
> +              selected memory device
same here

> +              3-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 2b0c8ca..b9e8752 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"
>  
> @@ -75,6 +76,7 @@ static uint64_t acpi_memory_hotplug_read(void *opaque, 
> hwaddr addr,
>      case 0x14: /* pack and return is_* fields */
>          val |= mdev->is_enabled   ? 1 : 0;
>          val |= mdev->is_inserting ? 2 : 0;
> +        val |= mdev->is_removing  ? 4 : 0;
>          trace_mhp_acpi_read_flags(mem_st->selector, val);
>          break;
>      default:
> @@ -90,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;
> @@ -121,21 +125,34 @@ static void acpi_memory_hotplug_write(void *opaque, 
> hwaddr addr, uint64_t data,
>          mdev = &mem_st->devs[mem_st->selector];
>          mdev->ost_status = data;
>          trace_mhp_acpi_write_ost_status(mem_st->selector, mdev->ost_status);
> -        /* TODO: implement memory removal on guest signal */
>  
>          info = acpi_memory_device_status(mem_st->selector, mdev);
>          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) { /* request removal of device */
add here a tracing event similar to above one ^^^

> +            /*
> +             * QEmu memory hot unplug is an asynchronized procedure. QEmu 
> first
> +             * calls pc-dimm unplug request cb to send a SCI to guest. When 
> the
> +             * Guest OS finished handling the SCI, it evaluates ACPI ej0, and
> +             * QEmu calls pc-dimm unplug cb to remove memory device.
> +             */
> +            dev = DEVICE(mdev->dimm);
> +            hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +            /* Call pc-dimm unplug cb. */
> +            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
>          }
> +
> +        break;
> +    default:
>          break;
>      }
> -
>  }
>  static const MemoryRegionOps acpi_memory_hotplug_ops = {
>      .read = acpi_memory_hotplug_read,
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 901f289..9f08fe6 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -223,7 +223,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/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 589bbe7..60f549e 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -261,6 +261,7 @@ void qdev_init_nofail(DeviceState *dev);
>  void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>                                   int required_for_version);
>  void qdev_unplug(DeviceState *dev, Error **errp);
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
>  void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                    DeviceState *dev, Error **errp);
>  void qdev_machine_creation_done(void);