<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="cid:filelist.xml@01D6FBB5.A1C89DA0"><link rel="Edit-Time-Data" href="cid:editdata.mso"><style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="tab-interval:.5in;word-wrap:break-word"> <div class="WordSection1"> <p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-font-family:Calibri;mso-bidi-font-family:"Times New Roman";mso-fareast-language:EN-US">My comment is only to make the history of changes easier to understand by separating the functional and non-functional changes.<o:p></o:p></span></p> <p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-font-family:Calibri;mso-bidi-font-family:"Times New Roman";mso-fareast-language:EN-US"><o:p> </o:p></span></p> <p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-font-family:Calibri;mso-bidi-font-family:"Times New Roman";mso-fareast-language:EN-US">Mike<o:p></o:p></span></p> <p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-font-family:Calibri;mso-bidi-font-family:"Times New Roman";mso-fareast-language:EN-US"><o:p> </o:p></span></p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> <p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Ni, Ray <ray.ni@intel.com> <br> <b>Sent:</b> Friday, February 5, 2021 10:38 AM<br> <b>To:</b> Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io<br> <b>Subject:</b> Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release<o:p></o:p></span></p> </div> </div> <p class="MsoNormal"><o:p> </o:p></p> <div> <div> <div> <p class="MsoNormal" style="background:white"><span style="color:black">Mike,<o:p></o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black">The clean up doesn’t cause any final instruction change and I have verified that.<o:p></o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black">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.<o:p></o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black">What potential issue do you see?<o:p></o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black">Thanks,<o:p></o:p></span></p> </div> <div> <p class="MsoNormal" style="background:white"><span style="color:black">Ray<o:p></o:p></span></p> </div> </div> <div> <p class="MsoNormal"><o:p> </o:p></p> </div> <div> <div> <p class="MsoNormal">thanks,<o:p></o:p></p> </div> <div> <p class="MsoNormal">ray<o:p></o:p></p> </div> </div> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> <p class="MsoNormal" style="mso-outline-level:1"><b><span lang="ZH-CN" style="font-size:11.0pt;mso-ascii-font-family:Calibri;mso-hansi-font-family:Calibri;mso-bidi-font-family:Calibri;color:black">发件人</span></b><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black">:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black"> Kinney, Michael D <<a href="mailto:michael.d.kinney@intel.com">michael.d.kinney@intel.com</a>><br> </span><b><span lang="ZH-CN" style="font-size:11.0pt;mso-ascii-font-family:Calibri;mso-hansi-font-family:Calibri;mso-bidi-font-family:Calibri;color:black">发送时间</span></b><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black">:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black"> Saturday, February 6, 2021 1:11:19 AM<br> </span><b><span lang="ZH-CN" style="font-size:11.0pt;mso-ascii-font-family:Calibri;mso-hansi-font-family:Calibri;mso-bidi-font-family:Calibri;color:black">收件人</span></b><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black">:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black"> <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> <<a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>>; Ni, Ray <<a href="mailto:ray.ni@intel.com">ray.ni@intel.com</a>>; Kinney, Michael D <<a href="mailto:michael.d.kinney@intel.com">michael.d.kinney@intel.com</a>><br> </span><b><span lang="ZH-CN" style="font-size:11.0pt;mso-ascii-font-family:Calibri;mso-hansi-font-family:Calibri;mso-bidi-font-family:Calibri;color:black">主题</span></b><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black">:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black"> RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release</span> <o:p> </o:p></p> <div> <p class="MsoNormal"> <o:p></o:p></p> </div> </div> <div> <div> <p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:11.0pt">Hi Ray,<br> <br> I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.<br> <br> 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?<br> <br> 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?<br> <br> Thanks,<br> <br> Mike<br> <br> > -----Original Message-----<br> > From: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> <<a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>> On Behalf Of Ni, Ray<br> > Sent: Thursday, February 4, 2021 11:58 PM<br> > To: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a><br> > Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release<br> > <br> > Patch #1 follows Laszlo's suggestion to add global NASM macros<br> > <br> > for NASM struc usage.<br> > <br> > Patch #2 changes all hardcode offset to use struc.<br> > <br> > Patch #3 doesn't have any change comparing to V1 except<br> > <br> > 1). dword/qword prefix is added.<br> > <br> > 2). the comments "program AP stack" is removed.<br> > <br> > Ray Ni (3):<br> > MdePkg/Nasm.inc: add macros for C types used in structure definition<br> > UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset<br> > UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release<br> > <br> > MdePkg/Include/Ia32/Nasm.inc | 38 ++++++<br> > MdePkg/Include/X64/Nasm.inc | 38 ++++++<br> > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-<br> > UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 -------<br> > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++---------<br> > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++<br> > UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -<br> > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-<br> > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-<br> > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------<br> > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------<br> > 11 files changed, 272 insertions(+), 211 deletions(-)<br> > delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc<br> > create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc<br> > delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc<br> > <br> > --<br> > 2.27.0.windows.1<br> > <br> > <br> > <br> > -=-=-=-=-=-=<br> > Groups.io Links: You receive all messages sent to this group.<br> > View/Reply Online (#71344): <a href="https://edk2.groups.io/g/devel/message/71344"> https://edk2.groups.io/g/devel/message/71344</a><br> > Mute This Topic: <a href="https://groups.io/mt/80401290/1643496">https://groups.io/mt/80401290/1643496</a><br> > Group Owner: <a href="mailto:devel+owner@edk2.groups.io">devel+owner@edk2.groups.io</a><br> > Unsubscribe: <a href="https://edk2.groups.io/g/devel/unsub">https://edk2.groups.io/g/devel/unsub</a> [michael.d.kinney@intel.com]<br> > -=-=-=-=-=-=<br> > <o:p></o:p></span></p> </div> </div> </div> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links:<p> You receive all messages sent to this group. <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/71373">View/Reply Online (#71373)</a> | | <a target="_blank" href="https://groups.io/mt/80401290/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>