From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web10.8876.1592873804271143233 for ; Mon, 22 Jun 2020 17:56:44 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: ray.ni@intel.com) IronPort-SDR: 8UVaWAhT9jlOWhVV9vqOph7AL0BrcPk49HYVE1US5BHiLz7Ds3EqlmbaEIa9/FZMvxeCtbrayM rxJxPkgrL2xg== X-IronPort-AV: E=McAfee;i="6000,8403,9660"; a="141423210" X-IronPort-AV: E=Sophos;i="5.75,268,1589266800"; d="scan'208";a="141423210" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Jun 2020 17:56:42 -0700 IronPort-SDR: ZXK9Ld2l9H9Z7+1GmCfSRbGsUxJRJFSGOtNoG7jgb7K+SbE5b9NF/cGnavEtvmqJXive7ilzaV Zm1nGZ6KEmfQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,268,1589266800"; d="scan'208";a="478672400" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 22 Jun 2020 17:56:42 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 22 Jun 2020 17:56:42 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.161]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.174]) with mapi id 14.03.0439.000; Tue, 23 Jun 2020 08:56:38 +0800 From: "Ni, Ray" To: "Cole, Deric" , "devel@edk2.groups.io" CC: "Dong, Eric" , Laszlo Ersek Subject: Re: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector Thread-Topic: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector Thread-Index: AQHWOGSP8Y87h/ISNUejqBh9DEUEQajEwRbggABBaYCAIH7qoA== Date: Tue, 23 Jun 2020 00:56:39 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C5CDBF2@SHSMSX104.ccr.corp.intel.com> References: <20200601223225.1925-1-deric.cole@intel.com> <734D49CCEBEEF84792F5B80ED585239D5C596480@SHSMSX104.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ray Ni > -----Original Message----- > From: Cole, Deric > Sent: Wednesday, June 3, 2020 12:42 AM > To: Ni, Ray ; devel@edk2.groups.io > Cc: Dong, Eric ; Laszlo Ersek > Subject: RE: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector >=20 > Ray, >=20 > The NOP is not a functional change, it's just for ease of debug. >=20 > Before, this file was padding with zeros in various places. When viewing = this memory using a disassembler, two > subsequent zero-bytes show up as an ADD instruction, which I found confus= ing. But worse, if the number of zero-bytes was > odd, the disassembler might try to "consume" part of the next (real) inst= ruction as an operand to the last hypothetical > ADD. >=20 > Since NOP is a 1-byte instruction, I used that instead, so it is easier t= o visually identify the real code versus the padding > when viewing disassembly. >=20 > -Deric >=20 > -----Original Message----- > From: Ni, Ray > Sent: Monday, June 1, 2020 9:51 PM > To: Cole, Deric ; devel@edk2.groups.io > Cc: Dong, Eric ; Laszlo Ersek > Subject: RE: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector >=20 > Deric, > Can you explain why changing all padding 0x0 to 0x90 (nop) in your patch? >=20 > Is it required to enable AP start up in pre-mem? >=20 > Thanks, > Ray >=20 > > -----Original Message----- > > From: Cole, Deric > > Sent: Tuesday, June 2, 2020 6:32 AM > > To: devel@edk2.groups.io > > Cc: Cole, Deric ; Dong, Eric > > ; Ni, Ray ; Laszlo Ersek > > > > Subject: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector > > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2776 > > > > Add a vector at 0xFF000 (0xFFFFF000) that can be used by > > Init-SIPI-SIPI to start an AP before memory is initialized. This > > vector jumps into the same SEC entry point as the ordinary reset > > vector, with a special value of "AP" in the DI register. The > > platform-specific SEC code is expected to check for that value and > > take a different path for APs, if this feature is supported by the plat= form. > > > > Cc: Eric Dong > > Cc: Ray Ni > > Cc: Laszlo Ersek > > Signed-off-by: Deric Cole > > --- > > UefiCpuPkg/SecCore/Ia32/ResetVec.nasmb | 31 > > ++++++++++++++++++++++++------- > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > diff --git a/UefiCpuPkg/SecCore/Ia32/ResetVec.nasmb > > b/UefiCpuPkg/SecCore/Ia32/ResetVec.nasmb > > index f41b9669d0..1dfc4efe4c 100644 > > --- a/UefiCpuPkg/SecCore/Ia32/ResetVec.nasmb > > +++ b/UefiCpuPkg/SecCore/Ia32/ResetVec.nasmb > > @@ -10,7 +10,7 @@ > > ; Abstract: > > > > ; > > > > ; Reset Vector Data structure > > > > -; This structure is located at 0xFFFFFFC0 > > > > +; This structure is located at 0xFFFFF000 > > > > ; > > > > > > ;--------------------------------------------------------------------- > > --------- > > > > > > > > @@ -23,19 +23,36 @@ USE16 > > ; > > > > > > > > ORG 0h > > > > + > > > > +; > > > > +; 0xFFFFF000 > > > > +; > > > > +; We enter here with CS:IP =3D 0xFF00:0x0000. Do a far-jump to change > > +CS to > > 0xF000 > > > > +; and IP to ApStartup. > > > > +; > > > > +ApVector: > > > > + mov di, "AP" > > > > + jmp 0xF000:0xF000+ApStartup > > > > + > > > > + TIMES 0xFC0-($-$$) nop > > > > + > > > > +; > > > > +; This should be at 0xFFFFFFC0 > > > > +; > > > > + > > > > ; > > > > ; Reserved > > > > ; > > > > ReservedData: DD 0eeeeeeeeh, 0eeeeeeeeh > > > > > > > > - TIMES 0x10-($-$$) DB 0 > > > > + TIMES 0xFD0-($-$$) nop > > > > ; > > > > -; This is located at 0xFFFFFFD0h > > > > +; This is located at 0xFFFFFFD0 > > > > ; > > > > mov di, "PA" > > > > jmp ApStartup > > > > > > > > - TIMES 0x20-($-$$) DB 0 > > > > + TIMES 0xFE0-($-$$) nop > > > > ; > > > > ; Pointer to the entry point of the PEI core > > > > ; It is located at 0xFFFFFFE0, and is fixed up by some build tool > > > > @@ -53,7 +70,7 @@ ASM_PFX(InterruptHandler): > > jmp $ > > > > iret > > > > > > > > - TIMES 0x30-($-$$) DB 0 > > > > + TIMES 0xFF0-($-$$) nop > > > > ; > > > > ; For IA32, the reset vector must be at 0xFFFFFFF0, i.e., 4G-16 byte > > > > ; Execution starts here upon power-on/platform-reset. > > > > @@ -74,7 +91,7 @@ ApStartup: > > DW -3 > > > > > > > > > > > > - TIMES 0x38-($-$$) DB 0 > > > > + TIMES 0xFF8-($-$$) nop > > > > ; > > > > ; Ap reset vector segment address is at 0xFFFFFFF8 > > > > ; This will be fixed up by some build tool, > > > > @@ -83,7 +100,7 @@ ApStartup: > > ; > > > > ApSegAddress: dd 12345678h > > > > > > > > - TIMES 0x3c-($-$$) DB 0 > > > > + TIMES 0xFFC-($-$$) nop > > > > ; > > > > ; BFV Base is at 0xFFFFFFFC > > > > ; This will be fixed up by some build tool, > > > > -- > > 2.26.2.windows.1 >=20 >=20