[edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release

Ni, Ray posted 3 patches 3 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20210205075810.981-1-ray.ni@intel.com
There is a newer version of this series
MdePkg/Include/Ia32/Nasm.inc                  |  38 ++++++
MdePkg/Include/X64/Nasm.inc                   |  38 ++++++
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   5 +-
UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc   |  43 -------
.../Library/MpInitLib/Ia32/MpFuncs.nasm       |  98 +++++++---------
UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |  99 ++++++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.c          |   1 -
UefiCpuPkg/Library/MpInitLib/MpLib.h          |   3 +-
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   5 +-
UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc    |  45 --------
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
11 files changed, 272 insertions(+), 211 deletions(-)
delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
[edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Posted by Ni, Ray 3 years, 2 months ago
Patch #1 follows Laszlo's suggestion to add global NASM macros
  for NASM struc usage.
Patch #2 changes all hardcode offset to use struc.
Patch #3 doesn't have any change comparing to V1 except
  1). dword/qword prefix is added.
  2). the comments "program AP stack" is removed.

Ray Ni (3):
  MdePkg/Nasm.inc: add macros for C types used in structure definition
  UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
  UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release

 MdePkg/Include/Ia32/Nasm.inc                  |  38 ++++++
 MdePkg/Include/X64/Nasm.inc                   |  38 ++++++
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   5 +-
 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc   |  43 -------
 .../Library/MpInitLib/Ia32/MpFuncs.nasm       |  98 +++++++---------
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |  99 ++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |   1 -
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |   3 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   5 +-
 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc    |  45 --------
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
 11 files changed, 272 insertions(+), 211 deletions(-)
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
 create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc

-- 
2.27.0.windows.1



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


Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Posted by Michael D Kinney 3 years, 2 months ago
Hi Ray,

I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.

Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?

Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
> 
> Patch #1 follows Laszlo's suggestion to add global NASM macros
> 
>   for NASM struc usage.
> 
> Patch #2 changes all hardcode offset to use struc.
> 
> Patch #3 doesn't have any change comparing to V1 except
> 
>   1). dword/qword prefix is added.
> 
>   2). the comments "program AP stack" is removed.
> 
> Ray Ni (3):
>   MdePkg/Nasm.inc: add macros for C types used in structure definition
>   UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
>   UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
> 
>  MdePkg/Include/Ia32/Nasm.inc                  |  38 ++++++
>  MdePkg/Include/X64/Nasm.inc                   |  38 ++++++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc   |  43 -------
>  .../Library/MpInitLib/Ia32/MpFuncs.nasm       |  98 +++++++---------
>  UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |  99 ++++++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   1 -
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |   3 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc    |  45 --------
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
>  11 files changed, 272 insertions(+), 211 deletions(-)
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> 
> --
> 2.27.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
> 



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


Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Posted by Ni, Ray 3 years, 2 months ago
Mike,

The clean up doesn’t cause any final instruction change and I have verified that.
The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets.

What potential issue do you see?

Thanks,
Ray

thanks,
ray
________________________________
发件人: Kinney, Michael D <michael.d.kinney@intel.com>
发送时间: Saturday, February 6, 2021 1:11:19 AM
收件人: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release

Hi Ray,

I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.

Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?

Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
>
> Patch #1 follows Laszlo's suggestion to add global NASM macros
>
>   for NASM struc usage.
>
> Patch #2 changes all hardcode offset to use struc.
>
> Patch #3 doesn't have any change comparing to V1 except
>
>   1). dword/qword prefix is added.
>
>   2). the comments "program AP stack" is removed.
>
> Ray Ni (3):
>   MdePkg/Nasm.inc: add macros for C types used in structure definition
>   UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
>   UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>
>  MdePkg/Include/Ia32/Nasm.inc                  |  38 ++++++
>  MdePkg/Include/X64/Nasm.inc                   |  38 ++++++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc   |  43 -------
>  .../Library/MpInitLib/Ia32/MpFuncs.nasm       |  98 +++++++---------
>  UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |  99 ++++++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   1 -
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |   3 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc    |  45 --------
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
>  11 files changed, 272 insertions(+), 211 deletions(-)
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>



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


Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Posted by Michael D Kinney 3 years, 2 months ago
My comment is only to make the history of changes easier to understand by separating the functional and non-functional changes.

Mike

From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, February 5, 2021 10:38 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release

Mike,

The clean up doesn’t cause any final instruction change and I have verified that.
The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets.

What potential issue do you see?

Thanks,
Ray

thanks,
ray
________________________________
发件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
发送时间: Saturday, February 6, 2021 1:11:19 AM
收件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release

Hi Ray,

I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.

Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?

Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
>
> Patch #1 follows Laszlo's suggestion to add global NASM macros
>
>   for NASM struc usage.
>
> Patch #2 changes all hardcode offset to use struc.
>
> Patch #3 doesn't have any change comparing to V1 except
>
>   1). dword/qword prefix is added.
>
>   2). the comments "program AP stack" is removed.
>
> Ray Ni (3):
>   MdePkg/Nasm.inc: add macros for C types used in structure definition
>   UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
>   UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>
>  MdePkg/Include/Ia32/Nasm.inc                  |  38 ++++++
>  MdePkg/Include/X64/Nasm.inc                   |  38 ++++++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc   |  43 -------
>  .../Library/MpInitLib/Ia32/MpFuncs.nasm       |  98 +++++++---------
>  UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |  99 ++++++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   1 -
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |   3 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc    |  45 --------
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
>  11 files changed, 272 insertions(+), 211 deletions(-)
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>


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


Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Posted by Ni, Ray 3 years, 2 months ago
I see. Will send a V3.

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Saturday, February 6, 2021 3:54 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release

My comment is only to make the history of changes easier to understand by separating the functional and non-functional changes.

Mike

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Friday, February 5, 2021 10:38 AM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release

Mike,

The clean up doesn’t cause any final instruction change and I have verified that.
The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets.

What potential issue do you see?

Thanks,
Ray

thanks,
ray
________________________________
发件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
发送时间: Saturday, February 6, 2021 1:11:19 AM
收件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release

Hi Ray,

I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.

Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?

Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
>
> Patch #1 follows Laszlo's suggestion to add global NASM macros
>
>   for NASM struc usage.
>
> Patch #2 changes all hardcode offset to use struc.
>
> Patch #3 doesn't have any change comparing to V1 except
>
>   1). dword/qword prefix is added.
>
>   2). the comments "program AP stack" is removed.
>
> Ray Ni (3):
>   MdePkg/Nasm.inc: add macros for C types used in structure definition
>   UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
>   UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>
>  MdePkg/Include/Ia32/Nasm.inc                  |  38 ++++++
>  MdePkg/Include/X64/Nasm.inc                   |  38 ++++++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc   |  43 -------
>  .../Library/MpInitLib/Ia32/MpFuncs.nasm       |  98 +++++++---------
>  UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |  99 ++++++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   1 -
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |   3 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   5 +-
>  UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc    |  45 --------
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
>  11 files changed, 272 insertions(+), 211 deletions(-)
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>


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