* [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
2023-11-20 4:22 [edk2-devel] [PATCH v3 0/1] Add support for XDSDT table Dhaval Sharma
@ 2023-11-20 4:22 ` Dhaval Sharma
0 siblings, 0 replies; 6+ messages in thread
From: Dhaval Sharma @ 2023-11-20 4:22 UTC (permalink / raw)
To: devel; +Cc: Liming Gao, Zhiguang Liu, Dandan Bi
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 (#111449): https://edk2.groups.io/g/devel/message/111449
Mute This Topic: https://groups.io/mt/102702076/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [edk2-devel] [PATCH v3 0/1] Add support for XDSDT table
@ 2023-11-20 4:24 Dhaval Sharma
2023-11-20 4:24 ` [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation Dhaval Sharma
0 siblings, 1 reply; 6+ messages in thread
From: Dhaval Sharma @ 2023-11-20 4:24 UTC (permalink / raw)
To: devel
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
2023-11-20 4:24 [edk2-devel] [PATCH v3 0/1] Add support for XDSDT table Dhaval Sharma
@ 2023-11-20 4:24 ` Dhaval Sharma
2023-11-28 18:41 ` Chiu, Chasel
2023-11-29 7:34 ` Pedro Falcato
0 siblings, 2 replies; 6+ messages in thread
From: Dhaval Sharma @ 2023-11-20 4:24 UTC (permalink / raw)
To: devel; +Cc: Liming Gao, Zhiguang Liu, Dandan Bi
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
2023-11-20 4:24 ` [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation Dhaval Sharma
@ 2023-11-28 18:41 ` Chiu, Chasel
2023-11-29 7:34 ` Pedro Falcato
1 sibling, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2023-11-28 18:41 UTC (permalink / raw)
To: devel@edk2.groups.io, dhaval@rivosinc.com
Cc: Gao, Liming, Liu, Zhiguang, Bi, Dandan
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
2023-11-20 4:24 ` [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation Dhaval Sharma
2023-11-28 18:41 ` Chiu, Chasel
@ 2023-11-29 7:34 ` Pedro Falcato
2023-11-29 14:25 ` Gerd Hoffmann
1 sibling, 1 reply; 6+ messages in thread
From: Pedro Falcato @ 2023-11-29 7:34 UTC (permalink / raw)
To: devel, dhaval; +Cc: Liming Gao, Zhiguang Liu, Dandan Bi, Gerd Hoffmann
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation
2023-11-29 7:34 ` Pedro Falcato
@ 2023-11-29 14:25 ` Gerd Hoffmann
0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2023-11-29 14:25 UTC (permalink / raw)
To: Pedro Falcato; +Cc: devel, dhaval, Liming Gao, Zhiguang Liu, Dandan Bi
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-29 14:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-20 4:24 [edk2-devel] [PATCH v3 0/1] Add support for XDSDT table Dhaval Sharma
2023-11-20 4:24 ` [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation Dhaval Sharma
2023-11-28 18:41 ` Chiu, Chasel
2023-11-29 7:34 ` Pedro Falcato
2023-11-29 14:25 ` Gerd Hoffmann
-- strict thread matches above, loose matches on Subject: below --
2023-11-20 4:22 [edk2-devel] [PATCH v3 0/1] Add support for XDSDT table Dhaval Sharma
2023-11-20 4:22 ` [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation Dhaval Sharma
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox