[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [QEMU PATCH v2 4/6] nvdimm: Implement ACPI NVDIMM Label Methods
|
From: |
Igor Mammedov |
|
Subject: |
Re: [QEMU PATCH v2 4/6] nvdimm: Implement ACPI NVDIMM Label Methods |
|
Date: |
Thu, 21 Jul 2022 10:58:37 +0200 |
On Fri, 01 Jul 2022 17:23:04 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:
> On Thu, 2022-06-16 at 14:32 +0200, Igor Mammedov wrote:
> > On Mon, 30 May 2022 11:40:45 +0800
> > Robert Hoo <robert.hu@linux.intel.com> wrote:
> >
> > > Recent ACPI spec [1] has defined NVDIMM Label Methods _LS{I,R,W},
> > > which
> > > depricates corresponding _DSM Functions defined by PMEM _DSM
> > > Interface spec
> > > [2].
> > >
> > > In this implementation, we do 2 things
> > > 1. Generalize the QEMU<->ACPI BIOS NVDIMM interface, wrap it with
> > > ACPI
> > > method dispatch, _DSM is one of the branches. This also paves the
> > > way for
> > > adding other ACPI methods for NVDIMM.
> > > 2. Add _LS{I,R,W} method in each NVDIMM device in SSDT.
> > > ASL form of SSDT changes can be found in next test/qtest/bios-
> > > table-test
> > > commit message.
> > >
> > > [1] ACPI Spec v6.4, 6.5.10 NVDIMM Label Methods
> > > https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf
> > > [2] Intel PMEM _DSM Interface Spec v2.0, 3.10 Deprecated Functions
> > > https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf
> > >
> > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> > > Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
> > > ---
> > > hw/acpi/nvdimm.c | 424 +++++++++++++++++++++++++++++++-----
> > > ----
> >
> > This patch is too large and doing to many things to be reviewable.
> > It needs to be split into smaller distinct chunks.
> > (however hold your horses and read on)
> >
> > The patch it is too intrusive and my hunch is that it breaks
> > ABI and needs a bunch of compat knobs to work properly and
> > that I'd like to avoid unless there is not other way around
> > the problem.
>
> Is the ABI here you mentioned the "struct NvdimmMthdIn{}" stuff?
> and the compat knobs refers to related functions' input/output params?
ABI are structures that guest and QEMU pass through information
between each other. And knobs in this case would be compat variable[s]
to keep old behavior in place for old machine types.
> My thoughts is that eventually, sooner or later, more ACPI methods will
> be implemented per request, although now we can play the trick of
> wrapper new methods over the pipe of old _DSM implementation.
> Though this changes a little on existing struct NvdimmDsmIn {}, it
> paves the way for the future; and actually the change is more an
> extension or generalization, not fundamentally changes the framework.
>
> In short, my point is the change/generalization/extension will be
> inevitable, even if not present.
Expanding ABI (interface between host&guest) has 2 drawbacks
* it exposes more attack surface of VMM to hostile guest
and rises chances that vulnerability would slip through
review/testing
* migration wise, QEMU has to support any ABI for years
and not only latest an greatest interface but also old
ones to keep guest started on older QEMU working across
migration, so any ABI change should be considered very
carefully before being implemented otherwise it all
quickly snowballs in unsupportable mess of compat
variables smeared across host/guest.
Reducing exposed ABI and constant need to expand it
was a reason why we have moved ACPI code from firmware
into QEMU, so we could describe hardware without costs
associated with of maintaining ABI.
There might be need to extend ABI eventually, but not in this case.
> > I was skeptical about this approach during v1 review and
> > now I'm pretty much sure it's over-engineered and we can
> > just repack data we receive from existing label _DSM functions
> > to provide _LS{I,R,W} like it was suggested in v1.
> > It will be much simpler and affect only AML side without
> > complicating ABI and without any compat cruft and will work
> > with ping-pong migration without any issues.
>
> Ostensibly it may looks simpler, actually not, I think. The AML "common
> pipe" NCAL() is already complex, it packs all _DSMs and NFIT() function
> logics there, packing new stuff in/through it will be bug-prone.
> Though this time we can avert touching it, as the new ACPI methods
> deprecating old _DSM functionally is almost the same.
> How about next time? are we going to always packing new methods logic
> in NCAL()?
> My point is that we should implement new methods as itself, of course,
> as a general programming rule, we can/should abstract common routines,
> but not packing them in one large function.
> >
> >
> > > include/hw/mem/nvdimm.h | 6 +
> > > 2 files changed, 338 insertions(+), 92 deletions(-)
> > >
> > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> > > index 59b42afcf1..50ee85866b 100644
> > > --- a/hw/acpi/nvdimm.c
> > > +++ b/hw/acpi/nvdimm.c
> > > @@ -416,17 +416,22 @@ static void nvdimm_build_nfit(NVDIMMState
> > > *state, GArray *table_offsets,
> > >
> > > #define NVDIMM_DSM_MEMORY_SIZE 4096
> > >
> > > -struct NvdimmDsmIn {
> > > +struct NvdimmMthdIn {
> > > uint32_t handle;
> > > + uint32_t method;
> > > + uint8_t args[4088];
> > > +} QEMU_PACKED;
> > > +typedef struct NvdimmMthdIn NvdimmMthdIn;
> > > +struct NvdimmDsmIn {
> > > uint32_t revision;
> > > uint32_t function;
> > > /* the remaining size in the page is used by arg3. */
> > > union {
> > > - uint8_t arg3[4084];
> > > + uint8_t arg3[4080];
> > > };
> > > } QEMU_PACKED;
> > > typedef struct NvdimmDsmIn NvdimmDsmIn;
> > > -QEMU_BUILD_BUG_ON(sizeof(NvdimmDsmIn) != NVDIMM_DSM_MEMORY_SIZE);
> > > +QEMU_BUILD_BUG_ON(sizeof(NvdimmMthdIn) != NVDIMM_DSM_MEMORY_SIZE);
> > >
> > > struct NvdimmDsmOut {
> > > /* the size of buffer filled by QEMU. */
> > > @@ -470,7 +475,8 @@ struct NvdimmFuncGetLabelDataIn {
> > > } QEMU_PACKED;
> > > typedef struct NvdimmFuncGetLabelDataIn NvdimmFuncGetLabelDataIn;
> > > QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelDataIn) +
> > > - offsetof(NvdimmDsmIn, arg3) >
> > > NVDIMM_DSM_MEMORY_SIZE);
> > > + offsetof(NvdimmDsmIn, arg3) +
> > > offsetof(NvdimmMthdIn, args) >
> > > + NVDIMM_DSM_MEMORY_SIZE);
> > >
> > > struct NvdimmFuncGetLabelDataOut {
> > > /* the size of buffer filled by QEMU. */
> > > @@ -488,14 +494,16 @@ struct NvdimmFuncSetLabelDataIn {
> > > } QEMU_PACKED;
> > > typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
> > > QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncSetLabelDataIn) +
> > > - offsetof(NvdimmDsmIn, arg3) >
> > > NVDIMM_DSM_MEMORY_SIZE);
> > > + offsetof(NvdimmDsmIn, arg3) +
> > > offsetof(NvdimmMthdIn, args) >
> > > + NVDIMM_DSM_MEMORY_SIZE);
> > >
> > > struct NvdimmFuncReadFITIn {
> > > uint32_t offset; /* the offset into FIT buffer. */
> > > } QEMU_PACKED;
> > > typedef struct NvdimmFuncReadFITIn NvdimmFuncReadFITIn;
> > > QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncReadFITIn) +
> > > - offsetof(NvdimmDsmIn, arg3) >
> > > NVDIMM_DSM_MEMORY_SIZE);
> > > + offsetof(NvdimmDsmIn, arg3) +
> > > offsetof(NvdimmMthdIn, args) >
> > > + NVDIMM_DSM_MEMORY_SIZE);
> > >
> > > struct NvdimmFuncReadFITOut {
> > > /* the size of buffer filled by QEMU. */
> > > @@ -636,7 +644,8 @@ static uint32_t
> > > nvdimm_get_max_xfer_label_size(void)
> > > * the max data ACPI can write one time which is transferred
> > > by
> > > * 'Set Namespace Label Data' function.
> > > */
> > > - max_set_size = dsm_memory_size - offsetof(NvdimmDsmIn, arg3) -
> > > + max_set_size = dsm_memory_size - offsetof(NvdimmMthdIn, args)
> > > -
> > > + offsetof(NvdimmDsmIn, arg3) -
> > > sizeof(NvdimmFuncSetLabelDataIn);
> > >
> > > return MIN(max_get_size, max_set_size);
> > > @@ -697,16 +706,15 @@ static uint32_t
> > > nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm,
> > > /*
> > > * DSM Spec Rev1 4.5 Get Namespace Label Data (Function Index 5).
> > > */
> > > -static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm,
> > > NvdimmDsmIn *in,
> > > - hwaddr dsm_mem_addr)
> > > +static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm,
> > > + NvdimmFuncGetLabelDataIn
> > > *get_label_data,
> > > + hwaddr dsm_mem_addr)
> > > {
> > > NVDIMMClass *nvc = NVDIMM_GET_CLASS(nvdimm);
> > > - NvdimmFuncGetLabelDataIn *get_label_data;
> > > NvdimmFuncGetLabelDataOut *get_label_data_out;
> > > uint32_t status;
> > > int size;
> > >
> > > - get_label_data = (NvdimmFuncGetLabelDataIn *)in->arg3;
> > > get_label_data->offset = le32_to_cpu(get_label_data->offset);
> > > get_label_data->length = le32_to_cpu(get_label_data->length);
> > >
> > > @@ -737,15 +745,13 @@ static void
> > > nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
> > > /*
> > > * DSM Spec Rev1 4.6 Set Namespace Label Data (Function Index 6).
> > > */
> > > -static void nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm,
> > > NvdimmDsmIn *in,
> > > +static void nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm,
> > > + NvdimmFuncSetLabelDataIn
> > > *set_label_data,
> > > hwaddr dsm_mem_addr)
> > > {
> > > NVDIMMClass *nvc = NVDIMM_GET_CLASS(nvdimm);
> > > - NvdimmFuncSetLabelDataIn *set_label_data;
> > > uint32_t status;
> > >
> > > - set_label_data = (NvdimmFuncSetLabelDataIn *)in->arg3;
> > > -
> > > set_label_data->offset = le32_to_cpu(set_label_data->offset);
> > > set_label_data->length = le32_to_cpu(set_label_data->length);
> > >
> > > @@ -760,19 +766,21 @@ static void
> > > nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
> > > }
> > >
> > > assert(offsetof(NvdimmDsmIn, arg3) + sizeof(*set_label_data) +
> > > - set_label_data->length <=
> > > NVDIMM_DSM_MEMORY_SIZE);
> > > + set_label_data->length <= NVDIMM_DSM_MEMORY_SIZE -
> > > + offsetof(NvdimmMthdIn, args));
> > >
> > > nvc->write_label_data(nvdimm, set_label_data->in_buf,
> > > set_label_data->length, set_label_data-
> > > >offset);
> > > nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_SUCCESS,
> > > dsm_mem_addr);
> > > }
> > >
> > > -static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr
> > > dsm_mem_addr)
> > > +static void nvdimm_dsm_device(uint32_t nv_handle, NvdimmDsmIn
> > > *dsm_in,
> > > + hwaddr dsm_mem_addr)
> > > {
> > > - NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(in-
> > > >handle);
> > > + NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(nv_handle);
> > >
> > > /* See the comments in nvdimm_dsm_root(). */
> > > - if (!in->function) {
> > > + if (!dsm_in->function) {
> > > uint32_t supported_func = 0;
> > >
> > > if (nvdimm && nvdimm->label_size) {
> > > @@ -794,7 +802,7 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in,
> > > hwaddr dsm_mem_addr)
> > > }
> > >
> > > /* Encode DSM function according to DSM Spec Rev1. */
> > > - switch (in->function) {
> > > + switch (dsm_in->function) {
> > > case 4 /* Get Namespace Label Size */:
> > > if (nvdimm->label_size) {
> > > nvdimm_dsm_label_size(nvdimm, dsm_mem_addr);
> > > @@ -803,13 +811,17 @@ static void nvdimm_dsm_device(NvdimmDsmIn
> > > *in, hwaddr dsm_mem_addr)
> > > break;
> > > case 5 /* Get Namespace Label Data */:
> > > if (nvdimm->label_size) {
> > > - nvdimm_dsm_get_label_data(nvdimm, in, dsm_mem_addr);
> > > + nvdimm_dsm_get_label_data(nvdimm,
> > > + (NvdimmFuncGetLabelDataIn
> > > *)dsm_in->arg3,
> > > + dsm_mem_addr);
> > > return;
> > > }
> > > break;
> > > case 0x6 /* Set Namespace Label Data */:
> > > if (nvdimm->label_size) {
> > > - nvdimm_dsm_set_label_data(nvdimm, in, dsm_mem_addr);
> > > + nvdimm_dsm_set_label_data(nvdimm,
> > > + (NvdimmFuncSetLabelDataIn *)dsm_in->arg3,
> > > + dsm_mem_addr);
> > > return;
> > > }
> > > break;
> > > @@ -819,67 +831,128 @@ static void nvdimm_dsm_device(NvdimmDsmIn
> > > *in, hwaddr dsm_mem_addr)
> > > }
> > >
> > > static uint64_t
> > > -nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
> > > +nvdimm_method_read(void *opaque, hwaddr addr, unsigned size)
> > > {
> > > - nvdimm_debug("BUG: we never read _DSM IO Port.\n");
> > > + nvdimm_debug("BUG: we never read NVDIMM Method IO Port.\n");
> > > return 0;
> > > }
> > >
> > > static void
> > > -nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned
> > > size)
> > > +nvdimm_dsm_handle(void *opaque, NvdimmMthdIn *method_in, hwaddr
> > > dsm_mem_addr)
> > > {
> > > NVDIMMState *state = opaque;
> > > - NvdimmDsmIn *in;
> > > - hwaddr dsm_mem_addr = val;
> > > + NvdimmDsmIn *dsm_in = (NvdimmDsmIn *)method_in->args;
> > >
> > > nvdimm_debug("dsm memory address 0x%" HWADDR_PRIx ".\n",
> > > dsm_mem_addr);
> > >
> > > - /*
> > > - * The DSM memory is mapped to guest address space so an evil
> > > guest
> > > - * can change its content while we are doing DSM emulation.
> > > Avoid
> > > - * this by copying DSM memory to QEMU local memory.
> > > - */
> > > - in = g_new(NvdimmDsmIn, 1);
> > > - cpu_physical_memory_read(dsm_mem_addr, in, sizeof(*in));
> > > -
> > > - in->revision = le32_to_cpu(in->revision);
> > > - in->function = le32_to_cpu(in->function);
> > > - in->handle = le32_to_cpu(in->handle);
> > > -
> > > - nvdimm_debug("Revision 0x%x Handler 0x%x Function 0x%x.\n",
> > > in->revision,
> > > - in->handle, in->function);
> > > + dsm_in->revision = le32_to_cpu(dsm_in->revision);
> > > + dsm_in->function = le32_to_cpu(dsm_in->function);
> > >
> > > + nvdimm_debug("Revision 0x%x Handler 0x%x Function 0x%x.\n",
> > > + dsm_in->revision, method_in->handle, dsm_in-
> > > >function);
> > > /*
> > > * Current NVDIMM _DSM Spec supports Rev1 and Rev2
> > > * Intel® OptanePersistent Memory Module DSM Interface,
> > > Revision 2.0
> > > */
> > > - if (in->revision != 0x1 && in->revision != 0x2) {
> > > + if (dsm_in->revision != 0x1 && dsm_in->revision != 0x2) {
> > > nvdimm_debug("Revision 0x%x is not supported, expect 0x1
> > > or 0x2.\n",
> > > - in->revision);
> > > + dsm_in->revision);
> > > nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_UNSUPPORT,
> > > dsm_mem_addr);
> > > - goto exit;
> > > + return;
> > > }
> > >
> > > - if (in->handle == NVDIMM_QEMU_RSVD_HANDLE_ROOT) {
> > > - nvdimm_dsm_handle_reserved_root_method(state, in,
> > > dsm_mem_addr);
> > > - goto exit;
> > > + if (method_in->handle == NVDIMM_QEMU_RSVD_HANDLE_ROOT) {
> > > + nvdimm_dsm_handle_reserved_root_method(state, dsm_in,
> > > dsm_mem_addr);
> > > + return;
> > > }
> > >
> > > /* Handle 0 is reserved for NVDIMM Root Device. */
> > > - if (!in->handle) {
> > > - nvdimm_dsm_root(in, dsm_mem_addr);
> > > - goto exit;
> > > + if (!method_in->handle) {
> > > + nvdimm_dsm_root(dsm_in, dsm_mem_addr);
> > > + return;
> > > }
> > >
> > > - nvdimm_dsm_device(in, dsm_mem_addr);
> > > + nvdimm_dsm_device(method_in->handle, dsm_in, dsm_mem_addr);
> > > +}
> > >
> > > -exit:
> > > - g_free(in);
> > > +static void nvdimm_lsi_handle(uint32_t nv_handle, hwaddr
> > > dsm_mem_addr)
> > > +{
> > > + NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(nv_handle);
> > > +
> > > + if (nvdimm->label_size) {
> > > + nvdimm_dsm_label_size(nvdimm, dsm_mem_addr);
> > > + }
> > > +
> > > + return;
> > > +}
> > > +
> > > +static void nvdimm_lsr_handle(uint32_t nv_handle,
> > > + void *data,
> > > + hwaddr dsm_mem_addr)
> > > +{
> > > + NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(nv_handle);
> > > + NvdimmFuncGetLabelDataIn *get_label_data = data;
> > > +
> > > + if (nvdimm->label_size) {
> > > + nvdimm_dsm_get_label_data(nvdimm, get_label_data,
> > > dsm_mem_addr);
> > > + }
> > > + return;
> > > +}
> > > +
> > > +static void nvdimm_lsw_handle(uint32_t nv_handle,
> > > + void *data,
> > > + hwaddr dsm_mem_addr)
> > > +{
> > > + NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(nv_handle);
> > > + NvdimmFuncSetLabelDataIn *set_label_data = data;
> > > +
> > > + if (nvdimm->label_size) {
> > > + nvdimm_dsm_set_label_data(nvdimm, set_label_data,
> > > dsm_mem_addr);
> > > + }
> > > + return;
> > > +}
> > > +
> > > +static void
> > > +nvdimm_method_write(void *opaque, hwaddr addr, uint64_t val,
> > > unsigned size)
> > > +{
> > > + NvdimmMthdIn *method_in;
> > > + hwaddr dsm_mem_addr = val;
> > > +
> > > + /*
> > > + * The DSM memory is mapped to guest address space so an evil
> > > guest
> > > + * can change its content while we are doing DSM emulation.
> > > Avoid
> > > + * this by copying DSM memory to QEMU local memory.
> > > + */
> > > + method_in = g_new(NvdimmMthdIn, 1);
> > > + cpu_physical_memory_read(dsm_mem_addr, method_in,
> > > sizeof(*method_in));
> > > +
> > > + method_in->handle = le32_to_cpu(method_in->handle);
> > > + method_in->method = le32_to_cpu(method_in->method);
> > > +
> > > + switch (method_in->method) {
> > > + case NVDIMM_METHOD_DSM:
> > > + nvdimm_dsm_handle(opaque, method_in, dsm_mem_addr);
> > > + break;
> > > + case NVDIMM_METHOD_LSI:
> > > + nvdimm_lsi_handle(method_in->handle, dsm_mem_addr);
> > > + break;
> > > + case NVDIMM_METHOD_LSR:
> > > + nvdimm_lsr_handle(method_in->handle, method_in->args,
> > > dsm_mem_addr);
> > > + break;
> > > + case NVDIMM_METHOD_LSW:
> > > + nvdimm_lsw_handle(method_in->handle, method_in->args,
> > > dsm_mem_addr);
> > > + break;
> > > + default:
> > > + nvdimm_debug("%s: Unkown method 0x%x\n", __func__,
> > > method_in->method);
> > > + break;
> > > + }
> > > +
> > > + g_free(method_in);
> > > }
> > >
> > > -static const MemoryRegionOps nvdimm_dsm_ops = {
> > > - .read = nvdimm_dsm_read,
> > > - .write = nvdimm_dsm_write,
> > > +static const MemoryRegionOps nvdimm_method_ops = {
> > > + .read = nvdimm_method_read,
> > > + .write = nvdimm_method_write,
> > > .endianness = DEVICE_LITTLE_ENDIAN,
> > > .valid = {
> > > .min_access_size = 4,
> > > @@ -899,12 +972,12 @@ void nvdimm_init_acpi_state(NVDIMMState
> > > *state, MemoryRegion *io,
> > > FWCfgState *fw_cfg, Object *owner)
> > > {
> > > state->dsm_io = dsm_io;
> > > - memory_region_init_io(&state->io_mr, owner, &nvdimm_dsm_ops,
> > > state,
> > > + memory_region_init_io(&state->io_mr, owner,
> > > &nvdimm_method_ops, state,
> > > "nvdimm-acpi-io", dsm_io.bit_width >>
> > > 3);
> > > memory_region_add_subregion(io, dsm_io.address, &state-
> > > >io_mr);
> > >
> > > state->dsm_mem = g_array_new(false, true /* clear */, 1);
> > > - acpi_data_push(state->dsm_mem, sizeof(NvdimmDsmIn));
> > > + acpi_data_push(state->dsm_mem, sizeof(NvdimmMthdIn));
> > > fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem-
> > > >data,
> > > state->dsm_mem->len);
> > >
> > > @@ -918,13 +991,22 @@ void nvdimm_init_acpi_state(NVDIMMState
> > > *state, MemoryRegion *io,
> > > #define NVDIMM_DSM_IOPORT "NPIO"
> > >
> > > #define NVDIMM_DSM_NOTIFY "NTFI"
> > > +#define NVDIMM_DSM_METHOD "MTHD"
> > > #define NVDIMM_DSM_HANDLE "HDLE"
> > > #define NVDIMM_DSM_REVISION "REVS"
> > > #define NVDIMM_DSM_FUNCTION "FUNC"
> > > #define NVDIMM_DSM_ARG3 "FARG"
> > >
> > > -#define NVDIMM_DSM_OUT_BUF_SIZE "RLEN"
> > > -#define NVDIMM_DSM_OUT_BUF "ODAT"
> > > +#define NVDIMM_DSM_OFFSET "OFST"
> > > +#define NVDIMM_DSM_TRANS_LEN "TRSL"
> > > +#define NVDIMM_DSM_IN_BUFF "IDAT"
> > > +
> > > +#define NVDIMM_DSM_OUT_BUF_SIZE "RLEN"
> > > +#define NVDIMM_DSM_OUT_BUF "ODAT"
> > > +#define NVDIMM_DSM_OUT_STATUS "STUS"
> > > +#define NVDIMM_DSM_OUT_LSA_SIZE "SIZE"
> > > +#define NVDIMM_DSM_OUT_MAX_TRANS "MAXT"
> > > +
> > >
> > > #define NVDIMM_DSM_RFIT_STATUS "RSTA"
> > >
> > > @@ -938,7 +1020,6 @@ static void nvdimm_build_common_dsm(Aml *dev,
> > > Aml *pckg, *pckg_index, *pckg_buf, *field, *dsm_out_buf,
> > > *dsm_out_buf_size;
> > > Aml *whilectx, *offset;
> > > uint8_t byte_list[1];
> > > - AmlRegionSpace rs;
> > >
> > > method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
> > > uuid = aml_arg(0);
> > > @@ -949,37 +1030,15 @@ static void nvdimm_build_common_dsm(Aml
> > > *dev,
> > >
> > > aml_append(method, aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR),
> > > dsm_mem));
> > >
> > > - if (nvdimm_state->dsm_io.space_id == AML_AS_SYSTEM_IO) {
> > > - rs = AML_SYSTEM_IO;
> > > - } else {
> > > - rs = AML_SYSTEM_MEMORY;
> > > - }
> > > -
> > > - /* map DSM memory and IO into ACPI namespace. */
> > > - aml_append(method, aml_operation_region(NVDIMM_DSM_IOPORT, rs,
> > > - aml_int(nvdimm_state->dsm_io.address),
> > > - nvdimm_state->dsm_io.bit_width >> 3));
> > > aml_append(method, aml_operation_region(NVDIMM_DSM_MEMORY,
> > > - AML_SYSTEM_MEMORY, dsm_mem, sizeof(NvdimmDsmIn)));
> > > -
> > > - /*
> > > - * DSM notifier:
> > > - * NVDIMM_DSM_NOTIFY: write the address of DSM memory and
> > > notify QEMU to
> > > - * emulate the access.
> > > - *
> > > - * It is the IO port so that accessing them will cause VM-
> > > exit, the
> > > - * control will be transferred to QEMU.
> > > - */
> > > - field = aml_field(NVDIMM_DSM_IOPORT, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > - AML_PRESERVE);
> > > - aml_append(field, aml_named_field(NVDIMM_DSM_NOTIFY,
> > > - nvdimm_state->dsm_io.bit_width));
> > > - aml_append(method, field);
> > > + AML_SYSTEM_MEMORY, dsm_mem, sizeof(NvdimmMthdIn)));
> > >
> > > /*
> > > * DSM input:
> > > * NVDIMM_DSM_HANDLE: store device's handle, it's zero if the
> > > _DSM call
> > > * happens on NVDIMM Root Device.
> > > + * NVDIMM_DSM_METHOD: ACPI method indicator, to distinguish
> > > _DSM and
> > > + * other ACPI methods.
> > > * NVDIMM_DSM_REVISION: store the Arg1 of _DSM call.
> > > * NVDIMM_DSM_FUNCTION: store the Arg2 of _DSM call.
> > > * NVDIMM_DSM_ARG3: store the Arg3 of _DSM call which is a
> > > Package
> > > @@ -991,13 +1050,16 @@ static void nvdimm_build_common_dsm(Aml
> > > *dev,
> > > field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > AML_PRESERVE);
> > > aml_append(field, aml_named_field(NVDIMM_DSM_HANDLE,
> > > - sizeof(typeof_field(NvdimmDsmIn, handle)) *
> > > BITS_PER_BYTE));
> > > + sizeof(typeof_field(NvdimmMthdIn, handle)) *
> > > BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_METHOD,
> > > + sizeof(typeof_field(NvdimmMthdIn, method)) *
> > > BITS_PER_BYTE));
> > > aml_append(field, aml_named_field(NVDIMM_DSM_REVISION,
> > > sizeof(typeof_field(NvdimmDsmIn, revision)) *
> > > BITS_PER_BYTE));
> > > aml_append(field, aml_named_field(NVDIMM_DSM_FUNCTION,
> > > sizeof(typeof_field(NvdimmDsmIn, function)) *
> > > BITS_PER_BYTE));
> > > aml_append(field, aml_named_field(NVDIMM_DSM_ARG3,
> > > - (sizeof(NvdimmDsmIn) - offsetof(NvdimmDsmIn, arg3)) *
> > > BITS_PER_BYTE));
> > > + (sizeof(NvdimmMthdIn) - offsetof(NvdimmMthdIn, args) -
> > > + offsetof(NvdimmDsmIn, arg3)) * BITS_PER_BYTE));
> > > aml_append(method, field);
> > >
> > > /*
> > > @@ -1065,6 +1127,7 @@ static void nvdimm_build_common_dsm(Aml *dev,
> > > * it reserves 0 for root device and is the handle for NVDIMM
> > > devices.
> > > * See the comments in nvdimm_slot_to_handle().
> > > */
> > > + aml_append(method, aml_store(aml_int(0),
> > > aml_name(NVDIMM_DSM_METHOD)));
> > > aml_append(method, aml_store(handle,
> > > aml_name(NVDIMM_DSM_HANDLE)));
> > > aml_append(method, aml_store(aml_arg(1),
> > > aml_name(NVDIMM_DSM_REVISION)));
> > > aml_append(method, aml_store(function,
> > > aml_name(NVDIMM_DSM_FUNCTION)));
> > > @@ -1250,6 +1313,7 @@ static void nvdimm_build_fit(Aml *dev)
> > > static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t
> > > ram_slots)
> > > {
> > > uint32_t slot;
> > > + Aml *method, *pkg, *field;
> > >
> > > for (slot = 0; slot < ram_slots; slot++) {
> > > uint32_t handle = nvdimm_slot_to_handle(slot);
> > > @@ -1266,6 +1330,155 @@ static void nvdimm_build_nvdimm_devices(Aml
> > > *root_dev, uint32_t ram_slots)
> > > * table NFIT or _FIT.
> > > */
> > > aml_append(nvdimm_dev, aml_name_decl("_ADR",
> > > aml_int(handle)));
> > > + aml_append(nvdimm_dev,
> > > aml_operation_region(NVDIMM_DSM_MEMORY,
> > > + AML_SYSTEM_MEMORY,
> > > aml_name(NVDIMM_ACPI_MEM_ADDR),
> > > + sizeof(NvdimmMthdIn)));
> > > +
> > > + /* ACPI 6.4: 6.5.10 NVDIMM Label Methods, _LS{I,R,W} */
> > > +
> > > + /* Begin of _LSI Block */
> > > + method = aml_method("_LSI", 0, AML_SERIALIZED);
> > > + /* _LSI Input field */
> > > + field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > + AML_PRESERVE);
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_HANDLE,
> > > + sizeof(typeof_field(NvdimmMthdIn, handle)) *
> > > BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_METHOD,
> > > + sizeof(typeof_field(NvdimmMthdIn, method)) *
> > > BITS_PER_BYTE));
> > > + aml_append(method, field);
> > > +
> > > + /* _LSI Output field */
> > > + field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > + AML_PRESERVE);
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF_SIZE,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelSizeOut,
> > > len)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_STATUS,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelSizeOut,
> > > + func_ret_status)) * BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_LSA_SIZE,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelSizeOut,
> > > label_size)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field,
> > > aml_named_field(NVDIMM_DSM_OUT_MAX_TRANS,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelSizeOut,
> > > max_xfer)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(method, field);
> > > +
> > > + aml_append(method, aml_store(aml_int(handle),
> > > + aml_name(NVDIMM_DSM_HANDLE))
> > > );
> > > + aml_append(method, aml_store(aml_int(0x100),
> > > + aml_name(NVDIMM_DSM_METHOD))
> > > );
> > > + aml_append(method,
> > > aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR),
> > > + aml_name(NVDIMM_DSM_NOTIFY))
> > > );
> > > +
> > > + pkg = aml_package(3);
> > > + aml_append(pkg, aml_name(NVDIMM_DSM_OUT_STATUS));
> > > + aml_append(pkg, aml_name(NVDIMM_DSM_OUT_LSA_SIZE));
> > > + aml_append(pkg, aml_name(NVDIMM_DSM_OUT_MAX_TRANS));
> > > +
> > > + aml_append(method, aml_name_decl("RPKG", pkg));
> > > +
> > > + aml_append(method, aml_return(aml_name("RPKG")));
> > > + aml_append(nvdimm_dev, method); /* End of _LSI Block */
> > > +
> > > +
> > > + /* Begin of _LSR Block */
> > > + method = aml_method("_LSR", 2, AML_SERIALIZED);
> > > +
> > > + /* _LSR Input field */
> > > + field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > + AML_PRESERVE);
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_HANDLE,
> > > + sizeof(typeof_field(NvdimmMthdIn, handle)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_METHOD,
> > > + sizeof(typeof_field(NvdimmMthdIn, method)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OFFSET,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelDataIn,
> > > offset)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_TRANS_LEN,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelDataIn,
> > > length)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(method, field);
> > > +
> > > + /* _LSR Output field */
> > > + field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > + AML_PRESERVE);
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF_SIZE,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelDataOut,
> > > len)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_STATUS,
> > > + sizeof(typeof_field(NvdimmFuncGetLabelDataOut,
> > > + func_ret_status)) * BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF,
> > > + (NVDIMM_DSM_MEMORY_SIZE -
> > > + offsetof(NvdimmFuncGetLabelDataOut, out_buf))
> > > *
> > > + BITS_PER_BYTE));
> > > + aml_append(method, field);
> > > +
> > > + aml_append(method, aml_store(aml_int(handle),
> > > + aml_name(NVDIMM_DSM_HANDLE))
> > > );
> > > + aml_append(method, aml_store(aml_int(0x101),
> > > + aml_name(NVDIMM_DSM_METHOD))
> > > );
> > > + aml_append(method, aml_store(aml_arg(0),
> > > aml_name(NVDIMM_DSM_OFFSET)));
> > > + aml_append(method, aml_store(aml_arg(1),
> > > + aml_name(NVDIMM_DSM_TRANS_LE
> > > N)));
> > > + aml_append(method,
> > > aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR),
> > > + aml_name(NVDIMM_DSM_NOTIFY))
> > > );
> > > +
> > > + aml_append(method, aml_store(aml_shiftleft(aml_arg(1),
> > > aml_int(3)),
> > > + aml_local(1)));
> > > + aml_append(method,
> > > aml_create_field(aml_name(NVDIMM_DSM_OUT_BUF),
> > > + aml_int(0), aml_local(1), "OBUF"));
> > > +
> > > + pkg = aml_package(2);
> > > + aml_append(pkg, aml_name(NVDIMM_DSM_OUT_STATUS));
> > > + aml_append(pkg, aml_name("OBUF"));
> > > + aml_append(method, aml_name_decl("RPKG", pkg));
> > > +
> > > + aml_append(method, aml_return(aml_name("RPKG")));
> > > + aml_append(nvdimm_dev, method); /* End of _LSR Block */
> > > +
> > > + /* Begin of _LSW Block */
> > > + method = aml_method("_LSW", 3, AML_SERIALIZED);
> > > + /* _LSW Input field */
> > > + field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > + AML_PRESERVE);
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_HANDLE,
> > > + sizeof(typeof_field(NvdimmMthdIn, handle)) *
> > > BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_METHOD,
> > > + sizeof(typeof_field(NvdimmMthdIn, method)) *
> > > BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OFFSET,
> > > + sizeof(typeof_field(NvdimmFuncSetLabelDataIn,
> > > offset)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_TRANS_LEN,
> > > + sizeof(typeof_field(NvdimmFuncSetLabelDataIn,
> > > length)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_IN_BUFF,
> > > 32640));
> > > + aml_append(method, field);
> > > +
> > > + /* _LSW Output field */
> > > + field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > + AML_PRESERVE);
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF_SIZE,
> > > + sizeof(typeof_field(NvdimmDsmFuncNoPayloadOut,
> > > len)) *
> > > + BITS_PER_BYTE));
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_OUT_STATUS,
> > > + sizeof(typeof_field(NvdimmDsmFuncNoPayloadOut,
> > > + func_ret_status)) * BITS_PER_BYTE));
> > > + aml_append(method, field);
> > > +
> > > + aml_append(method, aml_store(aml_int(handle),
> > > aml_name(NVDIMM_DSM_HANDLE)));
> > > + aml_append(method, aml_store(aml_int(0x102),
> > > aml_name(NVDIMM_DSM_METHOD)));
> > > + aml_append(method, aml_store(aml_arg(0),
> > > aml_name(NVDIMM_DSM_OFFSET)));
> > > + aml_append(method, aml_store(aml_arg(1),
> > > aml_name(NVDIMM_DSM_TRANS_LEN)));
> > > + aml_append(method, aml_store(aml_arg(2),
> > > aml_name(NVDIMM_DSM_IN_BUFF)));
> > > + aml_append(method,
> > > aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR),
> > > + aml_name(NVDIMM_DSM_NOTIFY))
> > > );
> > > +
> > > + aml_append(method,
> > > aml_return(aml_name(NVDIMM_DSM_OUT_STATUS)));
> > > + aml_append(nvdimm_dev, method); /* End of _LSW Block */
> > >
> > > nvdimm_build_device_dsm(nvdimm_dev, handle);
> > > aml_append(root_dev, nvdimm_dev);
> > > @@ -1278,7 +1491,8 @@ static void nvdimm_build_ssdt(GArray
> > > *table_offsets, GArray *table_data,
> > > uint32_t ram_slots, const char
> > > *oem_id)
> > > {
> > > int mem_addr_offset;
> > > - Aml *ssdt, *sb_scope, *dev;
> > > + Aml *ssdt, *sb_scope, *dev, *field;
> > > + AmlRegionSpace rs;
> > > AcpiTable table = { .sig = "SSDT", .rev = 1,
> > > .oem_id = oem_id, .oem_table_id = "NVDIMM"
> > > };
> > >
> > > @@ -1286,6 +1500,9 @@ static void nvdimm_build_ssdt(GArray
> > > *table_offsets, GArray *table_data,
> > >
> > > acpi_table_begin(&table, table_data);
> > > ssdt = init_aml_allocator();
> > > +
> > > + mem_addr_offset = build_append_named_dword(table_data,
> > > + NVDIMM_ACPI_MEM_ADD
> > > R);
> > > sb_scope = aml_scope("\\_SB");
> > >
> > > dev = aml_device("NVDR");
> > > @@ -1303,6 +1520,31 @@ static void nvdimm_build_ssdt(GArray
> > > *table_offsets, GArray *table_data,
> > > */
> > > aml_append(dev, aml_name_decl("_HID",
> > > aml_string("ACPI0012")));
> > >
> > > + if (nvdimm_state->dsm_io.space_id == AML_AS_SYSTEM_IO) {
> > > + rs = AML_SYSTEM_IO;
> > > + } else {
> > > + rs = AML_SYSTEM_MEMORY;
> > > + }
> > > +
> > > + /* map DSM memory and IO into ACPI namespace. */
> > > + aml_append(dev, aml_operation_region(NVDIMM_DSM_IOPORT, rs,
> > > + aml_int(nvdimm_state->dsm_io.address),
> > > + nvdimm_state->dsm_io.bit_width >> 3));
> > > +
> > > + /*
> > > + * DSM notifier:
> > > + * NVDIMM_DSM_NOTIFY: write the address of DSM memory and
> > > notify QEMU to
> > > + * emulate the access.
> > > + *
> > > + * It is the IO port so that accessing them will cause VM-
> > > exit, the
> > > + * control will be transferred to QEMU.
> > > + */
> > > + field = aml_field(NVDIMM_DSM_IOPORT, AML_DWORD_ACC,
> > > AML_NOLOCK,
> > > + AML_PRESERVE);
> > > + aml_append(field, aml_named_field(NVDIMM_DSM_NOTIFY,
> > > + nvdimm_state->dsm_io.bit_width));
> > > + aml_append(dev, field);
> > > +
> > > nvdimm_build_common_dsm(dev, nvdimm_state);
> > >
> > > /* 0 is reserved for root device. */
> > > @@ -1316,12 +1558,10 @@ static void nvdimm_build_ssdt(GArray
> > > *table_offsets, GArray *table_data,
> > >
> > > /* copy AML table into ACPI tables blob and patch header there
> > > */
> > > g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf-
> > > >len);
> > > - mem_addr_offset = build_append_named_dword(table_data,
> > > - NVDIMM_ACPI_MEM_ADD
> > > R);
> > >
> > > bios_linker_loader_alloc(linker,
> > > NVDIMM_DSM_MEM_FILE, nvdimm_state-
> > > >dsm_mem,
> > > - sizeof(NvdimmDsmIn), false /* high
> > > memory */);
> > > + sizeof(NvdimmMthdIn), false /* high
> > > memory */);
> > > bios_linker_loader_add_pointer(linker,
> > > ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
> > > NVDIMM_DSM_MEM_FILE, 0);
> > > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> > > index cf8f59be44..0206b6125b 100644
> > > --- a/include/hw/mem/nvdimm.h
> > > +++ b/include/hw/mem/nvdimm.h
> > > @@ -37,6 +37,12 @@
> > > } \
> > > } while (0)
> > >
> > > +/* NVDIMM ACPI Methods */
> > > +#define NVDIMM_METHOD_DSM 0
> > > +#define NVDIMM_METHOD_LSI 0x100
> > > +#define NVDIMM_METHOD_LSR 0x101
> > > +#define NVDIMM_METHOD_LSW 0x102
> > > +
> > > /*
> > > * The minimum label data size is required by NVDIMM Namespace
> > > * specification, see the chapter 2 Namespaces:
> >
> >
>