Fix backward incompatible CPU_MP_DATA struct change

[edk2-devel] [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct change

Posted by Wu, Hao A 4 years, 2 months ago

The series will resolve a backward compatibility issue with pre-built
binaries (e.g. FSP) introduced by commit 88bd0661661.

The relocation of 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
fields in structure CPU_MP_DATA may cause access issue for platforms that
use pre-built FSP binary, since the offset of these microcode related
fields in CPU_MP_DATA can be different between PEI phase (in the pre-built
binary) and DXE phase (in current code implementation).

The series will use the newly introduced EDKII microcode patch HOB instead
for the DXE phase to get the information of the loaded (done in PEI phase)
microcode patches data.

Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

Hao A Wu (2):
  Revert UefiCpuPkg/MpInitLib: Relocate microcode patch fields in
    CPU_MP_DATA
  UefiCpuPkg/MpInitLib: Not pass microcode info between archs in
    CPU_MP_DATA

 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  3 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.h          | 27 +++++++++++-
 UefiCpuPkg/Library/MpInitLib/Microcode.c      | 43 ++++++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLib.c          | 20 +++++----
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  3 +-
 5 files changed, 82 insertions(+), 14 deletions(-)

-- 
2.12.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53839): https://edk2.groups.io/g/devel/message/53839
Mute This Topic: https://groups.io/mt/71015994/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct change

Posted by Wu, Hao A 4 years, 2 months ago

> -----Original Message-----
> From: Wu, Hao A
> Sent: Thursday, February 06, 2020 1:24 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A; Kubacki, Michael A; Kinney, Michael D; Dong, Eric; Ni, Ray;
> Laszlo Ersek
> Subject: [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct
> change
> 
> The series will resolve a backward compatibility issue with pre-built
> binaries (e.g. FSP) introduced by commit 88bd0661661.
> 
> The relocation of 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
> fields in structure CPU_MP_DATA may cause access issue for platforms that
> use pre-built FSP binary, since the offset of these microcode related
> fields in CPU_MP_DATA can be different between PEI phase (in the pre-built
> binary) and DXE phase (in current code implementation).
> 
> The series will use the newly introduced EDKII microcode patch HOB instead
> for the DXE phase to get the information of the loaded (done in PEI phase)
> microcode patches data.


Sorry, I forgot to mention that

The series is also available at:
https://github.com/hwu25/edk2/tree/patch_mpinitlib_fsp_v1

Tests done for the series:
1. OS boot successfully on platform (not using FSP binary) when the EDKII
   microcode patch HOB is produced. Debug messages show that the microcode
   detection and application work properly.
2. OS boot successfully on platform (not using FSP binary) when the EDKII
   microcode patch HOB is NOT produced. Debug messages show that the microcode
   detection and application work properly.

Note:
At this moment, I am not able to verify the OS boot on platform that uses
pre-built FSP binary. I am seeking help to verify this case.

Best Regards,
Hao Wu


> 
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> 
> Hao A Wu (2):
>   Revert UefiCpuPkg/MpInitLib: Relocate microcode patch fields in
>     CPU_MP_DATA
>   UefiCpuPkg/MpInitLib: Not pass microcode info between archs in
>     CPU_MP_DATA
> 
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  3 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          | 27 +++++++++++-
>  UefiCpuPkg/Library/MpInitLib/Microcode.c      | 43 ++++++++++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 20 +++++----
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  3 +-
>  5 files changed, 82 insertions(+), 14 deletions(-)
> 
> --
> 2.12.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53843): https://edk2.groups.io/g/devel/message/53843
Mute This Topic: https://groups.io/mt/71015994/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct change

Posted by Wu, Hao A 4 years, 2 months ago

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wu, Hao A
> Sent: Thursday, February 06, 2020 2:21 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A; Kinney, Michael D; Dong, Eric; Ni, Ray; Laszlo Ersek
> Subject: Re: [edk2-devel] [PATCH v1 0/2] Fix backward incompatible
> CPU_MP_DATA struct change
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Thursday, February 06, 2020 1:24 PM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A; Kubacki, Michael A; Kinney, Michael D; Dong, Eric; Ni, Ray;
> > Laszlo Ersek
> > Subject: [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct
> > change
> >
> > The series will resolve a backward compatibility issue with pre-built
> > binaries (e.g. FSP) introduced by commit 88bd0661661.
> >
> > The relocation of 'MicrocodePatchRegionSize' and
> 'MicrocodePatchAddress'
> > fields in structure CPU_MP_DATA may cause access issue for platforms
> that
> > use pre-built FSP binary, since the offset of these microcode related
> > fields in CPU_MP_DATA can be different between PEI phase (in the pre-
> built
> > binary) and DXE phase (in current code implementation).
> >
> > The series will use the newly introduced EDKII microcode patch HOB
> instead
> > for the DXE phase to get the information of the loaded (done in PEI phase)
> > microcode patches data.
> 
> 
> Sorry, I forgot to mention that
> 
> The series is also available at:
> https://github.com/hwu25/edk2/tree/patch_mpinitlib_fsp_v1
> 
> Tests done for the series:
> 1. OS boot successfully on platform (not using FSP binary) when the EDKII
>    microcode patch HOB is produced. Debug messages show that the
> microcode
>    detection and application work properly.
> 2. OS boot successfully on platform (not using FSP binary) when the EDKII
>    microcode patch HOB is NOT produced. Debug messages show that the
> microcode
>    detection and application work properly.
> 
> Note:
> At this moment, I am not able to verify the OS boot on platform that uses
> pre-built FSP binary. I am seeking help to verify this case.


Additional information:
For the above note, I am able to verify that this series can resolve the boot
issue on platforms that use pre-built FSP binaries.

Since the series has already received the R-b tags from reviewers/maintainers,
I plan to push it soon.

Best Regards,
Hao Wu


> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> >
> > Hao A Wu (2):
> >   Revert UefiCpuPkg/MpInitLib: Relocate microcode patch fields in
> >     CPU_MP_DATA
> >   UefiCpuPkg/MpInitLib: Not pass microcode info between archs in
> >     CPU_MP_DATA
> >
> >  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  3 +-
> >  UefiCpuPkg/Library/MpInitLib/MpLib.h          | 27 +++++++++++-
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c      | 43
> ++++++++++++++++++++
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 20 +++++----
> >  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  3 +-
> >  5 files changed, 82 insertions(+), 14 deletions(-)
> >
> > --
> > 2.12.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54173): https://edk2.groups.io/g/devel/message/54173
Mute This Topic: https://groups.io/mt/71015994/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct change

Posted by Wu, Hao A 4 years, 2 months ago

> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, February 11, 2020 10:55 AM
> To: devel@edk2.groups.io; Wu, Hao A
> Cc: Kubacki, Michael A; Kinney, Michael D; Dong, Eric; Ni, Ray; Laszlo Ersek
> Subject: RE: [edk2-devel] [PATCH v1 0/2] Fix backward incompatible
> CPU_MP_DATA struct change
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Wu, Hao A
> > Sent: Thursday, February 06, 2020 2:21 PM
> > To: devel@edk2.groups.io
> > Cc: Kubacki, Michael A; Kinney, Michael D; Dong, Eric; Ni, Ray; Laszlo Ersek
> > Subject: Re: [edk2-devel] [PATCH v1 0/2] Fix backward incompatible
> > CPU_MP_DATA struct change
> >
> > > -----Original Message-----
> > > From: Wu, Hao A
> > > Sent: Thursday, February 06, 2020 1:24 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A; Kubacki, Michael A; Kinney, Michael D; Dong, Eric; Ni, Ray;
> > > Laszlo Ersek
> > > Subject: [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct
> > > change
> > >
> > > The series will resolve a backward compatibility issue with pre-built
> > > binaries (e.g. FSP) introduced by commit 88bd0661661.
> > >
> > > The relocation of 'MicrocodePatchRegionSize' and
> > 'MicrocodePatchAddress'
> > > fields in structure CPU_MP_DATA may cause access issue for platforms
> > that
> > > use pre-built FSP binary, since the offset of these microcode related
> > > fields in CPU_MP_DATA can be different between PEI phase (in the pre-
> > built
> > > binary) and DXE phase (in current code implementation).
> > >
> > > The series will use the newly introduced EDKII microcode patch HOB
> > instead
> > > for the DXE phase to get the information of the loaded (done in PEI phase)
> > > microcode patches data.
> >
> >
> > Sorry, I forgot to mention that
> >
> > The series is also available at:
> > https://github.com/hwu25/edk2/tree/patch_mpinitlib_fsp_v1
> >
> > Tests done for the series:
> > 1. OS boot successfully on platform (not using FSP binary) when the EDKII
> >    microcode patch HOB is produced. Debug messages show that the
> > microcode
> >    detection and application work properly.
> > 2. OS boot successfully on platform (not using FSP binary) when the EDKII
> >    microcode patch HOB is NOT produced. Debug messages show that the
> > microcode
> >    detection and application work properly.
> >
> > Note:
> > At this moment, I am not able to verify the OS boot on platform that uses
> > pre-built FSP binary. I am seeking help to verify this case.
> 
> 
> Additional information:
> For the above note, I am able to verify that this series can resolve the boot
> issue on platforms that use pre-built FSP binaries.
> 
> Since the series has already received the R-b tags from
> reviewers/maintainers,
> I plan to push it soon.


Thanks all.
The series has been pushed via commits ccb4c38a50..348a34d984.

Best Regards,
Hao Wu


> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > >
> > > Hao A Wu (2):
> > >   Revert UefiCpuPkg/MpInitLib: Relocate microcode patch fields in
> > >     CPU_MP_DATA
> > >   UefiCpuPkg/MpInitLib: Not pass microcode info between archs in
> > >     CPU_MP_DATA
> > >
> > >  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  3 +-
> > >  UefiCpuPkg/Library/MpInitLib/MpLib.h          | 27 +++++++++++-
> > >  UefiCpuPkg/Library/MpInitLib/Microcode.c      | 43
> > ++++++++++++++++++++
> > >  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 20 +++++----
> > >  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  3 +-
> > >  5 files changed, 82 insertions(+), 14 deletions(-)
> > >
> > > --
> > > 2.12.0.windows.1
> >
> >
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54179): https://edk2.groups.io/g/devel/message/54179
Mute This Topic: https://groups.io/mt/71015994/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 0/2] Fix backward incompatible CPU_MP_DATA struct change

Posted by Laszlo Ersek 4 years, 2 months ago

On 02/06/20 06:23, Hao A Wu wrote:
> The series will resolve a backward compatibility issue with pre-built
> binaries (e.g. FSP) introduced by commit 88bd0661661.
> 
> The relocation of 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
> fields in structure CPU_MP_DATA may cause access issue for platforms that
> use pre-built FSP binary, since the offset of these microcode related
> fields in CPU_MP_DATA can be different between PEI phase (in the pre-built
> binary) and DXE phase (in current code implementation).
> 
> The series will use the newly introduced EDKII microcode patch HOB instead
> for the DXE phase to get the information of the loaded (done in PEI phase)
> microcode patches data.
> 
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> 
> Hao A Wu (2):
>   Revert UefiCpuPkg/MpInitLib: Relocate microcode patch fields in
>     CPU_MP_DATA
>   UefiCpuPkg/MpInitLib: Not pass microcode info between archs in
>     CPU_MP_DATA
> 
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  3 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          | 27 +++++++++++-
>  UefiCpuPkg/Library/MpInitLib/Microcode.c      | 43 ++++++++++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 20 +++++----
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  3 +-
>  5 files changed, 82 insertions(+), 14 deletions(-)
> 

Looks plausible to me, but my review doesn't (and shouldn't) carry much
weight here.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53865): https://edk2.groups.io/g/devel/message/53865
Mute This Topic: https://groups.io/mt/71015994/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-