qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 06/26] arm: qemu: Add a devicetree file for qemu_arm64


From: François Ozog
Subject: Re: [PATCH v5 06/26] arm: qemu: Add a devicetree file for qemu_arm64
Date: Wed, 3 Nov 2021 15:44:35 +0100

Hi Simon, and team

On Wed, 3 Nov 2021 at 06:29, François Ozog <francois.ozog@linaro.org> wrote:
Hi Simon

Le mer. 3 nov. 2021 à 02:30, Simon Glass <sjg@chromium.org> a écrit :
Hi Tom,

On Tue, 2 Nov 2021 at 11:28, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Nov 02, 2021 at 09:00:53AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 1 Nov 2021 at 12:07, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Nov 01, 2021 at 06:33:35PM +0100, François Ozog wrote:
> > > > Hi Simon
> > > >
> > > > Le lun. 1 nov. 2021 à 17:58, Simon Glass <sjg@chromium.org> a écrit :
> > > >
> > > > > Hi Peter,
> > > > >
> > > > > On Mon, 1 Nov 2021 at 04:48, Peter Maydell <peter.maydell@linaro.org>
> > > > > wrote:
> > > > > >
> > > > > > On Tue, 26 Oct 2021 at 01:33, Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Add this file, generated from qemu, so there is a reference devicetree
> > > > > > > in the U-Boot tree.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > >
> > > > > > Note that the dtb you get from QEMU is only guaranteed to work if:
> > > > > >  1) you run it on the exact same QEMU version you generated it with
> > > > > >  2) you pass QEMU the exact same command line arguments you used
> > > > > >     when you generated it
> > > > >
> > > > > Yes, I certainly understand that. In general this is not safe, but in
> > > > > practice it works well enough for development and CI.
> > > >
> > > > You recognize that you hijack a product directory with development hack
> > > > facility. There is a test directory to keep things clear. There can be a
> > > > dev-dts or something similar for Dev time tools.
> > > > I have only seen push back on those fake dts files in the dts directory: I
> > > > guess that unless someone strongly favors a continuation of the discussion,
> > > > you may consider re-shaping the proposal to address concerns.
> > >
> > > Yes.  We need to document how to make development easier.  But I'm still
> > > not on board with the notion of including DTS files for platforms where
> > > the source of truth for the DTB is elsewhere.  That's going to bring us
> > > a lot more pain.
> >
> > Are you talking about QEMU specifically, or Raspberry Pi?
>
> I was using two of the more common and readily available platforms where
> the source of truth for the DTS/DTB is not (and will not be) U-Boot.
>
> > How can we get this resolved? I very much want to get to just having
> > OF_SEPARATE and OF_EMBED as the only available build-time options,
> > with OF_BOARD (and perhaps OF_PASSAGE) as something we can enable for
> > runtime support. I feel that separating the build-time and run-time
> > behaviour is very important. Over time perhaps we will have some
> > success in upstreaming bindings, but for now we have what we have.
> > There is still a lot of pushback on U-Boot having things in the
> > devicetree, although I do see that softening somewhat.
>
>
> To reiterate, the uniform bit of feedback on this series has been that
> everyone else disagrees with your notion that we _must_ have a dts
> in-tree.

[I would like everyone to take a deep breath and think about what this
actually impacts. I argue the impact outside U-Boot is approximately
zero. What are we actually discussing here?]

A few have responded that they can just add the files. I think that is
the case for everything except QEMU, right? I think even François
agrees with the documentation argument.
I do providing that the sample goes into documentation, not actionable as a build artifact in the dts directory.
There is no real burden in
adding the files so we can see what is going on, add a binman node,
SPL nodes, etc.

So I am going to stand my ground on that one. It affects:

- highbank
- qemu-ppce500
- rpi_4
- xilinx_versal_virt
- octeontx
- xenguest_arm64
- juno

So that is just 7 boards that I want to add devicetree files for. I
think that is reasonable and not a burden on these maintainers.

Let me deal with QEMU.

Let's imagine that we were in the state that I am proposing here,
which we would be if I were a better devicetree maintainer for U-Boot
and had not taken my eye off the ball, basically with my review of
[1], where I should have pushed to get a response on the questions
before it was applied. That might have triggered me to think about the
implications of this at the time.

Anyway, in the state that I am proposing, what problems would we have?

1. QEMU has a DT which only matches the 'standard' invocation as
documented at [2]


2. QEMU may get out of date if there is a new release.

I don't think (1) is really a problem. People will have to remove
CONFIG_OF_BOARD from configs/qemu_arm_spl_defconfig (or the like) to
get into this state, and we have a message now that prints out where
the devicetree comes from ("separate" in this case):

Core:  42 devices, 11 uclasses, devicetree: separate

For (2), I tested QEMU 6.1.50 and the only difference from 4.2.1 (a
year old) is:

kaslr-seed = <0x2037f53d 0x42c279e8>;

It doesn't affect anything here. It boots the lastest image fine.

Just for interest I went back to 2.5.0 which is nearly 6 years old!
There are a few differences like dma-coherent and gpio-keys being
added, but again it boots fine.

So in practice that doesn't seem to be a problem from what I can tell.
You are essentially saying “I don’t care about the system design, this DTS simplifies my development work for U-Boot and I checked it works on a useless ‘standard invocation’”

3. Anything else?

For qemu_arm_spl, it *does not boot* unless the U-Boot SPL properties
are present. There is no easy way to fix this.
one clean and easy way would be to upstream a Qemu change to merge a supplied overlay to the generated one. This the same idea as applying the NT_FW_COnFIG overlay in the TFA world. In both cases previous loaders do their job properly for U-Boot : setting up the stage as needed.
I was pointed to your proposal in Qemu mailing list and added my support to it.
I could not comment on the exact details of the proposal, but commented on the fact that merging a DT fragment provided to Qemu at runtime makes sense and has some precedent in TFA.
If we merge them into
qemu with dumpdtb, etc. we are assuming that the node we want to
update is present, so this is not really any better than having the
devicetree in U-Boot. Actually I think it is worse, since who knows
what changes might happen to the devicetree in QEMU which will stop
U-Boot from working.

QEMU cannot make changes that don't follow the bindings. U-Boot uses
the bindings, so we are good.

It just seems very clear to me that this approach is far superior to
U-Boot to the wonky business that we have today.

>
> > > It is important to make sure our "develop our project" workflow is sane
> > > and relatively pain free.  But that needs to not come by making
> > > sacrifices to the "use our project" outcome.  I would hope for example
> > > that the new Pi zero platform is just dtb changes, as far as the linux
> > > kernel is concerned which means that for rpi_arm64 (which uses run time
> > > dtb) it also just works.  And that's what we want to see.
> >
> > So long as OF_BOARD is enabled then the flow should work as you expect.
>
> Then we need to get things spun such that we can build the platforms
> where the dtb is given to us, complete and correct, at run time, to not
> require an in-tree dts that's not going to be used.  Documentation
> (another area we have much improved on these past few years and for
> which I am grateful for all of the effort behind!) is how we make the
> developer use-case for those platforms better.

I did not expect everyone to love this; this is how we got into the
mess we are in. A few people thinking it would be expedient to just do
their own thing. But I have come to this after an enormous amount of
thought and much discussion. I think sometimes people imagine that I
just throw things over the wall to get a reaction. That is not the
case.

We need some way to put U-Boot properties in there, at least until
things change upstream and within U-Boot. That envisages tooling that
is clearly not going to be in place for the upcoming release.

For the test, how about I create a separate qemu build (which I have
done, actually, qemu_arm_spl) and control the QEMU flags so we know it
will boot. It is for CI so we can keep it in sync and deal with any
breakages if people change things in QEMU (as above, this seems to be
a theoretical problem).

Again, please take a deep breath and consider how much this actually
affects TF-A, QEMU, etc. I'd argue not at all. We are talking about a
build-time devicetree in the U-Boot tree. We are not talking about
disabling OF_BOARD or removing that option.
It is less about development environnement and respective CIs and more about how we want to deliver products on the market and how the supply chain is organized. U-Boot is a key component of that chain. If something is missing from the previous stages of the chain, don’t try to work around the problem in U-Boot, just propose the change/correction in the relevant project (Qemu, TFA, RPI Videocore…) or ask for help. If nothing can be resolved “the clean way” then we can hack our way out (Qemu fork you proposed). But let’s try the clean path first.

Regards,
Simon

[1] http://patchwork.ozlabs.org/project/uboot/patch/20170402082520.32546-1-deymo@google.com/
[2] https://u-boot.readthedocs.io/en/latest/board/emulation/qemu-arm.html
--
François-Frédéric Ozog | Director Business Development
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog



--
François-Frédéric Ozog | Director Business Development
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog


reply via email to

[Prev in Thread] Current Thread [Next in Thread]