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

["Ni, Ray" <ray.ni@intel.com>]

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
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > >
> > >
> > >
> > > 
> > >