[SLOF] [PATCH v4] board-qemu: add private hcall to inform host on "phandle" update

[David Gibson]

On Thu, Sep 07, 2017 at 09:56:12AM +0200, Greg Kurz wrote:
> On Thu, 7 Sep 2017 16:38:00 +1000
> David Gibson <david at gibson.dropbear.id.au> wrote:
> 
> > On Wed, Sep 06, 2017 at 10:05:41AM -0500, Segher Boessenkool wrote:
> > > On Wed, Sep 06, 2017 at 04:40:58PM +0200, Greg Kurz wrote:  
> > > > > A phandle is an opaque cookie to everything but the firmware itself.
> > > > > Using pointers to some internal structure works just fine; most Open
> > > > > Firmware implementations do this.
> > > > > 
> > > > > Anything other than OF itself should *not* make up phandles.  There
> > > > > is no way to guarantee these will be unique, so it is a non-starter.
> > > > > 
> > > > > If QEMU wants to create a device node, it should ask OF to do it,
> > > > > say, via new-device.  
> > >   
> > > > Hmm... QEMU creates an FDT and writes it into the guest memory to pass
> > > > it to SLOF.  
> > > 
> > > It probably should skip that FDT intermediate step, just create all
> > > nodes it wants directly?  Sounds a lot simpler, and all current phandle
> > > problems will disappear too.  
> > 
> > No, it would be a complete PITA.  We'd have to have a way of running
> > the guest (running SLOF) concurrent with the qemu machine reset code,
> > communicating OF commands in, and responses back.  As it is now, we
> > just drop the FDT into guest memory before SLOF executes its first
> > instruction.
> > 
> > This is actually perfectly sound, even with an archaically OF centric
> > view of the world.  OF is allowed to build its DT from other
> > information available to it.  In this case that other data just
> > happens to be a FDT which will, in practice, look pretty similar to
> > OF's final DT.  The information flow is strictly in one direction:
> > qemu to OF to guest.
> > 
> > In other words OF *does* allocate all the phandles itself - in the
> > tree it presents to the guest.  The phandles allocated by qemu are
> > overwritten and obsolete by the time the guest gets the DT from OF.
> > 
> > The problem comes when we need to do hotplug, and qemu needs to inject
> > DT fragments long after SLOF is dead.  So far it works, because the
> > only things about the existing output-from-SLOF tree we rely on would
> > be completely perverse for SLOF to gratuitously change from the
> > input-to-SLOF DT.  To allow PHB hotplug, though, we need the master
> > XICS phandle, and SLOF *does* change all the phandles, so we don't
> > know it.  Hence this proposal.
> > 
> > I don't like this proposal precisely because it does make the DT
> > communication two way between qemu and SLOF, which has the prospect to
> > make things far, far uglier.
> > 
> > Unfortunately, I'm yet to see a better way to do this.
> > 
> > > > QEMU uses arbitrary and unique numbers when a phandle is
> > > > needed (we just need one actually for the interrupt controller which
> > > > is referenced by each PHB). SLOF then fixes all the QEMU-generated
> > > > phandles to "opaque cookies" (see commit 82954d4c1088)... the problem
> > > > is that if we want to hotplug another PHB later on, SLOF isn't involved
> > > > anymore and QEMU doesn't know about the "opaque cookie" to be put in
> > > > the node for this PHB... I hope I'm clear enough.
> > > > 
> > > > This being said, you partly answer the question when you say:
> > > > 
> > > > "A phandle is an opaque cookie to everything but the firmware itself."
> > > > 
> > > > and
> > > > 
> > > > "Anything other than OF itself should *not* make up phandles."
> > > > 
> > > > Even if QEMU takes care of only forging unique phandles,  
> > > 
> > > It cannot.  It cannot know about all phandles the firmware uses.  It
> > > can guess, sure, but that is very fragile.
> > >   
> > > > it already
> > > > does some arm-twisting with the OF specification... but would it be
> > > > acceptable for SLOF to have a translation table when using phandles
> > > > generated by QEMU ?  
> > 
> > I'm not sure exactly what you mean by this.  Converting SLOF to use
> > qemu supplied phandles everywhere would be possible, I think, but very
> > awkward and ugly.
> > 
> 
> I had this last mail from you in mind, when I posted the PHB hotplug series:
> 
> https://lists.nongnu.org/archive/html/qemu-ppc/2017-08/msg00005.html
> 
> "I'd be thinking of replacing the direct pointer dereferences for
> phandles with a simple lookup table of phandle to pointer, populated
> as we unflatten the tree."

Ah, right.  Since then I talked to Thomas and discovered this was
harder than I thought.

> > Applying some sort of translation just to the hotplugged fragments
> > isn't possible because SLOF is no longer around by the time of the
> > hotplug.  qemu talks directly to the guest.
> 
> Yes of course... hence the initial proposal of having SLOF to send any
> phandle value it *fixes* back to QEMU (only one with the current code).
> 
> > > Then you need to translate from "qhandle" to phandle every time anything
> > > changes, keep both up to date.  Why not just have actual phandles?
> > > Those stay up to date always (namely, they never change).
> > > 
> > > Why does QEMU think it is a good idea to make up phandles out of thin
> > > air?  Or, why does it do it if it is not a good idea?  
> > 
> > It doesn't.  The problem is that it needs to *know* a phandle that
> > SLOF made up in order to make a compatible hotplug fragment.  At
> > present it has no way to do that - qemu never sees the
> > output-from-SLOF DT.
> > 
> 
> <thinking aloud>
> Would it be more acceptable to send back the updated value of the
> XICS phandle through CAS ?
> </thinking aloud>

I don't think that makes sense.  I mean CAS is a negotiation between
guest OS and firmware.  We use a custom hypercall to punt most of it
from firmware to hypervisor.  We could piggyback on that to add
firmware -> hypervisor information, but I think it would be
functionally equivalent to an update phandle hypercall, just more
awkward.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/slof/attachments/20170907/3dc6348e/attachment.sig>