[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support
|
From: |
Michael S. Tsirkin |
|
Subject: |
Re: [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support |
|
Date: |
Thu, 3 Nov 2016 06:51:28 +0200 |
On Thu, Nov 03, 2016 at 12:25:01PM +0800, Xiao Guangrong wrote:
>
>
> On 11/03/2016 12:14 PM, Michael S. Tsirkin wrote:
> > On Thu, Nov 03, 2016 at 11:51:27AM +0800, Xiao Guangrong wrote:
> > > Resend these 3 patches to catch up release window...
> > >
> > > Igor,
> > >
> > > this is a open that i did not pass a buffer as parameter to RFIT as
> > > tried the way you suggested, but failed. May be i am not very good at
> > > ASL, i need more time to try. So let's keep the way as it is, i will
> > > improve it later.
> > >
> > > Thanks!
> >
> > And just for comparison, could you pls generate the
> > incremental diff as well?
>
> Hi Michael,
>
> Okay. This is the diff comparing with v3 and v4:
So I don't see how all this requires revert and reapply
and IMHO review of incremental patches would be easier
but if it makes Igor happier, so be it.
I'll wait until we have acks though.
> diff --git a/default-configs/mips-softmmu-common.mak
> b/default-configs/mips-softmmu-common.mak
> index 0394514..f0676f5 100644
> --- a/default-configs/mips-softmmu-common.mak
> +++ b/default-configs/mips-softmmu-common.mak
> @@ -17,6 +17,7 @@ CONFIG_FDC=y
> CONFIG_ACPI=y
> CONFIG_ACPI_X86=y
> CONFIG_ACPI_MEMORY_HOTPLUG=y
> +CONFIG_ACPI_NVDIMM=y
> CONFIG_ACPI_CPU_HOTPLUG=y
> CONFIG_APM=y
> CONFIG_I8257=y
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index cb26dd2..3df3620 100644
> --- a/docs/specs/acpi_mem_hotplug.txt
> +++ b/docs/specs/acpi_mem_hotplug.txt
> @@ -4,9 +4,6 @@ QEMU<->ACPI BIOS memory hotplug interface
> ACPI BIOS GPE.3 handler is dedicated for notifying OS about memory hot-add
> and hot-remove events.
>
> -ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device
> -hot-add and hot-remove events.
> -
> Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
> ---------------------------------------------------------------
> 0xa00:
> diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
> index 4aa5e3d..7887e57 100644
> --- a/docs/specs/acpi_nvdimm.txt
> +++ b/docs/specs/acpi_nvdimm.txt
> @@ -65,8 +65,8 @@ _FIT(Firmware Interface Table)
> The detailed definition of the structure can be found at ACPI 6.0: 5.2.25
> NVDIMM Firmware Interface Table (NFIT).
>
> -QEMU NVDIMM Implemention
> -========================
> +QEMU NVDIMM Implementation
> +==========================
> QEMU uses 4 bytes IO Port starting from 0x0a18 and a RAM-based memory page
> for NVDIMM ACPI.
>
> @@ -82,6 +82,16 @@ Memory:
> ACPI writes _DSM Input Data (based on the offset in the page):
> [0x0 - 0x3]: 4 bytes, NVDIMM Device Handle, 0 is reserved for NVDIMM
> Root device.
> +
> + The handle is completely QEMU internal thing, the values in
> + range [0, 0xFFFF] indicate nvdimm device (O means nvdimm
> + root device named NVDR), other values are reserved by other
> + purpose.
> +
> + Current reserved handle:
> + 0x10000 is reserved for QEMU internal DSM function called on
> + the root device.
> +
> [0x4 - 0x7]: 4 bytes, Revision ID, that is the Arg1 of _DSM method.
> [0x8 - 0xB]: 4 bytes. Function Index, that is the Arg2 of _DSM method.
> [0xC - 0xFFF]: 4084 bytes, the Arg3 of _DSM method.
> @@ -127,28 +137,22 @@ _DSM process diagram:
> | result from the page | | |
> +--------------------------+ +--------------+
>
> -Device Handle Reservation
> --------------------------
> -As we mentioned above, byte 0 ~ byte 3 in the DSM memory save NVDIMM device
> -handle. The handle is completely QEMU internal thing, the values in range
> -[0, 0xFFFF] indicate nvdimm device (O means nvdimm root device named NVDR),
> -other values are reserved by other purpose.
> -
> -Current reserved handle:
> -0x10000 is reserved for QEMU internal DSM function called on the root
> -device.
> +NVDIMM hotplug
> +--------------
> +ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device
> +hot-add and hot-remove events.
>
> QEMU internal use only _DSM function
> ------------------------------------
> -UUID, 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62, is reserved for QEMU internal
> -DSM function.
> -
> There is the function introduced by QEMU and only used by QEMU internal.
>
> 1) Read FIT
> - As we only reserved one page for NVDIMM ACPI it is impossible to map the
> - whole FIT data to guest's address space. This function is used by _FIT
> - method to read a piece of FIT data from QEMU.
> + UUID 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62 is reserved for Read_FIT DSM
> + function (private QEMU function)
> +
> + _FIT method uses Read_FIT function to fetch NFIT structures blob from
> + QEMU in 1 page sized increments which are then concatenated and returned
> + as _FIT method result.
>
> Input parameters:
> Arg0 – UUID {set to 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62}
> @@ -156,27 +160,29 @@ There is the function introduced by QEMU and only used
> by QEMU internal.
> Arg2 - Function Index, 0x1
> Arg3 - A package containing a buffer whose layout is as follows:
>
> -
> +----------+-------------+-------------+-----------------------------------+
> - | Filed | Byte Length | Byte Offset | Description
> |
> -
> +----------+-------------+-------------+-----------------------------------+
> - | offset | 4 | 0 | the offset of FIT buffer
> |
> -
> +----------+-------------+-------------+-----------------------------------+
> + +----------+--------+--------+-------------------------------------------+
> + | Field | Length | Offset | Description |
> + +----------+--------+--------+-------------------------------------------+
> + | offset | 4 | 0 | offset in QEMU's NFIT structures blob to |
> + | | | | read from |
> + +----------+--------+--------+-------------------------------------------+
>
> Output:
> -
> +----------+-------------+-------------+-----------------------------------+
> - | Filed | Byte Length | Byte Offset | Description
> |
> -
> +----------+-------------+-------------+-----------------------------------+
> - | | | | return status codes
> |
> - | | | | 0x100 indicates fit has been
> |
> - | status | 4 | 0 | updated
> |
> - | | | | other follows Chapter 3 in DSM
> |
> - | | | | Spec Rev1
> |
> -
> +----------+-------------+-------------+-----------------------------------+
> - | fit data | Varies | 4 | FIT data
> |
> - | | | |
> |
> -
> +----------+-------------+-------------+-----------------------------------+
> -
> - The FIT offset is maintained by the caller itself, current offset plugs
> + +----------+--------+--------+-------------------------------------------+
> + | Field | Length | Offset | Description |
> + +----------+--------+--------+-------------------------------------------+
> + | | | | return status codes |
> + | | | | 0x100 - error caused by NFIT update while |
> + | status | 4 | 0 | read by _FIT wasn't completed, other |
> + | | | | codes follow Chapter 3 in DSM Spec Rev1 |
> + +----------+--------+--------+-------------------------------------------+
> + | length | 4 | 4 | The fit size |
> + +----------+-----------------+-------------------------------------------+
> + | fit data | Varies | 8 | FIT data, its size is indicated by length |
> + | | | | filed above |
> + +----------+--------+--------+-------------------------------------------+
> +
> + The FIT offset is maintained by the OSPM itself, current offset plus
> the length returned by the function is the next offset we should read.
> When all the FIT data has been read out, zero length is returned.
>
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index e5a3c18..830c475 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -490,8 +490,12 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev,
> DeviceState *dev,
>
> if (lpc->pm.acpi_memory_hotplug.is_enabled &&
> object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> - acpi_memory_plug_cb(hotplug_dev, &lpc->pm.acpi_memory_hotplug,
> - dev, errp);
> + if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
> + nvdimm_acpi_plug_cb(hotplug_dev, dev);
> + } else {
> + acpi_memory_plug_cb(hotplug_dev, &lpc->pm.acpi_memory_hotplug,
> + dev, errp);
> + }
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> if (lpc->pm.cpu_hotplug_legacy) {
> legacy_acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.gpe_cpu, dev,
> errp);
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 70f6451..ec4e64b 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -2,7 +2,6 @@
> #include "hw/acpi/memory_hotplug.h"
> #include "hw/acpi/pc-hotplug.h"
> #include "hw/mem/pc-dimm.h"
> -#include "hw/mem/nvdimm.h"
> #include "hw/boards.h"
> #include "hw/qdev-core.h"
> #include "trace.h"
> @@ -233,8 +232,11 @@ void acpi_memory_plug_cb(HotplugHandler *hotplug_dev,
> MemHotplugState *mem_st,
> DeviceState *dev, Error **errp)
> {
> MemStatus *mdev;
> - AcpiEventStatusBits event;
> - bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
> + DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +
> + if (!dc->hotpluggable) {
> + return;
> + }
>
> mdev = acpi_memory_slot_status(mem_st, dev, errp);
> if (!mdev) {
> @@ -242,23 +244,10 @@ void acpi_memory_plug_cb(HotplugHandler *hotplug_dev,
> MemHotplugState *mem_st,
> }
>
> mdev->dimm = dev;
> -
> - /*
> - * do not set is_enabled and is_inserting if the slot is plugged with
> - * a nvdimm device to stop OSPM inquires memory region from the slot.
> - */
> - if (is_nvdimm) {
> - event = ACPI_NVDIMM_HOTPLUG_STATUS;
> - } else {
> - mdev->is_enabled = true;
> - event = ACPI_MEMORY_HOTPLUG_STATUS;
> - }
> -
> + mdev->is_enabled = true;
> if (dev->hotplugged) {
> - if (!is_nvdimm) {
> - mdev->is_inserting = true;
> - }
> - acpi_send_event(DEVICE(hotplug_dev), event);
> + mdev->is_inserting = true;
> + acpi_send_event(DEVICE(hotplug_dev), ACPI_MEMORY_HOTPLUG_STATUS);
> }
> }
>
> @@ -273,8 +262,6 @@ void acpi_memory_unplug_request_cb(HotplugHandler
> *hotplug_dev,
> return;
> }
>
> - /* nvdimm device hot unplug is not supported yet. */
> - assert(!object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM));
> mdev->is_removing = true;
> acpi_send_event(DEVICE(hotplug_dev), ACPI_MEMORY_HOTPLUG_STATUS);
> }
> @@ -289,8 +276,6 @@ void acpi_memory_unplug_cb(MemHotplugState *mem_st,
> return;
> }
>
> - /* nvdimm device hot unplug is not supported yet. */
> - assert(!object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM));
> mdev->is_enabled = false;
> mdev->dimm = NULL;
> }
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index fc1a012..b8548cc 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -38,11 +38,7 @@ static int nvdimm_plugged_device_list(Object *obj, void
> *opaque)
> GSList **list = opaque;
>
> if (object_dynamic_cast(obj, TYPE_NVDIMM)) {
> - DeviceState *dev = DEVICE(obj);
> -
> - if (dev->realized) { /* only realized NVDIMMs matter */
> - *list = g_slist_append(*list, DEVICE(obj));
> - }
> + *list = g_slist_append(*list, DEVICE(obj));
> }
>
> object_child_foreach(obj, nvdimm_plugged_device_list, opaque);
> @@ -375,17 +371,14 @@ static GArray *nvdimm_build_device_structure(void)
>
> static void nvdimm_init_fit_buffer(NvdimmFitBuffer *fit_buf)
> {
> - qemu_mutex_init(&fit_buf->lock);
> fit_buf->fit = g_array_new(false, true /* clear */, 1);
> }
>
> static void nvdimm_build_fit_buffer(NvdimmFitBuffer *fit_buf)
> {
> - qemu_mutex_lock(&fit_buf->lock);
> g_array_free(fit_buf->fit, true);
> fit_buf->fit = nvdimm_build_device_structure();
> fit_buf->dirty = true;
> - qemu_mutex_unlock(&fit_buf->lock);
> }
>
> void nvdimm_acpi_hotplug(AcpiNVDIMMState *state)
> @@ -399,11 +392,9 @@ static void nvdimm_build_nfit(AcpiNVDIMMState *state,
> GArray *table_offsets,
> NvdimmFitBuffer *fit_buf = &state->fit_buf;
> unsigned int header;
>
> - qemu_mutex_lock(&fit_buf->lock);
> -
> /* NVDIMM device is not plugged? */
> if (!fit_buf->fit->len) {
> - goto exit;
> + return;
> }
>
> acpi_add_table(table_offsets, table_data);
> @@ -417,9 +408,6 @@ static void nvdimm_build_nfit(AcpiNVDIMMState *state,
> GArray *table_offsets,
> build_header(linker, table_data,
> (void *)(table_data->data + header), "NFIT",
> sizeof(NvdimmNfitHeader) + fit_buf->fit->len, 1, NULL,
> NULL);
> -
> -exit:
> - qemu_mutex_unlock(&fit_buf->lock);
> }
>
> struct NvdimmDsmIn {
> @@ -497,7 +485,7 @@ QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncSetLabelDataIn) +
> offsetof(NvdimmDsmIn, arg3) > 4096);
>
> struct NvdimmFuncReadFITIn {
> - uint32_t offset; /* the offset of FIT buffer. */
> + uint32_t offset; /* the offset into FIT buffer. */
> } QEMU_PACKED;
> typedef struct NvdimmFuncReadFITIn NvdimmFuncReadFITIn;
> QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncReadFITIn) +
> @@ -507,6 +495,7 @@ struct NvdimmFuncReadFITOut {
> /* the size of buffer filled by QEMU. */
> uint32_t len;
> uint32_t func_ret_status; /* return status code. */
> + uint32_t length; /* the length of fit. */
> uint8_t fit[0]; /* the FIT data. */
> } QEMU_PACKED;
> typedef struct NvdimmFuncReadFITOut NvdimmFuncReadFITOut;
> @@ -532,7 +521,12 @@ nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr
> dsm_mem_addr)
> cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
> }
>
> -#define NVDIMM_QEMU_RSVD_HANDLE_ROOT 0x10000
> +#define NVDIMM_DSM_RET_STATUS_SUCCESS 0 /* Success */
> +#define NVDIMM_DSM_RET_STATUS_UNSUPPORT 1 /* Not Supported */
> +#define NVDIMM_DSM_RET_STATUS_INVALID 3 /* Invalid Input Parameters */
> +#define NVDIMM_DSM_RET_STATUS_FIT_CHANGED 0x100 /* FIT Changed */
> +
> +#define NVDIMM_QEMU_RSVD_HANDLE_ROOT 0x10000
>
> /* Read FIT data, defined in docs/specs/acpi_nvdimm.txt. */
> static void nvdimm_dsm_func_read_fit(AcpiNVDIMMState *state, NvdimmDsmIn *in,
> @@ -542,34 +536,32 @@ static void nvdimm_dsm_func_read_fit(AcpiNVDIMMState
> *state, NvdimmDsmIn *in,
> NvdimmFuncReadFITIn *read_fit;
> NvdimmFuncReadFITOut *read_fit_out;
> GArray *fit;
> - uint32_t read_len = 0, func_ret_status;
> + uint32_t read_len = 0, func_ret_status, offset;
> int size;
>
> read_fit = (NvdimmFuncReadFITIn *)in->arg3;
> - le32_to_cpus(&read_fit->offset);
> + offset = le32_to_cpu(read_fit->offset);
>
> - qemu_mutex_lock(&fit_buf->lock);
> fit = fit_buf->fit;
>
> nvdimm_debug("Read FIT: offset %#x FIT size %#x Dirty %s.\n",
> - read_fit->offset, fit->len, fit_buf->dirty ? "Yes" : "No");
> -
> - if (read_fit->offset > fit->len) {
> - func_ret_status = 3 /* Invalid Input Parameters */;
> - goto exit;
> - }
> + offset, fit->len, fit_buf->dirty ? "Yes" : "No");
>
> /* It is the first time to read FIT. */
> - if (!read_fit->offset) {
> + if (!offset) {
> fit_buf->dirty = false;
> } else if (fit_buf->dirty) { /* FIT has been changed during RFIT. */
> - func_ret_status = 0x100 /* fit changed */;
> + func_ret_status = NVDIMM_DSM_RET_STATUS_FIT_CHANGED;
> goto exit;
> }
>
> - func_ret_status = 0 /* Success */;
> - read_len = MIN(fit->len - read_fit->offset,
> - 4096 - sizeof(NvdimmFuncReadFITOut));
> + if (offset > fit->len) {
> + func_ret_status = NVDIMM_DSM_RET_STATUS_INVALID;
> + goto exit;
> + }
> +
> + func_ret_status = NVDIMM_DSM_RET_STATUS_SUCCESS;
> + read_len = MIN(fit->len - offset, 4096 - sizeof(NvdimmFuncReadFITOut));
>
> exit:
> size = sizeof(NvdimmFuncReadFITOut) + read_len;
> @@ -577,12 +569,12 @@ exit:
>
> read_fit_out->len = cpu_to_le32(size);
> read_fit_out->func_ret_status = cpu_to_le32(func_ret_status);
> - memcpy(read_fit_out->fit, fit->data + read_fit->offset, read_len);
> + read_fit_out->length = cpu_to_le32(read_len);
> + memcpy(read_fit_out->fit, fit->data + offset, read_len);
>
> cpu_physical_memory_write(dsm_mem_addr, read_fit_out, size);
>
> g_free(read_fit_out);
> - qemu_mutex_unlock(&fit_buf->lock);
> }
>
> static void nvdimm_dsm_reserved_root(AcpiNVDIMMState *state, NvdimmDsmIn *in,
> @@ -592,12 +584,12 @@ static void nvdimm_dsm_reserved_root(AcpiNVDIMMState
> *state, NvdimmDsmIn *in,
> case 0x0:
> nvdimm_dsm_function0(0x1 | 1 << 1 /* Read FIT */, dsm_mem_addr);
> return;
> - case 0x1 /*Read FIT */:
> + case 0x1 /* Read FIT */:
> nvdimm_dsm_func_read_fit(state, in, dsm_mem_addr);
> return;
> }
>
> - nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
> + nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_UNSUPPORT, dsm_mem_addr);
> }
>
> static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
> @@ -659,7 +651,7 @@ static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm,
> hwaddr dsm_mem_addr)
>
> nvdimm_debug("label_size %#x, max_xfer %#x.\n", label_size, mxfer);
>
> - label_size_out.func_ret_status = cpu_to_le32(0 /* Success */);
> + label_size_out.func_ret_status =
> cpu_to_le32(NVDIMM_DSM_RET_STATUS_SUCCESS);
> label_size_out.label_size = cpu_to_le32(label_size);
> label_size_out.max_xfer = cpu_to_le32(mxfer);
>
> @@ -670,7 +662,7 @@ static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm,
> hwaddr dsm_mem_addr)
> static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm,
> uint32_t offset, uint32_t length)
> {
> - uint32_t ret = 3 /* Invalid Input Parameters */;
> + uint32_t ret = NVDIMM_DSM_RET_STATUS_INVALID;
>
> if (offset + length < offset) {
> nvdimm_debug("offset %#x + length %#x is overflow.\n", offset,
> @@ -690,7 +682,7 @@ static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice
> *nvdimm,
> return ret;
> }
>
> - return 0 /* Success */;
> + return NVDIMM_DSM_RET_STATUS_SUCCESS;
> }
>
> /*
> @@ -714,7 +706,7 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice
> *nvdimm, NvdimmDsmIn *in,
>
> status = nvdimm_rw_label_data_check(nvdimm, get_label_data->offset,
> get_label_data->length);
> - if (status != 0 /* Success */) {
> + if (status != NVDIMM_DSM_RET_STATUS_SUCCESS) {
> nvdimm_dsm_no_payload(status, dsm_mem_addr);
> return;
> }
> @@ -724,7 +716,8 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice
> *nvdimm, NvdimmDsmIn *in,
> get_label_data_out = g_malloc(size);
>
> get_label_data_out->len = cpu_to_le32(size);
> - get_label_data_out->func_ret_status = cpu_to_le32(0 /* Success */);
> + get_label_data_out->func_ret_status =
> + cpu_to_le32(NVDIMM_DSM_RET_STATUS_SUCCESS);
> nvc->read_label_data(nvdimm, get_label_data_out->out_buf,
> get_label_data->length, get_label_data->offset);
>
> @@ -752,7 +745,7 @@ static void nvdimm_dsm_set_label_data(NVDIMMDevice
> *nvdimm, NvdimmDsmIn *in,
>
> status = nvdimm_rw_label_data_check(nvdimm, set_label_data->offset,
> set_label_data->length);
> - if (status != 0 /* Success */) {
> + if (status != NVDIMM_DSM_RET_STATUS_SUCCESS) {
> nvdimm_dsm_no_payload(status, dsm_mem_addr);
> return;
> }
> @@ -762,7 +755,7 @@ static void nvdimm_dsm_set_label_data(NVDIMMDevice
> *nvdimm, NvdimmDsmIn *in,
>
> nvc->write_label_data(nvdimm, set_label_data->in_buf,
> set_label_data->length, set_label_data->offset);
> - nvdimm_dsm_no_payload(0 /* Success */, dsm_mem_addr);
> + nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_SUCCESS, dsm_mem_addr);
> }
>
> static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
> @@ -813,7 +806,7 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr
> dsm_mem_addr)
> break;
> }
>
> - nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
> + nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_UNSUPPORT, dsm_mem_addr);
> }
>
> static uint64_t
> @@ -850,7 +843,7 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val,
> unsigned size)
> if (in->revision != 0x1 /* Currently we only support DSM Spec Rev1. */) {
> nvdimm_debug("Revision %#x is not supported, expect %#x.\n",
> in->revision, 0x1);
> - nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
> + nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_UNSUPPORT, dsm_mem_addr);
> goto exit;
> }
>
> @@ -881,6 +874,13 @@ static const MemoryRegionOps nvdimm_dsm_ops = {
> },
> };
>
> +void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev)
> +{
> + if (dev->hotplugged) {
> + acpi_send_event(DEVICE(hotplug_dev), ACPI_NVDIMM_HOTPLUG_STATUS);
> + }
> +}
> +
> void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
> FWCfgState *fw_cfg, Object *owner)
> {
> @@ -1031,7 +1031,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
> aml_append(unsupport, ifctx);
>
> /* No function is supported yet. */
> - byte_list[0] = 1 /* Not Supported */;
> + byte_list[0] = NVDIMM_DSM_RET_STATUS_UNSUPPORT;
> aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
> aml_append(method, unsupport);
>
> @@ -1094,7 +1094,7 @@ static void nvdimm_build_device_dsm(Aml *dev, uint32_t
> handle)
> aml_append(dev, method);
> }
>
> -static void nvdimm_build_fit(Aml *dev)
> +static void nvdimm_build_fit_method(Aml *dev)
> {
> Aml *method, *pkg, *buf, *buf_size, *offset, *call_result;
> Aml *whilectx, *ifcond, *ifctx, *elsectx, *fit;
> @@ -1103,13 +1103,11 @@ static void nvdimm_build_fit(Aml *dev)
> buf_size = aml_local(1);
> fit = aml_local(2);
>
> - aml_append(dev, aml_create_dword_field(aml_buffer(4, NULL),
> - aml_int(0), NVDIMM_DSM_RFIT_STATUS));
> + aml_append(dev, aml_name_decl(NVDIMM_DSM_RFIT_STATUS, aml_int(0)));
>
> /* build helper function, RFIT. */
> method = aml_method("RFIT", 1, AML_SERIALIZED);
> - aml_append(method, aml_create_dword_field(aml_buffer(4, NULL),
> - aml_int(0), "OFST"));
> + aml_append(method, aml_name_decl("OFST", aml_int(0)));
>
> /* prepare input package. */
> pkg = aml_package(1);
> @@ -1137,11 +1135,9 @@ static void nvdimm_build_fit(Aml *dev)
> aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
> aml_append(method, ifctx);
>
> - aml_append(method, aml_store(aml_sizeof(buf), buf_size));
> - aml_append(method, aml_subtract(buf_size,
> - aml_int(4) /* the size of "STAU" */,
> - buf_size));
> -
> + aml_append(method, aml_create_dword_field(buf,
> + aml_int(4) /* offset at byte 4 */, "LENG"));
> + aml_append(method, aml_store(aml_name("LENG"), buf_size));
> /* if we read the end of fit. */
> ifctx = aml_if(aml_equal(buf_size, aml_int(0)));
> aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
> @@ -1150,7 +1146,7 @@ static void nvdimm_build_fit(Aml *dev)
> aml_append(method, aml_store(aml_shiftleft(buf_size, aml_int(3)),
> buf_size));
> aml_append(method, aml_create_field(buf,
> - aml_int(4 * BITS_PER_BYTE), /* offset at byte
> 4.*/
> + aml_int(8 * BITS_PER_BYTE), /* offset at byte
> 4.*/
> buf_size, "BUFF"));
> aml_append(method, aml_return(aml_name("BUFF")));
> aml_append(dev, method);
> @@ -1171,7 +1167,7 @@ static void nvdimm_build_fit(Aml *dev)
> * again.
> */
> ifctx = aml_if(aml_equal(aml_name(NVDIMM_DSM_RFIT_STATUS),
> - aml_int(0x100 /* fit changed */)));
> + aml_int(NVDIMM_DSM_RET_STATUS_FIT_CHANGED)));
> aml_append(ifctx, aml_store(aml_buffer(0, NULL), fit));
> aml_append(ifctx, aml_store(aml_int(0), offset));
> aml_append(whilectx, ifctx);
> @@ -1251,7 +1247,7 @@ static void nvdimm_build_ssdt(GArray *table_offsets,
> GArray *table_data,
>
> /* 0 is reserved for root device. */
> nvdimm_build_device_dsm(dev, 0);
> - nvdimm_build_fit(dev);
> + nvdimm_build_fit_method(dev);
>
> nvdimm_build_nvdimm_devices(dev, ram_slots);
>
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 2adc246..17d36bd 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -378,7 +378,12 @@ static void piix4_device_plug_cb(HotplugHandler
> *hotplug_dev,
>
> if (s->acpi_memory_hotplug.is_enabled &&
> object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> - acpi_memory_plug_cb(hotplug_dev, &s->acpi_memory_hotplug, dev, errp);
> + if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
> + nvdimm_acpi_plug_cb(hotplug_dev, dev);
> + } else {
> + acpi_memory_plug_cb(hotplug_dev, &s->acpi_memory_hotplug,
> + dev, errp);
> + }
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> acpi_pcihp_device_plug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev,
> errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
> index ab34c19..17ac986 100644
> --- a/hw/core/hotplug.c
> +++ b/hw/core/hotplug.c
> @@ -35,17 +35,6 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
> }
> }
>
> -void hotplug_handler_post_plug(HotplugHandler *plug_handler,
> - DeviceState *plugged_dev,
> - Error **errp)
> -{
> - HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
> -
> - if (hdc->post_plug) {
> - hdc->post_plug(plug_handler, plugged_dev, errp);
> - }
> -}
> -
> void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
> DeviceState *plugged_dev,
> Error **errp)
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index d835e62..5783442 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -945,21 +945,10 @@ static void device_set_realized(Object *obj, bool
> value, Error **errp)
> goto child_realize_fail;
> }
> }
> -
> if (dev->hotplugged) {
> device_reset(dev);
> }
> dev->pending_deleted_event = false;
> - dev->realized = value;
> -
> - if (hotplug_ctrl) {
> - hotplug_handler_post_plug(hotplug_ctrl, dev, &local_err);
> - }
> -
> - if (local_err != NULL) {
> - dev->realized = value;
> - goto post_realize_fail;
> - }
> } else if (!value && dev->realized) {
> Error **local_errp = NULL;
> QLIST_FOREACH(bus, &dev->child_bus, sibling) {
> @@ -976,14 +965,13 @@ static void device_set_realized(Object *obj, bool
> value, Error **errp)
> }
> dev->pending_deleted_event = true;
> DEVICE_LISTENER_CALL(unrealize, Reverse, dev);
> + }
>
> - if (local_err != NULL) {
> - goto fail;
> - }
> -
> - dev->realized = value;
> + if (local_err != NULL) {
> + goto fail;
> }
>
> + dev->realized = value;
> return;
>
> child_realize_fail:
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 200963f..0403452 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1700,22 +1700,16 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
> goto out;
> }
>
> + if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
> + nvdimm_acpi_hotplug(&pcms->acpi_nvdimm_state);
> + }
> +
> hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
> hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &error_abort);
> out:
> error_propagate(errp, local_err);
> }
>
> -static void pc_dimm_post_plug(HotplugHandler *hotplug_dev,
> - DeviceState *dev, Error **errp)
> -{
> - PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> -
> - if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
> - nvdimm_acpi_hotplug(&pcms->acpi_nvdimm_state);
> - }
> -}
> -
> static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> @@ -1988,14 +1982,6 @@ static void pc_machine_device_plug_cb(HotplugHandler
> *hotplug_dev,
> }
> }
>
> -static void pc_machine_device_post_plug_cb(HotplugHandler *hotplug_dev,
> - DeviceState *dev, Error **errp)
> -{
> - if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> - pc_dimm_post_plug(hotplug_dev, dev, errp);
> - }
> -}
> -
> static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error
> **errp)
> {
> @@ -2330,7 +2316,6 @@ static void pc_machine_class_init(ObjectClass *oc, void
> *data)
> mc->reset = pc_machine_reset;
> hc->pre_plug = pc_machine_device_pre_plug_cb;
> hc->plug = pc_machine_device_plug_cb;
> - hc->post_plug = pc_machine_device_post_plug_cb;
> hc->unplug_request = pc_machine_device_unplug_request_cb;
> hc->unplug = pc_machine_device_unplug_cb;
> nc->nmi_monitor_handler = x86_nmi;
> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> index 10ca5b6..c0db869 100644
> --- a/include/hw/hotplug.h
> +++ b/include/hw/hotplug.h
> @@ -47,7 +47,6 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
> * @parent: Opaque parent interface.
> * @pre_plug: pre plug callback called at start of device.realize(true)
> * @plug: plug callback called at end of device.realize(true).
> - * @post_pug: post plug callback called after device is successfully plugged.
> * @unplug_request: unplug request callback.
> * Used as a means to initiate device unplug for devices
> that
> * require asynchronous unplug handling.
> @@ -62,7 +61,6 @@ typedef struct HotplugHandlerClass {
> /* <public> */
> hotplug_fn pre_plug;
> hotplug_fn plug;
> - hotplug_fn post_plug;
> hotplug_fn unplug_request;
> hotplug_fn unplug;
> } HotplugHandlerClass;
> @@ -85,14 +83,6 @@ void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> DeviceState *plugged_dev,
> Error **errp);
>
> -/**
> - * hotplug_handler_post_plug:
> - *
> - * Call #HotplugHandlerClass.post_plug callback of @plug_handler.
> - */
> -void hotplug_handler_post_plug(HotplugHandler *plug_handler,
> - DeviceState *plugged_dev,
> - Error **errp);
>
> /**
> * hotplug_handler_unplug_request:
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 33cd421..4b90584 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -103,16 +103,11 @@ typedef struct NVDIMMClass NVDIMMClass;
> * devices which is updated after the NVDIMM device is plugged or
> * unplugged.
> *
> - * Rules to use the buffer:
> - * 1) the user should hold the @lock to access the buffer.
> - * 2) mark @dirty whenever the buffer is updated.
> - *
> - * These rules preserve NVDIMM ACPI _FIT method to read incomplete
> - * or obsolete fit info if fit update happens during multiple RFIT
> - * calls.
> + * Mark @dirty whenever the buffer is updated so that it preserves NVDIMM
> + * ACPI _FIT method to read incomplete or obsolete fit info if fit update
> + * happens during multiple RFIT calls.
> */
> struct NvdimmFitBuffer {
> - QemuMutex lock;
> GArray *fit;
> bool dirty;
> };
> @@ -138,4 +133,5 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray
> *table_data,
> BIOSLinker *linker, AcpiNVDIMMState *state,
> uint32_t ram_slots);
> void nvdimm_acpi_hotplug(AcpiNVDIMMState *state);
> +void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev);
> #endif
>
- Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT, (continued)
- Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT, Igor Mammedov, 2016/11/03
- Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT, Xiao Guangrong, 2016/11/03
- Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT, Igor Mammedov, 2016/11/03
- Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT, Xiao Guangrong, 2016/11/03
- Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT, Igor Mammedov, 2016/11/03
[Qemu-devel] [PATCH v4 3/3] pc: memhp: enable nvdimm device hotplug, Xiao Guangrong, 2016/11/03
Re: [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support, Michael S. Tsirkin, 2016/11/03