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

[Michael D Kinney]

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 at intel.com>
Sent: Friday, February 5, 2021 10:38 AM
To: Kinney, Michael D <michael.d.kinney at intel.com>; devel at 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 at intel.com<mailto:michael.d.kinney at intel.com>>
发送时间: Saturday, February 6, 2021 1:11:19 AM
收件人: devel at edk2.groups.io<mailto:devel at edk2.groups.io> <devel at edk2.groups.io<mailto:devel at edk2.groups.io>>; Ni, Ray <ray.ni at intel.com<mailto:ray.ni at intel.com>>; Kinney, Michael D <michael.d.kinney at intel.com<mailto:michael.d.kinney at 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 at edk2.groups.io<mailto:devel at edk2.groups.io> <devel at edk2.groups.io<mailto:devel at edk2.groups.io>> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel at edk2.groups.io<mailto:devel at 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 at edk2.groups.io<mailto:devel+owner at edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210205/b6fb3261/attachment.htm>