On 2016-05-09 08:53:12, Yao, Jiewen wrote: > Jordan > Do you know why we have ArmPkg and ArmPlatformPkg? >
One reason is because we don't have a DriversPkg. Another reason is probably because the effort wasn't made to put them into common packages. > If you take a look at that, you will find many Arm specific driver > there, including CpuDxe/CpuPei/BaseMemoryLib/ > PeiServicesTablePointerLib/etc... > Should we create a package for IA32 and X64 to do the same? I think instead we put the IA32/X64 modules in other locations, and I think that made sense. If it made sense for IA32 and X64, then it should make sense for other architectures as well. > I do not think it is bad idea to have RiscVPkg, when we are not > clear on where to put it. > So this is the place to dump things that we don't know where else to put them? That doesn't inspire too much confidence. :) I agree that it might be needed, but can we try to minimize it? -Jordan > > > -----Original Message----- > > From: Justen, Jordan L > > Sent: Monday, May 9, 2016 11:46 PM > > To: Yao, Jiewen <[email protected]>; Chang, Abner (HPS SW/FW > > Technologist) <[email protected]>; [email protected] > > Cc: Gao, Liming <[email protected]> > > Subject: RE: [edk2] [PATCH] MdeModulePkg/DxeIplPeim: RISC-V arch DxeIpl. > > > > On 2016-05-09 08:12:20, Yao, Jiewen wrote: > > > Thank Abner. > > > For RiscVPkg, my personally feeling is that it should stick to RiscV > > > architecture, like ArmPkg. > > > > > > > I don't agree. Why don't we have an X64Pkg? I think it is because > > we've defined good locations already for most modules, and many of > > those modules already support multiple architectures. > > > > Can't the first try be to see if we can get by without adding new > > architecture specific packages? > > > > > Below module seems to be software concept only. They are independent > > > to RISV-V architecture, right? > > > I am not sure if it is proper to put to RiscVPkg. > > > > > > > RiscVPkg/Universal/Logo/Logo.uni | Bin 0 -> > > 1948 bytes > > > > RiscVPkg/Universal/Logo/LogoExtra.uni | Bin 0 -> > > 1342 bytes > > > > RiscVPkg/Universal/Logo/RiscvUefiLogo.bmp | Bin 0 -> > > 12446 bytes > > > > RiscVPkg/Universal/Logo/RiscvUefiLogo.inf | 34 + > > > > RiscVPkg/Universal/RiscvBadgingDxe/RiscvBadging.c | 107 +++ > > > > RiscVPkg/Universal/RiscvBadgingDxe/RiscvBadging.h | 32 + > > > > .../Universal/RiscvBadgingDxe/RiscvBadgingDxe.inf | 54 ++ > > > > > > Maybe we can put to a RiscV platform pkg? > > > > > > > I think it would be better to add the logo to a common location like, > > perhaps MdeModulePkg/Universal/Logo/RiscV. > > > > Actually, I don't think the new logo is needed. We don't have > > architecture specific logos in other cases, and I don't think it is > > needed here. > > > > -Jordan > > > > > > > > > -----Original Message----- > > > > From: edk2-devel [mailto:[email protected]] On Behalf > > Of > > > > Chang, Abner (HPS SW/FW Technologist) > > > > Sent: Monday, May 9, 2016 10:10 PM > > > > To: Justen, Jordan L <[email protected]>; > > [email protected] > > > > Cc: Yao, Jiewen <[email protected]>; Gao, Liming > > > > <[email protected]> > > > > Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeIplPeim: RISC-V arch > > DxeIpl. > > > > > > > > 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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

