| Message ID | 20180514100332.32990-2-aik@ozlabs.ru |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fdt: Support more QEMU-originated phandles | expand |
On 14.05.2018 12:03, Alexey Kardashevskiy wrote: > We generate a fake XICS phandle in QEMU and SLOF replaces that phandle > with the real one (i.e. SLOF's node address) in interrupt-parent and > interrupt-map properties. These properties are handled differently - > the interrupt-map is fixed in place while interrupt-parent is > decoded+encoded+set as a property. > > This changes interrupt-parent fixing code to do what the interrupt-map > code does because soon we are going to have more phandles to fix and some > contain an array of phandles (such as "ibm,npu"). > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > board-qemu/slof/fdt.fs | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs > index 8e8921f..5284ae6 100644 > --- a/board-qemu/slof/fdt.fs > +++ b/board-qemu/slof/fdt.fs > @@ -279,6 +279,23 @@ fdt-claim-reserve > 2drop > ; > > +: fdt-replace-l, 4 - swap 4 + swap ; Why "l," ? I thought the "comma" words would be used for compiling stuff into the dictionary, but apparently you're not doing this here? > +: (fdt-replace-phandles) ( old new propname propnamelen node ) Please use the ( before -- after ) notation here: ( old new propname propnamelen node -- ) > + get-property IF 2drop EXIT THEN > + BEGIN > + dup > + WHILE ( old new prop-addr prop-len ) > + over l@ > + 4 pick = IF > + 2 pick 2 pick l! \ replace old with new in place > + TRUE TO (fdt-phandle-replaced) > + THEN > + fdt-replace-l, Maybe you could simply use the "4 - swap 4 + swap" directly here? It's only one line of code, so not sure whether it's worth to put this into a separate Forth word. > + REPEAT > + 2drop 2drop > +; > + > \ Replace one phandle "old" with a phandle "new" in "node" and recursively > \ in its child nodes: > : fdt-replace-all-phandles ( old new node -- ) > @@ -288,14 +305,8 @@ fdt-claim-reserve > ( old new prop-addr prop-len R: node ) > fdt-replace-interrupt-map > THEN > - s" interrupt-parent" r@ get-property 0= IF > - ( old new prop-addr prop-len R: node ) > - decode-int -rot 2drop ( old new val R: node ) > - 2 pick = IF ( old new R: node ) > - dup encode-int s" interrupt-parent" r@ set-property > - TRUE TO (fdt-phandle-replaced) > - THEN > - THEN > + > + 2dup s" interrupt-parent" r@ (fdt-replace-phandles) > \ ... add more properties that have to be fixed here ... > r> > \ Now recurse over all child nodes: ( old new node ) Apart from the nits, this looks ok to me for small properties. But how do you make sure that you do not accidentally replace the wrong spot in bigger properties later? Thomas
On 19/5/18 2:10 am, Thomas Huth wrote: > On 14.05.2018 12:03, Alexey Kardashevskiy wrote: >> We generate a fake XICS phandle in QEMU and SLOF replaces that phandle >> with the real one (i.e. SLOF's node address) in interrupt-parent and >> interrupt-map properties. These properties are handled differently - >> the interrupt-map is fixed in place while interrupt-parent is >> decoded+encoded+set as a property. >> >> This changes interrupt-parent fixing code to do what the interrupt-map >> code does because soon we are going to have more phandles to fix and some >> contain an array of phandles (such as "ibm,npu"). >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> board-qemu/slof/fdt.fs | 27 +++++++++++++++++++-------- >> 1 file changed, 19 insertions(+), 8 deletions(-) >> >> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs >> index 8e8921f..5284ae6 100644 >> --- a/board-qemu/slof/fdt.fs >> +++ b/board-qemu/slof/fdt.fs >> @@ -279,6 +279,23 @@ fdt-claim-reserve >> 2drop >> ; >> >> +: fdt-replace-l, 4 - swap 4 + swap ; > > Why "l," ? I thought the "comma" words would be used for compiling stuff > into the dictionary, but apparently you're not doing this here? Not just dictionary, fdt-fl.fs uses this naming approach when adding to flatten DT. Segher suggested, looks nicer. > >> +: (fdt-replace-phandles) ( old new propname propnamelen node ) > > Please use the ( before -- after ) notation here: > > ( old new propname propnamelen node -- ) > >> + get-property IF 2drop EXIT THEN >> + BEGIN >> + dup >> + WHILE ( old new prop-addr prop-len ) >> + over l@ >> + 4 pick = IF >> + 2 pick 2 pick l! \ replace old with new in place >> + TRUE TO (fdt-phandle-replaced) >> + THEN >> + fdt-replace-l, > > Maybe you could simply use the "4 - swap 4 + swap" directly here? It's > only one line of code, so not sure whether it's worth to put this into a > separate Forth word. "l," kind of allocates some bytes in some heap. > >> + REPEAT >> + 2drop 2drop >> +; >> + >> \ Replace one phandle "old" with a phandle "new" in "node" and recursively >> \ in its child nodes: >> : fdt-replace-all-phandles ( old new node -- ) >> @@ -288,14 +305,8 @@ fdt-claim-reserve >> ( old new prop-addr prop-len R: node ) >> fdt-replace-interrupt-map >> THEN >> - s" interrupt-parent" r@ get-property 0= IF >> - ( old new prop-addr prop-len R: node ) >> - decode-int -rot 2drop ( old new val R: node ) >> - 2 pick = IF ( old new R: node ) >> - dup encode-int s" interrupt-parent" r@ set-property >> - TRUE TO (fdt-phandle-replaced) >> - THEN >> - THEN >> + >> + 2dup s" interrupt-parent" r@ (fdt-replace-phandles) >> \ ... add more properties that have to be fixed here ... >> r> >> \ Now recurse over all child nodes: ( old new node ) > > Apart from the nits, this looks ok to me for small properties. But how > do you make sure that you do not accidentally replace the wrong spot in > bigger properties later? Unlike already present and potentially dangerous "interrupt-xxx" hacking, the new properties I am adding are storing only phandles (one or two). It is worth commenting though, will add.
On 21.05.2018 09:17, Alexey Kardashevskiy wrote: > On 19/5/18 2:10 am, Thomas Huth wrote: >> On 14.05.2018 12:03, Alexey Kardashevskiy wrote: >>> We generate a fake XICS phandle in QEMU and SLOF replaces that phandle >>> with the real one (i.e. SLOF's node address) in interrupt-parent and >>> interrupt-map properties. These properties are handled differently - >>> the interrupt-map is fixed in place while interrupt-parent is >>> decoded+encoded+set as a property. >>> >>> This changes interrupt-parent fixing code to do what the interrupt-map >>> code does because soon we are going to have more phandles to fix and some >>> contain an array of phandles (such as "ibm,npu"). >>> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>> --- >>> board-qemu/slof/fdt.fs | 27 +++++++++++++++++++-------- >>> 1 file changed, 19 insertions(+), 8 deletions(-) >>> >>> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs >>> index 8e8921f..5284ae6 100644 >>> --- a/board-qemu/slof/fdt.fs >>> +++ b/board-qemu/slof/fdt.fs >>> @@ -279,6 +279,23 @@ fdt-claim-reserve >>> 2drop >>> ; >>> >>> +: fdt-replace-l, 4 - swap 4 + swap ; >> >> Why "l," ? I thought the "comma" words would be used for compiling stuff >> into the dictionary, but apparently you're not doing this here? > > Not just dictionary, fdt-fl.fs uses this naming approach when adding to > flatten DT. Segher suggested, looks nicer. Ok, sure, but you don't *add* anything here, e.g. you also don't call fdt-allot like in the other "fdt-something," function. IMHO the comma is rather confusing here. >>> +: (fdt-replace-phandles) ( old new propname propnamelen node ) >> >> Please use the ( before -- after ) notation here: >> >> ( old new propname propnamelen node -- ) >> >>> + get-property IF 2drop EXIT THEN >>> + BEGIN >>> + dup >>> + WHILE ( old new prop-addr prop-len ) >>> + over l@ >>> + 4 pick = IF >>> + 2 pick 2 pick l! \ replace old with new in place >>> + TRUE TO (fdt-phandle-replaced) >>> + THEN >>> + fdt-replace-l, >> >> Maybe you could simply use the "4 - swap 4 + swap" directly here? It's >> only one line of code, so not sure whether it's worth to put this into a >> separate Forth word. > > "l," kind of allocates some bytes in some heap. But your "fdt-replace-l," does not allocate anything, it just increases an address on the stack and a size on the stack at the same time. Thomas
On Tue, May 22, 2018 at 07:23:55AM +0200, Thomas Huth wrote: > On 21.05.2018 09:17, Alexey Kardashevskiy wrote: > > On 19/5/18 2:10 am, Thomas Huth wrote: > >> On 14.05.2018 12:03, Alexey Kardashevskiy wrote: > >>> +: fdt-replace-l, 4 - swap 4 + swap ; > >> > >> Why "l," ? I thought the "comma" words would be used for compiling stuff > >> into the dictionary, but apparently you're not doing this here? > > > > Not just dictionary, fdt-fl.fs uses this naming approach when adding to > > flatten DT. Segher suggested, looks nicer. > > Ok, sure, but you don't *add* anything here, e.g. you also don't call > fdt-allot like in the other "fdt-something," function. IMHO the comma is > rather confusing here. Agreed. > >> Maybe you could simply use the "4 - swap 4 + swap" directly here? It's > >> only one line of code, so not sure whether it's worth to put this into a > >> separate Forth word. > > > > "l," kind of allocates some bytes in some heap. > > But your "fdt-replace-l," does not allocate anything, it just increases > an address on the stack and a size on the stack at the same time. fdt-replace-l, is just /l /string so it is much easier to understand if you just type that directly. Segher
On 22/5/18 4:04 pm, Segher Boessenkool wrote: > On Tue, May 22, 2018 at 07:23:55AM +0200, Thomas Huth wrote: >> On 21.05.2018 09:17, Alexey Kardashevskiy wrote: >>> On 19/5/18 2:10 am, Thomas Huth wrote: >>>> On 14.05.2018 12:03, Alexey Kardashevskiy wrote: >>>>> +: fdt-replace-l, 4 - swap 4 + swap ; >>>> >>>> Why "l," ? I thought the "comma" words would be used for compiling stuff >>>> into the dictionary, but apparently you're not doing this here? >>> >>> Not just dictionary, fdt-fl.fs uses this naming approach when adding to >>> flatten DT. Segher suggested, looks nicer. >> >> Ok, sure, but you don't *add* anything here, e.g. you also don't call >> fdt-allot like in the other "fdt-something," function. IMHO the comma is >> rather confusing here. > > Agreed. Ok, I got it wrong there. >>>> Maybe you could simply use the "4 - swap 4 + swap" directly here? It's >>>> only one line of code, so not sure whether it's worth to put this into a >>>> separate Forth word. >>> >>> "l," kind of allocates some bytes in some heap. >> >> But your "fdt-replace-l," does not allocate anything, it just increases >> an address on the stack and a size on the stack at the same time. > > fdt-replace-l, is just > > /l /string > > so it is much easier to understand if you just type that directly. Is it? It is not a string I am advancing on and using something+string on that is misleading imho. I'll do simple "4 - swap 4 + swap".
On 19/5/18 2:10 am, Thomas Huth wrote: > On 14.05.2018 12:03, Alexey Kardashevskiy wrote: >> We generate a fake XICS phandle in QEMU and SLOF replaces that phandle >> with the real one (i.e. SLOF's node address) in interrupt-parent and >> interrupt-map properties. These properties are handled differently - >> the interrupt-map is fixed in place while interrupt-parent is >> decoded+encoded+set as a property. >> >> This changes interrupt-parent fixing code to do what the interrupt-map >> code does because soon we are going to have more phandles to fix and some >> contain an array of phandles (such as "ibm,npu"). [snip] >> + >> \ Replace one phandle "old" with a phandle "new" in "node" and recursively >> \ in its child nodes: >> : fdt-replace-all-phandles ( old new node -- ) >> @@ -288,14 +305,8 @@ fdt-claim-reserve >> ( old new prop-addr prop-len R: node ) >> fdt-replace-interrupt-map >> THEN >> - s" interrupt-parent" r@ get-property 0= IF >> - ( old new prop-addr prop-len R: node ) >> - decode-int -rot 2drop ( old new val R: node ) >> - 2 pick = IF ( old new R: node ) >> - dup encode-int s" interrupt-parent" r@ set-property >> - TRUE TO (fdt-phandle-replaced) >> - THEN >> - THEN >> + >> + 2dup s" interrupt-parent" r@ (fdt-replace-phandles) >> \ ... add more properties that have to be fixed here ... >> r> >> \ Now recurse over all child nodes: ( old new node ) > > Apart from the nits, this looks ok to me for small properties. But how > do you make sure that you do not accidentally replace the wrong spot in > bigger properties later? btw when I tried updating the commit log, I realized that I do not understand why we fix the "interrupt-parent" at all. Git points to https://github.com/aik/SLOF/commit/82954d but that does not explain it either, do you have any memories about that? :)
On 29.05.2018 10:06, Alexey Kardashevskiy wrote: [...] > btw when I tried updating the commit log, I realized that I do not > understand why we fix the "interrupt-parent" at all. Git points to > https://github.com/aik/SLOF/commit/82954d but that does not explain it > either, do you have any memories about that? :) "interrupt-parent" is a standard property defined in chapter 5 here: https://www.openbios.org/data/docs/rec.intmap.d09.pdf Looks like we're currently not using it in QEMU, but since it's a standard property, I guess I implemented support for this in SLOF anyway, just to be sure. Thomas
On Tue, May 29, 2018 at 05:59:14PM +1000, Alexey Kardashevskiy wrote: > On 22/5/18 4:04 pm, Segher Boessenkool wrote: > >>>> Maybe you could simply use the "4 - swap 4 + swap" directly here? It's > >>>> only one line of code, so not sure whether it's worth to put this into a > >>>> separate Forth word. > >>> > >>> "l," kind of allocates some bytes in some heap. > >> > >> But your "fdt-replace-l," does not allocate anything, it just increases > >> an address on the stack and a size on the stack at the same time. > > > > fdt-replace-l, is just > > > > /l /string > > > > so it is much easier to understand if you just type that directly. > > Is it? It is not a string I am advancing on and using something+string on > that is misleading imho. I'll do simple "4 - swap 4 + swap". "string" just means "memory region": an address and a length. "/string" shortens a memory region at the front (doing it at the end is just "-"). Segher
On Mon, May 14, 2018 at 08:03:31PM +1000, Alexey Kardashevskiy wrote: > We generate a fake XICS phandle in QEMU and SLOF replaces that phandle > with the real one (i.e. SLOF's node address) in interrupt-parent and > interrupt-map properties. These properties are handled differently - > the interrupt-map is fixed in place while interrupt-parent is > decoded+encoded+set as a property. > > This changes interrupt-parent fixing code to do what the interrupt-map > code does because soon we are going to have more phandles to fix and some > contain an array of phandles (such as "ibm,npu"). > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> Hmmmmmmmm..... Every time I see more of these contortions to deal with the mismatch between SLOF assigned and qemu assigned phandles, I wonder anew if the right solution isn't just for SLOF to keep the qemu assigned ones, and only assign its own for ones that don't have them in the first place. I mean, yes, it's very non-traditional from an OF point of view, but I really have a hard time seeing it being nastier than this growing collection of ad-hoc hacks. > --- > board-qemu/slof/fdt.fs | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs > index 8e8921f..5284ae6 100644 > --- a/board-qemu/slof/fdt.fs > +++ b/board-qemu/slof/fdt.fs > @@ -279,6 +279,23 @@ fdt-claim-reserve > 2drop > ; > > +: fdt-replace-l, 4 - swap 4 + swap ; > + > +: (fdt-replace-phandles) ( old new propname propnamelen node ) > + get-property IF 2drop EXIT THEN > + BEGIN > + dup > + WHILE ( old new prop-addr prop-len ) > + over l@ > + 4 pick = IF > + 2 pick 2 pick l! \ replace old with new in place > + TRUE TO (fdt-phandle-replaced) > + THEN > + fdt-replace-l, > + REPEAT > + 2drop 2drop > +; > + > \ Replace one phandle "old" with a phandle "new" in "node" and recursively > \ in its child nodes: > : fdt-replace-all-phandles ( old new node -- ) > @@ -288,14 +305,8 @@ fdt-claim-reserve > ( old new prop-addr prop-len R: node ) > fdt-replace-interrupt-map > THEN > - s" interrupt-parent" r@ get-property 0= IF > - ( old new prop-addr prop-len R: node ) > - decode-int -rot 2drop ( old new val R: node ) > - 2 pick = IF ( old new R: node ) > - dup encode-int s" interrupt-parent" r@ set-property > - TRUE TO (fdt-phandle-replaced) > - THEN > - THEN > + > + 2dup s" interrupt-parent" r@ (fdt-replace-phandles) > \ ... add more properties that have to be fixed here ... > r> > \ Now recurse over all child nodes: ( old new node )
On 5/6/18 3:19 pm, David Gibson wrote: > On Mon, May 14, 2018 at 08:03:31PM +1000, Alexey Kardashevskiy wrote: >> We generate a fake XICS phandle in QEMU and SLOF replaces that phandle >> with the real one (i.e. SLOF's node address) in interrupt-parent and >> interrupt-map properties. These properties are handled differently - >> the interrupt-map is fixed in place while interrupt-parent is >> decoded+encoded+set as a property. >> >> This changes interrupt-parent fixing code to do what the interrupt-map >> code does because soon we are going to have more phandles to fix and some >> contain an array of phandles (such as "ibm,npu"). >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > Hmmmmmmmm..... > > Every time I see more of these contortions to deal with the mismatch > between SLOF assigned and qemu assigned phandles, I wonder anew if the > right solution isn't just for SLOF to keep the qemu assigned ones, and > only assign its own for ones that don't have them in the first place. > > I mean, yes, it's very non-traditional from an OF point of view, but I > really have a hard time seeing it being nastier than this growing > collection of ad-hoc hacks. I made these patches to add nvlink2 pass through support which means I have to have cross references between PCI device nodes in the device tree. It turns out that if I simply put unique numbers to "phandle" properties in QEMU and not patch them, they remain unchanged and then the NVIDIA driver can actually find nodes by these QEMU-made phandles. I cannot find places in the (guest) kernel which would rely on phandles being physical addresses - the client interface is used from prom_init.c and that is it. In general, we have 2 types of phandles - "phandle" and "linux,phandle". If "phandle" is present at the prom_init stage, we keep that, if it is not, then prom_init creates "linux,phandle" so we end up having one of them but not both. We could probably change QEMU to store its phandles as "linux,phandle" and simply not touch "phandle" properties. Or use "ibm,phandle". But the comment at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/fdt.c?h=v4.16#n221 suggests these are not interchangeable and better be equal. Even though "ibm,phandle" values can differ from "phandle" ones. Hm. The comment came from there :) https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04b954a673dd0 > >> --- >> board-qemu/slof/fdt.fs | 27 +++++++++++++++++++-------- >> 1 file changed, 19 insertions(+), 8 deletions(-) >> >> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs >> index 8e8921f..5284ae6 100644 >> --- a/board-qemu/slof/fdt.fs >> +++ b/board-qemu/slof/fdt.fs >> @@ -279,6 +279,23 @@ fdt-claim-reserve >> 2drop >> ; >> >> +: fdt-replace-l, 4 - swap 4 + swap ; >> + >> +: (fdt-replace-phandles) ( old new propname propnamelen node ) >> + get-property IF 2drop EXIT THEN >> + BEGIN >> + dup >> + WHILE ( old new prop-addr prop-len ) >> + over l@ >> + 4 pick = IF >> + 2 pick 2 pick l! \ replace old with new in place >> + TRUE TO (fdt-phandle-replaced) >> + THEN >> + fdt-replace-l, >> + REPEAT >> + 2drop 2drop >> +; >> + >> \ Replace one phandle "old" with a phandle "new" in "node" and recursively >> \ in its child nodes: >> : fdt-replace-all-phandles ( old new node -- ) >> @@ -288,14 +305,8 @@ fdt-claim-reserve >> ( old new prop-addr prop-len R: node ) >> fdt-replace-interrupt-map >> THEN >> - s" interrupt-parent" r@ get-property 0= IF >> - ( old new prop-addr prop-len R: node ) >> - decode-int -rot 2drop ( old new val R: node ) >> - 2 pick = IF ( old new R: node ) >> - dup encode-int s" interrupt-parent" r@ set-property >> - TRUE TO (fdt-phandle-replaced) >> - THEN >> - THEN >> + >> + 2dup s" interrupt-parent" r@ (fdt-replace-phandles) >> \ ... add more properties that have to be fixed here ... >> r> >> \ Now recurse over all child nodes: ( old new node ) >
On Thu, Jun 07, 2018 at 03:30:20PM +1000, Alexey Kardashevskiy wrote: > On 5/6/18 3:19 pm, David Gibson wrote: > > On Mon, May 14, 2018 at 08:03:31PM +1000, Alexey Kardashevskiy wrote: > >> We generate a fake XICS phandle in QEMU and SLOF replaces that phandle > >> with the real one (i.e. SLOF's node address) in interrupt-parent and > >> interrupt-map properties. These properties are handled differently - > >> the interrupt-map is fixed in place while interrupt-parent is > >> decoded+encoded+set as a property. > >> > >> This changes interrupt-parent fixing code to do what the interrupt-map > >> code does because soon we are going to have more phandles to fix and some > >> contain an array of phandles (such as "ibm,npu"). > >> > >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > > > Hmmmmmmmm..... > > > > Every time I see more of these contortions to deal with the mismatch > > between SLOF assigned and qemu assigned phandles, I wonder anew if the > > right solution isn't just for SLOF to keep the qemu assigned ones, and > > only assign its own for ones that don't have them in the first place. > > > > I mean, yes, it's very non-traditional from an OF point of view, but I > > really have a hard time seeing it being nastier than this growing > > collection of ad-hoc hacks. > > > I made these patches to add nvlink2 pass through support which means I have > to have cross references between PCI device nodes in the device tree. > > It turns out that if I simply put unique numbers to "phandle" properties in > QEMU and not patch them, they remain unchanged and then the NVIDIA driver > can actually find nodes by these QEMU-made phandles. I cannot find places > in the (guest) kernel which would rely on phandles being physical addresses > - the client interface is used from prom_init.c and that is it. No, the phandles definitely should be opaque to the guest. They might not be to SLOF itself, though. > In general, we have 2 types of phandles - "phandle" and > "linux,phandle". Those aren't two types, just an older and newer name for the same thing. > If > "phandle" is present at the prom_init stage, we keep that, if it is not, > then prom_init creates "linux,phandle" so we end up having one of them but > not both. We could probably change QEMU to store its phandles as > "linux,phandle" and simply not touch "phandle" properties. Or use > "ibm,phandle". > > But the comment at > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/fdt.c?h=v4.16#n221 > suggests these are not interchangeable and better be equal. Rather they *are* interchangeable. If you have both they absolutely must be equal. > Even though > "ibm,phandle" values can differ from "phandle" ones. Hm. I have no idea what "ibm,phandle" is about or where they come from. > > The comment came from there :) > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04b954a673dd0 > > > > > >> --- > >> board-qemu/slof/fdt.fs | 27 +++++++++++++++++++-------- > >> 1 file changed, 19 insertions(+), 8 deletions(-) > >> > >> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs > >> index 8e8921f..5284ae6 100644 > >> --- a/board-qemu/slof/fdt.fs > >> +++ b/board-qemu/slof/fdt.fs > >> @@ -279,6 +279,23 @@ fdt-claim-reserve > >> 2drop > >> ; > >> > >> +: fdt-replace-l, 4 - swap 4 + swap ; > >> + > >> +: (fdt-replace-phandles) ( old new propname propnamelen node ) > >> + get-property IF 2drop EXIT THEN > >> + BEGIN > >> + dup > >> + WHILE ( old new prop-addr prop-len ) > >> + over l@ > >> + 4 pick = IF > >> + 2 pick 2 pick l! \ replace old with new in place > >> + TRUE TO (fdt-phandle-replaced) > >> + THEN > >> + fdt-replace-l, > >> + REPEAT > >> + 2drop 2drop > >> +; > >> + > >> \ Replace one phandle "old" with a phandle "new" in "node" and recursively > >> \ in its child nodes: > >> : fdt-replace-all-phandles ( old new node -- ) > >> @@ -288,14 +305,8 @@ fdt-claim-reserve > >> ( old new prop-addr prop-len R: node ) > >> fdt-replace-interrupt-map > >> THEN > >> - s" interrupt-parent" r@ get-property 0= IF > >> - ( old new prop-addr prop-len R: node ) > >> - decode-int -rot 2drop ( old new val R: node ) > >> - 2 pick = IF ( old new R: node ) > >> - dup encode-int s" interrupt-parent" r@ set-property > >> - TRUE TO (fdt-phandle-replaced) > >> - THEN > >> - THEN > >> + > >> + 2dup s" interrupt-parent" r@ (fdt-replace-phandles) > >> \ ... add more properties that have to be fixed here ... > >> r> > >> \ Now recurse over all child nodes: ( old new node ) > > > >
diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs index 8e8921f..5284ae6 100644 --- a/board-qemu/slof/fdt.fs +++ b/board-qemu/slof/fdt.fs @@ -279,6 +279,23 @@ fdt-claim-reserve 2drop ; +: fdt-replace-l, 4 - swap 4 + swap ; + +: (fdt-replace-phandles) ( old new propname propnamelen node ) + get-property IF 2drop EXIT THEN + BEGIN + dup + WHILE ( old new prop-addr prop-len ) + over l@ + 4 pick = IF + 2 pick 2 pick l! \ replace old with new in place + TRUE TO (fdt-phandle-replaced) + THEN + fdt-replace-l, + REPEAT + 2drop 2drop +; + \ Replace one phandle "old" with a phandle "new" in "node" and recursively \ in its child nodes: : fdt-replace-all-phandles ( old new node -- ) @@ -288,14 +305,8 @@ fdt-claim-reserve ( old new prop-addr prop-len R: node ) fdt-replace-interrupt-map THEN - s" interrupt-parent" r@ get-property 0= IF - ( old new prop-addr prop-len R: node ) - decode-int -rot 2drop ( old new val R: node ) - 2 pick = IF ( old new R: node ) - dup encode-int s" interrupt-parent" r@ set-property - TRUE TO (fdt-phandle-replaced) - THEN - THEN + + 2dup s" interrupt-parent" r@ (fdt-replace-phandles) \ ... add more properties that have to be fixed here ... r> \ Now recurse over all child nodes: ( old new node )
We generate a fake XICS phandle in QEMU and SLOF replaces that phandle with the real one (i.e. SLOF's node address) in interrupt-parent and interrupt-map properties. These properties are handled differently - the interrupt-map is fixed in place while interrupt-parent is decoded+encoded+set as a property. This changes interrupt-parent fixing code to do what the interrupt-map code does because soon we are going to have more phandles to fix and some contain an array of phandles (such as "ibm,npu"). Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- board-qemu/slof/fdt.fs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)