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

["Pedro Falcato" <pedro.falcato@gmail.com>]

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