[edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing tables

[edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing tables

[Dhaval Sharma]

As per ACPI Spec 6.5+ Table 5-9 if xDSDT is available,
it should be used first. Handle required flow when xDSDT
is absent or present.

Test: Tested on RISCV64 Qemu platform with xDSDT and booted to
linux kernel.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
Acked-by: Chasel Chiu <chasel.chiu@...>
---

Notes:
    v4:
    - Fix typos and commit message adding more clarity to patch subject
    v3:
    - Added description of ACPI spec clarification based on which this patch is created
    - Optimizing if-else flow
    v2:
    - Added proper indentation for else if

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 23 ++++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index e09bc9b704f5..61af6047a2a7 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -1892,14 +1892,23 @@ InstallAcpiTableFromHob (
           }
         }
 
-        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 0) {
+        //
+        // First check if xDSDT is available, as that is preferred as per
+        // ACPI Spec 6.5+ Table 5-9 X_DSDT definition
+        //
+        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt != 0) {
+          TableToInstall = (VOID *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt;
+        } else if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 0) {
           TableToInstall = (VOID *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt;
-          Status         = AddTableToList (AcpiTableInstance, TableToInstall, TRUE, Version, TRUE, &TableKey);
-          if (EFI_ERROR (Status)) {
-            DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI table DSDT\n"));
-            ASSERT_EFI_ERROR (Status);
-            break;
-          }
+        } else {
+          break;
+        }
+
+        Status = AddTableToList (AcpiTableInstance, TableToInstall, TRUE, Version, TRUE, &TableKey);
+        if (EFI_ERROR (Status)) {
+          DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI table DSDT\n"));
+          ASSERT_EFI_ERROR (Status);
+          break;
         }
       }
     }
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112003): https://edk2.groups.io/g/devel/message/112003
Mute This Topic: https://groups.io/mt/102965684/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 0/1] Prefer XDSDT table over DSDT if available

[Dhaval Sharma]

Enable detection of XDSDT table from ACPI HOB and use it to comply
with ACPI spec 6.5+ Table 5-9. https://github.com/tianocore/edk2/pull/5235

Dhaval (1):
  MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing
    tables

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 23 ++++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113400): https://edk2.groups.io/g/devel/message/113400
Mute This Topic: https://groups.io/mt/103598537/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing tables

[Dhaval Sharma]

As per ACPI Spec 6.5+ Table 5-9 if xDSDT is available,
it should be used first. Handle required flow when xDSDT
is absent or present.

Test: Tested on RISCV64 Qemu platform with xDSDT and booted to
linux kernel.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: devel@edk2.groups.io
Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
Acked-by: Chasel Chiu <chasel.chiu@...>
---

Notes:
    v4:
    - Fix typos and commit message adding more clarity to patch subject
    v3:
    - Added description of ACPI spec clarification based on which this patch is created
    - Optimizing if-else flow
    v2:
    - Added proper indentation for else if

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 23 ++++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index e09bc9b704f5..61af6047a2a7 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -1892,14 +1892,23 @@ InstallAcpiTableFromHob (
           }
         }
 
-        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 0) {
+        //
+        // First check if xDSDT is available, as that is preferred as per
+        // ACPI Spec 6.5+ Table 5-9 X_DSDT definition
+        //
+        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt != 0) {
+          TableToInstall = (VOID *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt;
+        } else if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 0) {
           TableToInstall = (VOID *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt;
-          Status         = AddTableToList (AcpiTableInstance, TableToInstall, TRUE, Version, TRUE, &TableKey);
-          if (EFI_ERROR (Status)) {
-            DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI table DSDT\n"));
-            ASSERT_EFI_ERROR (Status);
-            break;
-          }
+        } else {
+          break;
+        }
+
+        Status = AddTableToList (AcpiTableInstance, TableToInstall, TRUE, Version, TRUE, &TableKey);
+        if (EFI_ERROR (Status)) {
+          DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI table DSDT\n"));
+          ASSERT_EFI_ERROR (Status);
+          break;
         }
       }
     }
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113401): https://edk2.groups.io/g/devel/message/113401
Mute This Topic: https://groups.io/mt/103598583/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing tables

[Michael D Kinney]

Hi Liming,

I do not see any reviews of this patch.

What is the status?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dhaval
> Sharma
> Sent: Monday, January 8, 2024 7:00 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Pedro
> Falcato <pedro.falcato@gmail.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer
> xDSDT over DSDT when installing tables
> 
> As per ACPI Spec 6.5+ Table 5-9 if xDSDT is available,
> it should be used first. Handle required flow when xDSDT
> is absent or present.
> 
> Test: Tested on RISCV64 Qemu platform with xDSDT and booted to
> linux kernel.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: devel@edk2.groups.io
> Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> Acked-by: Chasel Chiu <chasel.chiu@...>
> ---
> 
> Notes:
>     v4:
>     - Fix typos and commit message adding more clarity to patch subject
>     v3:
>     - Added description of ACPI spec clarification based on which this
> patch is created
>     - Optimizing if-else flow
>     v2:
>     - Added proper indentation for else if
> 
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 23
> ++++++++++++++------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> index e09bc9b704f5..61af6047a2a7 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> @@ -1892,14 +1892,23 @@ InstallAcpiTableFromHob (
>            }
> 
>          }
> 
> 
> 
> -        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
> >Dsdt != 0) {
> 
> +        //
> 
> +        // First check if xDSDT is available, as that is preferred as
> per
> 
> +        // ACPI Spec 6.5+ Table 5-9 X_DSDT definition
> 
> +        //
> 
> +        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
> >XDsdt != 0) {
> 
> +          TableToInstall = (VOID
> *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
> >XDsdt;
> 
> +        } else if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> *)ChildTable)->Dsdt != 0) {
> 
>            TableToInstall = (VOID
> *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
> >Dsdt;
> 
> -          Status         = AddTableToList (AcpiTableInstance,
> TableToInstall, TRUE, Version, TRUE, &TableKey);
> 
> -          if (EFI_ERROR (Status)) {
> 
> -            DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add
> ACPI table DSDT\n"));
> 
> -            ASSERT_EFI_ERROR (Status);
> 
> -            break;
> 
> -          }
> 
> +        } else {
> 
> +          break;
> 
> +        }
> 
> +
> 
> +        Status = AddTableToList (AcpiTableInstance, TableToInstall,
> TRUE, Version, TRUE, &TableKey);
> 
> +        if (EFI_ERROR (Status)) {
> 
> +          DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add
> ACPI table DSDT\n"));
> 
> +          ASSERT_EFI_ERROR (Status);
> 
> +          break;
> 
>          }
> 
>        }
> 
>      }
> 
> --
> 2.39.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#113401):
> https://edk2.groups.io/g/devel/message/113401
> Mute This Topic: https://groups.io/mt/103598583/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114247): https://edk2.groups.io/g/devel/message/114247
Mute This Topic: https://groups.io/mt/103598583/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing tables

[gaoliming via groups.io]

Mike:
  Seemly, I miss this patch. 

Dhaval:
  I add one comment below. 
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael D
> Kinney
> 发送时间: 2024年1月24日 10:24
> 收件人: devel@edk2.groups.io; dhaval@rivosinc.com
> 抄送: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Pedro Falcato
> <pedro.falcato@gmail.com>; Chiu, Chasel <chasel.chiu@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> 主题: Re: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer
> xDSDT over DSDT when installing tables
> 
> Hi Liming,
> 
> I do not see any reviews of this patch.
> 
> What is the status?
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dhaval
> > Sharma
> > Sent: Monday, January 8, 2024 7:00 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> > <zhiguang.liu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Pedro
> > Falcato <pedro.falcato@gmail.com>; Chiu, Chasel <chasel.chiu@intel.com>
> > Subject: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer
> > xDSDT over DSDT when installing tables
> >
> > As per ACPI Spec 6.5+ Table 5-9 if xDSDT is available,
> > it should be used first. Handle required flow when xDSDT
> > is absent or present.
> >
> > Test: Tested on RISCV64 Qemu platform with xDSDT and booted to
> > linux kernel.
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: devel@edk2.groups.io
> > Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> > Acked-by: Chasel Chiu <chasel.chiu@...>
> > ---
> >
> > Notes:
> >     v4:
> >     - Fix typos and commit message adding more clarity to patch subject
> >     v3:
> >     - Added description of ACPI spec clarification based on which this
> > patch is created
> >     - Optimizing if-else flow
> >     v2:
> >     - Added proper indentation for else if
> >
> >  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 23
> > ++++++++++++++------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > index e09bc9b704f5..61af6047a2a7 100644
> > --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > @@ -1892,14 +1892,23 @@ InstallAcpiTableFromHob (
> >            }
> >
> >          }
> >
> >
> >
> > -        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> *)ChildTable)-
> > >Dsdt != 0) {
> >
> > +        //
> >
> > +        // First check if xDSDT is available, as that is preferred as
> > per
> >
> > +        // ACPI Spec 6.5+ Table 5-9 X_DSDT definition
> >
> > +        //
> >
> > +        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> *)ChildTable)-
> > >XDsdt != 0) {
> >
> > +          TableToInstall = (VOID
> > *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
> > >XDsdt;
> >
> > +        } else if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> > *)ChildTable)->Dsdt != 0) {
> >
> >            TableToInstall = (VOID
> > *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
> > >Dsdt;
> >
> > -          Status         = AddTableToList (AcpiTableInstance,
> > TableToInstall, TRUE, Version, TRUE, &TableKey);
> >
> > -          if (EFI_ERROR (Status)) {
> >
> > -            DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to
> add
> > ACPI table DSDT\n"));
> >
> > -            ASSERT_EFI_ERROR (Status);
> >
> > -            break;
> >
> > -          }
> >
> > +        } else {
> >
> > +          break;

[Liming] If XDsdt and Dsdt are both zero, here is break. The previous logic
will continue to look when Dsdt is zero. 
Please check whether this change is by purpose. If this change is by design,
I still think the return Status should be set to EFI_NOT_FOUND.

Thanks
Liming
> >
> > +        }
> >
> > +
> >
> > +        Status = AddTableToList (AcpiTableInstance, TableToInstall,
> > TRUE, Version, TRUE, &TableKey);
> >
> > +        if (EFI_ERROR (Status)) {
> >
> > +          DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to
> add
> > ACPI table DSDT\n"));
> >
> > +          ASSERT_EFI_ERROR (Status);
> >
> > +          break;
> >
> >          }
> >
> >        }
> >
> >      }
> >
> > --
> > 2.39.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#113401):
> > https://edk2.groups.io/g/devel/message/113401
> > Mute This Topic: https://groups.io/mt/103598583/1643496
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [michael.d.kinney@intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114306): https://edk2.groups.io/g/devel/message/114306
Mute This Topic: https://groups.io/mt/103932913/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-