| Message ID | 20220530034047.730356-4-robert.hu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support ACPI NVDIMM Label Methods | expand |
On Mon, 30 May 2022 11:40:44 +0800 Robert Hoo <robert.hu@linux.intel.com> wrote: > The Intel Optane PMem DSM Interface, Version 2.0 [1], is the up-to-date > spec for NVDIMM _DSM definition, which supports revision_id == 2. > > Nevertheless, Rev.2 of NVDIMM _DSM has no functional change on those Label > Data _DSM Functions, which are the only ones implemented for vNVDIMM. > So, simple change to support this revision_id == 2 case. > > [1] https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf pls enumerate functions that QEMU implement and that are supported by rev=2, do we really need rev2 ? also don't we need make sure that rev1 only function are excluded? /spec above says, functions 3-6 are deprecated and limited to rev1 only/ " Warning: This function has been deprecated in preference to the ACPI 6.2 _LSW (Label Storage Write) NVDIMM Device Interface and is only supported with Arg1 – Revision Id = 1. It is included here for backwards compatibility with existing Arg1 - Revision Id = 1 implementations. " > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > Reviewed-by: Jingqi Liu <jingqi.liu@intel.com> > --- > hw/acpi/nvdimm.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 0ab247a870..59b42afcf1 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -849,9 +849,13 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) > nvdimm_debug("Revision 0x%x Handler 0x%x Function 0x%x.\n", in->revision, > in->handle, in->function); > > - if (in->revision != 0x1 /* Currently we only support DSM Spec Rev1. */) { > - nvdimm_debug("Revision 0x%x is not supported, expect 0x%x.\n", > - in->revision, 0x1); > + /* > + * Current NVDIMM _DSM Spec supports Rev1 and Rev2 > + * Intel® OptanePersistent Memory Module DSM Interface, Revision 2.0 > + */ > + if (in->revision != 0x1 && in->revision != 0x2) { > + nvdimm_debug("Revision 0x%x is not supported, expect 0x1 or 0x2.\n", > + in->revision); > nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_UNSUPPORT, dsm_mem_addr); > goto exit; > }
On Thu, 2022-06-16 at 13:38 +0200, Igor Mammedov wrote: > On Mon, 30 May 2022 11:40:44 +0800 > Robert Hoo <robert.hu@linux.intel.com> wrote: > > > The Intel Optane PMem DSM Interface, Version 2.0 [1], is the up-to- > > date > > spec for NVDIMM _DSM definition, which supports revision_id == 2. > > > > Nevertheless, Rev.2 of NVDIMM _DSM has no functional change on > > those Label > > Data _DSM Functions, which are the only ones implemented for > > vNVDIMM. > > So, simple change to support this revision_id == 2 case. > > > > [1] > > https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf > > pls enumerate functions that QEMU implement and that are supported by > rev=2, > do we really need rev2 ? No matter rev.1 or rev.2, current QEMU implements only the three label methods: get namespace label data size (func index 4), get namespace label data (func index 5), set namespace label data (func index 6). In both rev.1 an rev.2, these 3 _DSM label methods are deprecated by ACPI Label methods. So, okay, we don't really need rev.2, at present. > > also don't we need make sure that rev1 only function are excluded? > /spec above says, functions 3-6 are deprecated and limited to rev1 > only/ > " > Warning: This function has been deprecated in preference to the ACPI > 6.2 _LSW (Label Storage Write) > NVDIMM Device Interface and is only supported with Arg1 – Revision Id > = 1. It is included here for > backwards compatibility with existing Arg1 - Revision Id = 1 > implementations. > " Well, they're deprecated, not obsoleted, so still included, I think. Anyway, as said above, we don't need this patch at this moment, let's keep it unchanged. > > > > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > > Reviewed-by: Jingqi Liu <jingqi.liu@intel.com> > > --- > > hw/acpi/nvdimm.c | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > > index 0ab247a870..59b42afcf1 100644 > > --- a/hw/acpi/nvdimm.c > > +++ b/hw/acpi/nvdimm.c > > @@ -849,9 +849,13 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, > > uint64_t val, unsigned size) > > nvdimm_debug("Revision 0x%x Handler 0x%x Function 0x%x.\n", > > in->revision, > > in->handle, in->function); > > > > - if (in->revision != 0x1 /* Currently we only support DSM Spec > > Rev1. */) { > > - nvdimm_debug("Revision 0x%x is not supported, expect > > 0x%x.\n", > > - in->revision, 0x1); > > + /* > > + * Current NVDIMM _DSM Spec supports Rev1 and Rev2 > > + * Intel® OptanePersistent Memory Module DSM Interface, > > Revision 2.0 > > + */ > > + if (in->revision != 0x1 && in->revision != 0x2) { > > + nvdimm_debug("Revision 0x%x is not supported, expect 0x1 > > or 0x2.\n", > > + in->revision); > > nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_UNSUPPORT, > > dsm_mem_addr); > > goto exit; > > } > >
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 0ab247a870..59b42afcf1 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -849,9 +849,13 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) nvdimm_debug("Revision 0x%x Handler 0x%x Function 0x%x.\n", in->revision, in->handle, in->function); - if (in->revision != 0x1 /* Currently we only support DSM Spec Rev1. */) { - nvdimm_debug("Revision 0x%x is not supported, expect 0x%x.\n", - in->revision, 0x1); + /* + * Current NVDIMM _DSM Spec supports Rev1 and Rev2 + * Intel® OptanePersistent Memory Module DSM Interface, Revision 2.0 + */ + if (in->revision != 0x1 && in->revision != 0x2) { + nvdimm_debug("Revision 0x%x is not supported, expect 0x1 or 0x2.\n", + in->revision); nvdimm_dsm_no_payload(NVDIMM_DSM_RET_STATUS_UNSUPPORT, dsm_mem_addr); goto exit; }