Re: [PATCH v6 25/37] board: emulation: Add QEMU sbsa support

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 6067 bytes --]

On Wed, Oct 02, 2024 at 04:55:34PM -0600, Simon Glass wrote:
> Hi Patrick,
> 
> On Wed, 2 Oct 2024 at 03:52, Patrick Rudolph
> <patrick.rudolph@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.
> > Due to this design a full DTB is added here as well that allows U-Boot's
> > driver to properly function. The DTB is appended at the end of the U-Boot
> > image and will be merged with the QEMU provided DTB.
> >
> > In addition 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.rudolph@9elements.com>
> > Cc: Peter Robinson <pbrobinson@gmail.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Tom Rini <trini@konsulko.com>
> > ---
> > Changelog v3:
> > - Add GIC and GIC-ITS to devicetree
> > - Select GICv3 driver
> > - Drop acpi_fill_madt and use driver model instead
> > Changelog v4:
> > - Drop CPU platform code
> > - Enhance the DT to allow MADT generation from DT
> > Changelog v5:
> > - Add full DT and place it at the end of U-Boot
> > - Merge DT with QEMU's DT
> > - Drop DT generation code
> > - Fix flash region length
> > - Drop enable_caches()
> > - Support platforms that do not pass FDT in x0
> > Changelog v6:
> > - Update header order
> > - Drop pad-byte from DT
> > - select BINMAN_FDT
> > - select E1000_NO_NVM
> > - drop config.h include
> > - drop a few CFG_ defines that were used for SPL
> >
> > ---
> >  arch/arm/Kconfig                            |   3 +-
> >  arch/arm/dts/qemu-sbsa.dts                  | 136 ++++++
> >  arch/arm/include/asm/arch-qemu-sbsa/boot0.h |  34 ++
> >  arch/arm/mach-qemu/Kconfig                  |  36 +-
> >  board/emulation/qemu-arm/MAINTAINERS        |   2 +
> >  board/emulation/qemu-sbsa/Kconfig           |  57 +++
> >  board/emulation/qemu-sbsa/Makefile          |   8 +
> >  board/emulation/qemu-sbsa/acpi.c            | 192 ++++++++
> >  board/emulation/qemu-sbsa/dsdt.asl          | 483 ++++++++++++++++++++
> >  board/emulation/qemu-sbsa/lowlevel_init.S   |  22 +
> >  board/emulation/qemu-sbsa/qemu-sbsa.c       | 311 +++++++++++++
> >  board/emulation/qemu-sbsa/qemu-sbsa.env     |  14 +
> >  board/emulation/qemu-sbsa/qemu-sbsa.h       |  38 ++
> >  board/emulation/qemu-sbsa/smc.c             |  71 +++
> >  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                 |  95 ++++
> >  19 files changed, 1605 insertions(+), 7 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
> 
> With the tweaks below:
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> [..]
> 
> > diff --git a/arch/arm/dts/qemu-sbsa.dts b/arch/arm/dts/qemu-sbsa.dts
> > new file mode 100644
> > index 0000000000..f743ea3863
> > --- /dev/null
> > +++ b/arch/arm/dts/qemu-sbsa.dts
> > @@ -0,0 +1,136 @@
> > +// SPDX-License-Identifier: GPL-2.0+ OR MIT
> > +/*
> > + * Devicetree with onboard devices for qemu_sbsa-ref for internal use only!
> > + * DO NOT PASS TO THE OS!
> > + *
> > + * As QEMU provides only a minimal devicetree this one is merged with
> > + * it and then fixed at runtime.
> > + *
> > + * Copyright 2024 9elements GmbH
> > + */
> > +#include "configs/qemu-sbsa.h"
> > +
> > +/dts-v1/;
> > +
> > +#define MAKE_PROP(x) (x>>32) (x&0xffffffff)
> > +
> > +/ {
> > +       #address-cells = <2>;
> > +       #size-cells = <2>;
> > +       interrupt-parent = <&intc>;
> > +       compatible = "linux,sbsa-ref";
> > +
> > +       binman: binman {
> > +               multiple-images;
> > +       };
> > +
> > +       cpus {
> > +               /* Filled by fdtdec_board_setup() */
> > +       };
> > +
> > +       memory {
> > +               /* Filled by fdtdec_board_setup() */
> > +       };
> > +
> > +       soc {
> > +               compatible = "simple-bus";
> > +               #address-cells = <2>;
> > +               #size-cells = <2>;
> > +               ranges;
> > +
> > +               uart0 {
> > +                       compatible = "arm,pl011";
> > +                       status = "okay";
> > +                       reg = <MAKE_PROP(SBSA_UART_BASE_ADDR)
> 
> Can you use
> 
> reg = /bits 64/ <0x60000000>
> 
> instead?
> 
> Same for those below.

Isn't that less readable ?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]