public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Cole, Deric" <deric.cole@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Dong, Eric" <eric.dong@intel.com>, Laszlo Ersek <lersek@redhat.com>
Subject: Re: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector
Date: Tue, 2 Jun 2020 16:41:43 +0000	[thread overview]
Message-ID: <MW3PR11MB46181DDB819145F57D3D2DC79D8B0@MW3PR11MB4618.namprd11.prod.outlook.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C596480@SHSMSX104.ccr.corp.intel.com>

Ray,

The NOP is not a functional change, it's just for ease of debug.

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 confusing. But worse, if the number of zero-bytes was odd, the disassembler might try to "consume" part of the next (real) instruction as an operand to the last hypothetical ADD.

Since NOP is a 1-byte instruction, I used that instead, so it is easier to visually identify the real code versus the padding when viewing disassembly.

-Deric

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, June 1, 2020 9:51 PM
To: Cole, Deric <deric.cole@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
Subject: RE: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector

Deric,
Can you explain why changing all padding 0x0 to 0x90 (nop) in your patch?

Is it required to enable AP start up in pre-mem?

Thanks,
Ray

> -----Original Message-----
> From: Cole, Deric <deric.cole@intel.com>
> Sent: Tuesday, June 2, 2020 6:32 AM
> To: devel@edk2.groups.io
> Cc: Cole, Deric <deric.cole@intel.com>; Dong, Eric 
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek 
> <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2776
> 
> 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 platform.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Deric Cole <deric.cole@intel.com>
> ---
>  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 = 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



  reply	other threads:[~2020-06-02 16:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-01 22:32 [PATCH] UefiCpuPkg/SecCore: Add pre-memory AP vector Deric Cole
2020-06-02  4:50 ` Ni, Ray
2020-06-02 16:41   ` Cole, Deric [this message]
2020-06-23  0:56     ` Ni, Ray
2020-06-23  2:14       ` Dong, Eric
2020-06-02 12:53 ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW3PR11MB46181DDB819145F57D3D2DC79D8B0@MW3PR11MB4618.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox