[v1] acpi/nvdimm: support NVDIMM _LS{I,R,W} methods

[PATCH 2/2] acpi/nvdimm: Fix aml_or() and aml_and() in if clause

Posted by Robert Hoo 3 years, 10 months ago

It should be some typo originally, where in If condition, using bitwise
and/or, rather than logical and/or.

The resulting change in AML code:

If (((Local6 == Zero) | (Arg0 != Local0)))
==>
If (((Local6 == Zero) || (Arg0 != Local0)))

If (((ObjectType (Arg3) == 0x04) & (SizeOf (Arg3) == One)))
==>
If (((ObjectType (Arg3) == 0x04) && (SizeOf (Arg3) == One)))

Fixes: 90623ebf603 ("nvdimm acpi: check UUID")
Fixes: 4568c948066 ("nvdimm acpi: save arg3 of _DSM method")
Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
---
 hw/acpi/nvdimm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 7cc419401b..2cd26bb9e9 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1040,7 +1040,7 @@ static void nvdimm_build_common_dsm(Aml *dev,
 
     uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
 
-    unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
+    unsupport = aml_if(aml_lor(unpatched, uuid_invalid));
 
     /*
      * function 0 is called to inquire what functions are supported by
@@ -1072,10 +1072,9 @@ static void nvdimm_build_common_dsm(Aml *dev,
      * in the DSM Spec.
      */
     pckg = aml_arg(3);
-    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
+    ifctx = aml_if(aml_land(aml_equal(aml_object_type(pckg),
                    aml_int(4 /* Package */)) /* It is a Package? */,
-                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */,
-                   NULL));
+                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */));
 
     pckg_index = aml_local(2);
     pckg_buf = aml_local(3);
-- 
2.31.1

Re: [PATCH 2/2] acpi/nvdimm: Fix aml_or() and aml_and() in if clause

Posted by Michael S. Tsirkin 3 years, 9 months ago

On Tue, Apr 12, 2022 at 02:57:53PM +0800, Robert Hoo wrote:
> It should be some typo originally, where in If condition, using bitwise
> and/or, rather than logical and/or.
> 
> The resulting change in AML code:
> 
> If (((Local6 == Zero) | (Arg0 != Local0)))
> ==>
> If (((Local6 == Zero) || (Arg0 != Local0)))
> 
> If (((ObjectType (Arg3) == 0x04) & (SizeOf (Arg3) == One)))
> ==>
> If (((ObjectType (Arg3) == 0x04) && (SizeOf (Arg3) == One)))
> 
> Fixes: 90623ebf603 ("nvdimm acpi: check UUID")
> Fixes: 4568c948066 ("nvdimm acpi: save arg3 of _DSM method")
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>

This changes existing AML, you need to do the dance
with updating bios test tables, see header of ./tests/qtest/bios-tables-test.c

> ---
>  hw/acpi/nvdimm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 7cc419401b..2cd26bb9e9 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -1040,7 +1040,7 @@ static void nvdimm_build_common_dsm(Aml *dev,
>  
>      uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
>  
> -    unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
> +    unsupport = aml_if(aml_lor(unpatched, uuid_invalid));
>  
>      /*
>       * function 0 is called to inquire what functions are supported by
> @@ -1072,10 +1072,9 @@ static void nvdimm_build_common_dsm(Aml *dev,
>       * in the DSM Spec.
>       */
>      pckg = aml_arg(3);
> -    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
> +    ifctx = aml_if(aml_land(aml_equal(aml_object_type(pckg),
>                     aml_int(4 /* Package */)) /* It is a Package? */,
> -                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */,
> -                   NULL));
> +                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */));
>  
>      pckg_index = aml_local(2);
>      pckg_buf = aml_local(3);
> -- 
> 2.31.1

Re: [PATCH 2/2] acpi/nvdimm: Fix aml_or() and aml_and() in if clause

Posted by Igor Mammedov 3 years, 9 months ago

On Tue, 12 Apr 2022 14:57:53 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> It should be some typo originally, where in If condition, using bitwise
> and/or, rather than logical and/or.
> 
> The resulting change in AML code:
> 
> If (((Local6 == Zero) | (Arg0 != Local0)))
> ==>  
> If (((Local6 == Zero) || (Arg0 != Local0)))
> 
> If (((ObjectType (Arg3) == 0x04) & (SizeOf (Arg3) == One)))
> ==>  
> If (((ObjectType (Arg3) == 0x04) && (SizeOf (Arg3) == One)))
> 
> Fixes: 90623ebf603 ("nvdimm acpi: check UUID")
> Fixes: 4568c948066 ("nvdimm acpi: save arg3 of _DSM method")
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/nvdimm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 7cc419401b..2cd26bb9e9 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -1040,7 +1040,7 @@ static void nvdimm_build_common_dsm(Aml *dev,
>  
>      uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
>  
> -    unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
> +    unsupport = aml_if(aml_lor(unpatched, uuid_invalid));
>  
>      /*
>       * function 0 is called to inquire what functions are supported by
> @@ -1072,10 +1072,9 @@ static void nvdimm_build_common_dsm(Aml *dev,
>       * in the DSM Spec.
>       */
>      pckg = aml_arg(3);
> -    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
> +    ifctx = aml_if(aml_land(aml_equal(aml_object_type(pckg),
>                     aml_int(4 /* Package */)) /* It is a Package? */,
> -                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */,
> -                   NULL));
> +                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */));
>  
>      pckg_index = aml_local(2);
>      pckg_buf = aml_local(3);