It looks good to me.
Reviewed-by: Guo Dong <guo.d...@intel.com> > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Tuesday, September 17, 2019 12:50 PM > To: edk2-devel-groups-io <devel@edk2.groups.io> > Cc: You, Benjamin <benjamin....@intel.com>; Dong, Guo > <guo.d...@intel.com>; Ma, Maurice <maurice...@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....@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_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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
