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

["John E Lofgren" <john.e.lofgren@intel.com>]

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

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


[-- Attachment #2: ac_split_lock.png --]
[-- Type: image/png, Size: 29368 bytes --]