PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure

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

Posted by Paweł Poławski 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96834): https://edk2.groups.io/g/devel/message/96834
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 1 month ago

Hi Pawel,

With suggestion from Vincent, let's remove ItaniumHealthFlags.

https://edk2.groups.io/g/devel/message/100812

I recommend adding a note in the comment block for the 
EFI_SEC_PLATFORM_INFORMATION_RECORD that with the removal of the 
ItaniumHealthFlags, this union has diverged from the PI
Specification definition.

With that one comment addition:

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100813): https://edk2.groups.io/g/devel/message/100813
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 1 month ago

Hi Pawel,

Can you please also reference the following BZ and commit in the commit message

https://bugzilla.tianocore.org/show_bug.cgi?id=1560
https://github.com/tianocore/edk2/commit/4e1daa60f5372c22a11503961061ffa569eaf873

If you post a branch with these small updates and my Rb and send it
through EDK II CI, I can do the push from there.

Thanks,

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, March 7, 2023 1:59 PM
> To: Paweł Poławski <ppolawsk@redhat.com>; devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent
> <vincent.zimmer@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 Pawel,
> 
> With suggestion from Vincent, let's remove ItaniumHealthFlags.
> 
> https://edk2.groups.io/g/devel/message/100812
> 
> I recommend adding a note in the comment block for the
> EFI_SEC_PLATFORM_INFORMATION_RECORD that with the removal of the
> ItaniumHealthFlags, this union has diverged from the PI
> Specification definition.
> 
> With that one comment addition:
> 
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100815): https://edk2.groups.io/g/devel/message/100815
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 1 month ago

Hi Pawel,

Any updates on this task?

I would like to see this go in soon so it can get some flight time before next stable tag.

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, March 7, 2023 2:25 PM
> To: Paweł Poławski <ppolawsk@redhat.com>; devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent <vincent.zimmer@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 Pawel,
> 
> Can you please also reference the following BZ and commit in the commit message
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1560
> https://github.com/tianocore/edk2/commit/4e1daa60f5372c22a11503961061ffa569eaf873
> 
> If you post a branch with these small updates and my Rb and send it
> through EDK II CI, I can do the push from there.
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Tuesday, March 7, 2023 1:59 PM
> > To: Paweł Poławski <ppolawsk@redhat.com>; devel@edk2.groups.io
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent
> > <vincent.zimmer@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 Pawel,
> >
> > With suggestion from Vincent, let's remove ItaniumHealthFlags.
> >
> > https://edk2.groups.io/g/devel/message/100812
> >
> > I recommend adding a note in the comment block for the
> > EFI_SEC_PLATFORM_INFORMATION_RECORD that with the removal of the
> > ItaniumHealthFlags, this union has diverged from the PI
> > Specification definition.
> >
> > With that one comment addition:
> >
> > Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101709): https://edk2.groups.io/g/devel/message/101709
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Paweł Poławski 1 year, 1 month ago

Hi Michael,

Sorry for the delay in my response. I just opened PR against EDK2 repo to
trigger CI:
https://github.com/tianocore/edk2/pull/4174

Best regards,
Pawel

On Thu, Mar 23, 2023 at 11:10 PM Kinney, Michael D <
michael.d.kinney@intel.com> wrote:

> Hi Pawel,
>
> Any updates on this task?
>
> I would like to see this go in soon so it can get some flight time before
> next stable tag.
>
> Mike
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Tuesday, March 7, 2023 2:25 PM
> > To: Paweł Poławski <ppolawsk@redhat.com>; devel@edk2.groups.io
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <
> zhiguang.liu@intel.com>; Zimmer, Vincent <vincent.zimmer@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 Pawel,
> >
> > Can you please also reference the following BZ and commit in the commit
> message
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1560
> >
> https://github.com/tianocore/edk2/commit/4e1daa60f5372c22a11503961061ffa569eaf873
> >
> > If you post a branch with these small updates and my Rb and send it
> > through EDK II CI, I can do the push from there.
> >
> > Thanks,
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Tuesday, March 7, 2023 1:59 PM
> > > To: Paweł Poławski <ppolawsk@redhat.com>; devel@edk2.groups.io
> > > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <
> zhiguang.liu@intel.com>; Zimmer, Vincent
> > > <vincent.zimmer@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 Pawel,
> > >
> > > With suggestion from Vincent, let's remove ItaniumHealthFlags.
> > >
> > > https://edk2.groups.io/g/devel/message/100812
> > >
> > > I recommend adding a note in the comment block for the
> > > EFI_SEC_PLATFORM_INFORMATION_RECORD that with the removal of the
> > > ItaniumHealthFlags, this union has diverged from the PI
> > > Specification definition.
> > >
> > > With that one comment addition:
> > >
> > > Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > 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
>
>

-- 

Paweł Poławski

Red Hat <https://www.redhat.com/> Virtualization

ppolawsk@redhat.com
@RedHat <https://twitter.com/redhat>   Red Hat
<https://www.linkedin.com/company/red-hat>  Red Hat
<https://www.facebook.com/RedHatInc>
<https://red.ht/sig>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101788): https://edk2.groups.io/g/devel/message/101788
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 1 month ago

Thanks Pawel,

I see there were some minor CI issues related to uncrustify formatting.

I am fixing those and will post a new PR with my Rb.

I will close 4174.

Mike

From: Pawel Polawski <ppolawsk@redhat.com>
Sent: Friday, March 24, 2023 7:46 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>
Subject: Re: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure

Hi Michael,

Sorry for the delay in my response. I just opened PR against EDK2 repo to trigger CI:
https://github.com/tianocore/edk2/pull/4174

Best regards,
Pawel

On Thu, Mar 23, 2023 at 11:10 PM Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Pawel,

Any updates on this task?

I would like to see this go in soon so it can get some flight time before next stable tag.

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Sent: Tuesday, March 7, 2023 2:25 PM
> To: Paweł Poławski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Zimmer, Vincent <vincent.zimmer@intel.com<mailto:vincent.zimmer@intel.com>>;
> Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Subject: RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure
>
> Hi Pawel,
>
> Can you please also reference the following BZ and commit in the commit message
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1560
> https://github.com/tianocore/edk2/commit/4e1daa60f5372c22a11503961061ffa569eaf873
>
> If you post a branch with these small updates and my Rb and send it
> through EDK II CI, I can do the push from there.
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Sent: Tuesday, March 7, 2023 1:59 PM
> > To: Paweł Poławski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Zimmer, Vincent
> > <vincent.zimmer@intel.com<mailto:vincent.zimmer@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Subject: RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure
> >
> > Hi Pawel,
> >
> > With suggestion from Vincent, let's remove ItaniumHealthFlags.
> >
> > https://edk2.groups.io/g/devel/message/100812
> >
> > I recommend adding a note in the comment block for the
> > EFI_SEC_PLATFORM_INFORMATION_RECORD that with the removal of the
> > ItaniumHealthFlags, this union has diverged from the PI
> > Specification definition.
> >
> > With that one comment addition:
> >
> > Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> >
> > Mike
> >
> >
> > > -----Original Message-----
> > > From: Paweł Poławski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>
> > > Sent: Thursday, December 1, 2022 7:36 AM
> > > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang
> > > <zhiguang.liu@intel.com<mailto: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<mailto:michael.d.kinney@intel.com>>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
> > >
> > > Signed-off-by: Paweł Poławski <ppolawsk@redhat.com<mailto: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


--

Paweł Poławski

Red Hat<https://www.redhat.com/> Virtualization

ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>
@RedHat<https://twitter.com/redhat>   Red Hat<https://www.linkedin.com/company/red-hat>  Red Hat<https://www.facebook.com/RedHatInc>
[https://static.redhat.com/libs/redhat/brand-assets/latest/corp/logo.png]<https://red.ht/sig>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101889): https://edk2.groups.io/g/devel/message/101889
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 1 month ago

Merged: https://github.com/tianocore/edk2/pull/4181
Commit: https://github.com/tianocore/edk2/commit/07fa6d1841abcfc58b226027d711ebb47559f442

Mike

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Saturday, March 25, 2023 6:05 PM
To: Pawel Polawski <ppolawsk@redhat.com>
Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure

Thanks Pawel,

I see there were some minor CI issues related to uncrustify formatting.

I am fixing those and will post a new PR with my Rb.

I will close 4174.

Mike

From: Pawel Polawski <ppolawsk@redhat.com>
Sent: Friday, March 24, 2023 7:46 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>
Subject: Re: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure

Hi Michael,

Sorry for the delay in my response. I just opened PR against EDK2 repo to trigger CI:
https://github.com/tianocore/edk2/pull/4174

Best regards,
Pawel

On Thu, Mar 23, 2023 at 11:10 PM Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Pawel,

Any updates on this task?

I would like to see this go in soon so it can get some flight time before next stable tag.

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Sent: Tuesday, March 7, 2023 2:25 PM
> To: Paweł Poławski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Zimmer, Vincent <vincent.zimmer@intel.com<mailto:vincent.zimmer@intel.com>>;
> Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Subject: RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure
>
> Hi Pawel,
>
> Can you please also reference the following BZ and commit in the commit message
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1560
> https://github.com/tianocore/edk2/commit/4e1daa60f5372c22a11503961061ffa569eaf873
>
> If you post a branch with these small updates and my Rb and send it
> through EDK II CI, I can do the push from there.
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Sent: Tuesday, March 7, 2023 1:59 PM
> > To: Paweł Poławski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Zimmer, Vincent
> > <vincent.zimmer@intel.com<mailto:vincent.zimmer@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Subject: RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover data structure
> >
> > Hi Pawel,
> >
> > With suggestion from Vincent, let's remove ItaniumHealthFlags.
> >
> > https://edk2.groups.io/g/devel/message/100812
> >
> > I recommend adding a note in the comment block for the
> > EFI_SEC_PLATFORM_INFORMATION_RECORD that with the removal of the
> > ItaniumHealthFlags, this union has diverged from the PI
> > Specification definition.
> >
> > With that one comment addition:
> >
> > Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> >
> > Mike
> >
> >
> > > -----Original Message-----
> > > From: Paweł Poławski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>
> > > Sent: Thursday, December 1, 2022 7:36 AM
> > > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang
> > > <zhiguang.liu@intel.com<mailto: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<mailto:michael.d.kinney@intel.com>>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
> > >
> > > Signed-off-by: Paweł Poławski <ppolawsk@redhat.com<mailto: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


--

Paweł Poławski

Red Hat<https://www.redhat.com/> Virtualization

ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>
@RedHat<https://twitter.com/redhat>   Red Hat<https://www.linkedin.com/company/red-hat>  Red Hat<https://www.facebook.com/RedHatInc>
[https://static.redhat.com/libs/redhat/brand-assets/latest/corp/logo.png]<https://red.ht/sig>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101890): https://edk2.groups.io/g/devel/message/101890
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Paweł Poławski 1 year ago

Hi Mike,

Thank you for taking care of this.
Also thanks to everyone involved in the discussion on how to address this
issue.

Best regards,
Pawel

On Sun, Mar 26, 2023 at 4:04 AM Michael D Kinney <michael.d.kinney@intel.com>
wrote:

> Merged: https://github.com/tianocore/edk2/pull/4181
>
> Commit:
> https://github.com/tianocore/edk2/commit/07fa6d1841abcfc58b226027d711ebb47559f442
>
>
>
> Mike
>
>
>
> *From:* Kinney, Michael D <michael.d.kinney@intel.com>
> *Sent:* Saturday, March 25, 2023 6:05 PM
> *To:* Pawel Polawski <ppolawsk@redhat.com>
> *Cc:* devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent <
> vincent.zimmer@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> *Subject:* RE: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover
> data structure
>
>
>
> Thanks Pawel,
>
>
>
> I see there were some minor CI issues related to uncrustify formatting.
>
>
>
> I am fixing those and will post a new PR with my Rb.
>
>
>
> I will close 4174.
>
>
>
> Mike
>
>
>
> *From:* Pawel Polawski <ppolawsk@redhat.com>
> *Sent:* Friday, March 24, 2023 7:46 AM
> *To:* Kinney, Michael D <michael.d.kinney@intel.com>
> *Cc:* devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Zimmer, Vincent <
> vincent.zimmer@intel.com>
> *Subject:* Re: [edk2-devel] PATCH v1 1/1 MdePkg: Remove Itanium leftover
> data structure
>
>
>
> Hi Michael,
>
>
>
> Sorry for the delay in my response. I just opened PR against EDK2 repo to
> trigger CI:
>
> https://github.com/tianocore/edk2/pull/4174
>
>
>
> Best regards,
>
> Pawel
>
>
>
> On Thu, Mar 23, 2023 at 11:10 PM Kinney, Michael D <
> michael.d.kinney@intel.com> wrote:
>
> Hi Pawel,
>
> Any updates on this task?
>
> I would like to see this go in soon so it can get some flight time before
> next stable tag.
>
> Mike
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Tuesday, March 7, 2023 2:25 PM
> > To: Paweł Poławski <ppolawsk@redhat.com>; devel@edk2.groups.io
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <
> zhiguang.liu@intel.com>; Zimmer, Vincent <vincent.zimmer@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 Pawel,
> >
> > Can you please also reference the following BZ and commit in the commit
> message
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1560
> >
> https://github.com/tianocore/edk2/commit/4e1daa60f5372c22a11503961061ffa569eaf873
> >
> > If you post a branch with these small updates and my Rb and send it
> > through EDK II CI, I can do the push from there.
> >
> > Thanks,
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Tuesday, March 7, 2023 1:59 PM
> > > To: Paweł Poławski <ppolawsk@redhat.com>; devel@edk2.groups.io
> > > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <
> zhiguang.liu@intel.com>; Zimmer, Vincent
> > > <vincent.zimmer@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 Pawel,
> > >
> > > With suggestion from Vincent, let's remove ItaniumHealthFlags.
> > >
> > > https://edk2.groups.io/g/devel/message/100812
> > >
> > > I recommend adding a note in the comment block for the
> > > EFI_SEC_PLATFORM_INFORMATION_RECORD that with the removal of the
> > > ItaniumHealthFlags, this union has diverged from the PI
> > > Specification definition.
> > >
> > > With that one comment addition:
> > >
> > > Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > 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
>
>
>
> --
>
> *Paweł Poławski*
>
> Red Hat <https://www.redhat.com/> Virtualization
>
> ppolawsk@redhat.com
>
> @RedHat <https://twitter.com/redhat>   Red Hat
> <https://www.linkedin.com/company/red-hat>  Red Hat
> <https://www.facebook.com/RedHatInc>
>
> <https://red.ht/sig>
> 
>
>

-- 

Paweł Poławski

Red Hat <https://www.redhat.com/> Virtualization

ppolawsk@redhat.com
@RedHat <https://twitter.com/redhat>   Red Hat
<https://www.linkedin.com/company/red-hat>  Red Hat
<https://www.facebook.com/RedHatInc>
<https://red.ht/sig>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101984): https://edk2.groups.io/g/devel/message/101984
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96838): https://edk2.groups.io/g/devel/message/96838
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Paweł Poławski 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96840): https://edk2.groups.io/g/devel/message/96840
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Paweł Poławski 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97287): https://edk2.groups.io/g/devel/message/97287
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97321): https://edk2.groups.io/g/devel/message/97321
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97660): https://edk2.groups.io/g/devel/message/97660
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ni, Ray 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97678): https://edk2.groups.io/g/devel/message/97678
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Paweł Poławski 1 year, 3 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98237): https://edk2.groups.io/g/devel/message/98237
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ni, Ray 1 year, 3 months ago

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

The CPU health record is stored in PPI first by SecCore, then saved to HOB by CpuMpPei module.
Then CpuDxe gets the record from HOB.

If SecCore, CpuMpPei, CpuDxe are built with different structure definition,
the compatibility issue appears.

The very well know binary separation module today for x86 is Intel's FSP.
I am not sure if today customer might use a pre-built FSP binary which is built from
version #1 of edk2 while the SecCore and CpuDxe in platform binary are built from
a different version of edk2.
I don't know how AMD distributes the binary module. + Tom

If binary distribution is not adopted yet by industry, I prefer we just update the
structure definition.

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

I don't like this approach because it still breaks the case when CPU >=1024 and binary
distribution is used.

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

CpuDxe driver is one of the consumers.
1. Old CpuMpPei (in FSP binary) + new CpuDxe (In Platform binary)
     This doesn't work because CpuMpPei still cannot produce the huge-size HOB.
2. New CpuMpPei (in FSP binary) + old CpuDxe (In Platform binary)
     This doesn't work because old CpuDxe doesn't look for multiple HOBs.



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pawel Polawski
> Sent: Tuesday, January 10, 2023 4:19 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@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 everyone,
> 
> If there is a chance you have some evaluation about this problem already?
> 
> Best regards,
> Pawel
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98243): https://edk2.groups.io/g/devel/message/98243
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Lendacky, Thomas via groups.io 1 year, 3 months ago

+ Garret

On 1/10/23 03:36, Ni, Ray via groups.io wrote:
>>>>> 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.
> 
> The CPU health record is stored in PPI first by SecCore, then saved to HOB by CpuMpPei module.
> Then CpuDxe gets the record from HOB.
> 
> If SecCore, CpuMpPei, CpuDxe are built with different structure definition,
> the compatibility issue appears.
> 
> The very well know binary separation module today for x86 is Intel's FSP.
> I am not sure if today customer might use a pre-built FSP binary which is built from
> version #1 of edk2 while the SecCore and CpuDxe in platform binary are built from
> a different version of edk2.
> I don't know how AMD distributes the binary module. + Tom
> 
> If binary distribution is not adopted yet by industry, I prefer we just update the
> structure definition.
> 
>>>>>
>>>>> 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.
> 
> I don't like this approach because it still breaks the case when CPU >=1024 and binary
> distribution is used.
> 
>>>>>
>>>>> 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.
> 
> CpuDxe driver is one of the consumers.
> 1. Old CpuMpPei (in FSP binary) + new CpuDxe (In Platform binary)
>       This doesn't work because CpuMpPei still cannot produce the huge-size HOB.
> 2. New CpuMpPei (in FSP binary) + old CpuDxe (In Platform binary)
>       This doesn't work because old CpuDxe doesn't look for multiple HOBs.
> 
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pawel Polawski
>> Sent: Tuesday, January 10, 2023 4:19 PM
>> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent
>> <vincent.zimmer@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 everyone,
>>
>> If there is a chance you have some evaluation about this problem already?
>>
>> Best regards,
>> Pawel
>>
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98247): https://edk2.groups.io/g/devel/message/98247
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Paweł Poławski 1 year, 3 months ago

Hi all,

Just to let you know: I performed a test with this modified version
of EDK2 and 1500 vCPU set in Qemu config. The test ended up with 
success. I can confirm that space saved by removing Itanium data 
structure allows to allocate more than 1024 vCPU using KVM accelerator.

On thing to note - to test this KVM needs to be modified, the same with 
Qemu. Both - by default has vCPU limits so low that they will be 
bottleneck for vCPU now.

Best regards,
Pawel

W dniu 10.01.2023 o 16:32, Lendacky, Thomas via groups.io pisze:
> + Garret
> 
> On 1/10/23 03:36, Ni, Ray via groups.io wrote:
>>>>>> 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.
>>
>> The CPU health record is stored in PPI first by SecCore, then saved to 
>> HOB by CpuMpPei module.
>> Then CpuDxe gets the record from HOB.
>>
>> If SecCore, CpuMpPei, CpuDxe are built with different structure 
>> definition,
>> the compatibility issue appears.
>>
>> The very well know binary separation module today for x86 is Intel's FSP.
>> I am not sure if today customer might use a pre-built FSP binary which 
>> is built from
>> version #1 of edk2 while the SecCore and CpuDxe in platform binary are 
>> built from
>> a different version of edk2.
>> I don't know how AMD distributes the binary module. + Tom
>>
>> If binary distribution is not adopted yet by industry, I prefer we 
>> just update the
>> structure definition.
>>
>>>>>>
>>>>>> 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.
>>
>> I don't like this approach because it still breaks the case when CPU 
>> >=1024 and binary
>> distribution is used.
>>
>>>>>>
>>>>>> 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.
>>
>> CpuDxe driver is one of the consumers.
>> 1. Old CpuMpPei (in FSP binary) + new CpuDxe (In Platform binary)
>>       This doesn't work because CpuMpPei still cannot produce the 
>> huge-size HOB.
>> 2. New CpuMpPei (in FSP binary) + old CpuDxe (In Platform binary)
>>       This doesn't work because old CpuDxe doesn't look for multiple 
>> HOBs.
>>
>>
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pawel 
>>> Polawski
>>> Sent: Tuesday, January 10, 2023 4:19 PM
>>> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael 
>>> D <michael.d.kinney@intel.com>; Zimmer, Vincent
>>> <vincent.zimmer@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 everyone,
>>>
>>> If there is a chance you have some evaluation about this problem 
>>> already?
>>>
>>> Best regards,
>>> Pawel
>>>
>>
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98682): https://edk2.groups.io/g/devel/message/98682
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Gerd Hoffmann 1 year, 3 months ago

On Tue, Jan 10, 2023 at 09:36:59AM +0000, Ni, Ray wrote:
> > >>> 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.
> 
> The CPU health record is stored in PPI first by SecCore, then saved to HOB by CpuMpPei module.
> Then CpuDxe gets the record from HOB.
> 
> If SecCore, CpuMpPei, CpuDxe are built with different structure definition,
> the compatibility issue appears.
> 
> The very well know binary separation module today for x86 is Intel's FSP.
> I am not sure if today customer might use a pre-built FSP binary which is built from
> version #1 of edk2 while the SecCore and CpuDxe in platform binary are built from
> a different version of edk2.
> I don't know how AMD distributes the binary module. + Tom
> 
> If binary distribution is not adopted yet by industry, I prefer we just update the
> structure definition.

Yes.  *That* is the big question.  Just removing ITANIUM_HANDOFF_STATUS
is source compatible but not binary compatible.

For OVMF just updating the structure definition is not a problem at all
because the complete firmware image is built in one go from the source
code, so there are never incompatible modules.

So if compatibility at source level works for everybody we can do just
that, and maybe additionally change the GUIDs so any incompatibilities
would not go unnoticed.

> > >>> 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.
> 
> I don't like this approach because it still breaks the case when CPU >=1024 and binary
> distribution is used.

Well.  We already have EFI_SEC_PLATFORM_INFORMATION_RECORD +
EFI_SEC_PLATFORM_INFORMATION_RECORD2 and some code which is able to work
with both formats.  See for example CollectBistDataFromPpi().

So in case binary compatibility is required we probably have to add
EFI_SEC_PLATFORM_INFORMATION_RECORD3  Maybe it is possible to remove
support for EFI_SEC_PLATFORM_INFORMATION_RECORD at the same time.

take care,
  Gerd

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98244): https://edk2.groups.io/g/devel/message/98244
Mute This Topic: https://groups.io/mt/95384808/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-