On Tue, Aug 6, 2024 at 7:05 AM Gregor Haas <gregorhaas1997@gmail.com> wrote:
>
> This patch series adds support for specifying OpenSBI domains on the QEMU
> command line. A simple example of what this looks like is below, including
> mapping the board's UART into the secondary domain:
Thanks for the patch, sorry it took me so long to look into this
No worries -- thanks for your review!
>
> qemu-system-riscv64 -machine virt -bios fw_jump.bin -cpu max -smp 2 -m 4G -nographic \
> -device opensbi-memregion,id=mem,base=0xBC000000,order=26,mmio=false \
> -device opensbi-memregion,id=uart,base=0x10000000,order=12,mmio=true,device0="/soc/serial@10000000" \
> -device opensbi-domain,id=domain,possible-harts=0-1,boot-hart=0x0,next-addr=0xBC000000,next-mode=1,region0=mem,perms0=0x3f,region1=uart,perms1=0x3f
This will need documentation added under docs (probably under
docs/system/riscv) of how this should be used.
I've just sent a v4 patch series which includes documentation! Please let
me know what you think.
I'm not convinced this is something we want though. A user can dump
the QEMU DTB and edit it to support OpenSBI domains if they want.
My worry is that the command line method is complex for users to get
right and will be fragile and prone to breakage as parts of QEMU's DTB
changes.
I've found this patch series really useful for programmatically generating test
fixtures in an isolation system I'm working on, which is built on top of OpenSBI
domains. In that sense, I've found generating the correct flags is easier rather
than manually editing or generating several dozen device tree files for each
test configuration.
I take your point that these flags are hard to get right, and there may be more
user-friendly ways to do this. FWIW, I that this will only rarely break if QEMU's
DTB changes -- the only part that really depends on QEMU's DTB (rather than
just adding new information to it) is the device-linking part for memregions, and
it gives a loud, direct error if it cannot find the user-specified device.
Also, how are users supposed to know what options to use? Maybe some
documentation will help clear up how this should be used by users
>
> As a result of the above configuration, QEMU will add the following subnodes to
> the device tree:
>
> chosen {
> opensbi-domains {
> compatible = "opensbi,domain,config";
>
> domain {
> next-mode = <0x01>;
> next-addr = <0x00 0xbc000000>;
> boot-hart = <0x03>;
> regions = <0x8000 0x3f 0x8002 0x3f>;
> possible-harts = <0x03 0x01>;
> phandle = <0x8003>;
> compatible = "opensbi,domain,instance";
> };
>
> uart {
> phandle = <0x8002>;
> devices = <0x1800000>;
> mmio;
> order = <0x0c>;
> base = <0x00 0x10000000>;
> compatible = "opensbi,domain,memregion";
> };
>
> mem {
> phandle = <0x8000>;
> order = <0x1a>;
> base = <0x00 0xbc000000>;
> compatible = "opensbi,domain,memregion";
> };
> };
> };
>
> This results in OpenSBI output as below, where regions 01-03 are inherited from
> the root domain and regions 00 and 04 correspond to the user specified ones:
>
> Domain1 Name : domain
> Domain1 Boot HART : 0
> Domain1 HARTs : 0,1
> Domain1 Region00 : 0x0000000010000000-0x0000000010000fff M: (I,R,W,X) S/U: (R,W,X)
> Domain1 Region01 : 0x0000000002000000-0x000000000200ffff M: (I,R,W) S/U: ()
> Domain1 Region02 : 0x0000000080080000-0x000000008009ffff M: (R,W) S/U: ()
> Domain1 Region03 : 0x0000000080000000-0x000000008007ffff M: (R,X) S/U: ()
> Domain1 Region04 : 0x00000000bc000000-0x00000000bfffffff M: (R,W,X) S/U: (R,W,X)
> Domain1 Next Address : 0x00000000bc000000
> Domain1 Next Arg1 : 0x0000000000000000
> Domain1 Next Mode : S-mode
> Domain1 SysReset : no
> Domain1 SysSuspend : no
>
> v3:
> - Addressed review comments from v2 by adding default values to new properties.
> This results in concrete errors at QEMU configuration time if a mandatory
> property (as mandated by the OpenSBI spec) is not provided.
> - Changed command line encoding for the possible-harts field from a CPU bitmask
> (e.g. where bit X is set if CPU X is a possible hart) to a range format (e.g.
> the possible harts should be CPUs X-Y, where Y >= X). This does constrain the
> hart assignment to consecutive ranges of harts, but this constraint is also
> present for other QEMU subsystems (such as NUMA).
> - Added create_fdt_one_device(), which is invoked when scanning the device tree
> for a memregion's devices. This function allocates a phandle for a region's
> device if one does not yet exist.
>
> v2:
> - Addressed review comments from v1. Specifically, renamed domain.{c,h} ->
> opensbi_domain.{c,h} to increase clarity of what these files do. Also, more
> consistently use g_autofree for dynamically allocated variables
> - Added an "assign" flag to OpenSBIDomainState, which indicates whether to
> assign the domain's boot hart to it at domain parsing time.
>
> Gregor Haas (1):
> Add support for generating OpenSBI domains in the device tree
>
> MAINTAINERS | 7 +
> hw/riscv/Kconfig | 4 +
> hw/riscv/meson.build | 1 +
> hw/riscv/opensbi_domain.c | 542 ++++++++++++++++++++++++++++++
There should be a license comment at the start of new files. Have a
look at other QEMU files for examples
I've included this in my v4 patch series as well!
Alistair
> hw/riscv/virt.c | 3 +
> include/hw/riscv/opensbi_domain.h | 50 +++
> 6 files changed, 607 insertions(+)
> create mode 100644 hw/riscv/opensbi_domain.c
> create mode 100644 include/hw/riscv/opensbi_domain.h
>
> --
> 2.45.2
>
>
Thanks,
Gregor