Hi Jordan,
I will send out another draft version of patches which address all comments. 
Then upstream RISC-V code to the branch.
Sure, I am pleased to check if there is any reusable modules for RISC-V.
Thanks
Abner

-----Original Message-----
From: Jordan Justen [mailto:[email protected]] 
Sent: Monday, May 09, 2016 2:25 PM
To: Chang, Abner (HPS SW/FW Technologist) <[email protected]>; 
[email protected]
Cc: [email protected]; Yao, Jiewen <[email protected]>
Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeIplPeim: RISC-V arch DxeIpl.

On 2016-05-08 04:19:08, Chang, Abner (HPS SW/FW Technologist) wrote:
> Hi Jordan,
> The UEFI/PI ECR for RISC-V is ready but not yet send to UEFI for 
> review. I have been told to upstream RISC-V code first and then submit 
> the spec. I will confirm this again.

You were told to upstream EDK II support before the UEFI spec documented the 
architecture? Or, maybe to just prepare the EDK II code so it would be ready 
for upstream once it is in the spec?

Maybe EDK II could adopt a new idea of an "unsupported architecture", but it 
seems like EDK II might make some mistakes if it doesn't wait for UEFI to 
standardize the architecture. For example, what if the calling convention 
changes before it makes it into the spec?

I think Jiewen's idea of edk2-staging is good. But, before that, it seems you 
should make another draft version of the patches in fork of EDK II on github.

In a few cases, I asked if the modules could be added to more generic 
locations. How about trying to look over everything in a RiscV*Pkg, and ask if 
it could instead live in Mde*Pkg, UefiCpuPkg or OvmfPkg?
(And, hopefully by maximally reusing existing modules with architecture 
specific sources as needed...)

For example, we don't have a X64*Pkg...

-Jordan

> Below response to your comments,
> > ---
> >  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf            |  6 +-
> >  MdeModulePkg/Core/DxeIplPeim/Riscv64/DxeLoadFunc.c | 79
> > ++++++++++++++++++++++
> >>>Is it Riscv64 or RiscV64?
> >>>Like MdePkg, I think MdeModulePkg only upports achitectures in the UEFI 
> >>>spec.
> 
> This is typo, should be RiscV64. RISC-V arch UEFI spec is ready for review.
> 
> > 
> > ---
> >  RiscVPkg/Include/RiscV.h                           | 105 +++
> >  .../PeiServicesTablePointer.c                      | 121 +++
> >  .../PeiServicesTablePointerLib.inf                 |  46 ++
> >  .../PeiServicesTablePointerLib.uni                 | Bin 0 -> 2520 bytes
> >
> >>> utf-8. (See other my other email.)
> >>>
> >>>I think this library should live in MdePkg, except MdePkg only 
> >>>supports architectures that are in the UEFI spec.
> PeiServerTablePointer lib is very processor depends. PI spec changes for this 
> is ready for review as well.
> 
> >>> RiscVPkg,
> >>>This package seems to be mash up of things that possibly should be 
> >>>in MdePkg, UefiCpuPkg, a chipset package, and possibly platform packages.
> >>>
> Modules and libraries under RiscVPkg are all processor specific. Like Timer 
> is provided by RISC-V processor itself instead of using something else on 
> platform. ResetVector, Sec, exception and etc. are all the same.
> UefiCpuPkg seems to me still not clean enough for common CPU usage. Like sec 
> and ResetVector, we still have to use the modules from other processor 
> package. So put RISC-V modules under its own package is mush clean in my 
> opinion for now. 
> 
> >>>What is the status of the QEMU port?
> >>>I don't think we should put this upstream into EDK II until QEMU 
> >>>has it upstream. Maybe https://github.com/tianocore/edk2-platforms 
> >>>in a ovmf-riscv branch?
> There is no official QEMU supports RISC-V yet. The QEMU which supports RISC-V 
> QEMU is on RISC-V Github. However, I have another RISC-V QEMU port for PC/AT 
> which uses PCI as the platform bus spec because RISC-V platform spec is not 
> yet ready.
> The RISC-V QEMU PC/AT port is on my github. The detail is mentioned in 
> the readme file in RiscVVirtPkg. You also can refer to this link 
> https://github.com/AbnerChang/RiscVQemuPcat
> 
> >>>What would prevent this from living under OvmfPkg? Say 
> >>>OvmfPkg/OvmfPkgRiscV64.dsc?
> I think it should be no problem to use OvmfPkg as common processor/platform 
> emulator. Just you still can see few Intel things. ARM QEMU implementation 
> must be considered as well if we move RISC-V to OvmfPkg, thus we can have the 
> consistent implementation.
> RISC-V QEMU implementation just follows ARM QEMU implement, that is to have a 
> separate package. But I agree with you that all processors/platforms emulator 
> should be under OvmfPkg. 
> 
> >>>This should be utf-8:
> >>>BaseTools/Scripts/ConvertUni.py
> I will fix this.
> 
> I think I set the configurations when submit patches. Will figure it out.
> Thanks
> Abner
> 
> -----Original Message-----
> From: Jordan Justen [mailto:[email protected]]
> Sent: Sunday, May 08, 2016 1:53 PM
> To: Chang, Abner (HPS SW/FW Technologist) <[email protected]>; 
> [email protected]
> Cc: [email protected]
> Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeIplPeim: RISC-V arch DxeIpl.
> 
> On 2016-05-07 21:23:36, Abner Chang wrote:
> > From: AbnerChang <[email protected]>
> > 
> 
> git config settings. :)
> 
> > The implementation of RISC-V DxeIpl.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Abner Chang<[email protected]>
> 
> space before <
> 
> > ---
> >  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf            |  6 +-
> >  MdeModulePkg/Core/DxeIplPeim/Riscv64/DxeLoadFunc.c | 79
> > ++++++++++++++++++++++
> 
> Is it Riscv64 or RiscV64?
> 
> Like MdePkg, I think MdeModulePkg only supports achitectures in the UEFI spec.
> 
> -Jordan
> 
> >  2 files changed, 84 insertions(+), 1 deletion(-)  create mode 
> > 100644 MdeModulePkg/Core/DxeIplPeim/Riscv64/DxeLoadFunc.c
> > 
> > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> > index 04ad928..13c0852 100644
> > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> > @@ -6,6 +6,7 @@
> >  #  needed to run the DXE Foundation.
> >  #
> >  #  Copyright (c) 2006 - 2015, Intel Corporation. All rights 
> > reserved.<BR>
> > +#  Copyright (c) 2016, Hewlett Packard Enterprise Development LP. 
> > +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 @@ -29,7 +30,7 @@  #  # The following information is 
> > for reference only and not required by the build tools.
> >  #
> > -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC (EBC is for build 
> > only) AARCH64
> > +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC (EBC is for build 
> > only) AARCH64 RISCV64
> >  #
> >  
> >  [Sources]
> > @@ -57,6 +58,9 @@
> >  [Sources.ARM, Sources.AARCH64]
> >    Arm/DxeLoadFunc.c
> >  
> > +[Sources.RISCV64]
> > +  Riscv64/DxeLoadFunc.c
> > +
> >  [Packages]
> >    MdePkg/MdePkg.dec
> >    MdeModulePkg/MdeModulePkg.dec
> > diff --git a/MdeModulePkg/Core/DxeIplPeim/Riscv64/DxeLoadFunc.c
> > b/MdeModulePkg/Core/DxeIplPeim/Riscv64/DxeLoadFunc.c
> > new file mode 100644
> > index 0000000..6b42e44
> > --- /dev/null
> > +++ b/MdeModulePkg/Core/DxeIplPeim/Riscv64/DxeLoadFunc.c
> > @@ -0,0 +1,79 @@
> > +/** @file
> > +  RISC-V specific functionality for DxeLoad.
> > +
> > +  Copyright (c) 2016, Hewlett Packard Enterprise Development LP. 
> > + 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  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" 
> > +BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> > IMPLIED.
> > +**/
> > +
> > +#include "DxeIpl.h"
> > +
> > +typedef
> > +VOID*
> > +(EFIAPI *DXEENTRYPOINT) (
> > +  IN  VOID *HobStart
> > +  );
> > +
> > +/**
> > +   Transfers control to DxeCore.
> > +
> > +   This function performs a CPU architecture specific operations to execute
> > +   the entry point of DxeCore with the parameters of HobList.
> > +   It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
> > +
> > +   @param DxeCoreEntryPoint         The entry point of DxeCore.
> > +   @param HobList                   The start of HobList passed to DxeCore.
> > +
> > +**/
> > +VOID
> > +HandOffToDxeCore (
> > +  IN EFI_PHYSICAL_ADDRESS   DxeCoreEntryPoint,
> > +  IN EFI_PEI_HOB_POINTERS   HobList
> > +  )
> > +{
> > +  VOID                            *BaseOfStack;
> > +  VOID                            *TopOfStack;
> > +  EFI_STATUS                      Status;
> > +  //
> > +  //
> > +  // Allocate 128KB for the Stack
> > +  //
> > +  BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
> > +  ASSERT (BaseOfStack != NULL);
> > +
> > +  //
> > +  // Compute the top of the stack we were allocated. Pre-allocate a 
> > + UINTN  // for safety.
> > +  //
> > +  TopOfStack = (VOID *) ((UINTN) BaseOfStack + EFI_SIZE_TO_PAGES
> > + (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);  TopOfStack = 
> > + ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
> > +
> > +  //
> > +  // End of PEI phase signal
> > +  //
> > +  Status = PeiServicesInstallPpi (&gEndOfPeiSignalPpi); 
> > + ASSERT_EFI_ERROR (Status);
> > +
> > +  //
> > +  // Update the contents of BSP stack HOB to reflect the real stack info 
> > passed to DxeCore.
> > +  //    
> > +  UpdateStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN) BaseOfStack, 
> > + STACK_SIZE);
> > +
> > +  DEBUG ((EFI_D_INFO, "DXE Core new stack at %x, stack pointer at 
> > + %x\n", BaseOfStack, TopOfStack));
> > +
> > +  //
> > +  // Transfer the control to the entry point of DxeCore.
> > +  //
> > +  SwitchStack (
> > +    (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeCoreEntryPoint,
> > +    HobList.Raw,
> > +    NULL,
> > +    TopOfStack
> > +    );
> > +}
> > --
> > 1.9.5.msysgit.0
> > 
> > _______________________________________________
> > edk2-devel mailing list
> > [email protected]
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to