On Wed, Mar 4, 2020 at 7:22 PM Schaefer, Daniel (DualStudy) <[email protected]> wrote: > > 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 ?
Both. I have verified bootefi boot on Qemu and Unleashed. > Maybe the failure is unrelated to the new patches - we've never booted Linux > from EDKII yet. > I have never tried EDKII patches as well. I will give it a try. You can add a quick hack can be added in OpenSBI to add the chosen node easily. By doing that, you ensure that EDK2 is unchanged and try my EFI stub patches. I might have missed something in EFI stub as well :). > > 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? yes. > 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. > I don't know how systemd-boot works but grub doesn't need to modify DT. The stage loading the grub must have already made that modification. > 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) > <[email protected]> wrote: > > -----Original Message----- > From: Ard Biesheuvel [mailto:[email protected]] > Sent: Tuesday, February 25, 2020 4:48 PM > To: Chang, Abner (HPS SW/FW Technologist) <[email protected]> > Cc: Atish Patra <[email protected]>; Heinrich Schuchardt > <[email protected]>; Atish Patra <[email protected]>; U-Boot Mailing > List <[email protected]>; Alexander Graf <[email protected]>; Anup Patel > <[email protected]>; Bin Meng <[email protected]>; Joe > Hershberger <[email protected]>; Loic Pallardy > <[email protected]>; Lukas Auer <[email protected]>; > Marek Behún <[email protected]>; Marek Vasut > <[email protected]>; Patrick Delaunay <[email protected]>; > Peng Fan <[email protected]>; Philippe Reynes > <[email protected]>; Simon Glass <[email protected]>; > Simon Goldschmidt <[email protected]>; Stefano Babic > <[email protected]>; Thierry Reding <[email protected]>; Tom Rini > <[email protected]>; [email protected]; Schaefer, Daniel (DualStudy) > <[email protected]> > 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) > <[email protected]> wrote: > > -----Original Message----- > From: Atish Patra [mailto:[email protected]] > > <snip header soup> > > On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel > <[email protected]> wrote: > > On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt > <[email protected]> > > 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. -- Regards, Atish
