From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: philmd@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 26 Sep 2019 06:17:55 -0700 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F33191108 for ; Thu, 26 Sep 2019 13:17:54 +0000 (UTC) Received: by mail-wm1-f70.google.com with SMTP id s25so1031387wmh.1 for ; Thu, 26 Sep 2019 06:17:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=q1aZL6TKtT+rVXEuKgHT5rngFu16Ut8z7YihgVJ6F9U=; b=AeEdU9OAScrmNzLUOIwd8mYZuTymUz0ugr5DF9hbMZjnhKXscnEVQx/ChYO12UNF6D gHvHHUjDiml2VoFpnDM13A6tbO67JA9+c5iRdpLpNV2EHzDHgSgNWX1jolHmW8jGY86l UzqZ9js1NtAfMiAQnIAPy4In5KMGecHPEVNO4IKMXsYEc3m2bifZmZT4q2+LyWbKynq/ HbSYjUhG2alfUK2FosaJMR2PMNXc6IwmwsjPSbm6OrTrhyRbjviXH+RPZEZUewHAinjg xq7T1r22dgL7vsd1pt9LzHDESC3Lz7K92Ga3KvhIw4FSfCiEE1ueTuo/UtOyz5U67YfI 97fg== X-Gm-Message-State: APjAAAVd7eerg7+wISaPY4RG6TYlcjUpZnQwunMyLaHCtMWwQOTJ+VJV Upye83qb7/c/wkpxq/vyukUkvoWKm7VkqbPuarkgByX2j21BcZn+pOK7jDsSr4ALmxFaiZ6LDYE L3ELv60k/dAXrOw== X-Received: by 2002:a5d:61d1:: with SMTP id q17mr1076864wrv.53.1569503873689; Thu, 26 Sep 2019 06:17:53 -0700 (PDT) X-Google-Smtp-Source: APXvYqxikfz5i18ZghKbh8lSl7GnraWM0xV4/CUibYNqpOX9HTGRa+tAW0LvfWLmF/6dThBrRoB36Q== X-Received: by 2002:a5d:61d1:: with SMTP id q17mr1076844wrv.53.1569503873480; Thu, 26 Sep 2019 06:17:53 -0700 (PDT) Received: from [192.168.1.35] (240.red-88-21-68.staticip.rima-tde.net. [88.21.68.240]) by smtp.gmail.com with ESMTPSA id 33sm5430183wra.41.2019.09.26.06.17.51 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 26 Sep 2019 06:17:52 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 34/35] UefiPayloadPkg/BlSupportPei: fix MMCONFIG assignment from XSDT To: devel@edk2.groups.io, lersek@redhat.com Cc: Benjamin You , Guo Dong , Maurice Ma References: <20190917194935.24322-1-lersek@redhat.com> <20190917194935.24322-35-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: Date: Thu, 26 Sep 2019 15:17:51 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <20190917194935.24322-35-lersek@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Cc: Guo Dong > Cc: Maurice Ma > Signed-off-by: Laszlo Ersek > --- > > 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 > + 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")); > } > >