[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
Cc: Zhiguang Liu
Cc: Dandan Bi
Signed-off-by: Dhaval Sharma
---
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 (#111449): https://edk2.groups.io/g/devel/message/111449
Mute This Topic: https://groups.io/mt/102702076/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[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
Cc: Zhiguang Liu
Cc: Dandan Bi
Signed-off-by: Dhaval Sharma
---
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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
Thanks for update! Change looks good to me.
Acked-by: Chasel Chiu
> -Original Message-
> From: 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 ; Liu, Zhiguang
> ; Bi, Dandan
> 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
> Cc: Zhiguang Liu
> Cc: Dandan Bi
> Signed-off-by: Dhaval Sharma
> ---
>
> 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+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.c...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
On Mon, Nov 20, 2023 at 4:24 AM Dhaval Sharma 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
> Cc: Zhiguang Liu
> Cc: Dandan Bi
> Signed-off-by: Dhaval Sharma
> ---
>
> 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
