| Message ID | 149968907271.10573.9258257305369267903.stgit@bahia.lan |
|---|---|
| State | Superseded |
| Headers | show |
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@redhat.com> > Signed-off-by: Greg Kurz <groug@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. > + 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
On Mon, Jul 10, 2017 at 04:57:57PM +0200, Thomas Huth wrote: > On 10.07.2017 14:17, Greg Kurz wrote: > > --- 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. In general, this needs to be much better factored if it wants to be readable at all. As soon as you need indentation to make any sense of your structure, you need to factor more. Segher
On Mon, 10 Jul 2017 16:57:57 +0200 Thomas Huth <thuth@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@redhat.com> > > Signed-off-by: Greg Kurz <groug@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. > Oh, I didn't know about ?dup but I'd gladly use it from now on ! > > + \ 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. > Ok. > > + 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) > Sure, I'll do that. > > +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 :-) > Indeed :) I'll fix that. > > + 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. > Thanks for the review! Cheers, -- Greg > Thomas
On Mon, 10 Jul 2017 10:13:31 -0500 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Mon, Jul 10, 2017 at 04:57:57PM +0200, Thomas Huth wrote: > > On 10.07.2017 14:17, Greg Kurz wrote: > > > --- 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. > > In general, this needs to be much better factored if it wants to be > readable at all. As soon as you need indentation to make any sense > of your structure, you need to factor more. > Not sure I have enough forth background to understand... are you suggesting that each level of indentation calls for the introduction of a new word ? > > Segher
On Mon, Jul 10, 2017 at 05:40:14PM +0200, Greg Kurz wrote: > > > Dito - use ?dup and remove the "ELSE drop" part. > > > > In general, this needs to be much better factored if it wants to be > > readable at all. As soon as you need indentation to make any sense > > of your structure, you need to factor more. > > Not sure I have enough forth background to understand... are > you suggesting that each level of indentation calls for the > introduction of a new word ? Not exactly. I am saying that if you make your word bodies huge and complex, you have a hard time understanding them. Indenting stuff (and adding stack comments for each line, or even after each word) helps a little bit, but it's a crutch. If you have a hard time modifying some code because it is too complex, first factor it a bit better. Segher
On Mon, 10 Jul 2017 11:19:49 -0500 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Mon, Jul 10, 2017 at 05:40:14PM +0200, Greg Kurz wrote: > > > > Dito - use ?dup and remove the "ELSE drop" part. > > > > > > In general, this needs to be much better factored if it wants to be > > > readable at all. As soon as you need indentation to make any sense > > > of your structure, you need to factor more. > > > > Not sure I have enough forth background to understand... are > > you suggesting that each level of indentation calls for the > > introduction of a new word ? > > Not exactly. I am saying that if you make your word bodies huge and > complex, you have a hard time understanding them. Indenting stuff > (and adding stack comments for each line, or even after each word) > helps a little bit, but it's a crutch. > Ok, I get it better now. > If you have a hard time modifying some code because it is too complex, > first factor it a bit better. How did you guess I had hard time modifying this code ? ;) I'll try to factor. Cheers, -- Greg > > > Segher
On Mon, 10 Jul 2017 16:57:57 +0200 Thomas Huth <thuth@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@redhat.com> > > Signed-off-by: Greg Kurz <groug@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
On 24.07.2017 13:19, Greg Kurz wrote: > On Mon, 10 Jul 2017 16:57:57 +0200 > Thomas Huth <thuth@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@redhat.com> >>> Signed-off-by: Greg Kurz <groug@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 ? Sorry, my bad, I somehow overlooked the -2 during my review. Thomas
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 + \ Ignore hcall not implemented error, print error otherwise + dup -2 <> IF ." HV-UPDATE-PHANDLE error: " . cr ELSE drop THEN + 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 + +PRIM(hv_X2d_update_X2d_phandle) + uint32_t old_phandle = TOS.u; POP; + uint32_t new_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__
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> --- 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(-)