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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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

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

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