On Tue, 2014-09-23 at 07:01 +0000, Fan, Jeff wrote: 
> Chen,
> 
> Please use this updated one. Thanks!

Thanks for your job. I will review it and
then merge to my code base.

Thanks,
Chen

> 
> Jeff
> 
> -----Original Message-----
> From: Fan, Jeff [mailto:[email protected]] 
> Sent: Tuesday, September 23, 2014 2:51 PM
> To: Chen, Fan
> Cc: [email protected]
> Subject: Re: [edk2] [RFC v3 00/19] Introduce Mp Service protocol to UefiCpuPkg
> 
> Chen,
> 
> I attached the updated CpuDxe driver to pass VS2008 and nasm building and 
> testing. Also, I updated some minor format.
> 
> Please review it and merge back to your code base.
> 
> Moreover, I have two more comments as below.
> 
> 1. Please use the following style to define enum CPU_STATE per EDKII C coding 
> standard spec. Do not use all UPPERCASE.
> typedef enum {
>   EnumMenberOne,  ///< First member descript.
>   EnumMemberTwo,  ///< Second member description.
>   EnumMemberMax ///< Number of members in this enum } ENUMERATE_TYPE ;
> 
> 2. Please use the following comments style define in EDKII C coding standard 
> spec for your code comments.
> •  Local comments must use the C++ comment ‘//.’
> •  Local comments must have a blank comment line, ‘//’, before and after the 
> comment block.
> 
> For example:
>    //
>    //  Comments
>    //
> 
> Jordan,
> 
> Do you still want to provide .asm version for MSFT based on Chen's the latest 
> patch? :-)
> 
> Thanks!
> Jeff
> 
> -----Original Message-----
> From: Chen, Fan [mailto:[email protected]]
> Sent: Tuesday, September 23, 2014 1:22 PM
> To: Fan, Jeff
> Cc: Andrew Fish; [email protected]; Jordan Justen
> Subject: Re: [RFC v3 00/19] Introduce Mp Service protocol to UefiCpuPkg
> 
> On Tue, 2014-09-23 at 03:34 +0000, Fan, Jeff wrote: 
> > Andrew,
> > 
> > gBS->Stall () implementation has no bug.
> > 
> > Actually, gBS->Stall () will consume EFI Metronome Arch Protocol that is 
> > published by Metronome module. But Metronome module is dispatched after 
> > CpuDxe module dispatched on Ovmf platform.
> > 
> > Before Metronome module dispatched, gBS->Stall () is not available. 
> > 
> > EFI_STATUS
> > EFIAPI
> > CoreStall (
> >   IN UINTN            Microseconds
> >   )
> >  {
> >   if (gMetronome == NULL) {
> >     return EFI_NOT_AVAILABLE_YET;
> >   }
> > }
> > 
> > Since CpuDxe also needs to publish CPU Arch protocol, it cannot assume 
> > gBS->Stall() is ready always. 
> > 
> > I suggest to use TimerLib Service instead of gBS->Stall in CpuDxe here.
> 
> I will update this suggestion to my github patches firstly.
> 
> Thanks,
> Chen
> 
> 
> > 
> > Another long term option, producing CPU MP protocol in another Cpu MP 
> > driver instead of in CpuDxe driver that is produce CPU ARCH protocol only.
> > 
> > Thanks!
> > Jeff
> > 
> > -----Original Message-----
> > From: Andrew Fish [mailto:[email protected]]
> > Sent: Monday, September 22, 2014 11:58 PM
> > To: Fan, Jeff
> > Cc: Chen, Fan; [email protected]; Jordan Justen
> > Subject: Re: [RFC v3 00/19] Introduce Mp Service protocol to 
> > UefiCpuPkg
> > 
> > 
> > On Sep 22, 2014, at 12:23 AM, Fan, Jeff <[email protected]> wrote:
> > 
> > > Yes. Timeout is root reason here.
> > > 
> > > It seemed that gBS->Stall() does not delay enough time as expected.
> > > I used MicroSecondDelay (100 * 1000) to replace gBS->Stall() in 
> > > StartApsStackless() to delay 100ms.  
> > > 
> > > The image ovmf.fd built by GCC could work on Windows Qemu now.
> > > C:\qemu\qemu-system-x86_64.exe -pflash OVMF64.fd -smp 4 -serial COM8
> > > 
> > 
> > gBS->Stall() should always work. We should fix the bug in the gBS->Stall().
> > 
> > Thanks,
> > 
> > Andrew Fish
> > 
> > > I will do some verification more. Thanks!
> > > 
> > > Jeff
> > > -----Original Message-----
> > > From: Chen, Fan [mailto:[email protected]]
> > > Sent: Monday, September 22, 2014 1:46 PM
> > > To: Fan, Jeff
> > > Cc: [email protected]; Jordan Justen; Andrew Fish
> > > Subject: Re: [RFC v3 00/19] Introduce Mp Service protocol to 
> > > UefiCpuPkg
> > > 
> > > Hi Jeff,
> > > 
> > > On Mon, 2014-09-22 at 05:12 +0000, Fan, Jeff wrote: 
> > >> Chen,
> > >> 
> > >> Thanks. I used the real platform to verify Cpu MP. It seemed that one AP 
> > >> is good but 3 APs still failed. Do you know what else need to consider?
> > >> Or could you tell me your test environment?
> > >> 
> > >> 1. -smp 4 faile, please see the attached log and console output $ 
> > >> sudo
> > >> qemu-system-x86_64 -bios OVMF64.fd -smp 4 --enable-kvm -serial 
> > >> file:ovmf64_4.log
> > >> 
> > > 
> > > Thanks for your testing, from the log file I can think of is the more CPU 
> > > would need more time to execute the start-up code, but MP just waited for 
> > > 100ms, I encountered the same issue when I set the number of cpu more 
> > > than 4. was the detect count of CPUs always two each time?
> > > 
> > > I used OVMF to test:
> > > command line: 
> > > ./OvmfPkg/build.sh qemu --enable-kvm -m 1024 -hda 
> > > /home/data/ovmf-rhel6.3.img -debugcon file:debug.log -global
> > > isa-debugcon.iobase=0x402 -smp 4
> > > 
> > > # /bin/qemu-system-x86_64 --version
> > > QEMU emulator version 2.1.50
> > > 
> > > Thanks,
> > > Chen
> > > 
> > > 
> > >> 2. -smp 2 succeed, please see the attached log.
> > >> $ sudo qemu-system-x86_64 -bios OVMF64.fd -smp 2 --enable-kvm 
> > >> -serial file:ovmf64_2.log
> > >> 
> > >> For IA32 arch, I got the exact result.
> > >> 
> > >> Thanks!
> > >> Jeff
> > >> -----Original Message-----
> > >> From: Chen, Fan [mailto:[email protected]]
> > >> Sent: Friday, September 19, 2014 5:24 PM
> > >> To: Fan, Jeff
> > >> Cc: [email protected]; Jordan Justen; Andrew Fish
> > >> Subject: Re: [RFC v3 00/19] Introduce Mp Service protocol to 
> > >> UefiCpuPkg
> > >> 
> > >> On Fri, 2014-09-19 at 07:07 +0000, Fan, Jeff wrote: 
> > >>> Chen & Jordan,
> > >>> 
> > >>> I encountered on exception as blew on Ubuntu12.04 (on virtual machine 
> > >>> by Virtual Box, VT-x enabled) with GCC46.  Log is attached. Do you have 
> > >>> any idea on it?
> > >>> 
> > >>> $ qemu-system-i386 -smp 4 -serial file:ovmf.log
> > >>> 
> > >>> Could not access KVM kernel module: No such file or directory 
> > >>> failed to initialize KVM: No such file or directory
> > >> 
> > >> I think you probably did not load the KVM modules. and used the qemu 
> > >> command with --enable-kvm.
> > >> 
> > >> Thanks,
> > >> Chen
> > >> 
> > >> 
> > >>> Back to tcg accelerator.
> > >>> qemu: fatal: Trying to execute code outside RAM or ROM at
> > >>> 0x000a0000
> > >>> 
> > >>> EAX=00000000 EBX=00000000 ECX=00000000 EDX=00000633
> > >>> ESI=00000000 EDI=00001ff8 EBP=00000000 ESP=00000000 EIP=00000ffc
> > >>> EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0000
> > >>> 00000000 0000ffff 00009300 CS =9f00 0009f000 0000ffff 00009b00 SS
> > >>> =0000
> > >>> 00000000 0000ffff 00009300 DS =0000 00000000 0000ffff 00009300 FS
> > >>> =0000 00000000 0000ffff 00009300 GS =0000 00000000 0000ffff
> > >>> 00009300
> > >>> LDT=0000 00000000 0000ffff 00008200 TR =0000 00000000 0000ffff
> > >>> 00008b00
> > >>> GDT=     00000000 0000ffff
> > >>> IDT=     00000000 0000ffff
> > >>> CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
> > >>> DR0=00000000 DR1=00000000 DR2=00000000 DR3=00000000
> > >>> DR6=ffff0ff0 DR7=00000400
> > >>> CCS=00000000 CCD=00000000 CCO=SUBW    
> > >>> EFER=0000000000000000
> > >>> 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
> > >>> Aborted (core dumped)
> > >>> 
> > >>> Thanks!
> > >>> Jeff
> > >>> -----Original Message-----
> > >>> From: Chen Fan [mailto:[email protected]]
> > >>> Sent: Thursday, September 18, 2014 11:12 AM
> > >>> To: [email protected]
> > >>> Cc: Jordan Justen; Fan, Jeff; Andrew Fish
> > >>> Subject: [RFC v3 00/19] Introduce Mp Service protocol to 
> > >>> UefiCpuPkg
> > >>> 
> > >>> This series patchset try to implement Mp Service protocol in 
> > >>> UefiCpuPkg, Jordan had implemented the startup APs code, and I try to 
> > >>> add more initialization code to let all APs work up, this Mp Service 
> > >>> protocol's implementation used EmulatorPkg/MpService for reference.
> > >>> this patches works on my github:
> > >>>  https://github.com/ChenFanFnst/edk2/tree/cpu-mp-service
> > >>> 
> > >>> V2-V3:
> > >>>  1. rebase codes due to Jordan'tree updated:
> > >>>     https://github.com/jljusten/edk2/tree/ap-startup-example
> > >>>  2. add supported on Ia32 arch
> > >>>  3. add a new Lock to replace present SpinLock mechanisms in mutilple
> > >>>     processors, maybe the SpinLock mechanisms is not MP safe.
> > >>>  4. add function header
> > >>>  5. add StartupAllAPs() supported
> > >>>  6. add SwitchBSP() function, which is unsupported.
> > >>> 
> > >>> V1-V2:
> > >>>  1. do not call anything EFI API from APs.
> > >>>  2. add AP busy-wait for task assignment from BSP and get rid of
> > >>>     IPI sent mechanism.
> > >>> 
> > >>> TODO:
> > >>>  1) StartupThisAP/StartAllAps: when Timeout expires before AP returns
> > >>>     from Procedure, constrainedly terminate the executed Procedure.
> > >>> 
> > >>> Any Comment is welcome.
> > >>> 
> > >>> Chen Fan (13):
> > >>>  UefiCpuPkg/CpuDxe: Introduce AsmApDoneWithCommonStack
> > >>>  UefiCpuPkg/CpuDxe: Waiting for Aps initialization done
> > >>>  UefiCpuPkg/CpuDxe: Switch Ap CommonStack with NewStack
> > >>>  UefiCpuPkg/CpuDxe: install Mp Service protocol
> > >>>  UefiCpuPkg/CpuDxe: implement Mp Protocol: GetNumberOfProcessors()
> > >>>  UefiCpuPkg/CpuDxe: implement Mp Protocol: WhoAmI()
> > >>>  UefiCpuPkg/CpuDxe: implement Mp Services: GetProcessorInfo()
> > >>>  UefiCpuPkg/CpuDxe: implement Mp Protocol: EnableDisableAP()
> > >>>  UefiCpuPkg/CpuDxe: implement Mp Protocol: StartupThisAP()
> > >>>  UefiCpuPkg/CpuDxe: introduce CPU bus lock for sync data
> > >>>  UefiCpuPkg/CpuDxe: implement Mp Services: StartupAllAPs()
> > >>>  UefiCpuPkg/CpuDxe: implement Mp Services: SwitchBSP()
> > >>>  UefiCpuPkg/CpuDxe: add Mp Service TestCase
> > >>> 
> > >>> Jordan Justen (6):
> > >>>  UefiCpuPkg/CpuDxe: Add no-op InitializeMpSupport
> > >>>  UefiCpuPkg/CpuDxe: Add ApEntryPointInC
> > >>>  UefiCpuPkg/CpuDxe: Add stackless assembly AP entry points
> > >>>  UefiCpuPkg/CpuDxe: Move GDT structures into CpuGdt.h
> > >>>  UefiCpuPkg/CpuDxe: Add StartApsStackless routine
> > >>>  UefiCpuPkg/CpuDxe: Startup APs
> > >>> 
> > >>> UefiCpuPkg/CpuDxe/ApStartup.asm   |  111 +++
> > >>> UefiCpuPkg/CpuDxe/ApStartup.c     |  209 ++++++
> > >>> UefiCpuPkg/CpuDxe/CpuDxe.c        |    5 +
> > >>> UefiCpuPkg/CpuDxe/CpuDxe.inf      |    8 +
> > >>> UefiCpuPkg/CpuDxe/CpuGdt.c        |   52 +-
> > >>> UefiCpuPkg/CpuDxe/CpuGdt.h        |   72 ++
> > >>> UefiCpuPkg/CpuDxe/CpuMp.c         | 1349 
> > >>> +++++++++++++++++++++++++++++++++++++
> > >>> UefiCpuPkg/CpuDxe/CpuMp.h         |  643 ++++++++++++++++++
> > >>> UefiCpuPkg/CpuDxe/CpuMpTest.c     |   76 +++
> > >>> UefiCpuPkg/CpuDxe/Ia32/MpAsm.S    |  109 +++
> > >>> UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm |  144 ++++
> > >>> UefiCpuPkg/CpuDxe/X64/MpAsm.S     |  111 +++
> > >>> UefiCpuPkg/CpuDxe/X64/MpAsm.nasm  |  142 ++++
> > >>> 13 files changed, 2980 insertions(+), 51 deletions(-)  create mode
> > >>> 100644 UefiCpuPkg/CpuDxe/ApStartup.asm  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/ApStartup.c  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/CpuGdt.h  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/CpuMp.c  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/CpuMp.h  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/CpuMpTest.c  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/Ia32/MpAsm.S  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/X64/MpAsm.S  create mode 100644 
> > >>> UefiCpuPkg/CpuDxe/X64/MpAsm.nasm
> > >>> 
> > >>> --
> > >>> 1.9.3
> > >>> 
> > >> 
> > > 
> > 
> 

------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to