| Message ID | 1488583692-31827-1-git-send-email-mdroth@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 04/03/17 10:28, Michael Roth wrote: > This is needed to ensure VFIO passthrough devices are able to > offload MMIO accesses to KVM. > > Cc: Segher Boessenkool <segher@kernel.crashing.org> > Cc: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > v2: > * default to 0 min-align instead of 64K (David) After some discussions on IRC, it seems safer to do the following: 1. first read the qemu,mem-bar-min-align property and use that for alignment; 2. if no property is present, than default to 64K. This should work assuming that normally newer SLOF should not appear in real world together with old QEMU (so we do not change old QEMU behaviour), and this SLOF change is targeted for QEMU v2.10, and one way or another we will address BAR alignment in QEMU in that timeframe - QEMU will either put a property to the device tree (64k for new machines and 1 for old machines) or just do BAR allocation. If you agree, could you make another patch combining the property and 64k default? If not, speak :) Thanks! > * refactor assign-var* for readability (Segher) > * add/utilize umax instead of max (Segher) > --- > board-qemu/slof/pci-phb.fs | 4 ++++ > slof/engine.in | 1 + > slof/fs/pci-properties.fs | 38 ++++++++++++++++++++++++++++++-------- > slof/fs/pci-scan.fs | 3 +++ > 4 files changed, 38 insertions(+), 8 deletions(-) > > diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs > index 667514e..698bf5a 100644 > --- a/board-qemu/slof/pci-phb.fs > +++ b/board-qemu/slof/pci-phb.fs > @@ -331,6 +331,10 @@ setup-puid > my-puid TO puid \ Set current puid > phb-parse-ranges > 1 TO pci-hotplug-enabled > + s" qemu,mem-bar-min-align" get-node get-property 0= IF > + decode-int TO pci-mem-bar-min-align > + 2drop > + THEN > s" qemu,phb-enumerated" get-node get-property 0<> IF > 1 0 (probe-pci-host-bridge) > ELSE > diff --git a/slof/engine.in b/slof/engine.in > index 0344a38..549e409 100644 > --- a/slof/engine.in > +++ b/slof/engine.in > @@ -145,6 +145,7 @@ col(D2/ >R U2/ R@ LIT(8*CELLSIZE-1) LSHIFT OR R> 2/) > col(NEGATE 0 SWAP -) > col(ABS DUP 0< 0BRANCH(1) NEGATE) > col(MAX 2DUP < 0BRANCH(1) SWAP DROP) > +col(UMAX 2DUP U< 0BRANCH(1) SWAP DROP) > col(MIN 2DUP > 0BRANCH(1) SWAP DROP) > col(U* *) > col(1+ 1 +) > diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs > index 7faa714..e6fd843 100644 > --- a/slof/fs/pci-properties.fs > +++ b/slof/fs/pci-properties.fs > @@ -91,17 +91,37 @@ > \ *************************************************************************************** > \ align the current mem and set var to next mem > \ align with a size of 0 returns 0 !!! > -: assign-var ( size var -- al-mem ) > - 2dup @ \ ( size var size cur-mem ) read current free mem > - swap #aligned \ ( size var al-mem ) align the mem to the size > - dup 2swap -rot + \ ( al-mem var new-mem ) add size to aligned mem > - swap ! \ ( al-mem ) set variable to new mem > +: assign-var-align ( size align var -- al-mem ) > + dup >r @ \ ( size align cur-mem ) > + swap #aligned \ ( size al-mem ) > + tuck + \ ( al-mem new-mem ) > + r> ! \ ( al-mem ) > +; > + > +: assign-var-min-align ( size min-align var -- al-mem ) > + >r over umax \ ( size align ) > + r> assign-var-align \ ( al-mem ) > ; > > \ set bar to current free mem ( in variable ) and set variable to next free mem > : assign-bar-value32 ( bar size var -- 4 ) > over IF \ IF size > 0 > - assign-var \ | ( bar al-mem ) set variable to next mem > + >r \ | ( bar size ) > + pci-mem-bar-min-align \ | ( bar size min-align ) > + r> assign-var-min-align \ | ( bar al-mem ) set variable to next mem > + swap rtas-config-l! \ | ( -- ) set the bar to al-mem > + ELSE \ ELSE > + 2drop drop \ | clear stack > + THEN \ FI > + 4 \ size of the base-address-register > +; > + > +\ set bar to current free mem ( in variable ) and set variable to next free mem > +: assign-io-bar-value32 ( bar size var -- 4 ) > + over IF \ IF size > 0 > + >r \ | ( bar size ) > + dup \ | ( bar size size-align ) > + r> assign-var-align \ | ( bar al-mem ) set variable to next mem > swap rtas-config-l! \ | ( -- ) set the bar to al-mem > ELSE \ ELSE > 2drop drop \ | clear stack > @@ -112,7 +132,9 @@ > \ set bar to current free mem ( in variable ) and set variable to next free mem > : assign-bar-value64 ( bar size var -- 8 ) > over IF \ IF size > 0 > - assign-var \ | ( bar al-mem ) set variable to next mem > + >r \ | ( bar size ) > + pci-mem-bar-min-align \ | ( bar size min-align ) > + r> assign-var-min-align \ | ( bar al-mem ) set variable to next mem > swap \ | ( al-mem addr ) calc config-addr of this bar > 2dup rtas-config-l! \ | ( al-mem addr ) set the Lower part of the bar to al-mem > 4 + swap 20 rshift \ | ( al-mem>>32 addr ) prepare the upper part of the al-mem > @@ -163,7 +185,7 @@ > : assign-io-bar ( bar-addr -- 4 ) > dup pci-bar-size-io \ fetch size > pci-next-io \ var to change > - assign-bar-value32 \ and set it all > + assign-io-bar-value32 \ and set it all > ; > > \ Setup an Expansion ROM bar > diff --git a/slof/fs/pci-scan.fs b/slof/fs/pci-scan.fs > index a528c8e..bd8cc0d 100644 > --- a/slof/fs/pci-scan.fs > +++ b/slof/fs/pci-scan.fs > @@ -24,6 +24,9 @@ VARIABLE pci-max-io > VARIABLE pci-next-mem64 \ prefetchable 64-bit memory mapped > VARIABLE pci-max-mem64 > > +\ 0 to default to natural alignment > +0 VALUE pci-mem-bar-min-align > + > \ Counter of busses found > 0 VALUE pci-bus-number > \ Counter of devices found >
Quoting Alexey Kardashevskiy (2017-03-07 23:30:34) > On 04/03/17 10:28, Michael Roth wrote: > > This is needed to ensure VFIO passthrough devices are able to > > offload MMIO accesses to KVM. > > > > Cc: Segher Boessenkool <segher@kernel.crashing.org> > > Cc: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > > Cc: David Gibson <david@gibson.dropbear.id.au> > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > --- > > v2: > > * default to 0 min-align instead of 64K (David) > > > After some discussions on IRC, it seems safer to do the following: > 1. first read the qemu,mem-bar-min-align property and use that for alignment; > 2. if no property is present, than default to 64K. > > This should work assuming that normally newer SLOF should not appear in > real world together with old QEMU (so we do not change old QEMU behaviour), > and this SLOF change is targeted for QEMU v2.10, and one way or another we > will address BAR alignment in QEMU in that timeframe - QEMU will either put > a property to the device tree (64k for new machines and 1 for old machines) > or just do BAR allocation. > > If you agree, could you make another patch combining the property and 64k > default? If not, speak :) Thanks! This is somewhat opposed to David's concerns WRT requiring new device-tree properties to maintain current behavior, instead of the opposite. But if it's under the understanding that we do plan to address this on the QEMU side in a way that won't require such a property (by doing BAR assignment in QEMU as you mentioned), then perhaps this approach is acceptable... the property would only be a last-resort. > > > > > * refactor assign-var* for readability (Segher) > > * add/utilize umax instead of max (Segher) > > --- > > board-qemu/slof/pci-phb.fs | 4 ++++ > > slof/engine.in | 1 + > > slof/fs/pci-properties.fs | 38 ++++++++++++++++++++++++++++++-------- > > slof/fs/pci-scan.fs | 3 +++ > > 4 files changed, 38 insertions(+), 8 deletions(-) > > > > diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs > > index 667514e..698bf5a 100644 > > --- a/board-qemu/slof/pci-phb.fs > > +++ b/board-qemu/slof/pci-phb.fs > > @@ -331,6 +331,10 @@ setup-puid > > my-puid TO puid \ Set current puid > > phb-parse-ranges > > 1 TO pci-hotplug-enabled > > + s" qemu,mem-bar-min-align" get-node get-property 0= IF > > + decode-int TO pci-mem-bar-min-align > > + 2drop > > + THEN > > s" qemu,phb-enumerated" get-node get-property 0<> IF > > 1 0 (probe-pci-host-bridge) > > ELSE > > diff --git a/slof/engine.in b/slof/engine.in > > index 0344a38..549e409 100644 > > --- a/slof/engine.in > > +++ b/slof/engine.in > > @@ -145,6 +145,7 @@ col(D2/ >R U2/ R@ LIT(8*CELLSIZE-1) LSHIFT OR R> 2/) > > col(NEGATE 0 SWAP -) > > col(ABS DUP 0< 0BRANCH(1) NEGATE) > > col(MAX 2DUP < 0BRANCH(1) SWAP DROP) > > +col(UMAX 2DUP U< 0BRANCH(1) SWAP DROP) > > col(MIN 2DUP > 0BRANCH(1) SWAP DROP) > > col(U* *) > > col(1+ 1 +) > > diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs > > index 7faa714..e6fd843 100644 > > --- a/slof/fs/pci-properties.fs > > +++ b/slof/fs/pci-properties.fs > > @@ -91,17 +91,37 @@ > > \ *************************************************************************************** > > \ align the current mem and set var to next mem > > \ align with a size of 0 returns 0 !!! > > -: assign-var ( size var -- al-mem ) > > - 2dup @ \ ( size var size cur-mem ) read current free mem > > - swap #aligned \ ( size var al-mem ) align the mem to the size > > - dup 2swap -rot + \ ( al-mem var new-mem ) add size to aligned mem > > - swap ! \ ( al-mem ) set variable to new mem > > +: assign-var-align ( size align var -- al-mem ) > > + dup >r @ \ ( size align cur-mem ) > > + swap #aligned \ ( size al-mem ) > > + tuck + \ ( al-mem new-mem ) > > + r> ! \ ( al-mem ) > > +; > > + > > +: assign-var-min-align ( size min-align var -- al-mem ) > > + >r over umax \ ( size align ) > > + r> assign-var-align \ ( al-mem ) > > ; > > > > \ set bar to current free mem ( in variable ) and set variable to next free mem > > : assign-bar-value32 ( bar size var -- 4 ) > > over IF \ IF size > 0 > > - assign-var \ | ( bar al-mem ) set variable to next mem > > + >r \ | ( bar size ) > > + pci-mem-bar-min-align \ | ( bar size min-align ) > > + r> assign-var-min-align \ | ( bar al-mem ) set variable to next mem > > + swap rtas-config-l! \ | ( -- ) set the bar to al-mem > > + ELSE \ ELSE > > + 2drop drop \ | clear stack > > + THEN \ FI > > + 4 \ size of the base-address-register > > +; > > + > > +\ set bar to current free mem ( in variable ) and set variable to next free mem > > +: assign-io-bar-value32 ( bar size var -- 4 ) > > + over IF \ IF size > 0 > > + >r \ | ( bar size ) > > + dup \ | ( bar size size-align ) > > + r> assign-var-align \ | ( bar al-mem ) set variable to next mem > > swap rtas-config-l! \ | ( -- ) set the bar to al-mem > > ELSE \ ELSE > > 2drop drop \ | clear stack > > @@ -112,7 +132,9 @@ > > \ set bar to current free mem ( in variable ) and set variable to next free mem > > : assign-bar-value64 ( bar size var -- 8 ) > > over IF \ IF size > 0 > > - assign-var \ | ( bar al-mem ) set variable to next mem > > + >r \ | ( bar size ) > > + pci-mem-bar-min-align \ | ( bar size min-align ) > > + r> assign-var-min-align \ | ( bar al-mem ) set variable to next mem > > swap \ | ( al-mem addr ) calc config-addr of this bar > > 2dup rtas-config-l! \ | ( al-mem addr ) set the Lower part of the bar to al-mem > > 4 + swap 20 rshift \ | ( al-mem>>32 addr ) prepare the upper part of the al-mem > > @@ -163,7 +185,7 @@ > > : assign-io-bar ( bar-addr -- 4 ) > > dup pci-bar-size-io \ fetch size > > pci-next-io \ var to change > > - assign-bar-value32 \ and set it all > > + assign-io-bar-value32 \ and set it all > > ; > > > > \ Setup an Expansion ROM bar > > diff --git a/slof/fs/pci-scan.fs b/slof/fs/pci-scan.fs > > index a528c8e..bd8cc0d 100644 > > --- a/slof/fs/pci-scan.fs > > +++ b/slof/fs/pci-scan.fs > > @@ -24,6 +24,9 @@ VARIABLE pci-max-io > > VARIABLE pci-next-mem64 \ prefetchable 64-bit memory mapped > > VARIABLE pci-max-mem64 > > > > +\ 0 to default to natural alignment > > +0 VALUE pci-mem-bar-min-align > > + > > \ Counter of busses found > > 0 VALUE pci-bus-number > > \ Counter of devices found > > > > > -- > Alexey >
diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs index 667514e..698bf5a 100644 --- a/board-qemu/slof/pci-phb.fs +++ b/board-qemu/slof/pci-phb.fs @@ -331,6 +331,10 @@ setup-puid my-puid TO puid \ Set current puid phb-parse-ranges 1 TO pci-hotplug-enabled + s" qemu,mem-bar-min-align" get-node get-property 0= IF + decode-int TO pci-mem-bar-min-align + 2drop + THEN s" qemu,phb-enumerated" get-node get-property 0<> IF 1 0 (probe-pci-host-bridge) ELSE diff --git a/slof/engine.in b/slof/engine.in index 0344a38..549e409 100644 --- a/slof/engine.in +++ b/slof/engine.in @@ -145,6 +145,7 @@ col(D2/ >R U2/ R@ LIT(8*CELLSIZE-1) LSHIFT OR R> 2/) col(NEGATE 0 SWAP -) col(ABS DUP 0< 0BRANCH(1) NEGATE) col(MAX 2DUP < 0BRANCH(1) SWAP DROP) +col(UMAX 2DUP U< 0BRANCH(1) SWAP DROP) col(MIN 2DUP > 0BRANCH(1) SWAP DROP) col(U* *) col(1+ 1 +) diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs index 7faa714..e6fd843 100644 --- a/slof/fs/pci-properties.fs +++ b/slof/fs/pci-properties.fs @@ -91,17 +91,37 @@ \ *************************************************************************************** \ align the current mem and set var to next mem \ align with a size of 0 returns 0 !!! -: assign-var ( size var -- al-mem ) - 2dup @ \ ( size var size cur-mem ) read current free mem - swap #aligned \ ( size var al-mem ) align the mem to the size - dup 2swap -rot + \ ( al-mem var new-mem ) add size to aligned mem - swap ! \ ( al-mem ) set variable to new mem +: assign-var-align ( size align var -- al-mem ) + dup >r @ \ ( size align cur-mem ) + swap #aligned \ ( size al-mem ) + tuck + \ ( al-mem new-mem ) + r> ! \ ( al-mem ) +; + +: assign-var-min-align ( size min-align var -- al-mem ) + >r over umax \ ( size align ) + r> assign-var-align \ ( al-mem ) ; \ set bar to current free mem ( in variable ) and set variable to next free mem : assign-bar-value32 ( bar size var -- 4 ) over IF \ IF size > 0 - assign-var \ | ( bar al-mem ) set variable to next mem + >r \ | ( bar size ) + pci-mem-bar-min-align \ | ( bar size min-align ) + r> assign-var-min-align \ | ( bar al-mem ) set variable to next mem + swap rtas-config-l! \ | ( -- ) set the bar to al-mem + ELSE \ ELSE + 2drop drop \ | clear stack + THEN \ FI + 4 \ size of the base-address-register +; + +\ set bar to current free mem ( in variable ) and set variable to next free mem +: assign-io-bar-value32 ( bar size var -- 4 ) + over IF \ IF size > 0 + >r \ | ( bar size ) + dup \ | ( bar size size-align ) + r> assign-var-align \ | ( bar al-mem ) set variable to next mem swap rtas-config-l! \ | ( -- ) set the bar to al-mem ELSE \ ELSE 2drop drop \ | clear stack @@ -112,7 +132,9 @@ \ set bar to current free mem ( in variable ) and set variable to next free mem : assign-bar-value64 ( bar size var -- 8 ) over IF \ IF size > 0 - assign-var \ | ( bar al-mem ) set variable to next mem + >r \ | ( bar size ) + pci-mem-bar-min-align \ | ( bar size min-align ) + r> assign-var-min-align \ | ( bar al-mem ) set variable to next mem swap \ | ( al-mem addr ) calc config-addr of this bar 2dup rtas-config-l! \ | ( al-mem addr ) set the Lower part of the bar to al-mem 4 + swap 20 rshift \ | ( al-mem>>32 addr ) prepare the upper part of the al-mem @@ -163,7 +185,7 @@ : assign-io-bar ( bar-addr -- 4 ) dup pci-bar-size-io \ fetch size pci-next-io \ var to change - assign-bar-value32 \ and set it all + assign-io-bar-value32 \ and set it all ; \ Setup an Expansion ROM bar diff --git a/slof/fs/pci-scan.fs b/slof/fs/pci-scan.fs index a528c8e..bd8cc0d 100644 --- a/slof/fs/pci-scan.fs +++ b/slof/fs/pci-scan.fs @@ -24,6 +24,9 @@ VARIABLE pci-max-io VARIABLE pci-next-mem64 \ prefetchable 64-bit memory mapped VARIABLE pci-max-mem64 +\ 0 to default to natural alignment +0 VALUE pci-mem-bar-min-align + \ Counter of busses found 0 VALUE pci-bus-number \ Counter of devices found
This is needed to ensure VFIO passthrough devices are able to offload MMIO accesses to KVM. Cc: Segher Boessenkool <segher@kernel.crashing.org> Cc: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> Cc: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- v2: * default to 0 min-align instead of 64K (David) * refactor assign-var* for readability (Segher) * add/utilize umax instead of max (Segher) --- board-qemu/slof/pci-phb.fs | 4 ++++ slof/engine.in | 1 + slof/fs/pci-properties.fs | 38 ++++++++++++++++++++++++++++++-------- slof/fs/pci-scan.fs | 3 +++ 4 files changed, 38 insertions(+), 8 deletions(-)