From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: eric.dong@intel.com) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by groups.io with SMTP; Sun, 08 Sep 2019 17:34:15 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Sep 2019 17:34:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,483,1559545200"; d="scan'208";a="267914200" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga001.jf.intel.com with ESMTP; 08 Sep 2019 17:34:13 -0700 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 8 Sep 2019 17:34:13 -0700 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 8 Sep 2019 17:34:13 -0700 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Sun, 8 Sep 2019 17:34:12 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.113]) by SHSMSX105.ccr.corp.intel.com ([169.254.11.23]) with mapi id 14.03.0439.000; Mon, 9 Sep 2019 08:34:10 +0800 From: "Dong, Eric" To: "Lofgren, John E" , "devel@edk2.groups.io" CC: "Ni, Ray" , "Laszlo Ersek (lersek@redhat.com)" Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock Thread-Topic: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock Thread-Index: AQHVYoPfQh+SzfBrdE6h+lBq9mDor6ccsqWAgAHM9ACABASnoA== Date: Mon, 9 Sep 2019 00:34:10 +0000 Message-ID: References: <20190903181524.1132-1-john.e.lofgren@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 ; devel@edk2.groups.io > Cc: Ni, Ray ; Laszlo Ersek (lersek@redhat.com) > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix > #AC split lock >=20 > Hi Eric, >=20 > 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 1= 0 bytes (8 > bytes =3D base, 2 bytes =3D limit) of data to memory. In the code, its s= eparating the > base and limit to their own 64 bit space. Since base is offset by 2 byte= s 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. >=20 > 2. no, we don't need to worry about those two xchg because it works wit= h > aligned memory. >=20 >=20 > Thank you, > John >=20 > >-----Original Message----- > >From: Dong, Eric > >Sent: Thursday, September 5, 2019 12:37 AM > >To: devel@edk2.groups.io; Lofgren, John E > >Cc: Ni, Ray ; Laszlo Ersek (lersek@redhat.com) > >; Dong, Eric > >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=3D2150 > >> > >> 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 > >> --- > >> > >> > >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 > >> > >> > >>=20