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

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;
     }
-- 
2.31.1

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;
> >      }
> 
>