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

Simon Glass
2024-10-04T01:29:16Z

Hi Tom,

On Thu, 3 Oct 2024 at 16:25, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Oct 03, 2024 at 02:46:03PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 3 Oct 2024 at 13:17, Tom Rini <trini@konsulko.com> wrote:
> [snip]
> > > We don't, but this isn't quite a normal board?
> >
> > Don't be surprised if this becomes more common. For better or worse,
> > ARM has decided on ACPI for its servers and RISC-V is following. We
> > also have the EFI app, which has a devicetree, although of course no
> > actual addresses.
>
> True. And U-Boot being able to take and reasonably as far as the OS is
> concerned be the UEFI implementation in UEFI+ACPI boot opens us up to
> being able to be part of the software stack that's certified for
> SystemReady ES and SR. On that real hardware we can certainly expect to
> mandate that the previous part of the firmware stack gives us a device
> tree. It will be TF-A and so the "pass over a device tree" part
> shouldn't be an issue. And since yes, really (and for the future
> record), real hardware isn't allowed to fake this out like we're
> allowing here, today.

Indeed.

>
> Heck, it's possible that down the line and it's important to a lot more
> people, we can get the TF-A build for QEMU SBSA to have an option to
> include a device tree for U-Boot, because it's an important part of the
> stack for that project to be able to validate too.

We'll see!

>
> > > > Breaking down the SBSA_UART_BASE_ADDR:
> > > >
> > > > - this is in the uart0 node so obviously relates to the UART
> > > > - it is a reg property, so BASE_ADDR is obvious
> > > > - SBSA is the board name, which is in the filename, so don't really add anything
> > > >
> > > > Normally where the devicetree needs a value defined, we put it in the
> > > > dt-bindings directory. But even then, we don't typically put addresses
> > > > there. It is just for cases where we need the code and the devicetree
> > > > to be in sync about a property value. With addresses, the code reads
> > > > them from the devicetree, so doesn't need to have a #define to access
> > > > them...in fact that would defeat the purpose.
> > > >
> > > > If we did this sort of thing with other boards, we would have a heap
> > > > of binding files containing addresses that are only used in one place.
> > > > I just don't see the point.
> > > >
> > > > However for SBSA_PCIE_MMIO_BASE_ADDR I see that it is used in the .asl
> > > > and the memory map, so it seems fair enough to define that, at least
> > > > unless/until we can refer to devicetree addresses in the .asl file.
> > > > Even with this one though, I would expect something like:
> > > >
> > > > ranges = <0x02000000>, /bits/ 64 <SBSA_PCIE_MMIO_BASE_ADDR
> > > >
> > > > SBSA_PCIE_MMIO_BASE_ADDR>;
> > > >
> > > > But in this case I can see how a macro might be more convenient and
> > > > I'm not sure I have a strong opinion here.
> > >
> > > If you're strongly against using the macros here, OK, fine. But since
> > > this tree is only going to be read by humans in U-Boot, and consumed by
> > > U-Boot, I think it makes sense to deviate from the norms a little, in
> > > favour of human readability.
> >
> > How about only using them when there is code that need the same address?
> >
> > There are two lots of macros so just to check we are talking about the
> > same thing:
> >
> > - xxx_BASE_ADDR - IMO they should be open-coded into the devicetree if
> > it is the only user
> > - MAKE_PROP - IMO we should use /bit/ 64 instead
>
> OK.
>
> > > > > > But I very much doubt upstream would accept it with a macro. Other
> > > > > > 64-bit boards seem to use '/bits/ 64'.
> > > > >
> > > > > What upstream are you referring to here? I'm not sure what other project
> > > > > will want a device tree for the "no device tree only acpi" virtual
> > > > > reference platform.
> > > >
> > > > It depends if they want to run U-Boot or not. U-Boot always needs a
> > > > devicetree, as you know. It cannot use ACPI tables to configure itself
> > > > and I would rather it doesn't start.
> > > >
> > > > I don't actually see any reason why this devicetree could not be
> > > > upstreamed, except that QEMU actually creates a Linux devicetree right
> > > > now (on the ARM side) and takes no notice of what U-Boot needs... I
> > > > don't see any U-Boot patches from the maintainers, for example,
> > > > although I may have missed some. It seems very focussed on UEFI. I
> > > > would love that to change, as QEMU is widely used in U-Boot on ARM...
> > >
> > > I feel like we're lost the point here. This platform in question is the
> > > "never use device tree, only use ACPI" reference platform.
> >
> > never use device tree for Linux
> >
> > (U-Boot always uses it)
>
> True.
>
> > > This is not
> > > the regular "virt" platform, even. U-Boot needs a device tree because
> > > internally we use it for configuration. I do not see the QEMU project
> > > agreeing to have this platform generate a device tree for us to consume
> > > in this case, declaring instead that it's a U-Boot internal need and
> > > that no, it's fine if U-Boot can't use it. _We_ want it because it makes
> > > testing of the aarch64 + ACPI use case easy because it provides us with
> > > the ability to use the reference platform too and make sure that we
> > > aren't breaking the OS in unexpected ways.
> >
> > OK, well as you say this is not really the main issue, so that's fine.
> > I agree (and was lamenting) that QEMU seems unwilling at present to do
> > anything to assist U-Boot.
>
> At least today, I don't think this is even a QEMU issue, honestly. A
> point could be made in the future that EDKII will ignore TF-A passing a
> device tree, and that also needs to be confirmed / validated because
> <physical hardware platform> supports EDKII or U-Boot for ES/SR
> certification. That's a much stronger I think position to start from in
> asking QEMU to pass along the device tree that it could make.

Yes, sure. I do feel that if there were a few more U-Boot users in the
QEMU world then it might have already happened. I am almost tempted to
send a patch to add a pre-built u-boot.bin to QEMU :-)

Regards,
Simon