UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150
V3 changes:
change to mov instruction (non locking instuction) instead
of xchg to simplify design.
V2 changes:
Add xchg 16 bit instructions to handle sgdt and sidt base
63:48 bits and 47:32 bits.
Add comment to explain why xchg 64bit isnt being used
Split lock happens when a locking instruction is used on mis-aligned data
that crosses two cachelines. If close source platform enables Alignment Check
Exception(#AC), They can hit a double fault due to split lock being in
CpuExceptionHandlerLib.
sigt and sgdt saves 10 bytes to memory, 8 bytes is base and 2 bytes is limit.
The data is mis-aligned, can cross two cacheline, and a xchg
instruction(locking instuction) is being utilize.
Signed-off-by: John E Lofgren <john.e.lofgren@intel.com>
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 4db1a09f28..7b7642b290 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -180,21 +180,29 @@ HasErrorCode:
push qword [rbp + 24]
;; UINT64 Gdtr[2], Idtr[2];
+ ; sidt and sgdt saves 10 bytes to memory, 8 bytes = base and 2 bytes = limit.
+ ; To avoid #AC split lock when separating base and limit into their
+ ; own separate 64 bit memory, we can’t use 64 bit xchg since base [63:48] bits
+ ; may cross the cache line.
xor rax, rax
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]
+ xchg ax, [rsp + 6]
+ xchg ax, [rsp + 4]
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]
+ xchg ax, [rsp + 6]
+ xchg ax, [rsp + 4]
;; UINT64 Ldtr, Tr;
xor rax, rax
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47438): https://edk2.groups.io/g/devel/message/47438
Mute This Topic: https://groups.io/mt/34181976/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 09/18/19 00:49, John E Lofgren wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150 > V3 changes: > change to mov instruction (non locking instuction) instead > of xchg to simplify design. I think something's wrong -- the v3 update described above isn't actually implemented in the patch (it continues using XCHG, rather than MOV). Thanks Laszlo > > V2 changes: > Add xchg 16 bit instructions to handle sgdt and sidt base > 63:48 bits and 47:32 bits. > Add comment to explain why xchg 64bit isnt being used > > Split lock happens when a locking instruction is used on mis-aligned data > that crosses two cachelines. If close source platform enables Alignment Check > Exception(#AC), They can hit a double fault due to split lock being in > CpuExceptionHandlerLib. > > sigt and sgdt saves 10 bytes to memory, 8 bytes is base and 2 bytes is limit. > The data is mis-aligned, can cross two cacheline, and a xchg > instruction(locking instuction) is being utilize. > > Signed-off-by: John E Lofgren <john.e.lofgren@intel.com> > --- > UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm > index 4db1a09f28..7b7642b290 100644 > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm > @@ -180,21 +180,29 @@ HasErrorCode: > push qword [rbp + 24] > > ;; UINT64 Gdtr[2], Idtr[2]; > + ; sidt and sgdt saves 10 bytes to memory, 8 bytes = base and 2 bytes = limit. > + ; To avoid #AC split lock when separating base and limit into their > + ; own separate 64 bit memory, we can’t use 64 bit xchg since base [63:48] bits > + ; may cross the cache line. > xor rax, rax > 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] > + xchg ax, [rsp + 6] > + xchg ax, [rsp + 4] > > 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] > + xchg ax, [rsp + 6] > + xchg ax, [rsp + 4] > > ;; UINT64 Ldtr, Tr; > xor rax, rax > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47472): https://edk2.groups.io/g/devel/message/47472 Mute This Topic: https://groups.io/mt/34181976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Sorry. I forgot amend it to the commit. Ill fix it. Sorry Again, John >-----Original Message----- >From: Laszlo Ersek [mailto:lersek@redhat.com] >Sent: Wednesday, September 18, 2019 1:52 AM >To: devel@edk2.groups.io; Lofgren, John E <john.e.lofgren@intel.com> >Subject: Re: [edk2-devel] [Patch V3] UefiCpuPkg/CpuExceptionHandlerLib: Fix >split lock > >On 09/18/19 00:49, John E Lofgren wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150 >> V3 changes: >> change to mov instruction (non locking instuction) instead of xchg to >> simplify design. > >I think something's wrong -- the v3 update described above isn't actually >implemented in the patch (it continues using XCHG, rather than MOV). > >Thanks >Laszlo > >> >> V2 changes: >> Add xchg 16 bit instructions to handle sgdt and sidt base >> 63:48 bits and 47:32 bits. >> Add comment to explain why xchg 64bit isnt being used >> >> Split lock happens when a locking instruction is used on mis-aligned >> data that crosses two cachelines. If close source platform enables >> Alignment Check Exception(#AC), They can hit a double fault due to >> split lock being in CpuExceptionHandlerLib. >> >> sigt and sgdt saves 10 bytes to memory, 8 bytes is base and 2 bytes is limit. >> The data is mis-aligned, can cross two cacheline, and a xchg >> instruction(locking instuction) is being utilize. >> >> Signed-off-by: John E Lofgren <john.e.lofgren@intel.com> >> --- >> >> >UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas >m >> | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git >> >a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na >> sm >> >b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na >> sm >> index 4db1a09f28..7b7642b290 100644 >> --- >> >a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na >> sm >> +++ >b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAs >> +++ m.nasm >> @@ -180,21 +180,29 @@ HasErrorCode: >> push qword [rbp + 24] >> >> ;; UINT64 Gdtr[2], Idtr[2]; >> + ; sidt and sgdt saves 10 bytes to memory, 8 bytes = base and 2 bytes = >limit. >> + ; To avoid #AC split lock when separating base and limit into their >> + ; own separate 64 bit memory, we can’t use 64 bit xchg since base [63:48] >bits >> + ; may cross the cache line. >> xor rax, rax >> 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] >> + xchg ax, [rsp + 6] >> + xchg ax, [rsp + 4] >> >> 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] >> + xchg ax, [rsp + 6] >> + xchg ax, [rsp + 4] >> >> ;; UINT64 Ldtr, Tr; >> xor rax, rax >> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47483): https://edk2.groups.io/g/devel/message/47483 Mute This Topic: https://groups.io/mt/34181976/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.