It looks good to me.
Reviewed-by: Guo Dong <guo.dong@intel.com>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, September 17, 2019 12:50 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: You, Benjamin <benjamin.you@intel.com>; Dong, Guo
> <guo.dong@intel.com>; Ma, Maurice <maurice.ma@intel.com>
> Subject: [PATCH 34/35] UefiPayloadPkg/BlSupportPei: fix MMCONFIG
> assignment from XSDT
>
> (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.you@intel.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@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_HE
> ADER *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_H
> EADER *)(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_H
> EADER
> + *)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_H
> EADER *)(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_H
> EADER
> + *)Signature;
> DEBUG ((DEBUG_INFO, "Found MM config address in Xsdt\n"));
> }
>
> --
> 2.19.1.3.g30247aa5d201
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47785): https://edk2.groups.io/g/devel/message/47785
Mute This Topic: https://groups.io/mt/34180239/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-