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

Reply via email to