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

[Alistair Francis <alistair23@gmail.com>]

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