[edk2-devel] [PATCH v3 0/1] Add support for XDSDT table

[edk2-devel] [PATCH v3 0/1] Add support for XDSDT table

[Dhaval Sharma]

Enable detection of XDSDT table from ACPI HOB and use it to comply
with ACPI spec 6.5+ Table 5-9.

Dhaval (1):
  MdeModulePkg: Fix issue with ACPI table creation

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

-- 
2.39.2



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

[edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation

[Dhaval Sharma]

As per ACPI Spec 6.5+ Table 5-9 if xDSDT is avaialble,
it should be used first. Handle required flow when xDSDT
is abscent 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>
---

Notes:
    v2:
    - Added proper indentation for else if

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

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index e09bc9b704f5..ead8376177c9 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -1892,14 +1892,22 @@ InstallAcpiTableFromHob (
           }
         }
 
-        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 0) {
+        //
+        // First check if xDSDT is available 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 (#111451): https://edk2.groups.io/g/devel/message/111451
Mute This Topic: https://groups.io/mt/102702109/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation

[Chiu, Chasel]

Thanks for update! Change looks good to me.
Acked-by: Chasel Chiu <chasel.chiu@intel.com>


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dhaval
> Sharma
> Sent: Sunday, November 19, 2023 8:24 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Bi, Dandan <dandan.bi@intel.com>
> Subject: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table
> creation
> 
> As per ACPI Spec 6.5+ Table 5-9 if xDSDT is avaialble, it should be used first.
> Handle required flow when xDSDT is abscent 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>
> ---
> 
> Notes:
>     v2:
>     - Added proper indentation for else if
> 
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 22
> +++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> index e09bc9b704f5..ead8376177c9 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> @@ -1892,14 +1892,22 @@ InstallAcpiTableFromHob (
>            }         } -        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> *)ChildTable)->Dsdt != 0) {+        //+        // First check if xDSDT is available 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 (#111451): https://edk2.groups.io/g/devel/message/111451
> Mute This Topic: https://groups.io/mt/102702109/1777047
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.chiu@intel.com] -=-
> =-=-=-=-=
> 



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

Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation

[Pedro Falcato]

On Mon, Nov 20, 2023 at 4:24 AM Dhaval Sharma <dhaval@rivosinc.com> wrote:
>
> As per ACPI Spec 6.5+ Table 5-9 if xDSDT is avaialble,

nit: available

> it should be used first. Handle required flow when xDSDT
> is abscent or present.

nit: absent

Separate nit: Please update the patch's subject to something more
descriptive. "Fix issue with ..." is really generic and useless for
anyone reading the log/blame.
Maybe something like "MdeModulePkg/AcpiTableDxe: Prefer xDSDT over
DSDT when installing tables"?

>
> 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>
> ---
>
> Notes:
>     v2:
>     - Added proper indentation for else if
>
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 22 +++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> index e09bc9b704f5..ead8376177c9 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> @@ -1892,14 +1892,22 @@ InstallAcpiTableFromHob (
>            }
>          }
>
> -        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 0) {
> +        //
> +        // First check if xDSDT is available that is preferred as per

nit: available, as that is preferred...

> +        // 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;

(+CC Gerd for qemu)
Is it possible that XDsdt may come filled out with a > 32-bit address
on 32-bit platforms/builds? In other words, is truncation of the
address a problem here? Assuming all of these tables are coming from
qemu + OvmfPkg/AcpiPlatformDxe, that is. I would not expect real
platforms to ever do such a thing.

For what it's worth, I checked the spec, and it clearly mentions that
the *OSPM* must ignore DSDT over X_DSDT. But we're not OSPM, so we're
not exactly bound by their constraints.

-- 
Pedro


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

Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation

[Gerd Hoffmann]

  Hi,

> > +        // 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;
> 
> (+CC Gerd for qemu)
> Is it possible that XDsdt may come filled out with a > 32-bit address
> on 32-bit platforms/builds? In other words, is truncation of the
> address a problem here?

I don't think this is possible.  qemu provides a script together with
the acpi tables, which contains instructions for pointer fixups, so the
firmware is free to choose where it places the tables in memory.  32bit
ovmf builds will surely place the tables below 4G.

take care,
  Gerd



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

[edk2-devel] [PATCH v3 0/1] Add support for XDSDT table

[Dhaval Sharma]

Enable detection of XDSDT table from ACPI HOB and use it to comply
with ACPI spec 6.5+ Table 5-9.

Dhaval (1):
  MdeModulePkg: Fix issue with ACPI table creation

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

-- 
2.39.2



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