Re: [edk2-devel] [edk2-platforms PATCH 1/2] WhitleyOpenBoardPkg: remove <LegacyBiosMpTable.h> references

["Laszlo Ersek" <lersek@redhat.com>]

Hi Chasel,

On 11/10/23 02:13, Chiu, Chasel wrote:
> 
> Hi Laszlo,
> 
> I verified and encountered build failure as some files still consuming definitions from LegacyBiosMpTable.h, for example:
> https://github.com/tianocore/edk2-platforms/blob/899a9dc97cd54690513380ad01ee8b2609dbefd5/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/Common/Dxe/SystemBoardInfoDxe/SystemBoardInfoDxe.c#L22
> 
> Any suggestion that we can reduce impact to existing platforms?

thanks for testing the build!

Here's my problem: this information seems to be exposed
firmware-globally, ultimately. We have a UbaConfigProtocol->AddData()
call, which exposes SystemBoardInfoData. SystemBoardInfoData exposes
SystemBoardInfoCallback(). That one in turn exposes SystemBoardInfoTable
(of type DXE_SYSTEM_BOARD_INFO).

And so on and so on, and ultimately we publicly expose
DEVICE_DATA_HW_LOCAL_INT.

That structure type is defined in
"Platform/Intel/WhitleyOpenBoardPkg/Include/PlatDevData.h", and it only
has the following comment:

"Platform hardwired data describing connection of interrupt sources to
local APICs"

So I can't tell if this structure type officially and explicitly defers
to the MP Table specification, or just ad-hoc reuses the same values.

In the former case, we cannot remove LegacyBiosMpTable.h from edk2 (or
perhaps we need to move it over to edk2-platforms).

In the latter case, we should just replace the enum constants with their
integer values (or perhaps replace them with some different macros, like
ACPI spec macros or similar), and then we can delete LegacyBiosMpTable.h.

That is, we need to figure out where the DEVICE_DATA_HW_LOCAL_INT type
definition originates from.

The particular "DeviceDataHwLocalInt1" array comes from edk2-platforms
commit 3584efd25110 ("WhitleyOpenBoardPkg: Add UBA Modules",
2021-07-14). The commit message is empty -- so it's a dead-end.

The type definition comes from edk2-platforms commit 41bfa85f527a
("WhitleyOpenBoardPkg: Add Includes and Libraries", 2021-07-14). The
commit message is similarly empty.

Laszlo

> 
> Thanks,
> Chasel
> 
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
>> Sent: Thursday, November 9, 2023 4:06 AM
>> To: devel@edk2.groups.io
>> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Desimone, Nathaniel
>> L <nathaniel.l.desimone@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
>> Subject: [edk2-devel] [edk2-platforms PATCH 1/2] WhitleyOpenBoardPkg:
>> remove <LegacyBiosMpTable.h> references
>>
>> For removing "MdePkg/Include/IndustryStandard/LegacyBiosMpTable.h" from
>> edk2, first remove the edk2-platforms references to that header file.
>>
>> I can't build-test this change. As far as I can tell, building the CooperCityRvp and
>> WilsonCityRvp platforms with "build_bios.py" should build these changes;
>> however, both platforms fail to build without FSP blobs.
>>
>> I think there's a fair chance that this patch should work nonetheless;
>> <LegacyBiosMpTable.h> introduces names prefixed with
>> EFI_LEGACY_MP_TABLE_, and edk2-platforms doesn't contain that string. (The
>> one exception is FEATUREBYTE2_5, which is also absent from edk2-platforms.)
>>
>> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
>> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
>> Cc: Chasel Chiu <chasel.chiu@intel.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1754
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/Common/Dxe/SystemBoar
>> dInfoDxe/SystemBoardInfoDxe.h | 1 -
>>
>> Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/StaticSkuDataDxe/Platform
>> DeviceDataSRP10nm.c       | 1 -
>>  2 files changed, 2 deletions(-)
>>
>> diff --git
>> a/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/Common/Dxe/SystemBo
>> ardInfoDxe/SystemBoardInfoDxe.h
>> b/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/Common/Dxe/SystemBo
>> ardInfoDxe/SystemBoardInfoDxe.h
>> index 32c16ff9110a..d8c209a57f75 100644
>> ---
>> a/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/Common/Dxe/SystemBo
>> ardInfoDxe/SystemBoardInfoDxe.h
>> +++
>> b/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/Common/Dxe/SystemBo
>> +++ ardInfoDxe/SystemBoardInfoDxe.h
>> @@ -27,7 +27,6 @@
>>  #include <Platform.h>
>>  #include <Ppi/PchPolicy.h>
>>
>> -#include <IndustryStandard/LegacyBiosMpTable.h>
>>  #include <UncoreCommonIncludes.h>
>>
>>  #endif  //_SYSTEM_BOARD_INFO_DXE_H_
>> diff --git
>> a/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/StaticSkuDataDxe/Platfor
>> mDeviceDataSRP10nm.c
>> b/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/StaticSkuDataDxe/Platfor
>> mDeviceDataSRP10nm.c
>> index ed9f80734cd7..b69ae1736bb8 100644
>> ---
>> a/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/StaticSkuDataDxe/Platfor
>> mDeviceDataSRP10nm.c
>> +++ b/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/StaticSkuDataDxe/Pl
>> +++ atformDeviceDataSRP10nm.c
>> @@ -8,7 +8,6 @@
>>
>>  #include <PlatPirqData.h>
>>  #include <PlatDevData.h>
>> -#include <IndustryStandard/LegacyBiosMpTable.h>
>>
>>  #ifndef V_INTEL_VID
>>  #define V_INTEL_VID               0x8086
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111132): https://edk2.groups.io/g/devel/message/111132
Mute This Topic: https://groups.io/mt/102483850/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-