Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RiscV64 Support In UPL #4367

Closed

Conversation

dhaval-rivos
Copy link
Contributor

This patchset contains modifications required in UPL boot flow to
enable RiscV64 support. Squashing earlier series of patches
related to arch specific removal from UPL path to this release such
that all chnages can be viewed in the entirety.has following
proposed changes in addition to arch specific removal patchset:

  1. Add required infra to support RiscV64
  2. New UPL proposal supports multiple Archs and accordingly ABI
    is also different. This is a proposal to accomodate these changes.
    This patch adds a hook to parse RiscV64 specific IN params and extract
    FDT from the same.
  3. Take FDT received from BL and parse it to create required HOBs that
    are later consumed by UPL.

P.S. This patch is review only at this point as it is tested in a limited
fashion for RV to get to early debug logs from FDT described serial device.
It will require modifications to work on other Archs.

According to UPL spec BL should initialize FP init meaning UPL
does not need to initialize it. Besides this is arch specific init
and needs to be moved out of UPL common flow. In order to not break
current BL implementations, for now just moving the init to later
point of time but for both x32 and x64 eventually this should be
removed once BL impelement this logic.

Test: Verified booting  UEFI shell on coreboot on qemu.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>

Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>

Reviewed-by: Gua Guo <gua.guo@...>
Reviewed-by: James Lu <james.lu@...>
8259 is very arch specific programming. It needs to be moved out to
the respective arch flow. Added in both x64 and x32 paths

Test: Able to boot UEFI shell with Coreboot Tianocore payload on
x86 qemu

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>

Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>

Reviewed-by: Gua Guo <gua.guo@...>
Reviewed-by: James Lu <james.lu@...>
Add support for RiscV64 Arch for UPL. The Patch primarily
allows one to compile UPL with RV64 GCC tools.
It builds on top of earlier patches submitted for UPL
boot flow modifications where arch specific initialization
was removed. Follow up patches will be added to further the
boot flow.

Test: Tested that code compiles UPL Pkg with RV64 GCC5

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Cc: Sunil V <sunilvl@ventanamicro.com>

Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
According to UPL spec for various archs ABIs we have different
input passing arguments. We provide Arch specific hooks to
ensure FDT is populated from the correct argument following the
spec. TODO: Will have to create one parser per arch.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Cc: Sunil V <sunilvl@ventanamicro.com>
Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
Provide an arch specific hook which will consume IN params
to UPL and populate FDT variable properly. Each Arch has its
own ABI and accordingly input param changes. First part, This
hook will ensure correct input param is used to populate FDT
value. Second part, after finding proper FDT, it uses FDT to
create HOBs that are later consumed by UPL.New implementation
may remove HOB creation as per latest spec. First part will
still be relevant.

Test: Able to parse FDT, create HOBs and get serial debug logs.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Cc: Sunil V <sunilvl@ventanamicro.com>

Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
RV CPU driver requires access to HartID and FDT passed by BL.
Set it through FirmwareContext. In future this should be passed
as part of FDT itself to avoid any custome structures.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Cc: Sunil V <sunilvl@ventanamicro.com>
Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
UPL expects to have Dxe info from gUniversalPayloadExtraDataGuid HOB.
With new FDT proposal this requirement should go away. But for now
satisfying this requirement through RV hook.

Test: Able to enter DxeMain function after finding correct DxeFv.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Cc: Sunil V <sunilvl@ventanamicro.com>
Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
Added required Dxe and Arch Proto drivers to ensure we
are able to boot to Shell.

Test: Able to boot to UEFI Shell

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Cc: Sunil V <sunilvl@ventanamicro.com>
Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
Copy link
Contributor

@gguo11837463 gguo11837463 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, I mean Spec 1.0, it will enable FDT and FIT both based on UniversalPayload Spec 1.0.

  1. FitUniversalPayloadEntry.inf will exist in that time, we can add these changes there.
  2. Currently, have two dsc file UefiPayloadPkg.dsc and UefiPayloadPkgRV64.dsc. it does not make sense for me.

Could you help to merge UefiPayloadPkg.dsc and UefiPayloadPkgRV64.dsc into UefiPayloadPkg.dsc follow below rule ?
[Components] -> IA32/X64/RISCV64
[Components.IA32] -> For IA32
[Components.X64] -> For X64
[Components.RISCV64] -> RISCV64

  1. The change will hold on after formal spec release for everyone have access right. In that time, it will make sense to integrate it.

  2. "BuildBlHobs" return success here may have some problem for IA32/X64 directly.
    UefiPayloadPkg/UefiPayloadEntry/Ia32/Ia32FdtParserLib.c
    UefiPayloadPkg/UefiPayloadEntry/X64/X64FdtParserLib.c

  3. FdtLib you may need to prevent to use FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
    Recommend you may use "FdtLib|MdePkg/Library/BaseFdtLib/BaseFdtLib.inf", we need to prevent UefiPayloadPkg have EmbeddedPkg dependency.

@dhaval-rivos
Copy link
Contributor Author

  1. FitUniversalPayloadEntry.inf

For comments 1-3, you suggest we integrate these changes when we have FIT implementation in place? For now let us continue with the review though and flush out other implementation?

@dhaval-rivos
Copy link
Contributor Author

dhaval-rivos commented May 16, 2023

4. BuildBlHobs

Yes. This is just a placeholder for now. Eventually I expect every BL/Arch to have correct implementation in place. With FIT implementation it will become obvious. In fact I am thinking of changing the name of the function to something like ParseUplInputParams(). Does it make sense? How do we align to ensure FIT based implementation that is WIP from our side is complying to this?

@dhaval-rivos
Copy link
Contributor Author

5. FdtLib

Sounds good. I am ready with the change. Once all comments are discussed with spin off another version.

@gguo11837463
Copy link
Contributor

  1. BuildBlHobs

Yes. This is just a placeholder for now. Eventually I expect every BL/Arch to have correct implementation in place. With FIT implementation it will become obvious. In fact I am thinking of changing the name of the function to something like ParseUplInputParams(). Does it make sense? How do we align to ensure FIT based implementation that is WIP from our side is complying to this?

ParseUplInputParams() should be good.

FIT POC is on the PR #4245 maybe you can try to integrated into the PR. FDT is still in discussion.

@mergify
Copy link

mergify bot commented Jul 24, 2023

PR can not be merged due to conflict. Please rebase and resubmit

@lersek lersek closed this Oct 4, 2023
@dhaval-rivos dhaval-rivos deleted the upl-rv64-enable-compilation-v1 branch November 30, 2023 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants