Re: [PATCH v2 0/2] Move device tree files in a subdir in pc-bios

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Apr 23, 2025 at 02:54:26PM +0200, BALATON Zoltan wrote:
> On Wed, 23 Apr 2025, Daniel P. Berrangé wrote:
> > On Wed, Apr 23, 2025 at 01:23:28PM +0200, Philippe Mathieu-Daudé wrote:
> > > Hi Mark,
> > > 
> > > On 23/4/25 12:18, Mark Cave-Ayland wrote:
> > > > On 23/04/2025 11:02, BALATON Zoltan wrote:
> > > > 
> > > > > Simple series doing what the subject says.
> > > > > 
> > > > > v2:
> > > > > - Added changes to qemu.nsi (Philippe)
> > > > > - Changed order of enum to keep it sorted. This changes value of
> > > > > existing define but the value is not relevant, always used by name.
> > > > > 
> > > > > BALATON Zoltan (2):
> > > > >    system/datadir: Add new type constant for DTB files
> > > > >    pc-bios: Move device tree files in their own subdir
> > > > > 
> > > > >   MAINTAINERS                                |   2 +-
> > > > >   hw/microblaze/boot.c                       |   2 +-
> > > > >   hw/ppc/ppc440_bamboo.c                     |   2 +-
> > > > >   hw/ppc/sam460ex.c                          |   2 +-
> > > > >   hw/ppc/virtex_ml507.c                      |   2 +-
> > > > >   include/qemu/datadir.h                     |  11 +++++++---
> > > > >   pc-bios/{ => dtb}/bamboo.dtb               | Bin
> > > > >   pc-bios/{ => dtb}/bamboo.dts               |   0
> > > > >   pc-bios/{ => dtb}/canyonlands.dtb          | Bin
> > > > >   pc-bios/{ => dtb}/canyonlands.dts          |   0
> > > > >   pc-bios/dtb/meson.build                    |  23 +++++++++++++++++++++
> > > > >   pc-bios/{ => dtb}/petalogix-ml605.dtb      | Bin
> > > > >   pc-bios/{ => dtb}/petalogix-ml605.dts      |   0
> > > > >   pc-bios/{ => dtb}/petalogix-s3adsp1800.dtb | Bin
> > > > >   pc-bios/{ => dtb}/petalogix-s3adsp1800.dts |   0
> > > > >   pc-bios/meson.build                        |  23 +--------------------
> > > > >   qemu.nsi                                   |   2 +-
> > > > >   system/datadir.c                           |   5 ++++-
> > > > >   18 files changed, 42 insertions(+), 32 deletions(-)
> > > > >   rename pc-bios/{ => dtb}/bamboo.dtb (100%)
> > > > >   rename pc-bios/{ => dtb}/bamboo.dts (100%)
> > > > >   rename pc-bios/{ => dtb}/canyonlands.dtb (100%)
> > > > >   rename pc-bios/{ => dtb}/canyonlands.dts (100%)
> > > > >   create mode 100644 pc-bios/dtb/meson.build
> > > > >   rename pc-bios/{ => dtb}/petalogix-ml605.dtb (100%)
> > > > >   rename pc-bios/{ => dtb}/petalogix-ml605.dts (100%)
> > > > >   rename pc-bios/{ => dtb}/petalogix-s3adsp1800.dtb (100%)
> > > > >   rename pc-bios/{ => dtb}/petalogix-s3adsp1800.dts (100%)
> > > > 
> > > > In previous discussions we've had around what to do with pc-bios, wasn't
> > > > the consensus that we should aim towards dividing up the directory on a
> > > > per-target basis? I'm wondering if this is going in right direction, as
> > > > I can certainly see that a per-target split would be more useful to
> > > > packagers.
> 
> One problem is that pc-bios doesn't only contain machine firmware but also
> card ROMs which would belong to more targets (or archs) as e.g. PCI cards
> work on multiple archs. So it's not trivial to split by target, you'd still
> have a lot of files not easily assigned to any target.
> 
> This series is in preparation for another that will add a dtb for pegasos2
> and I did not want to increase the mess and took the opportunity to try to
> tidy it a bit. I don't intend to do any major refactoring of the pc-bios
> dir, that's out of scope of these patches.
> 
> > > pc-bios/ is already a mess, packagers usually take it as a whole. This
> > > series isn't making the current situation worse.
> > > 
> > > I don't recall a per-target split discussion, but one moving firmware
> > > blobs out of tree in a more adapted storage like git-lfs.
> > 
> > Talking about the pc-bios dir in general is a bit of a can of worms
> > and we never make concrete progress historically :-(
> > 
> > Probably best to split up the problem to some extent.
> > 
> > The device tree files are conceptually quite different from the
> > 3rd party pre-built firmware images, which are diffferent from
> > the keymaps.
> > 
> > IIUC, device tree files are tied to specific machine types, so
> > I wonder if they should not simply live alongside their machine
> > type .c impl file, completely outside of pc-bios ?
> > 
> > eg
> > 
> >  petalogix-ml605.{dts,dtb} live alongside hw/microblaze/petalogix_ml605_mmu.c
> >  babmboo.{dts,dtb} live alongside ./hw/ppc/ppc440_bamboo.c
> 
> You need the dtbs at run time and the dir where we can look files up is the
> pc-bios. So these need to be installed there at the end. We could scatter
> them around in the source to put them next their machines but that would
> make installation of them more difficult than having it in one dir.
> 
> > For the keymaps it feels like an probable easy win to move them to a
> > ui/keymaps/ directory instead.
> 
> Currently you can run a git build directly from build dir and it will find
> the roms/dtbs/keymaps. You can also run a binary copied elsewhere if you
> pass -L path/to/pc-bios. Moving things out of it would break this and may
> cause more problems than it would solve.


This is just describing a limitation of the current resource locating
implementation. For running in tree there's no reason why we can't
look in a different directory for keymaps/dtbs - we just took the
lazy option historically of putting them alongside firmware. That
can be fixed.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|