Hi,

I have started to implement the corresponding changes in EDK2: 
https://github.com/changab/edk2-staging-riscv/compare/5f63e9249751ccb9302514455b9a1a7038f34547...RISC-V-DT-fixup
What happens is: The DTB is embedded in the FW image and passed to sbi_init in 
SEC phase. We initialize OpenSBI as early as possible and because it also wants 
to modify the device tree, I have to pass it the DTB as next_arg1.
Then it's passed to PEI in the firmware context and further to DXE via a HOB. 
In DXE the boot-hartid is inserted and it's installed into the EFI system 
config table from where the EFISTUB reads it.

Unfortunately I don't get any console output after the EFISTUB calls 
ExitBootServices, so my patch is still WIP.
Atish, which platform file in OpenSBI did you use to test your changes? 
platform/qemu/virt/platform.c or platform/sifive/fu540/platform.c ?
Maybe the failure is unrelated to the new patches - we've never booted Linux 
from EDKII yet.


I'm a bit skeptical whether DT is the best way to pass the boothartid to the 
kernel. Ard has argued that it's good because it's independent from UEFI, but 
the proper kernel doesn't read the hartid from there - it gets it from a0. Only 
the EFISTUB reads the hartid from the device tree. Therefore the solution we 
need is EFI specific anyways, right?
One of the design goals is that we don't want to force bootloaders, which load 
the EFISTUB (e.g. UEFI Shell, grub chainloading, systemd-boot), to change.

If we let the firmware embed the hartid in the DT, the user cannot swap out the 
DT later for their custom one. They need to use the one given by the firmware.
Of course we could add commands and config to bootloaders to load and fixup 
(embed hartid) the device tree... but, as mentioned earlier, we want to avoid 
that.
Additionally from EDKII side we're also planning to run later stages, including 
the bootloader, in S-Mode, where they wouldn't even have access to mhartid and 
thus couldn't fixup the DT.

If instead the firmware writes the hartid into the EFI system config table, the 
EFISTUB can read it from there, just like it does the device tree already. Then 
bootloaders can put a user supplied DT in the config table, too, like they 
always have.

What do you all think - does that make sense?

- Daniel
On 2/25/20 10:07 AM, Ard Biesheuvel wrote:

On Tue, 25 Feb 2020 at 09:59, Chang, Abner (HPS SW/FW Technologist)
<abner.ch...@hpe.com><mailto:abner.ch...@hpe.com> wrote:


-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Tuesday, February 25, 2020 4:48 PM
To: Chang, Abner (HPS SW/FW Technologist) 
<abner.ch...@hpe.com><mailto:abner.ch...@hpe.com>
Cc: Atish Patra <ati...@atishpatra.org><mailto:ati...@atishpatra.org>; Heinrich 
Schuchardt
<xypron.g...@gmx.de><mailto:xypron.g...@gmx.de>; Atish Patra 
<atish.pa...@wdc.com><mailto:atish.pa...@wdc.com>; U-Boot Mailing
List <u-boot@lists.denx.de><mailto:u-boot@lists.denx.de>; Alexander Graf 
<ag...@csgraf.de><mailto:ag...@csgraf.de>; Anup Patel
<anup.pa...@wdc.com><mailto:anup.pa...@wdc.com>; Bin Meng 
<bmeng...@gmail.com><mailto:bmeng...@gmail.com>; Joe
Hershberger <joe.hershber...@ni.com><mailto:joe.hershber...@ni.com>; Loic 
Pallardy
<loic.palla...@st.com><mailto:loic.palla...@st.com>; Lukas Auer 
<lukas.a...@aisec.fraunhofer.de><mailto:lukas.a...@aisec.fraunhofer.de>;
Marek BehĂșn <marek.be...@nic.cz><mailto:marek.be...@nic.cz>; Marek Vasut
<marek.va...@gmail.com><mailto:marek.va...@gmail.com>; Patrick Delaunay 
<patrick.delau...@st.com><mailto:patrick.delau...@st.com>;
Peng Fan <peng....@nxp.com><mailto:peng....@nxp.com>; Philippe Reynes
<philippe.rey...@softathome.com><mailto:philippe.rey...@softathome.com>; Simon 
Glass <s...@chromium.org><mailto:s...@chromium.org>;
Simon Goldschmidt 
<simon.k.r.goldschm...@gmail.com><mailto:simon.k.r.goldschm...@gmail.com>; 
Stefano Babic
<sba...@denx.de><mailto:sba...@denx.de>; Thierry Reding 
<tred...@nvidia.com><mailto:tred...@nvidia.com>; Tom Rini
<tr...@konsulko.com><mailto:tr...@konsulko.com>; 
l...@nuviainc.com<mailto:l...@nuviainc.com>; Schaefer, Daniel (DualStudy)
<daniel.schae...@hpe.com><mailto:daniel.schae...@hpe.com>
Subject: Re: [RFC PATCH 0/1] Add boot hartid to a Device tree

On Tue, 25 Feb 2020 at 09:28, Chang, Abner (HPS SW/FW Technologist)
<abner.ch...@hpe.com><mailto:abner.ch...@hpe.com> wrote:


-----Original Message-----
From: Atish Patra [mailto:ati...@atishpatra.org]


<snip header soup>



On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
<ard.biesheu...@linaro.org><mailto:ard.biesheu...@linaro.org> wrote:


On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt
<xypron.g...@gmx.de><mailto:xypron.g...@gmx.de>


wrote:


On 2/24/20 11:19 PM, Atish Patra wrote:


The RISC-V booting protocol requires the hart id to be present in


"a0"


register. This is not a problem for bootm/booti commands as
they directly jump to Linux kernel. However, bootefi jumps to
a EFI boot stub code in Linux kernel which acts a loader and
jumps to real Linux after terminating the boot services. This
boot stub code has to be aware of the boot hart id so that it
can set it in "a0" before jumping to Linux kernel. Currently,
UEFI protocol doesn't have any mechanism to pass the boot hart
id to an EFI executable. We should keep it this way as this is
a RISC-V specific requirement rather than a UEFI requirement.
Out of the all


possible options, device tree seemed to be the best choice to do this job.


The detailed discussion can be found in the following thread.

INVALID URI REMOVED



abs.org_patch_1233664_&d=DwIBaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_S


N6FZB


N4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=J8GY_HS3fV_cJH9duXP739y


0hDK


3qfHGNx2Dpcf-UBY&s=iVpRlpTOME_A-


O5STNbXXawkW24gxy2yi56Q8AtZ2bI&e=


The above mentioned patch is obsoleted by the new suggestion.



Thanks for pointing that out to avoid confusion.



This patch updates the device tree in arch_fixup_fdt() which
is common for all booting commands. As a result, the DT
modification doesn't require any efi related arch specific
functions and all DT related modifications are contained at
one place. However, the hart id node will be available for
Linux even if the kernel is booted using


bootm command.


If that is not acceptable, we can always move the code to an
efi specific function.


Does a related Linux patch already exist?


Yes. But in my local tree ;). It will be included in RISC-V EFI stub
support series which I am planning to post in a couple of days.



How about EDK2?



RISC-V is not supported at all yet in EDK2.



The EDK2 patches are out there and reviewed. I guess it will be
available in mainline EDK2 pretty soon.


Yes, currently we are working on edk2 CI testing for RISCV64 arch.  We


hope edk2 RISC-V port could be in mainstream in Mar.


Excellent! Is this core support? Or do you have a platform implemented as
well that can be upstreamed?


Yes we do have platform implementations to be upstreamed, below is the latest 
status of RISC-V edk2 port. We will have to update status later because we just 
merged OpenSBI tag 0.6 to edk2 RISC-V.
https://github.com/riscv/riscv-uefi-edk2-docs



Good to know! I saw some patches going by on the mailing list, but it
is hard to derive the current state of affairs from that.

I'm glad to see you did not make the same mistake we made on ARM and
omit the PEI phase entirely.

What I did notice is the use of APRIORI PEI and APRIORI DXE sections
in your platform descriptions. I recommend you try to avoid that, as
it is a maintenance burden going forward: instead, please use dummy
protocols and NULL library class resolutions if you need to make
generic components depend on platform specific protocols. Also, please
document this - the APRIORI section does not explain *why* you have to
circumvent the ordinary dependency tree based module dispatch.

Reply via email to