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

[Tom Rini <trini@konsulko.com>]

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

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.

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.

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

-- 
Tom

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