Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock

["Dong, Eric" <eric.dong@intel.com>]

Hi John,

Thanks for your explanation.  I agree with your analysis. I think you can submit your next version patch.

Thanks,
Eric
> -----Original Message-----
> From: Lofgren, John E
> Sent: Saturday, September 7, 2019 3:01 AM
> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek (lersek@redhat.com)
> <lersek@redhat.com>
> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix
> #AC split lock
> 
> Hi Eric,
> 
> 1. you are correct, we need to need to handle 63-32 bits. I attach image which
> I think will help explain it better.  sidt and sgdt instructions, save 10 bytes (8
> bytes = base, 2 bytes = limit) of data to memory. In the code, its separating the
> base and limit to their own 64 bit space. Since base is offset by 2 bytes and
> 64bit, it can cross a cache line causing #ac split lock.  We can use two addition
> 16 bit xchg to fix it.  We need to use 16bit version since the split of cache line is
> occurring between 63-48 and 47-32.
> 
> 2. no, we don't need to worry about those two xchg  because it works with
> aligned memory.
> 
> 
> Thank you,
> John
> 
> >-----Original Message-----
> >From: Dong, Eric
> >Sent: Thursday, September 5, 2019 12:37 AM
> >To: devel@edk2.groups.io; Lofgren, John E <john.e.lofgren@intel.com>
> >Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek (lersek@redhat.com)
> ><lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> >Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib:
> >Fix #AC split lock
> >
> >Hi John,
> >
> >I'm not sure whether I understand the code correctly. If not, please
> >correct me.
> >
> >1. You change to the code to only exchange 32 bits(eax) instead of 64
> >bits(rax). After your change, how to handle the above 32 bits value
> >(from bit
> >32 to bit 63)?
> >2. In this file, also have another two xchg codes, do them need to be
> >updated also?
> >
> >Thanks,
> >Eric
> >> -----Original Message-----
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >> John E Lofgren
> >> Sent: Wednesday, September 4, 2019 2:15 AM
> >> To: devel@edk2.groups.io
> >> Subject: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix
> >> #AC split lock
> >>
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150
> >>
> >> Fix #AC split lock's caused by seperating base and limit from sgdt
> >> and sidt by changing xchg operands to 32-bit to stop from crossing
> cacheline.
> >>
> >> Signed-off-by: John E Lofgren <john.e.lofgren@intel.com>
> >> ---
> >>
> >>
> >UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
> >m
> >> | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git
> >>
> >a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na
> >> s
> >> m
> >>
> >b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na
> >> s
> >> m
> >> index 4db1a09f28..6d83dca4b9 100644
> >> ---
> >>
> >a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na
> >> s
> >> m
> >> +++
> >b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.
> >> +++ nasm
> >> @@ -184,17 +184,17 @@ HasErrorCode:
> >>      push    rax
> >>      push    rax
> >>      sidt    [rsp]
> >> -    xchg    rax, [rsp + 2]
> >> -    xchg    rax, [rsp]
> >> -    xchg    rax, [rsp + 8]
> >> +    xchg    eax, [rsp + 2]
> >> +    xchg    eax, [rsp]
> >> +    xchg    eax, [rsp + 8]
> >>
> >>      xor     rax, rax
> >>      push    rax
> >>      push    rax
> >>      sgdt    [rsp]
> >> -    xchg    rax, [rsp + 2]
> >> -    xchg    rax, [rsp]
> >> -    xchg    rax, [rsp + 8]
> >> +    xchg    eax, [rsp + 2]
> >> +    xchg    eax, [rsp]
> >> +    xchg    eax, [rsp + 8]
> >>
> >>  ;; UINT64  Ldtr, Tr;
> >>      xor     rax, rax
> >> --
> >> 2.16.2.windows.1
> >>
> >>
> >>