On 9/17/19 9:49 PM, Laszlo Ersek wrote:
> (This patch is unrelated to the rest of this series; its purpose is to
> enable building the UefiPayloadPkg DSC files with GCC.)
>
> When building "UefiPayloadPkg/UefiPayloadPkgIa32.dsc" with GCC48 for the
> DEBUG target, the compiler reports that "Entry32" may be used
> uninitialized in ParseAcpiInfo(), in the XSDT branch.
>
> Code inspection proves the compiler right. In the XSDT branch, the code
> from the RSDT branch must have been duplicated, and "Entry32" references
> were replaced with "Entry64" -- except where "MmCfgHdr" is assigned.
>
> Fix this bug by introducing a helper variable called "Signature", so that
> we have to refer to "Entry32" or "Entry64" only once per loop body.
>
> Cc: Benjamin You <benjamin....@intel.com>
> Cc: Guo Dong <guo.d...@intel.com>
> Cc: Maurice Ma <maurice...@intel.com>
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
>
> Notes:
> build-tested only
>
> UefiPayloadPkg/BlSupportPei/BlSupportPei.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
> b/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
> index 90433b609f22..22972453117a 100644
> --- a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
> +++ b/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
> @@ -164,6 +164,7 @@ ParseAcpiInfo (
> UINT64 *Entry64;
> UINTN Entry64Num;
> UINTN Idx;
> + UINT32 *Signature;
> EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *MmCfgHdr;
>
> EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE
> *MmCfgBase;
>
> @@ -181,13 +182,14 @@ ParseAcpiInfo (
> Entry32 = (UINT32 *)(Rsdt + 1);
> Entry32Num = (Rsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) >> 2;
> for (Idx = 0; Idx < Entry32Num; Idx++) {
> - if (*(UINT32 *)(UINTN)(Entry32[Idx]) ==
> EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
> - Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> *)(UINTN)(Entry32[Idx]);
> + Signature = (UINT32 *)(UINTN)Entry32[Idx];
> + if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE)
> {
> + Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
> DEBUG ((DEBUG_INFO, "Found Fadt in Rsdt\n"));
> }
>
> - if (*(UINT32 *)(UINTN)(Entry32[Idx]) ==
> EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE)
> {
> - MmCfgHdr =
> (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER
> *)(UINTN)(Entry32[Idx]);
> + if (*Signature ==
> EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE)
> {
> + MmCfgHdr =
> (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature;
> DEBUG ((DEBUG_INFO, "Found MM config address in Rsdt\n"));
> }
>
> @@ -205,13 +207,14 @@ ParseAcpiInfo (
> Entry64 = (UINT64 *)(Xsdt + 1);
> Entry64Num = (Xsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) >> 3;
> for (Idx = 0; Idx < Entry64Num; Idx++) {
> - if (*(UINT32 *)(UINTN)(Entry64[Idx]) ==
> EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
> - Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> *)(UINTN)(Entry64[Idx]);
> + Signature = (UINT32 *)(UINTN)Entry64[Idx];
> + if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE)
> {
> + Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
> DEBUG ((DEBUG_INFO, "Found Fadt in Xsdt\n"));
> }
>
> - if (*(UINT32 *)(UINTN)(Entry64[Idx]) ==
> EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE)
> {
> - MmCfgHdr =
> (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER
> *)(UINTN)(Entry32[Idx]);
Ah! Thanks GCC.
Reviewed-by: Philippe Mathieu-Daude <phi...@redhat.com>
> + if (*Signature ==
> EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE)
> {
> + MmCfgHdr =
> (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature;
> DEBUG ((DEBUG_INFO, "Found MM config address in Xsdt\n"));
> }
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48108): https://edk2.groups.io/g/devel/message/48108
Mute This Topic: https://groups.io/mt/34180239/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
