Hi Tom, On Tue, 24 Sept 2024 at 16:29, Tom Rini <tr...@konsulko.com> wrote: > > On Tue, Sep 24, 2024 at 03:17:15PM +0200, Patrick Rudolph wrote: > > On Fri, Sep 20, 2024 at 5:59 PM Simon Glass <s...@chromium.org> wrote: > > > > > > Hi Patrick, > > > > > > On Fri, 20 Sept 2024 at 12:29, Patrick Rudolph > > > <patrick.rudo...@9elements.com> wrote: > > > > > > > > On Fri, Sep 20, 2024 at 11:37 AM Simon Glass <s...@chromium.org> wrote: > > > > > > > > > > +Peter Maydell for comment on the below > > > > > > > > > > Hi Patrick, > > > > > > > > > > On Fri, 20 Sept 2024 at 09:54, Patrick Rudolph > > > > > <patrick.rudo...@9elements.com> wrote: > > > > > > > > > > > > On Thu, Sep 19, 2024 at 4:10 PM Simon Glass <s...@chromium.org> > > > > > > wrote: > > > > > > > > > > > > > > Hi Patrick, > > > > > > > > > > > > > > On Wed, 18 Sept 2024 at 08:29, Patrick Rudolph > > > > > > > <patrick.rudo...@9elements.com> wrote: > > > > > > > > > > > > > > > > On Mon, Sep 16, 2024 at 5:41 PM Simon Glass <s...@chromium.org> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Patrick, > > > > > > > > > > > > > > > > > > On Thu, 12 Sept 2024 at 00:23, Patrick Rudolph > > > > > > > > > <patrick.rudo...@9elements.com> wrote: > > > > > > > > > > > > > > > > > > > > On Thu, Sep 12, 2024 at 2:58 AM Simon Glass > > > > > > > > > > <s...@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Patrick, > > > > > > > > > > > > > > > > > > > > > > On Wed, 11 Sept 2024 at 00:26, Patrick Rudolph > > > > > > > > > > > <patrick.rudo...@9elements.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > Add support for Arm sbsa [1] v0.3+ that is supported by > > > > > > > > > > > > QEMU [2]. > > > > > > > > > > > > > > > > > > > > > > > > Unlike other Arm based platforms the machine only > > > > > > > > > > > > provides a minimal > > > > > > > > > > > > FDT that contains number of CPUs, ammount of memory and > > > > > > > > > > > > machine-version. > > > > > > > > > > > > The boot firmware has to provide ACPI tables to the OS. > > > > > > > > > > > > > > > > > > > > > > > > Add a minimal amount of devicetree nodes to make the > > > > > > > > > > > > board useable within > > > > > > > > > > > > U-Boot, provide documentation how to use, enable binman > > > > > > > > > > > > to fabricate both > > > > > > > > > > > > ROMs that are required to boot and add ACPI tables to > > > > > > > > > > > > make it full compatible > > > > > > > > > > > > to the EDK2 reference implementation. > > > > > > > > > > > > > > > > > > > > > > > > The board was tested using Fedora 40 Aarch64 > > > > > > > > > > > > Workstation. It's able > > > > > > > > > > > > to boot from USB and AHCI or network. > > > > > > > > > > > > > > > > > > > > > > > > Tested and found working: > > > > > > > > > > > > - serial > > > > > > > > > > > > - PCI > > > > > > > > > > > > - xHCI > > > > > > > > > > > > - Bochs display > > > > > > > > > > > > - AHCI > > > > > > > > > > > > - network using e1000e > > > > > > > > > > > > - CPU init > > > > > > > > > > > > - Booting Fedora 40 > > > > > > > > > > > > > > > > > > > > > > > > 1: Server Base System Architecture (SBSA) > > > > > > > > > > > > 2: https://www.qemu.org/docs/master/system/arm/sbsa.html > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Patrick Rudolph > > > > > > > > > > > > <patrick.rudo...@9elements.com> > > > > > > > > > > > > Cc: Peter Robinson <pbrobin...@gmail.com> > > > > > > > > > > > > Cc: Simon Glass <s...@chromium.org> > > > > > > > > > > > > Cc: Tom Rini <tr...@konsulko.com> > > > > > > > > > > > > --- > > > > > > > > > > > > Changelog v3: > > > > > > > > > > > > - Add GIC and GIC-ITS to devicetree > > > > > > > > > > > > - Select GICv3 driver > > > > > > > > > > > > - Drop acpi_fill_madt and use driver model instead > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > arch/arm/Kconfig | 1 + > > > > > > > > > > > > arch/arm/dts/qemu-sbsa.dts | 49 ++ > > > > > > > > > > > > arch/arm/include/asm/arch-qemu-sbsa/boot0.h | 33 ++ > > > > > > > > > > > > arch/arm/mach-qemu/Kconfig | 36 +- > > > > > > > > > > > > board/emulation/qemu-arm/MAINTAINERS | 2 + > > > > > > > > > > > > board/emulation/qemu-sbsa/Kconfig | 58 +++ > > > > > > > > > > > > board/emulation/qemu-sbsa/Makefile | 8 + > > > > > > > > > > > > board/emulation/qemu-sbsa/acpi.c | 235 > > > > > > > > > > > > ++++++++++ > > > > > > > > > > > > board/emulation/qemu-sbsa/dsdt.asl | 483 > > > > > > > > > > > > ++++++++++++++++++++ > > > > > > > > > > > > board/emulation/qemu-sbsa/lowlevel_init.S | 22 + > > > > > > > > > > > > board/emulation/qemu-sbsa/qemu-sbsa.c | 481 > > > > > > > > > > > > +++++++++++++++++++ > > > > > > > > > > > > board/emulation/qemu-sbsa/qemu-sbsa.env | 14 + > > > > > > > > > > > > board/emulation/qemu-sbsa/qemu-sbsa.h | 38 ++ > > > > > > > > > > > > board/emulation/qemu-sbsa/smc.c | 72 +++ > > > > > > > > > > > > configs/qemu-arm-sbsa_defconfig | 10 + > > > > > > > > > > > > doc/board/emulation/index.rst | 1 + > > > > > > > > > > > > doc/board/emulation/qemu-sbsa.rst | 98 ++++ > > > > > > > > > > > > doc/develop/driver-model/virtio.rst | 1 + > > > > > > > > > > > > include/configs/qemu-sbsa.h | 98 ++++ > > > > > > > > > > > > 19 files changed, 1734 insertions(+), 6 deletions(-) > > > > > > > > > > > > create mode 100644 arch/arm/dts/qemu-sbsa.dts > > > > > > > > > > > > create mode 100644 > > > > > > > > > > > > arch/arm/include/asm/arch-qemu-sbsa/boot0.h > > > > > > > > > > > > create mode 100644 board/emulation/qemu-sbsa/Kconfig > > > > > > > > > > > > create mode 100644 board/emulation/qemu-sbsa/Makefile > > > > > > > > > > > > create mode 100644 board/emulation/qemu-sbsa/acpi.c > > > > > > > > > > > > create mode 100644 board/emulation/qemu-sbsa/dsdt.asl > > > > > > > > > > > > create mode 100644 > > > > > > > > > > > > board/emulation/qemu-sbsa/lowlevel_init.S > > > > > > > > > > > > create mode 100644 > > > > > > > > > > > > board/emulation/qemu-sbsa/qemu-sbsa.c > > > > > > > > > > > > create mode 100644 > > > > > > > > > > > > board/emulation/qemu-sbsa/qemu-sbsa.env > > > > > > > > > > > > create mode 100644 > > > > > > > > > > > > board/emulation/qemu-sbsa/qemu-sbsa.h > > > > > > > > > > > > create mode 100644 board/emulation/qemu-sbsa/smc.c > > > > > > > > > > > > create mode 100644 configs/qemu-arm-sbsa_defconfig > > > > > > > > > > > > create mode 100644 doc/board/emulation/qemu-sbsa.rst > > > > > > > > > > > > create mode 100644 include/configs/qemu-sbsa.h > > > > > > > > > > > > > > > > > > > > > > Wow there is a lot in this patch! > > > > > > > > > > > > > > > > > > > > > > I don't think this should be in a separate subdir, just > > > > > > > > > > > using > > > > > > > > > > > board/emulation/qemu-arm should be OK? > > > > > > > > > > Why would it share the code base with qemu-arm, as it's a > > > > > > > > > > completely > > > > > > > > > > different board? > > > > > > > > > > > > > > > > > > Perhaps I misunderstand this...what kind of board is it? How > > > > > > > > > do you > > > > > > > > > run qemu for this board? > > > > > > > > > > > > > > > > > For QEMU SBSA you use -machine sbsa-ref, but for qemu-arm you > > > > > > > > use -machine virt. > > > > > > > > > > > > > > > > Both can use a cortex-a57 as CPU, but that's all those boards > > > > > > > > share. They have a > > > > > > > > different memory layout, virt provides a complete FDT, while > > > > > > > > sbsa-ref doesn't. > > > > > > > > sbsa-ref needs an Arm-TF and nonsecure boot firmware, while > > > > > > > > virt works > > > > > > > > without a firmware. > > > > > > > > > > > > > > OK I see > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I see you are setting plat data in the board file...is > > > > > > > > > > > there not a CPU > > > > > > > > > > > driver which can read it from the devicetree or set it > > > > > > > > > > > based on the > > > > > > > > > > > compatible string? > > > > > > > > > > The CPU driver could read the data from devicetree, but > > > > > > > > > > that would require a new > > > > > > > > > > DT binding. Is that the preferred method over using plat > > > > > > > > > > data? > > > > > > > > > > > > > > > > > > It is fine to use plat data...but it should be set up in the > > > > > > > > > driver > > > > > > > > > where possible. In this case if you don't have a binding you > > > > > > > > > can check > > > > > > > > > the compatible string of the device (or even the root > > > > > > > > > compatible-string) and add your plat data if it matches. > > > > > > > > > > > > > > > > I was able to parse everything from the DT, except for PMU > > > > > > > > IRQs. Those > > > > > > > > aren't needed to > > > > > > > > boot the platform and can be added later on. > > > > > > > > Will send out a new revision soon. > > > > > > > > > > > > > > OK > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > For FDT fixup, can you please use the event and the > > > > > > > > > > > ofnode interface? > > > > > > > > > > > See EVT_FT_FIXUP and bootmeth_vbe_ft_fixup() as an > > > > > > > > > > > example. > > > > > > > > > > > > > > > > It looks like EVT_FT_FIXUP is run rather late in the bootflow, > > > > > > > > used to > > > > > > > > updated the FDT for OS usage, > > > > > > > > but it must be run early to update the minimal FDT passed by > > > > > > > > QEMU and > > > > > > > > add the missing nodes > > > > > > > > for U-Boot drivers. > > > > > > > > > > > > > > If the QEMU FDT is not suitable, shouldn't we use the U-Boot/Linux > > > > > > > one? Do we have to use the one passed in by QEMU? > > > > > > > > > > > > > There's no U-Boot/Linux FDT, since this is an "ACPI only" platform. > > > > > > In EDK2 firmware there's everything hard-coded as well to generate > > > > > > proper ACPI code, but since > > > > > > U-Boot relies on FDT, I decided to "fix" it and add the required > > > > > > missing nodes. This allows to make > > > > > > use of U-Boot existing infrastructure around FDT. > > > > > > > > > > > > > > > > So wouldn't it be better to pass in a u-boot.bin with the DT attached? > > > > > Adding the thing in code seems pretty ugly. > > > > > > > > > Would that make use of CONFIG_OF_EMBED? > > > > I decided against this option as it "is not recommended for production > > > > devices." > > > > However I agree that it's possible and would be much cleaner and less > > > > code. > > > > Will look into this. > > > > > > Well, indeed, it would be better not to use that. Does QEMU support a > > > flat binary or does it require an ELF? Given the choice between > > > OF_EMBED and your DT-building code I would go with the latter. > > The first executed instruction is at the start of the flash, thus it's > > not possible to use anything that > > has a header. Since this is an emulated platform that will never > > provide a full DT it might be > > OK to use OF_EMBED here. It would be much less C code and might be > > easier to maintain. > > > > > > > > > > > > > > > The QEMU generated FDT contains a bare minimum of nodes and isn't > > > > > > suitable to boot an OS. > > > > > > > > > > Yes, I am not very pleased about that, particularly as my patch to > > > > > allow passing a proper FDT to QEMU[1] has still not been accepted. > > > > > > > > > The SBSA ref machine was designed that way, to make sure that only ACPI > > > > is used > > > > to boot the OS. > > > > > > Er, but doesn't firmware run before the OS? It seems like a strange > > > design decision. > > I guess it was designed with EDK2 in mind as UEFI is mandatory to pass > > the ACPI table pointer. > > This is the reference platform for the "never give the OS a Device Tree" > Arm platform specification, so, it makes sense that no, really, there's > no device tree associated with it. Since it's also the case that we > better never give the OS a device tree, maybe this is a place where > OF_EMBED is fine and makes sense?
Not in my opinion...that only really makes sense for debugging. The correct thing here is to provide the devicetree to U-Boot. Since QEMU is unable to do so, some mechanism is needed. I suggest disabling OF_BOARD, so that it doesn't get the (missing) devicetree from the start of RAM, but instead gets it from the end of the U-Boot image, as is usually done with other boards. Regards, Simon
