From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Wed, 18 Sep 2019 01:52:15 -0700 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 54E34C057FA6; Wed, 18 Sep 2019 08:52:15 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-2.rdu2.redhat.com [10.10.120.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9406060C05; Wed, 18 Sep 2019 08:52:14 +0000 (UTC) Subject: Re: [edk2-devel] [Patch V3] UefiCpuPkg/CpuExceptionHandlerLib: Fix split lock To: devel@edk2.groups.io, john.e.lofgren@intel.com References: <20190917224910.28040-1-john.e.lofgren@intel.com> From: "Laszlo Ersek" Message-ID: <0b464724-85d4-c8bb-d5db-c164ac51c1f5@redhat.com> Date: Wed, 18 Sep 2019 10:52:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190917224910.28040-1-john.e.lofgren@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 18 Sep 2019 08:52:15 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 09/18/19 00:49, John E Lofgren wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2150 > 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 >=20 > 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 >=20 > Split lock happens when a locking instruction is used on mis-aligned da= ta > 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. >=20 > 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. >=20 > Signed-off-by: John E Lofgren > --- > UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm= | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) >=20 > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHan= dlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHan= dlerAsm.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] > =20 > ;; UINT64 Gdtr[2], Idtr[2]; > + ; sidt and sgdt saves 10 bytes to memory, 8 bytes =3D base and 2 b= ytes =3D limit. > + ; To avoid #AC split lock when separating base and limit into thei= r > + ; own separate 64 bit memory, we can=E2=80=99t use 64 bit xchg sin= ce 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] > =20 > 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] > =20 > ;; UINT64 Ldtr, Tr; > xor rax, rax >=20