Sorry. I forgot amend it to the commit. Ill fix it. Sorry Again, John
>-----Original Message----- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Wednesday, September 18, 2019 1:52 AM >To: devel@edk2.groups.io; Lofgren, John E <john.e.lofg...@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.lofg...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-