public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ruiyu" <ruiyu.ni@Intel.com>
To: Andrew Fish <afish@apple.com>, Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel <edk2-devel@lists.01.org>,
	"Eric Dong" <eric.dong@intel.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	"Bandan Das" <bsd@redhat.com>,
	"Jiewen Yao" <jiewen.yao@intel.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH] UefiCpuPkg/MpInitLib: AP uses memory preceding IDT to store CpuMpData
Date: Wed, 27 Jun 2018 13:06:52 +0800	[thread overview]
Message-ID: <935660f2-e111-e871-f408-fcbd3e73da3d@Intel.com> (raw)
In-Reply-To: <4DFBB17A-3FCF-448D-B8F0-C4D66A33CF9F@apple.com>

On 6/27/2018 1:20 AM, Andrew Fish wrote:
> 
> 
>> On Jun 26, 2018, at 10:06 AM, Laszlo Ersek <lersek@redhat.com 
>> <mailto:lersek@redhat.com>> wrote:
>>
>> (replying again to the patch email directly, for keeping context --
>> adding some people to the CC list. Comments below.)
>>
>> On 06/25/18 04:54, Ruiyu Ni wrote:
>>> Today's MpInitLib PEI implementation directly calls
>>> PeiServices->GetHobList() from AP which may cause racing issue.
>>>
>>> This patch fixes this issue by storing the CpuMpData to memory
>>> preceding IDT. Pointer to PeiServices pointer is stored there,
>>> so after AP procedure returns, the PeiServices pointer should be
>>> restored.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com <mailto:ruiyu.ni@intel.com>>
>>> Cc: Jeff Fan <vanjeff_919@hotmail.com <mailto:vanjeff_919@hotmail.com>>
>>> Cc: Eric Dong <eric.dong@intel.com <mailto:eric.dong@intel.com>>
>>> Cc: Jiewen Yao <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com>>
>>> Cc: Fish Andrew <afish@apple.com <mailto:afish@apple.com>>
>>> ---
>>> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 33 ++++++++++++++++++-
>>> UefiCpuPkg/Library/MpInitLib/MpLib.c    |  8 +++++
>>> UefiCpuPkg/Library/MpInitLib/MpLib.h    | 27 +++++++++++++++-
>>> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 56 
>>> +++++++++++++++++++++++++++++++--
>>> 4 files changed, 119 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
>>> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>>> index e7ed21c6cd..26fead2c66 100644
>>> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>>> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>>> @@ -1,7 +1,7 @@
>>> /** @file
>>>   MP initialize support functions for DXE phase.
>>>
>>> -  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>>> +  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>>>   This program and the accompanying materials
>>>   are licensed and made available under the terms and conditions of 
>>> the BSD License
>>>   which accompanies this distribution.  The full text of the license 
>>> may be found at
>>> @@ -75,6 +75,37 @@ SaveCpuMpData (
>>>   mCpuMpData = CpuMpData;
>>> }
>>>
>>> +/**
>>> +  Push the CpuMpData for AP to use.
>>> +
>>> +  @param[in]  The pointer to CPU MP Data structure will be pushed.
>>> +  @param[out] The pointer to the context which will be passed to 
>>> PopCpuMpData().
>>> +
>>> +  @return  The pointer value which was stored in where the CPU MP 
>>> Data is pushed.
>>> +**/
>>> +VOID *
>>> +PushCpuMpData (
>>> +  IN  CPU_MP_DATA    *CpuMpData,
>>> +  OUT VOID           **Context
>>> +  )
>>> +{
>>> +  return NULL;
>>> +}
>>> +
>>> +/**
>>> +  Pop the CpuMpData.
>>> +
>>> +  @param[in] Pointer  The pointer value which was stored in where 
>>> the CPU MP Data is pushed.
>>> +  @param[in] Context  The context of push/pop operation.
>>> +**/
>>> +VOID
>>> +PopCpuMpData (
>>> +  IN VOID           *Pointer,
>>> +  IN VOID           *Context
>>> +  )
>>> +{
>>> +}
>>> +
>>> /**
>>>   Get available system memory below 1MB by specified size.
>>>
>>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
>>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>>> index f2ff40417a..786a7825d5 100644
>>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>>> @@ -580,6 +580,8 @@ ApWakeupFunction (
>>>   CPU_INFO_IN_HOB            *CpuInfoInHob;
>>>   UINT64                     ApTopOfStack;
>>>   UINTN                      CurrentApicMode;
>>> +  VOID                       *BackupPtr;
>>> +  VOID                       *Context;
>>>
>>>   //
>>>   // AP finished assembly code and begin to execute C code
>>> @@ -659,8 +661,14 @@ ApWakeupFunction (
>>>           EnableDebugAgent ();
>>>           //
>>>           // Invoke AP function here
>>> +          // Use a BSP owned area (PeiServices Pointer storage) to 
>>> store the CpuMpData.
>>> +          // It's required in PEI phase because CpuMpData cannot be 
>>> cached in global variable as in DXE phase.
>>> +          // DXE version of Pushxxx andPopxxx is dummy implementation.
>>>           //
>>> +          BackupPtr = PushCpuMpData (CpuMpData, &Context);
>>>           Procedure (Parameter);
>>> +          PopCpuMpData (BackupPtr, Context);
>>> +
>>>           CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) 
>>> CpuMpData->CpuInfoInHob;
>>>           if (CpuMpData->SwitchBspFlag) {
>>>             //
>>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
>>> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>>> index e7f9a4de0a..270d62ff20 100644
>>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
>>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>>> @@ -1,7 +1,7 @@
>>> /** @file
>>>   Common header file for MP Initialize Library.
>>>
>>> -  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
>>> +  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>>>   This program and the accompanying materials
>>>   are licensed and made available under the terms and conditions of 
>>> the BSD License
>>>   which accompanies this distribution.  The full text of the license 
>>> may be found at
>>> @@ -321,6 +321,31 @@ SaveCpuMpData (
>>>   IN CPU_MP_DATA   *CpuMpData
>>>   );
>>>
>>> +/**
>>> +  Push the CpuMpData for AP to use.
>>> +
>>> +  @param[in]  The pointer to CPU MP Data structure will be pushed.
>>> +  @param[out] The pointer to the context which will be passed to 
>>> PopCpuMpData().
>>> +
>>> +  @return  The pointer value which was stored in where the CPU MP 
>>> Data is pushed.
>>> +**/
>>> +VOID *
>>> +PushCpuMpData (
>>> +  IN  CPU_MP_DATA    *CpuMpData,
>>> +  OUT VOID           **Context
>>> +  );
>>> +
>>> +/**
>>> +  Pop the CpuMpData.
>>> +
>>> +  @param[in] Pointer  The pointer value which was stored in where 
>>> the CPU MP Data is pushed.
>>> +  @param[in] Context  The context of push/pop operation.
>>> +**/
>>> +VOID
>>> +PopCpuMpData (
>>> +  IN VOID           *Pointer,
>>> +  IN VOID           *Context
>>> +  );
>>>
>>> /**
>>>   Get available system memory below 1MB by specified size.
>>> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 
>>> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
>>> index 791ae9db6e..5c9c4b3b1e 100644
>>> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
>>> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
>>> @@ -27,6 +27,9 @@ EnableDebugAgent (
>>>
>>> /**
>>>   Get pointer to CPU MP Data structure.
>>> +  For BSP, the pointer is retrieved from HOB.
>>> +  For AP, the pointer is retrieved from the location which stores 
>>> the PeiServices pointer.
>>> +  It's safe because BSP is blocking and has no chance to use 
>>> PeiServices pointer when AP is executing.
>>>
>>>   @return  The pointer to CPU MP Data structure.
>>> **/
>>> @@ -35,9 +38,17 @@ GetCpuMpData (
>>>   VOID
>>>   )
>>> {
>>> -  CPU_MP_DATA      *CpuMpData;
>>> -
>>> -  CpuMpData = GetCpuMpDataFromGuidedHob ();
>>> +  CPU_MP_DATA                  *CpuMpData;
>>> +  MSR_IA32_APIC_BASE_REGISTER  ApicBaseMsr;
>>> +  IA32_DESCRIPTOR              Idtr;
>>> +
>>> +  ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
>>> +  if (ApicBaseMsr.Bits.BSP == 1) {
>>> +    CpuMpData = GetCpuMpDataFromGuidedHob ();
>>> +  } else {
>>> +    AsmReadIdtr (&Idtr);
>>> +    CpuMpData = (CPU_MP_DATA *)(*(UINTN *) (Idtr.Base - sizeof 
>>> (UINTN)));
>>> +  }
>>>   ASSERT (CpuMpData != NULL);
>>>   return CpuMpData;
>>> }
>>> @@ -64,6 +75,45 @@ SaveCpuMpData (
>>>     );
>>> }
>>>
>>> +/**
>>> +  Push the CpuMpData for AP to use.
>>> +
>>> +  @param[in]  The pointer to CPU MP Data structure will be pushed.
>>> +  @param[out] The pointer to the context which will be passed to 
>>> PopCpuMpData().
>>> +
>>> +  @return  The pointer value which was stored in where the CPU MP 
>>> Data is pushed.
>>> +**/
>>> +VOID *
>>> +PushCpuMpData (
>>> +  IN  CPU_MP_DATA    *CpuMpData,
>>> +  OUT VOID           **Context
>>> +  )
>>> +{
>>> +  EFI_PEI_SERVICES  **PeiServices;
>>> +  IA32_DESCRIPTOR   Idtr;
>>> +
>>> +  AsmReadIdtr (&Idtr);
>>> +  *Context = (VOID *) (Idtr.Base - sizeof (UINTN));
>>> +  PeiServices = (EFI_PEI_SERVICES **)(*(UINTN *)(*Context));
>>> +  *(UINTN *)(*Context) = (UINTN)CpuMpData;
>>> +  return PeiServices;
>>> +}
>>> +
>>> +/**
>>> +  Pop the CpuMpData.
>>> +
>>> +  @param[in] Pointer  The pointer value which was stored in where 
>>> the CPU MP Data is pushed.
>>> +  @param[in] Context  The context of push/pop operation.
>>> +**/
>>> +VOID
>>> +PopCpuMpData (
>>> +  IN VOID           *Pointer,
>>> +  IN VOID           *Context
>>> +  )
>>> +{
>>> +  *(UINTN *)Context = (UINTN)Pointer;
>>> +}
>>> +
>>> /**
>>>   Check if AP wakeup buffer is overlapped with existing allocated buffer.
>>>
>>>
>>
>> I captured a KVM trace while the guest was stuck; the following messages
>> repeat infinitely:
>>
>>> CPU-8401  [000]  5171.301018: kvm_entry:            vcpu 0
>>> CPU-8401  [000]  5171.301019: kvm_exit:             reason DR_ACCESS 
>>> rip 0xbff0b28d info 17 0
>>> CPU-8401  [000]  5171.301019: kvm_entry:            vcpu 0
>>> CPU-8401  [000]  5171.301050: kvm_exit:             reason 
>>> EXCEPTION_NMI rip 0xbff03d30 info 0 80000306
>>> CPU-8401  [000]  5171.301051: kvm_emulate_insn:     0:bff03d30: 60
>>> CPU-8401  [000]  5171.301051: kvm_inj_exception:    #UD (0x0)
>>
>> The final part of the OVMF log is,
>>
>>> Loading PEIM at 0x000BFF05000 EntryPoint=0x000BFF0ADC6 CpuMpPei.efi
>>> AP Loop Mode is 1
>>> WakeupBufferStart = 9F000, WakeupBufferSize = 1000
>>> TimedWaitForApFinish: reached FinishedApLimit=7 in 0 microseconds
>>> APIC MODE is 1
>>> MpInitLib: Find 8 processors in system.
>>> Does not find any stored CPU BIST information from PPI!
>>>  APICID - 0x00000000, BIST - 0x00000000
>>>  APICID - 0x00000001, BIST - 0x00000000
>>>  APICID - 0x00000002, BIST - 0x00000000
>>>  APICID - 0x00000003, BIST - 0x00000000
>>>  APICID - 0x00000004, BIST - 0x00000000
>>>  APICID - 0x00000005, BIST - 0x00000000
>>>  APICID - 0x00000006, BIST - 0x00000000
>>>  APICID - 0x00000007, BIST - 0x00000000
>>> Install PPI: 9E9F374B-8F16-4230-9824-5846EE766A97
>>> Install PPI: EE16160A-E8BE-47A6-820A-C6900DB0250A
>>> Notify: PPI Guid: EE16160A-E8BE-47A6-820A-C6900DB0250A, Peim notify 
>>> entry point: 8524F8
>>> PlatformPei: OnMpServicesAvailable
>>
>> Note that the first address in the KVM trace, 0xBFF0B28D, is valid. It
>> is offset 0x628D bytes from the CpuMpPei.efi load address (0xBFF05000),
>> and the disassembly for the PEIM is consistent with the "DR_ACCESS"
>> trap:
>>
>>> 00000000000061e8 <HasErrorCode>:
>>>    61e8:55 push   %rbp
>>>    61e9:48 89 e5 mov    %rsp,%rbp
>>>    61ec:6a 00 pushq  $0x0
>>>    61ee:6a 00 pushq  $0x0
>>>    61f0:41 57 push   %r15
>>>    61f2:41 56 push   %r14
>>>    61f4:41 55 push   %r13
>>>    61f6:41 54 push   %r12
>>>    61f8:41 53 push   %r11
>>>    61fa:41 52 push   %r10
>>>    61fc:41 51 push   %r9
>>>    61fe:41 50 push   %r8
>>>    6200:50 push   %rax
>>>    6201:ff 75 08 pushq  0x8(%rbp)
>>>    6204:52 push   %rdx
>>>    6205:53 push   %rbx
>>>    6206:ff 75 30 pushq  0x30(%rbp)
>>>    6209:ff 75 00 pushq  0x0(%rbp)
>>>    620c:56 push   %rsi
>>>    620d:57 push   %rdi
>>>    620e:48 0f b7 45 38 movzwq 0x38(%rbp),%rax
>>>    6213:50 push   %rax
>>>    6214:48 0f b7 45 20 movzwq 0x20(%rbp),%rax
>>>    6219:50 push   %rax
>>>    621a:8c d8 mov    %ds,%eax
>>>    621c:50 push   %rax
>>>    621d:8c c0 mov    %es,%eax
>>>    621f:50 push   %rax
>>>    6220:8c e0 mov    %fs,%eax
>>>    6222:50 push   %rax
>>>    6223:8c e8 mov    %gs,%eax
>>>    6225:50 push   %rax
>>>    6226:48 89 4d 08 mov    %rcx,0x8(%rbp)
>>>    622a:ff 75 18 pushq  0x18(%rbp)
>>>    622d:48 31 c0 xor    %rax,%rax
>>>    6230:50 push   %rax
>>>    6231:50 push   %rax
>>>    6232:0f 01 0c 24 sidt   (%rsp)
>>>    6236:48 87 44 24 02 xchg   %rax,0x2(%rsp)
>>>    623b:48 87 04 24 xchg   %rax,(%rsp)
>>>    623f:48 87 44 24 08 xchg   %rax,0x8(%rsp)
>>>    6244:48 31 c0 xor    %rax,%rax
>>>    6247:50 push   %rax
>>>    6248:50 push   %rax
>>>    6249:0f 01 04 24 sgdt   (%rsp)
>>>    624d:48 87 44 24 02 xchg   %rax,0x2(%rsp)
>>>    6252:48 87 04 24 xchg   %rax,(%rsp)
>>>    6256:48 87 44 24 08 xchg   %rax,0x8(%rsp)
>>>    625b:48 31 c0 xor    %rax,%rax
>>>    625e:66 0f 00 c8 str    %ax
>>>    6262:50 push   %rax
>>>    6263:66 0f 00 c0 sldt   %ax
>>>    6267:50 push   %rax
>>>    6268:ff 75 28 pushq  0x28(%rbp)
>>>    626b:44 0f 20 c0 mov    %cr8,%rax
>>>    626f:50 push   %rax
>>>    6270:0f 20 e0 mov    %cr4,%rax
>>>    6273:48 0d 08 02 00 00 or     $0x208,%rax
>>>    6279:0f 22 e0 mov    %rax,%cr4
>>>    627c:50 push   %rax
>>>    627d:0f 20 d8 mov    %cr3,%rax
>>>    6280:50 push   %rax
>>>    6281:0f 20 d0 mov    %cr2,%rax
>>>    6284:50 push   %rax
>>>    6285:48 31 c0 xor    %rax,%rax
>>>    6288:50 push   %rax
>>>    6289:0f 20 c0 mov    %cr0,%rax
>>>    628c:50 push   %rax
>>>    628d:0f 21 f8 mov    %db7,%rax <-------- here
>>>    6290:50 push   %rax
>>>    6291:0f 21 f0 mov    %db6,%rax
>>>    6294:50 push   %rax
>>>    6295:0f 21 d8 mov    %db3,%rax
>>>    6298:50 push   %rax
>>>    6299:0f 21 d0 mov    %db2,%rax
>>>    629c:50 push   %rax
>>>    629d:0f 21 c8 mov    %db1,%rax
>>>    62a0:50 push   %rax
>>>    62a1:0f 21 c0 mov    %db0,%rax
>>>    62a4:50 push   %rax
>>>    62a5:48 81 ec 00 02 00 00sub    $0x200,%rsp
>>>    62ac:48 89 e7 mov    %rsp,%rdi
>>>    62af:0f ae 07 fxsave (%rdi)
>>>    62b2:fc cld
>>>    62b3:ff 75 10 pushq  0x10(%rbp)
>>>    62b6:48 8b 4d 08 mov    0x8(%rbp),%rcx
>>>    62ba:48 89 e2 mov    %rsp,%rdx
>>>    62bd:48 83 ec 28 sub    $0x28,%rsp
>>>    62c1:e8 61 0c 00 00 callq  6f27 <CommonExceptionHandler>
>>> 62c2: R_X86_64_PC32CommonExceptionHandler-0x4
>>>    62c6:48 83 c4 28 add    $0x28,%rsp
>>>    62ca:fa cli
>>>    62cb:48 83 c4 08 add    $0x8,%rsp
>>>    62cf:48 89 e6 mov    %rsp,%rsi
>>>    62d2:0f ae 0e fxrstor (%rsi)
>>>    62d5:48 81 c4 00 02 00 00add    $0x200,%rsp
>>>    62dc:48 83 c4 30 add    $0x30,%rsp
>>>    62e0:58 pop    %rax
>>>    62e1:0f 22 c0 mov    %rax,%cr0
>>>    62e4:48 83 c4 08 add    $0x8,%rsp
>>>    62e8:58 pop    %rax
>>>    62e9:0f 22 d0 mov    %rax,%cr2
>>>    62ec:58 pop    %rax
>>>    62ed:0f 22 d8 mov    %rax,%cr3
>>>    62f0:58 pop    %rax
>>>    62f1:0f 22 e0 mov    %rax,%cr4
>>>    62f4:58 pop    %rax
>>>    62f5:44 0f 22 c0 mov    %rax,%cr8
>>>    62f9:8f 45 28 popq   0x28(%rbp)
>>>    62fc:48 83 c4 30 add    $0x30,%rsp
>>>    6300:8f 45 18 popq   0x18(%rbp)
>>>    6303:58 pop    %rax
>>>    6304:58 pop    %rax
>>>    6305:58 pop    %rax
>>>    6306:8e c0 mov    %eax,%es
>>>    6308:58 pop    %rax
>>>    6309:8e d8 mov    %eax,%ds
>>>    630b:8f 45 20 popq   0x20(%rbp)
>>>    630e:8f 45 38 popq   0x38(%rbp)
>>>    6311:5f pop    %rdi
>>>    6312:5e pop    %rsi
>>>    6313:48 83 c4 08 add    $0x8,%rsp
>>>    6317:8f 45 30 popq   0x30(%rbp)
>>>    631a:5b pop    %rbx
>>>    631b:5a pop    %rdx
>>>    631c:59 pop    %rcx
>>>    631d:58 pop    %rax
>>>    631e:41 58 pop    %r8
>>>    6320:41 59 pop    %r9
>>>    6322:41 5a pop    %r10
>>>    6324:41 5b pop    %r11
>>>    6326:41 5c pop    %r12
>>>    6328:41 5d pop    %r13
>>>    632a:41 5e pop    %r14
>>>    632c:41 5f pop    %r15
>>>    632e:48 89 ec mov    %rbp,%rsp
>>>    6331:5d pop    %rbp
>>>    6332:48 83 c4 10 add    $0x10,%rsp
>>>    6336:48 83 7c 24 e0 00 cmpq   $0x0,-0x20(%rsp)
>>>    633c:74 14 je     6352 <DoReturn>
>>>    633e:48 83 7c 24 d8 01 cmpq   $0x1,-0x28(%rsp)
>>>    6344:74 04 je     634a <ErrorCode>
>>>    6346:ff 64 24 e0 jmpq   *-0x20(%rsp)
>>
>> (This function is from
>> "UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.S"
>> -- I guess it's already a problem that we are in that file at all?)
>>
>> However, the opcode 0x60 at address 0xBFF03D30, which triggers the #UD
>> exception ("invalid opcode"), is *below* the "CpuMpPei.efi" load address
>> (by 0x12D0 bytes).
>>
>>
>> Ray, can you please explain how this patch is supposed to work? Are you
>> re-purposing an otherwise unused (un-exercised) entry in the interrupt
>> descriptor table, for storing a generic pointer?
>>
>> ... The commit message says, "memory preceding IDT", and the patch says
>> "(Idtr.Base - sizeof (UINTN))". What memory is supposed to be there?
>>
> 
> Laszlo,
> 
> Some context. A few weeks ago I reported that the APs in this code were 
> using PEI Services to get a HOB to store the CPU context structure. PEI 
> Services are not defined as MP Safe so this is not a good long term 
> direction. Given this is PEI Code and can run XIP global variables may 
> not be writable so this code needs some why (other than a HOB) to have a 
> pointer.
> 
> Per the PI Spec Idtr.Base - sizeof(UINTN) is the address of the PEI 
> Services Table pointer. We add that so every PEI API did not need to 
> pass the PEI Services table pointer. It is what enables things like 
> ASSERT() and DEBUG() to work in PEI. Prior to this we had to have 
> PEI_ASSERT()/PEI_DEBUG() and pass the PEI Services Table pointer.
> 
> Ironically I'm in the process of making a change that uses Itdr.Base - 
> (2*sizeof(UINTN)) to store debugging context. Not sure if that is what 
> this code is doing?

Similar. This patch uses the location which stores PeiServices** to 
store the CpuMpData pointer because AP doesn't need to use PeiServices.

> 
> Thanks,
> 
> Andrew Fish
> 
> 
>> Here's a register dump, to see where the IDT is:
>>
>>> $ virsh qemu-monitor-command ovmf.fedora --hmp 'info registers'
>>>
>>> RAX=0000000000000000 RBX=00000000008524f8 RCX=00000000bfeebd30 
>>> RDX=ffffffffffffffff
>>> RSI=00000000bbf1c068 RDI=00000000bfeebd30 RBP=00000000bbf1bee0 
>>> RSP=00000000bbf1bea0
>>> R8 =0000000000000001 R9 =0000000000000000 R10=0000000000000000 
>>> R11=00000000000000b0
>>> R12=00000000bff14b60 R13=0000000000000000 R14=0000000000000000 
>>> R15=0000000000000000
>>> RIP=00000000bff090b3 RFL=00000093 [--S-A-C] CPL=0 II=0 A20=1 SMM=0 HLT=0
>>> ES =0008 0000000000000000 ffffffff 00c09300 DPL=0 DS   [-WA]
>>> CS =0018 0000000000000000 ffffffff 00a09b00 DPL=0 CS64 [-RA]
>>> SS =0008 0000000000000000 ffffffff 00c09300 DPL=0 DS   [-WA]
>>> DS =0008 0000000000000000 ffffffff 00c09300 DPL=0 DS   [-WA]
>>> FS =0008 0000000000000000 ffffffff 00c09300 DPL=0 DS   [-WA]
>>> GS =0008 0000000000000000 ffffffff 00c09300 DPL=0 DS   [-WA]
>>> LDT=0000 0000000000000000 0000ffff 00008200 DPL=0 LDT
>>> TR =0000 0000000000000000 0000ffff 00008b00 DPL=0 TSS64-busy
>>> GDT=     00000000ffffff80 0000001f
>>> IDT=     00000000bbf1dd58 0000021f
>>> CR0=80000033 CR2=0000000000000000 CR3=0000000000800000 CR4=00000668
>>> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 
>>> DR3=0000000000000000
>>> DR6=00000000ffff0ff0 DR7=0000000000000400
>>> EFER=0000000000000500
>>> FCW=037f FSW=0000 [ST=0] FTW=00 MXCSR=00001f80
>>> FPR0=0000000000000000 0000 FPR1=0000000000000000 0000
>>> FPR2=0000000000000000 0000 FPR3=0000000000000000 0000
>>> FPR4=0000000000000000 0000 FPR5=0000000000000000 0000
>>> FPR6=0000000000000000 0000 FPR7=0000000000000000 0000
>>> XMM00=00000000000000000000000000000000 
>>> XMM01=00000000000000000000000000000000
>>> XMM02=00000000000000000000000000000000 
>>> XMM03=00000000000000000000000000000000
>>> XMM04=00000000000000000000000000000000 
>>> XMM05=00000000000000000000000000000000
>>> XMM06=00000000000000000000000000000000 
>>> XMM07=00000000000000000000000000000000
>>> XMM08=00000000000000000000000000000000 
>>> XMM09=00000000000000000000000000000000
>>> XMM10=00000000000000000000000000000000 
>>> XMM11=00000000000000000000000000000000
>>> XMM12=00000000000000000000000000000000 
>>> XMM13=00000000000000000000000000000000
>>> XMM14=00000000000000000000000000000000 
>>> XMM15=00000000000000000000000000000000
>>
>> The IDT base address 0xBBF1DD58 doesn't tell me anything, unfortunately.
>> Here's a dump of the memory starting at (0xBBF1DD58 - 8):
>>
>>> $ virsh qemu-monitor-command ovmf.fedora --hmp 'xp /32gx 0xBBF1DD50'
>>>
>>> 00000000bbf1dd50: 0x00000000bbf1cac8 0xbff08e000018afb0
>>> 00000000bbf1dd60: 0x0000000000000000 0xbff08e000018afbf
>>> 00000000bbf1dd70: 0x0000000000000000 0xbff08e000018afce
>>> 00000000bbf1dd80: 0x0000000000000000 0xbff08e000018afdd
>>> 00000000bbf1dd90: 0x0000000000000000 0xbff08e000018afec
>>> 00000000bbf1dda0: 0x0000000000000000 0xbff08e000018affb
>>> 00000000bbf1ddb0: 0x0000000000000000 0xbff08e000018b00a
>>> 00000000bbf1ddc0: 0x0000000000000000 0xbff08e000018b019
>>> 00000000bbf1ddd0: 0x0000000000000000 0xbff08e000018b028
>>> 00000000bbf1dde0: 0x0000000000000000 0xbff08e000018b037
>>> 00000000bbf1ddf0: 0x0000000000000000 0xbff08e000018b046
>>> 00000000bbf1de00: 0x0000000000000000 0xbff08e000018b055
>>> 00000000bbf1de10: 0x0000000000000000 0xbff08e000018b064
>>> 00000000bbf1de20: 0x0000000000000000 0xbff08e000018b073
>>> 00000000bbf1de30: 0x0000000000000000 0xbff08e000018b082
>>> 00000000bbf1de40: 0x0000000000000000 0xbff08e000018b091
>>
>> Thanks
>> Laszlo
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
> 


-- 
Thanks,
Ray


  parent reply	other threads:[~2018-06-27  5:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-25  2:54 [PATCH] UefiCpuPkg/MpInitLib: AP uses memory preceding IDT to store CpuMpData Ruiyu Ni
2018-06-25 16:01 ` Laszlo Ersek
2018-06-25 17:01   ` Laszlo Ersek
2018-06-26  7:50     ` Ni, Ruiyu
2018-06-26 12:52       ` Laszlo Ersek
2018-06-26 17:06 ` Laszlo Ersek
2018-06-26 17:20   ` Andrew Fish
2018-06-26 18:57     ` Laszlo Ersek
2018-06-27  6:00       ` Ni, Ruiyu
2018-06-27 12:00         ` Laszlo Ersek
2018-06-29  9:36           ` Ni, Ruiyu
2018-06-27  5:06     ` Ni, Ruiyu [this message]
2018-06-27  4:50   ` Ni, Ruiyu

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=935660f2-e111-e871-f408-fcbd3e73da3d@Intel.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