diff mbox

[v3,3/3] board-qemu: add private hcall to inform host on "phandle" update

Message ID 150001528273.32021.11084260558190071450.stgit@bahia.lan
State Superseded
Headers show

Commit Message

Greg Kurz July 14, 2017, 6:54 a.m. UTC
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@redhat.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
v3: - drop "<> 0" before IF

v2: - use ?dup
    - switch the order of the parameters of hv-update-phandle
    - added stack comment to hv-update-phandle
---
 board-qemu/slof/fdt.fs    |   14 ++++++++++++--
 lib/libhvcall/hvcall.code |    7 +++++++
 lib/libhvcall/hvcall.in   |    1 +
 lib/libhvcall/libhvcall.h |    1 +
 4 files changed, 21 insertions(+), 2 deletions(-)

Comments

Greg Kurz July 15, 2017, 12:06 p.m. UTC | #1
On Fri, 14 Jul 2017 08:54:42 +0200
Greg Kurz <groug@kaod.org> 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@redhat.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> v3: - drop "<> 0" before IF
> 
> v2: - use ?dup
>     - switch the order of the parameters of hv-update-phandle
>     - added stack comment to hv-update-phandle
> ---
>  board-qemu/slof/fdt.fs    |   14 ++++++++++++--
>  lib/libhvcall/hvcall.code |    7 +++++++
>  lib/libhvcall/hvcall.in   |    1 +
>  lib/libhvcall/libhvcall.h |    1 +
>  4 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> index 851645eae299..a78e8ff208f6 100644
> --- a/board-qemu/slof/fdt.fs
> +++ b/board-qemu/slof/fdt.fs
> @@ -308,18 +308,28 @@ fdt-claim-reserve
>     3drop
>  ;
>  
> +\ Tell QEMU about the updated phandle:
> +: fdt-hv-update-phandle ( old new -- )
> +   hv-update-phandle ?dup IF
> +      \ Ignore hcall not implemented error, print error otherwise
> +      ?dup -2 <> IF ." HV-UPDATE-PHANDLE error: " . cr THEN
          ^^
This is wrong... :-\

fdt-hv-update-phandle doesn't return anything: we shouldn't ?dup the return
value (which we know to be non-null because of the previous ?dup BTW).

> +   THEN
> +;
> +
>  \ Replace one FDT phandle "val" with a OF1275 phandle "node" in the
>  \ whole tree:
>  : fdt-update-phandle ( val node -- )
>     >r  
>     FALSE TO (fdt-phandle-replaced)
> -   r@ s" /" find-node               ( val node root )
> -   fdt-replace-all-phandles
> +   r@ 2dup s" /" find-node          ( val node val node root )
> +   fdt-replace-all-phandles         ( val node )
>     (fdt-phandle-replaced) IF
> +      fdt-hv-update-phandle
>        r@ set-node
>        s" phandle" delete-property
>        s" linux,phandle" delete-property
>     ELSE
> +      2drop
>        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..834974862ef5 100644
> --- a/lib/libhvcall/hvcall.code
> +++ b/lib/libhvcall/hvcall.code
> @@ -129,3 +129,10 @@ PRIM(check_X2d_and_X2d_patch_X2d_sc1)
>  
>  	patch_broken_sc1((void*)start, (void*)end, (void*)patch_ins);
>  MIRP
> +
> +// : hv-update-phandle ( old_phandle new_phandle -- res )
> +PRIM(hv_X2d_update_X2d_phandle)
> +	uint32_t new_phandle = TOS.u; POP;
> +	uint32_t old_phandle = TOS.u;
> +	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__
>  
> 
> _______________________________________________
> SLOF mailing list
> SLOF@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/slof
Alexey Kardashevskiy July 17, 2017, 3:40 a.m. UTC | #2
On 15/07/17 22:06, Greg Kurz wrote:
> On Fri, 14 Jul 2017 08:54:42 +0200
> Greg Kurz <groug@kaod.org> 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@redhat.com>
>> Signed-off-by: Greg Kurz <groug@kaod.org>
>> ---
>> v3: - drop "<> 0" before IF
>>
>> v2: - use ?dup
>>     - switch the order of the parameters of hv-update-phandle
>>     - added stack comment to hv-update-phandle
>> ---
>>  board-qemu/slof/fdt.fs    |   14 ++++++++++++--
>>  lib/libhvcall/hvcall.code |    7 +++++++
>>  lib/libhvcall/hvcall.in   |    1 +
>>  lib/libhvcall/libhvcall.h |    1 +
>>  4 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
>> index 851645eae299..a78e8ff208f6 100644
>> --- a/board-qemu/slof/fdt.fs
>> +++ b/board-qemu/slof/fdt.fs
>> @@ -308,18 +308,28 @@ fdt-claim-reserve
>>     3drop
>>  ;
>>  
>> +\ Tell QEMU about the updated phandle:
>> +: fdt-hv-update-phandle ( old new -- )
>> +   hv-update-phandle ?dup IF
>> +      \ Ignore hcall not implemented error, print error otherwise
>> +      ?dup -2 <> IF ." HV-UPDATE-PHANDLE error: " . cr THEN
>           ^^
> This is wrong... :-\
> 
> fdt-hv-update-phandle doesn't return anything: we shouldn't ?dup the return
> value (which we know to be non-null because of the previous ?dup BTW).

Does this mean you are reposting it soon?
For now, I've picked 1/3 and 2/3 so no need to repost, thanks.


> 
>> +   THEN
>> +;
>> +
>>  \ Replace one FDT phandle "val" with a OF1275 phandle "node" in the
>>  \ whole tree:
>>  : fdt-update-phandle ( val node -- )
>>     >r  
>>     FALSE TO (fdt-phandle-replaced)
>> -   r@ s" /" find-node               ( val node root )
>> -   fdt-replace-all-phandles
>> +   r@ 2dup s" /" find-node          ( val node val node root )
>> +   fdt-replace-all-phandles         ( val node )
>>     (fdt-phandle-replaced) IF
>> +      fdt-hv-update-phandle
>>        r@ set-node
>>        s" phandle" delete-property
>>        s" linux,phandle" delete-property
>>     ELSE
>> +      2drop
>>        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..834974862ef5 100644
>> --- a/lib/libhvcall/hvcall.code
>> +++ b/lib/libhvcall/hvcall.code
>> @@ -129,3 +129,10 @@ PRIM(check_X2d_and_X2d_patch_X2d_sc1)
>>  
>>  	patch_broken_sc1((void*)start, (void*)end, (void*)patch_ins);
>>  MIRP
>> +
>> +// : hv-update-phandle ( old_phandle new_phandle -- res )
>> +PRIM(hv_X2d_update_X2d_phandle)
>> +	uint32_t new_phandle = TOS.u; POP;
>> +	uint32_t old_phandle = TOS.u;
>> +	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__
>>  
>>
>> _______________________________________________
>> SLOF mailing list
>> SLOF@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/slof
> 
> 
> 
> _______________________________________________
> SLOF mailing list
> SLOF@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/slof
>
Greg Kurz July 24, 2017, 9:28 a.m. UTC | #3
On Mon, 17 Jul 2017 13:40:56 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 15/07/17 22:06, Greg Kurz wrote:
> > On Fri, 14 Jul 2017 08:54:42 +0200
> > Greg Kurz <groug@kaod.org> 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@redhat.com>
> >> Signed-off-by: Greg Kurz <groug@kaod.org>
> >> ---
> >> v3: - drop "<> 0" before IF
> >>
> >> v2: - use ?dup
> >>     - switch the order of the parameters of hv-update-phandle
> >>     - added stack comment to hv-update-phandle
> >> ---
> >>  board-qemu/slof/fdt.fs    |   14 ++++++++++++--
> >>  lib/libhvcall/hvcall.code |    7 +++++++
> >>  lib/libhvcall/hvcall.in   |    1 +
> >>  lib/libhvcall/libhvcall.h |    1 +
> >>  4 files changed, 21 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> >> index 851645eae299..a78e8ff208f6 100644
> >> --- a/board-qemu/slof/fdt.fs
> >> +++ b/board-qemu/slof/fdt.fs
> >> @@ -308,18 +308,28 @@ fdt-claim-reserve
> >>     3drop
> >>  ;
> >>  
> >> +\ Tell QEMU about the updated phandle:
> >> +: fdt-hv-update-phandle ( old new -- )
> >> +   hv-update-phandle ?dup IF
> >> +      \ Ignore hcall not implemented error, print error otherwise
> >> +      ?dup -2 <> IF ." HV-UPDATE-PHANDLE error: " . cr THEN  
> >           ^^
> > This is wrong... :-\
> > 
> > fdt-hv-update-phandle doesn't return anything: we shouldn't ?dup the return
> > value (which we know to be non-null because of the previous ?dup BTW).  
> 
> Does this mean you are reposting it soon?

I was on vacation last week but, yes, I plan to repost very soon.

> For now, I've picked 1/3 and 2/3 so no need to repost, thanks.
> 

Thanks!

> 
> >   
> >> +   THEN
> >> +;
> >> +
> >>  \ Replace one FDT phandle "val" with a OF1275 phandle "node" in the
> >>  \ whole tree:
> >>  : fdt-update-phandle ( val node -- )  
> >>     >r    
> >>     FALSE TO (fdt-phandle-replaced)
> >> -   r@ s" /" find-node               ( val node root )
> >> -   fdt-replace-all-phandles
> >> +   r@ 2dup s" /" find-node          ( val node val node root )
> >> +   fdt-replace-all-phandles         ( val node )
> >>     (fdt-phandle-replaced) IF
> >> +      fdt-hv-update-phandle
> >>        r@ set-node
> >>        s" phandle" delete-property
> >>        s" linux,phandle" delete-property
> >>     ELSE
> >> +      2drop
> >>        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..834974862ef5 100644
> >> --- a/lib/libhvcall/hvcall.code
> >> +++ b/lib/libhvcall/hvcall.code
> >> @@ -129,3 +129,10 @@ PRIM(check_X2d_and_X2d_patch_X2d_sc1)
> >>  
> >>  	patch_broken_sc1((void*)start, (void*)end, (void*)patch_ins);
> >>  MIRP
> >> +
> >> +// : hv-update-phandle ( old_phandle new_phandle -- res )
> >> +PRIM(hv_X2d_update_X2d_phandle)
> >> +	uint32_t new_phandle = TOS.u; POP;
> >> +	uint32_t old_phandle = TOS.u;
> >> +	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__
> >>  
> >>
> >> _______________________________________________
> >> SLOF mailing list
> >> SLOF@lists.ozlabs.org
> >> https://lists.ozlabs.org/listinfo/slof  
> > 
> > 
> > 
> > _______________________________________________
> > SLOF mailing list
> > SLOF@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/slof
> >   
> 
>
diff mbox

Patch

diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
index 851645eae299..a78e8ff208f6 100644
--- a/board-qemu/slof/fdt.fs
+++ b/board-qemu/slof/fdt.fs
@@ -308,18 +308,28 @@  fdt-claim-reserve
    3drop
 ;
 
+\ Tell QEMU about the updated phandle:
+: fdt-hv-update-phandle ( old new -- )
+   hv-update-phandle ?dup IF
+      \ Ignore hcall not implemented error, print error otherwise
+      ?dup -2 <> IF ." HV-UPDATE-PHANDLE error: " . cr THEN
+   THEN
+;
+
 \ Replace one FDT phandle "val" with a OF1275 phandle "node" in the
 \ whole tree:
 : fdt-update-phandle ( val node -- )
    >r
    FALSE TO (fdt-phandle-replaced)
-   r@ s" /" find-node               ( val node root )
-   fdt-replace-all-phandles
+   r@ 2dup s" /" find-node          ( val node val node root )
+   fdt-replace-all-phandles         ( val node )
    (fdt-phandle-replaced) IF
+      fdt-hv-update-phandle
       r@ set-node
       s" phandle" delete-property
       s" linux,phandle" delete-property
    ELSE
+      2drop
       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..834974862ef5 100644
--- a/lib/libhvcall/hvcall.code
+++ b/lib/libhvcall/hvcall.code
@@ -129,3 +129,10 @@  PRIM(check_X2d_and_X2d_patch_X2d_sc1)
 
 	patch_broken_sc1((void*)start, (void*)end, (void*)patch_ins);
 MIRP
+
+// : hv-update-phandle ( old_phandle new_phandle -- res )
+PRIM(hv_X2d_update_X2d_phandle)
+	uint32_t new_phandle = TOS.u; POP;
+	uint32_t old_phandle = TOS.u;
+	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__