[PATCH v3 0/1] Add support for generating OpenSBI domains in the device tree

Gregor Haas posted 1 patch 1 month, 1 week ago
There is a newer version of this series
MAINTAINERS                       |   7 +
hw/riscv/Kconfig                  |   4 +
hw/riscv/meson.build              |   1 +
hw/riscv/opensbi_domain.c         | 542 ++++++++++++++++++++++++++++++
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
[PATCH v3 0/1] Add support for generating OpenSBI domains in the device tree
Posted by Gregor Haas 1 month, 1 week ago
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:

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

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 ++++++++++++++++++++++++++++++
 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
Re: [PATCH v3 0/1] Add support for generating OpenSBI domains in the device tree
Posted by Alistair Francis 1 week ago
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

>
> 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'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.

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

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
>
>
Re: [PATCH v3 0/1] Add support for generating OpenSBI domains in the device tree
Posted by Gregor Haas 5 days, 21 hours ago
Hi Alistair!

On Sun, Sep 8, 2024 at 8:27 PM Alistair Francis <alistair23@gmail.com>
wrote:

> 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
Re: [PATCH v3 0/1] Add support for generating OpenSBI domains in the device tree
Posted by Alistair Francis 5 days, 15 hours ago
On Wed, Sep 11, 2024 at 7:08 AM Gregor Haas <gregorhaas1997@gmail.com> wrote:
>
> Hi Alistair!
>
> On Sun, Sep 8, 2024 at 8:27 PM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> 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.

That's fair

>
> 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.

Maybe some unit tests would help here as well, to make sure we catch
the error when the change happens.

Alistair
Re: [PATCH v3 0/1] Add support for generating OpenSBI domains in the device tree
Posted by Daniel Henrique Barboza 3 weeks, 3 days ago

On 8/5/24 6:04 PM, Gregor Haas 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:
> 
> 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
> 
> 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

I believe we need OpenSBI patches for this output, don't we? If I try this example using stock
OpenSBI 1.5.1 from QEMU this happens:


OpenSBI v1.5.1
    ____                    _____ ____ _____
   / __ \                  / ____|  _ \_   _|
  | |  | |_ __   ___ _ __ | (___ | |_) || |
  | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
  | |__| | |_) |  __/ | | |____) | |_) || |_
   \____/| .__/ \___|_| |_|_____/|____/_____|
         | |
         |_|

sbi_domain_finalize: platform domains_init() failed (error -3)
init_coldboot: domain finalize failed (error -3)
(--- hangs ---)

It's not a big deal or a blocker to have this merged in QEMU regardless, but it would be nice
to have this documented somewhere (perhaps a new doc file). This would prevent users from trying
to use the device without the proper support.

This can be done after this patch is queued though. Thanks,


Daniel


> 
> 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 ++++++++++++++++++++++++++++++
>   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
>
Re: [PATCH v3 0/1] Add support for generating OpenSBI domains in the device tree
Posted by Gregor Haas 3 weeks, 3 days ago
Hi Daniel,

That's correct -- I believe this [1] patch on the OpenSBI mailing list
addresses
this issue. I am currently waiting for it to be reviewed.

Thanks,
Gregor

[1] http://lists.infradead.org/pipermail/opensbi/2024-August/007240.html

On Thu, Aug 22, 2024 at 2:49 PM Daniel Henrique Barboza <
dbarboza@ventanamicro.com> wrote:

>
>
> On 8/5/24 6:04 PM, Gregor Haas 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:
> >
> > 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
> >
> > 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
>
> I believe we need OpenSBI patches for this output, don't we? If I try this
> example using stock
> OpenSBI 1.5.1 from QEMU this happens:
>
>
> OpenSBI v1.5.1
>     ____                    _____ ____ _____
>    / __ \                  / ____|  _ \_   _|
>   | |  | |_ __   ___ _ __ | (___ | |_) || |
>   | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
>   | |__| | |_) |  __/ | | |____) | |_) || |_
>    \____/| .__/ \___|_| |_|_____/|____/_____|
>          | |
>          |_|
>
> sbi_domain_finalize: platform domains_init() failed (error -3)
> init_coldboot: domain finalize failed (error -3)
> (--- hangs ---)
>
> It's not a big deal or a blocker to have this merged in QEMU regardless,
> but it would be nice
> to have this documented somewhere (perhaps a new doc file). This would
> prevent users from trying
> to use the device without the proper support.
>
> This can be done after this patch is queued though. Thanks,
>
>
> Daniel
>
>
> >
> > 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 ++++++++++++++++++++++++++++++
> >   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
> >
>