[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] -=-=-=-=-=-=-=-=-=-=-=-