On Tue, Oct 13, 2020 at 3:11 AM Alex Bennée <alex.ben...@linaro.org> wrote: > > > Edgar E. Iglesias <edgar.igles...@gmail.com> writes: > > > On Mon, Oct 12, 2020 at 05:02:57PM +0100, Alex Bennée wrote: > >> > >> Alistair Francis <alistai...@gmail.com> writes: > >> > >> > On Fri, Oct 9, 2020 at 10:07 AM Alex Bennée <alex.ben...@linaro.org> > >> > wrote: > >> >> > >> >> Hi, > >> >> > >> >> This series adds the ability to append FDT data for blobs loaded by > >> >> the generic loader. My principle use-case was to be able to directly > >> >> boot Xen with a kernel image which avoided having to: > >> >> > >> >> - get the kernel image into my system image > >> >> - deal with bugs in FDT munging by -bios firmware and/or grub > <snip> > >> >> "Polluting" the generic loader arguments > >> >> > >> >> This was mainly out of convenience as the loader already knows the > >> >> size of the blob being loaded. However you could certainly argue it > >> >> makes sense to have a more generic "FDT expander" virtual device that > >> >> could for example query the QOM model somehow to find the details it > >> >> needs. > >> > > >> > That seems like a better option. Why not have a generic way to modify > >> > the device tree with a specific argument? It could either be -device > >> > loader,file=fdt,... or -fdt ... > >> > >> Well it comes down to how much of a special case this is? Pretty much > >> all FDT (and ACPI for the matter) is basically down to the board level > >> models - and not all of them just the funky configurable ones. For other > >> board models we just expect the user to pass the FDT they got with their > >> kernel blob. > >> > >> For modern VirtIO systems the only thing you really need to expose is > >> the root PCI bus because the rest of the devices are discover-able > >> there. > >> > >> So the real question is are there any other -devices that we want to be > >> able to graft FDT entries on or is the generic loader enough of a > >> special case that we keep all the logic in there? > >> > > > > Hi, > > > > Another option is to allow the user to pass along a DTB overlay with the > > generic loader option (or with a separate option as Alistair suggested). > > With overlways we wouldn't need all the command-line options to enable > > construction of dtb fragments, it would be more or less transparent to > > QEMU. There may be limitations with the overlay flows that I'm not aware > > of though... > > So the problem of adding DTB overlays is it's not that much easier than > the current options of using -machine dumpdtb and then hand hacking the > magic values and rebuilding, for example:
I agree with this. If a user is changing a DTB from a command line they probably only want small changes and are unlikely to need the full power of an overlay. > > https://medium.com/@denisobrezkov/xen-on-arm-and-qemu-1654f24dea75 > > Unless we come up with some sort of template support that allows QEMU to > insert numbers like address and size while processing the template. But > that seems a little too over engineered and likely introduces complexity > into the system. This though sounds interesting :) > > Given the generic-loader is so simple I'm leaning towards another > approach of just c&p'ing generic-loader into a new "magic" device (maybe > guest-loader) and stripping out the bits we don't need (data, data-len, > be etc) and making the options more tuned what we are trying to achieve. > For example: > > -device guest-loader,kernel=path/to/Image,args="command > line",addr=0x47000000,hyp=xen > -device guest-loader,initrd=path/to/initrd,addr=0x42000000,hyp=xen At first I'm not thrilled of adding a new loader. In saying that, there are lots of times where -kernel and friends doesn't do what I want it to do and I have to fall back to the generic loader and code changes to QEMU so maybe this is a good idea for image loading. I'm guessing this would be the same for every platform which would be a nice change from -kernel. Alistair > > And then we can embed the smarts in the loader to set either DTB or ACPI > entries as required and if we need additional magic to support different > hypervisors (which hopefully you don't but...) you can modulate the > hyp=FOO variable. > > There may be an argument for having a -hypervisor as a sugar alias for > -kernel (which maps to machine.kernel) but currently I see no practical > differences you need to launch it except maybe forcing the > virtualisation property to true if it exists - but that seems a little > ARM focused. > > -- > Alex Bennée