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

[François Ozog]

Hi Simon, and team

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

> Hi Simon
>
> Le mer. 3 nov. 2021 à 02:30, Simon Glass <sjg at chromium.org> a écrit :
>
>> Hi Tom,
>>
>> On Tue, 2 Nov 2021 at 11:28, Tom Rini <trini at 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 at 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 at chromium.org> a
>> écrit :
>> > > > >
>> > > > > > Hi Peter,
>> > > > > >
>> > > > > > On Mon, 1 Nov 2021 at 04:48, Peter Maydell <
>> peter.maydell at linaro.org>
>> > > > > > wrote:
>> > > > > > >
>> > > > > > > On Tue, 26 Oct 2021 at 01:33, Simon Glass <sjg at 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 at 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 at linaro.org | Skype: ffozog
>
>

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