Re: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure

["Paweł Poławski" <ppolawsk@redhat.com>]

Hi everyone,

If there is a chance you have some evaluation about this problem already?

Best regards,
Pawel

W dniu 21.12.2022 o 09:03, Ni, Ray pisze:
> sure Mike. I will.
> 
>> -----Original Message-----
>> From: Kinney, Michael D <michael.d.kinney@intel.com>
>> Sent: Wednesday, December 21, 2022 11:51 AM
>> To: devel@edk2.groups.io; ppolawsk@redhat.com; Ni, Ray <ray.ni@intel.com>; Zimmer, Vincent
>> <vincent.zimmer@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Subject: RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure
>>
>> Hi Ray,
>>
>> Can you please help investigate/evaluate options to solve this problem?
>>
>> Thanks,
>>
>> Mike
>>
>>> -----Original Message-----
>>> From: Kinney, Michael D <michael.d.kinney@intel.com>
>>> Sent: Tuesday, December 13, 2022 1:40 PM
>>> To: devel@edk2.groups.io; ppolawsk@redhat.com; Ni, Ray <ray.ni@intel.com>; Zimmer, Vincent
>> <vincent.zimmer@intel.com>;
>>> Kinney, Michael D <michael.d.kinney@intel.com>
>>> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>>> Subject: RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure
>>>
>>> Hi Pawel,
>>>
>>> Let's add Ray and Vincent to this topic to help pick the best option.
>>>
>>> If everything is rebuilt from sources, then just removing the union
>>> member will work.
>>>
>>> The challenge is that the non-Itanium CPU archs that may depend on the
>>> current binary layout of these structures.  This could be a binary
>>> PEI CPU module, DXE CPU module, SMM CPU module, MM CPU modules,
>>> UEFI App/Shell App that use the PPI or HOB.
>>>
>>> Even if we define a new HOB format, we have to decide when it is
>>> used to support compatibility.  Perhaps always keep the current
>>> logic if < 1024 CPUs.  If number of CPUs >= 1024, then produce
>>> the new format of CPU information and update all consumers to
>>> look for new format and use that with higher priority than the
>>> old format.
>>>
>>> Another option is to keep the current format and allow multiple
>>> HOBs to be produced if the CPU information does not fit in the
>>> single HOB size limit of 64KB.  Then update all consumers to
>>> look for 1 or more HOBs to collect all the information.  This
>>> approach removes the CPU number limit as long as there is enough
>>> temp RAM for the multiple HOBs.
>>>
>>> Best regards,
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pawel Polawski
>>>> Sent: Monday, December 12, 2022 9:18 PM
>>>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
>>>> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>>>> Subject: Re: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure
>>>>
>>>> Hi Mike
>>>>
>>>> If there is a chance you were able to check if I should introduce new
>>>> PPI with a new GUID for my pull request? Or we can push forward with
>>>> exiting version?
>>>>
>>>> In case of some changes needed - I could start implementing them right
>>>> away so we could try to process everything before Christmas break.
>>>>
>>>> Let me know what is your opinion.
>>>>
>>>> Best regards,
>>>> Pawel
>>>>
>>>> W dniu 1.12.2022 o 21:23, Paweł Poławski pisze:
>>>>> Hi Mike,
>>>>>
>>>>> Thank you for the fast response.
>>>>> I will be waiting for your guidance on this topic.
>>>>>
>>>>> Btw - I am too new in the EDK2 world to witness this Itanium
>>>>> deprecation back in 2019. How such process looks like?
>>>>> If it starts with update in documentation followed by the changes
>>>>> in the code? How long is the intermediate part where both - new and old
>>>>> functionality needs to be supported?
>>>>>
>>>>> Best regards,
>>>>> Pawel
>>>>>
>>>>>
>>>>> W dniu 1.12.2022 o 18:01, Michael D Kinney pisze:
>>>>>> Hi Pawel,
>>>>>>
>>>>>> Thank you for the patch.  I agree this fixes the size issue.
>>>>>>
>>>>>> However, this structure and the associated PPI are defined in the PI Spec
>>>>>> and other modules may depend on the structure layout and would have to be
>>>>>> rebuilt after this structure change.
>>>>>>
>>>>>> It would be better to have a backwards compatible change here and I do
>>>>>> not have a specific proposal yet that both reduces the size and
>>>>>> preserves backwards compatibility.
>>>>>>
>>>>>> There have been examples in the past where we have had to introduce
>>>>>> a new version of a PPI with a new GUID for this type of change.
>>>>>>
>>>>>> I will get back to you on this topic in a few days.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Mike
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Paweł Poławski <ppolawsk@redhat.com>
>>>>>>> Sent: Thursday, December 1, 2022 7:36 AM
>>>>>>> To: devel@edk2.groups.io
>>>>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>>>>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang
>>>>>>> <zhiguang.liu@intel.com>
>>>>>>> Subject: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover
>>>>>>> data structure
>>>>>>>
>>>>>>> Itanium support has been removed from EDK2 aroun 2019.
>>>>>>> ITANIUM_HANDOFF_STATUS data structure looks to be
>>>>>>> some leftover from that process.
>>>>>>>
>>>>>>> There is also positive sidefect of this data structure removal.
>>>>>>> Due to HOB allocation type used in PEI stage there is a limit
>>>>>>> how much data about virtual CPU can be hold. This limit result
>>>>>>> in only 1024 vCPU can be used by VM.
>>>>>>> With Itanium related data structure removed more allocated space
>>>>>>> can be used for vCPU data and with current allocation limit
>>>>>>> will change from 1024 to around 8k vCPUs.
>>>>>>>
>>>>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>>>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>>>>>>>
>>>>>>> Signed-off-by: Paweł Poławski <ppolawsk@redhat.com>
>>>>>>> ---
>>>>>>>    MdePkg/Include/Ppi/SecPlatformInformation.h | 44 --------------------
>>>>>>>    1 file changed, 44 deletions(-)
>>>>>>>
>>>>>>> diff --git a/MdePkg/Include/Ppi/SecPlatformInformation.h
>>>>>>> b/MdePkg/Include/Ppi/SecPlatformInformation.h
>>>>>>> index 02b0711f189e..fbcd205acd96 100644
>>>>>>> --- a/MdePkg/Include/Ppi/SecPlatformInformation.h
>>>>>>> +++ b/MdePkg/Include/Ppi/SecPlatformInformation.h
>>>>>>> @@ -84,49 +84,6 @@ typedef union {
>>>>>>>
>>>>>>>    typedef EFI_HEALTH_FLAGS X64_HANDOFF_STATUS;
>>>>>>>    typedef EFI_HEALTH_FLAGS IA32_HANDOFF_STATUS;
>>>>>>> -///
>>>>>>> -/// The hand-off status structure for Itanium architecture.
>>>>>>> -///
>>>>>>> -typedef struct {
>>>>>>> -  ///
>>>>>>> -  /// SALE_ENTRY state : 3 = Recovery_Check
>>>>>>> -  /// and 0 = RESET or Normal_Boot phase.
>>>>>>> -  ///
>>>>>>> -  UINT8     BootPhase;
>>>>>>> -  ///
>>>>>>> -  /// Firmware status on entry to SALE.
>>>>>>> -  ///
>>>>>>> -  UINT8     FWStatus;
>>>>>>> -  UINT16    Reserved1;
>>>>>>> -  UINT32    Reserved2;
>>>>>>> -  ///
>>>>>>> -  /// Geographically significant unique processor ID assigned by PAL.
>>>>>>> -  ///
>>>>>>> -  UINT16    ProcId;
>>>>>>> -  UINT16    Reserved3;
>>>>>>> -  UINT8     IdMask;
>>>>>>> -  UINT8     EidMask;
>>>>>>> -  UINT16    Reserved4;
>>>>>>> -  ///
>>>>>>> -  /// Address to make PAL calls.
>>>>>>> -  ///
>>>>>>> -  UINT64    PalCallAddress;
>>>>>>> -  ///
>>>>>>> -  /// If the entry state is RECOVERY_CHECK, this contains the PAL_RESET
>>>>>>> -  /// return address, and if entry state is RESET, this contains
>>>>>>> -  /// address for PAL_authentication call.
>>>>>>> -  ///
>>>>>>> -  UINT64    PalSpecialAddress;
>>>>>>> -  ///
>>>>>>> -  /// GR35 from PALE_EXIT state.
>>>>>>> -  ///
>>>>>>> -  UINT64    SelfTestStatus;
>>>>>>> -  ///
>>>>>>> -  /// GR37 from PALE_EXIT state.
>>>>>>> -  ///
>>>>>>> -  UINT64    SelfTestControl;
>>>>>>> -  UINT64    MemoryBufferRequired;
>>>>>>> -} ITANIUM_HANDOFF_STATUS;
>>>>>>>
>>>>>>>    ///
>>>>>>>    /// EFI_SEC_PLATFORM_INFORMATION_RECORD.
>>>>>>> @@ -134,7 +91,6 @@ typedef struct {
>>>>>>>    typedef union {
>>>>>>>      IA32_HANDOFF_STATUS       IA32HealthFlags;
>>>>>>>      X64_HANDOFF_STATUS        x64HealthFlags;
>>>>>>> -  ITANIUM_HANDOFF_STATUS    ItaniumHealthFlags;
>>>>>>>    } EFI_SEC_PLATFORM_INFORMATION_RECORD;
>>>>>>>
>>>>>>>    /**
>>>>>>> --
>>>>>>> 2.38.1
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
> 
> 
> 
> 
> 
>