UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

[Kirkendall, Garrett]

[AMD Public Use]

There are currently three instances of StandardSignatureIsAuthenticAMD Function in the below locations.  I need to propose another change that will require the same function in PiSmmCpuDxeSmm.  I would like to see this function move to a single "LibraryClass" location.  I think no one knew where to propose the function, so we ended up with three instances so far.  The best place I can find would be UefiCpuPkg/BaseUefiCpuLib/.  Thoughts?  Is there a better function name such that a similar Intel identifier function could be created when needed in the future?

I'm thinking this will be 3 patches
1. Add StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
2. Move current instances to Library function
3. Propose code change in PiSmmCpuDxeSmm

UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c: StandardSignatureIsAuthenticAMD (
UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c: StandardSignatureIsAuthenticAMD (
UefiCpuPkg/Library/MpInitLib/MpLib.c: StandardSignatureIsAuthenticAMD (

GARRETT KIRKENDALL
SMTS Firmware Engineer | CTE
7171 Southwest Parkway, Austin, TX 78735 USA 
AMD   facebook  |  amd.com

Re: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

[Ni, Ray]

Since all platform DSCs have already listed UefiCpuLib.inf so this change doesn't impact platform DSC.
I don't see an issue with this change.

Eric,
What's your opinion?

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kirkendall, Garrett
> Sent: Wednesday, June 10, 2020 10:18 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib
> 
> [AMD Public Use]
> 
> There are currently three instances of StandardSignatureIsAuthenticAMD Function in the below locations.  I need to
> propose another change that will require the same function in PiSmmCpuDxeSmm.  I would like to see this function move
> to a single "LibraryClass" location.  I think no one knew where to propose the function, so we ended up with three
> instances so far.  The best place I can find would be UefiCpuPkg/BaseUefiCpuLib/.  Thoughts?  Is there a better function
> name such that a similar Intel identifier function could be created when needed in the future?
> 
> I'm thinking this will be 3 patches
> 1. Add StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
> 2. Move current instances to Library function
> 3. Propose code change in PiSmmCpuDxeSmm
> 
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/MpInitLib/MpLib.c: StandardSignatureIsAuthenticAMD (
> 
> GARRETT KIRKENDALL
> SMTS Firmware Engineer | CTE
> 7171 Southwest Parkway, Austin, TX 78735 USA
> AMD   facebook  |  amd.com
> 
> 

Re: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

[Dong, Eric]

[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]

This proposal is ok to me.

I'm not sure whether it can be added to MdePkg, just like add to CpuLib. I found all the definitions and functions used in StandardSignatureIsAuthenticAMD function defined in MdePkg.

If it can be added to MdePkg, all code base can use this function, if it been added to UefiCpuPkg, other core packages may can't use it.

Thanks,
Eric
From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, June 11, 2020 9:12 AM
To: devel@edk2.groups.io; garrett.kirkendall@amd.com
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

Since all platform DSCs have already listed UefiCpuLib.inf so this change doesn't impact platform DSC.
I don't see an issue with this change.

Eric,
What's your opinion?

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Kirkendall, Garrett
> Sent: Wednesday, June 10, 2020 10:18 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Subject: [edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib
>
> [AMD Public Use]
>
> There are currently three instances of StandardSignatureIsAuthenticAMD Function in the below locations.  I need to
> propose another change that will require the same function in PiSmmCpuDxeSmm.  I would like to see this function move
> to a single "LibraryClass" location.  I think no one knew where to propose the function, so we ended up with three
> instances so far.  The best place I can find would be UefiCpuPkg/BaseUefiCpuLib/.  Thoughts?  Is there a better function
> name such that a similar Intel identifier function could be created when needed in the future?
>
> I'm thinking this will be 3 patches
> 1. Add StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
> 2. Move current instances to Library function
> 3. Propose code change in PiSmmCpuDxeSmm
>
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/MpInitLib/MpLib.c: StandardSignatureIsAuthenticAMD (
>
> GARRETT KIRKENDALL
> SMTS Firmware Engineer | CTE
> 7171 Southwest Parkway, Austin, TX 78735 USA
> AMD   facebook  |  amd.com
>
> 

[-- Attachment #2: Type: text/html, Size: 8017 bytes --]

Re: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 3184 bytes --]

UefiCpuLib is in UefiCpuPkg.
All consumers are in UefiCpuPkg. I think we can keep its location unchanged.

From: Dong, Eric <eric.dong@intel.com>
Sent: Thursday, June 11, 2020 9:29 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; garrett.kirkendall@amd.com
Cc: Laszlo Ersek <lersek@redhat.com>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

This proposal is ok to me.

I'm not sure whether it can be added to MdePkg, just like add to CpuLib. I found all the definitions and functions used in StandardSignatureIsAuthenticAMD function defined in MdePkg.

If it can be added to MdePkg, all code base can use this function, if it been added to UefiCpuPkg, other core packages may can't use it.

Thanks,
Eric
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, June 11, 2020 9:12 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>
Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

Since all platform DSCs have already listed UefiCpuLib.inf so this change doesn't impact platform DSC.
I don't see an issue with this change.

Eric,
What's your opinion?

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Kirkendall, Garrett
> Sent: Wednesday, June 10, 2020 10:18 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Subject: [edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib
>
> [AMD Public Use]
>
> There are currently three instances of StandardSignatureIsAuthenticAMD Function in the below locations.  I need to
> propose another change that will require the same function in PiSmmCpuDxeSmm.  I would like to see this function move
> to a single "LibraryClass" location.  I think no one knew where to propose the function, so we ended up with three
> instances so far.  The best place I can find would be UefiCpuPkg/BaseUefiCpuLib/.  Thoughts?  Is there a better function
> name such that a similar Intel identifier function could be created when needed in the future?
>
> I'm thinking this will be 3 patches
> 1. Add StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
> 2. Move current instances to Library function
> 3. Propose code change in PiSmmCpuDxeSmm
>
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/MpInitLib/MpLib.c: StandardSignatureIsAuthenticAMD (
>
> GARRETT KIRKENDALL
> SMTS Firmware Engineer | CTE
> 7171 Southwest Parkway, Austin, TX 78735 USA
> AMD   facebook  |  amd.com
>
> 

[-- Attachment #2: Type: text/html, Size: 9197 bytes --]

Re: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

[Dong, Eric]

[-- Attachment #1: Type: text/plain, Size: 3813 bytes --]

I mean there is already a BaseCpuLib in MdePkg. Is it possible to add it to there?

If no usage outside of UefiCpuPkg, I'm ok to move this function to UefiCpuLib.

Thanks,
Eric

From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, June 11, 2020 12:56 PM
To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io; garrett.kirkendall@amd.com
Cc: Laszlo Ersek <lersek@redhat.com>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

UefiCpuLib is in UefiCpuPkg.
All consumers are in UefiCpuPkg. I think we can keep its location unchanged.

From: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>
Sent: Thursday, June 11, 2020 9:29 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

This proposal is ok to me.

I'm not sure whether it can be added to MdePkg, just like add to CpuLib. I found all the definitions and functions used in StandardSignatureIsAuthenticAMD function defined in MdePkg.

If it can be added to MdePkg, all code base can use this function, if it been added to UefiCpuPkg, other core packages may can't use it.

Thanks,
Eric
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, June 11, 2020 9:12 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>
Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

Since all platform DSCs have already listed UefiCpuLib.inf so this change doesn't impact platform DSC.
I don't see an issue with this change.

Eric,
What's your opinion?

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Kirkendall, Garrett
> Sent: Wednesday, June 10, 2020 10:18 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Subject: [edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib
>
> [AMD Public Use]
>
> There are currently three instances of StandardSignatureIsAuthenticAMD Function in the below locations.  I need to
> propose another change that will require the same function in PiSmmCpuDxeSmm.  I would like to see this function move
> to a single "LibraryClass" location.  I think no one knew where to propose the function, so we ended up with three
> instances so far.  The best place I can find would be UefiCpuPkg/BaseUefiCpuLib/.  Thoughts?  Is there a better function
> name such that a similar Intel identifier function could be created when needed in the future?
>
> I'm thinking this will be 3 patches
> 1. Add StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
> 2. Move current instances to Library function
> 3. Propose code change in PiSmmCpuDxeSmm
>
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/MpInitLib/MpLib.c: StandardSignatureIsAuthenticAMD (
>
> GARRETT KIRKENDALL
> SMTS Firmware Engineer | CTE
> 7171 Southwest Parkway, Austin, TX 78735 USA
> AMD   facebook  |  amd.com
>
> 

[-- Attachment #2: Type: text/html, Size: 10625 bytes --]

Re: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

[Laszlo Ersek]

On 06/11/20 09:07, Dong, Eric wrote:
> I mean there is already a BaseCpuLib in MdePkg. Is it possible to add it to there?
> 
> If no usage outside of UefiCpuPkg, I'm ok to move this function to UefiCpuLib.

Sounds OK to me.

Thanks
Laszlo

Re: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 4741 bytes --]

Comments in BaseCpuLib tell that this library is separated from BaseLib due to IPF.

  The CPU Library provides services to flush CPU TLBs and place the CPU in a sleep state.
  The implementation of these services on Itanium processors requires the use of PAL Calls.
  PAL Calls require PEI and DXE specific mechanisms to look up PAL Entry Point.
  As a result, these services could not be defined in the Base Library.

So I think we should avoid adding API to this library.

From: Dong, Eric <eric.dong@intel.com>
Sent: Thursday, June 11, 2020 3:07 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; garrett.kirkendall@amd.com
Cc: Laszlo Ersek <lersek@redhat.com>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

I mean there is already a BaseCpuLib in MdePkg. Is it possible to add it to there?

If no usage outside of UefiCpuPkg, I'm ok to move this function to UefiCpuLib.

Thanks,
Eric

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, June 11, 2020 12:56 PM
To: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

UefiCpuLib is in UefiCpuPkg.
All consumers are in UefiCpuPkg. I think we can keep its location unchanged.

From: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>
Sent: Thursday, June 11, 2020 9:29 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

This proposal is ok to me.

I'm not sure whether it can be added to MdePkg, just like add to CpuLib. I found all the definitions and functions used in StandardSignatureIsAuthenticAMD function defined in MdePkg.

If it can be added to MdePkg, all code base can use this function, if it been added to UefiCpuPkg, other core packages may can't use it.

Thanks,
Eric
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, June 11, 2020 9:12 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>
Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Subject: RE: UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib

Since all platform DSCs have already listed UefiCpuLib.inf so this change doesn't impact platform DSC.
I don't see an issue with this change.

Eric,
What's your opinion?

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Kirkendall, Garrett
> Sent: Wednesday, June 10, 2020 10:18 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Subject: [edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD function to BaseUefiCpuLib
>
> [AMD Public Use]
>
> There are currently three instances of StandardSignatureIsAuthenticAMD Function in the below locations.  I need to
> propose another change that will require the same function in PiSmmCpuDxeSmm.  I would like to see this function move
> to a single "LibraryClass" location.  I think no one knew where to propose the function, so we ended up with three
> instances so far.  The best place I can find would be UefiCpuPkg/BaseUefiCpuLib/.  Thoughts?  Is there a better function
> name such that a similar Intel identifier function could be created when needed in the future?
>
> I'm thinking this will be 3 patches
> 1. Add StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
> 2. Move current instances to Library function
> 3. Propose code change in PiSmmCpuDxeSmm
>
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c: StandardSignatureIsAuthenticAMD (
> UefiCpuPkg/Library/MpInitLib/MpLib.c: StandardSignatureIsAuthenticAMD (
>
> GARRETT KIRKENDALL
> SMTS Firmware Engineer | CTE
> 7171 Southwest Parkway, Austin, TX 78735 USA
> AMD   facebook  |  amd.com
>
> 

[-- Attachment #2: Type: text/html, Size: 13562 bytes --]