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

[Greg Kurz]

On Mon, 10 Jul 2017 16:57:57 +0200
Thomas Huth <thuth at redhat.com> wrote:

> On 10.07.2017 14:17, Greg Kurz wrote:
> > The "interrupt-map" property in each PHB node references the "phandle"
> > property of the "interrupt-controller" node. This is used by the guest
> > OS to setup IRQs for any PCI device plugged into the PHB. QEMU sets this
> > property to an arbitrary value in the flattened DT passed to SLOF.
> > 
> > Since commit 82954d4c1088, SLOF has some generic code to convert all
> > references to any "phandle" property to a SLOF specific value.
> > 
> > This is is perfectly okay for coldplug devices, since the guest OS only
> > sees the converted value in "interrupt-map". It is a problem though for
> > hotplug devices. Since they don't go through SLOF, the guest OS receives
> > the arbitrary value set by QEMU and fails to setup IRQs.
> > 
> > In order to support PHB hotplug, this patch introduces a new private
> > hcall, which allows SLOF to tell QEMU that a "phandle" was converted
> > from an old value to a new value.
> > 
> > Suggested-by: Thomas Huth <thuth at redhat.com>
> > Signed-off-by: Greg Kurz <groug at kaod.org>
> > ---
> >  board-qemu/slof/fdt.fs    |   14 +++++++++++---
> >  lib/libhvcall/hvcall.code |    6 ++++++
> >  lib/libhvcall/hvcall.in   |    1 +
> >  lib/libhvcall/libhvcall.h |    1 +
> >  4 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> > index 8d4635f30495..8ce0a59879ac 100644
> > --- a/board-qemu/slof/fdt.fs
> > +++ b/board-qemu/slof/fdt.fs
> > @@ -316,14 +316,22 @@ fdt-claim-reserve
> >     s" phandle" r@ get-property 0= IF
> >        decode-int                       ( p-addr2 p-len2 val )
> >        \ ." found phandle: " dup . cr
> > -      r@ s" /" find-node               ( p-addr2 p-len2 val node root )  
> > -      fdt-replace-all-phandles         ( p-addr2 p-len2 )
> > -      2drop
> > +      dup                              ( p-addr2 p-len2 val val )
> > +      r@ s" /" find-node               ( p-addr2 p-len2 val val node root )
> > +      fdt-replace-all-phandles         ( p-addr2 p-len2 val )
> > +      -rot 2drop                       ( val )
> >        (fdt-phandle-replaced) IF
> >           r@ set-node
> >           s" phandle" delete-property
> >           s" linux,phandle" delete-property
> > +         r@ swap hv-update-phandle dup 0 <> IF  
> 
> Instead of "dup 0 <> IF" you can use "?dup IF" here and remove the
> corresponding "ELSE drop" below.
> 
> > +	    \ Ignore hcall not implemented error, print error otherwise
> > +	    dup -2 <> IF ." HV-UPDATE-PHANDLE error: " . cr ELSE drop THEN  
> 
> Dito - use ?dup and remove the "ELSE drop" part.
> 

We want to print the value if it's not -2. I don't see how ?dup can help
here actually or am I missing something ?

> > +         ELSE
> > +	    drop
> > +         THEN
> >        ELSE
> > +         drop
> >           diagnostic-mode? IF
> >              cr ." Warning: Did not replace phandle in " r@ node>path type cr
> >           THEN
> > diff --git a/lib/libhvcall/hvcall.code b/lib/libhvcall/hvcall.code
> > index 0ff50f27e8a9..9f4d53c7237b 100644
> > --- a/lib/libhvcall/hvcall.code
> > +++ b/lib/libhvcall/hvcall.code
> > @@ -129,3 +129,9 @@ PRIM(check_X2d_and_X2d_patch_X2d_sc1)
> >  
> >  	patch_broken_sc1((void*)start, (void*)end, (void*)patch_ins);
> >  MIRP  
> 
> Could you add a stack comment before that function, please? (like it is
> done for hv-reg-crq for example already)
> 
> > +PRIM(hv_X2d_update_X2d_phandle)
> > +	uint32_t old_phandle = TOS.u; POP;
> > +	uint32_t new_phandle = TOS.u;  
> 
> If you switch the order of the parameters, you could get rid of the
> "swap" before calling this function :-)
> 
> > +	TOS.u = hv_generic(KVMPPC_H_UPDATE_PHANDLE, old_phandle, new_phandle);
> > +MIRP
> > diff --git a/lib/libhvcall/hvcall.in b/lib/libhvcall/hvcall.in
> > index 4437b77f001d..ab7513af8977 100644
> > --- a/lib/libhvcall/hvcall.in
> > +++ b/lib/libhvcall/hvcall.in
> > @@ -31,4 +31,5 @@ cod(RX!)
> >  cod(hv-logical-memop)
> >  cod(hv-cas)
> >  cod(hv-rtas-update)
> > +cod(hv-update-phandle)
> >  cod(get-print-version)
> > diff --git a/lib/libhvcall/libhvcall.h b/lib/libhvcall/libhvcall.h
> > index b2ea3f6bf944..5776a2b772f7 100644
> > --- a/lib/libhvcall/libhvcall.h
> > +++ b/lib/libhvcall/libhvcall.h
> > @@ -25,6 +25,7 @@
> >  /* Client Architecture support */
> >  #define KVMPPC_H_CAS            (KVMPPC_HCALL_BASE + 0x2)
> >  #define KVMPPC_H_RTAS_UPDATE    (KVMPPC_HCALL_BASE + 0x3)
> > +#define KVMPPC_H_UPDATE_PHANDLE (KVMPPC_HCALL_BASE + 0x4)
> >  
> >  #ifndef __ASSEMBLY__  
> 
> Apart from the nits, the patch looks fine to me.
> 
>  Thomas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/slof/attachments/20170724/277b9d33/attachment.sig>