diff mbox

pci: Fix secondary and subordinate PCI bus enumeration with board-qemu

Message ID 1474977142-14564-1-git-send-email-thuth@redhat.com
State Accepted
Headers show

Commit Message

Thomas Huth Sept. 27, 2016, 11:52 a.m. UTC
SLOF currently fails to correctly initialize the secondary and
subordinate bus number registers in the config space of PCI
bridges, so that for example with the following command line,
none of the PCI devices is usable:

 qemu-system-ppc64 -nodefaults -nographic -serial mon:stdio \
    -device pci-bridge,chassis_nr=1,id=bridge0,addr=0x3 \
    -device pci-bridge,chassis_nr=2,id=bridge1,addr=0x4 \
    -device virtio-balloon,bus=bridge1,addr=0x1 \
    -device virtio-net,bus=bridge0,addr=0x2 \
    -device virtio-rng,bus=bridge0,addr=0x5 \
    -device pci-bridge,chassis_nr=3,id=br2,addr=0x6,bus=bridge1 \
    -device e1000,bus=br2,addr=0x1

This is because SLOF tries to enumerate the PCI bus numbers
that are reachable via a bridge. In the function pci-bridge-probe,
it increases the pci-bus-number counter and writes that value into
the secondary bus number register of the PCI config space, and
after probing all attached bridges, it fills the number of the
last enumerated bus number into the subordinate bus number register.
This works fine if the whole bus enumeration is done by SLOF,
however on board-qemu, we nowadays rely on the pre-initialized PCI
device tree from QEMU - and the numbers that SLOF is trying to use
here do not match with the bus numbers that QEMU already assigned
to the bus segments (QEMU provides the device tree nodes in
descending order, but SLOF tries to enumerate the bus numbers in
ascending order here instead).

To fix this issue, we should simply stop setting up the secondary
and subordinate config space registers of the bridge in SLOF - since
this is done by QEMU already! Thus we replace the "pci-bridge-probe"
function with a board-qemu-specific function "phb-pci-bridge-probe",
that does not call "pci-bus!" and "pci-bus-subo!" anymore. (And since
pci-bridge-probe was the only spot that called phb-pci-probe-bus, we
can get rid of that wrapper now, too, and call phb-pci-walk-bridge
from phb-pci-bridge-probe directly instead).

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1377083
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 board-qemu/slof/pci-phb.fs | 15 +++++++++++----
 slof/fs/pci-properties.fs  |  2 +-
 slof/fs/pci-scan.fs        |  2 ++
 3 files changed, 14 insertions(+), 5 deletions(-)

Comments

Nikunj A Dadhania Sept. 28, 2016, 8:21 a.m. UTC | #1
Thomas Huth <thuth@redhat.com> writes:

> SLOF currently fails to correctly initialize the secondary and
> subordinate bus number registers in the config space of PCI
> bridges, so that for example with the following command line,
> none of the PCI devices is usable:
>
>  qemu-system-ppc64 -nodefaults -nographic -serial mon:stdio \
>     -device pci-bridge,chassis_nr=1,id=bridge0,addr=0x3 \
>     -device pci-bridge,chassis_nr=2,id=bridge1,addr=0x4 \
>     -device virtio-balloon,bus=bridge1,addr=0x1 \
>     -device virtio-net,bus=bridge0,addr=0x2 \
>     -device virtio-rng,bus=bridge0,addr=0x5 \
>     -device pci-bridge,chassis_nr=3,id=br2,addr=0x6,bus=bridge1 \
>     -device e1000,bus=br2,addr=0x1
>
> This is because SLOF tries to enumerate the PCI bus numbers
> that are reachable via a bridge. In the function pci-bridge-probe,
> it increases the pci-bus-number counter and writes that value into
> the secondary bus number register of the PCI config space, and
> after probing all attached bridges, it fills the number of the
> last enumerated bus number into the subordinate bus number register.
> This works fine if the whole bus enumeration is done by SLOF,
> however on board-qemu, we nowadays rely on the pre-initialized PCI
> device tree from QEMU - and the numbers that SLOF is trying to use
> here do not match with the bus numbers that QEMU already assigned
> to the bus segments (QEMU provides the device tree nodes in
> descending order, but SLOF tries to enumerate the bus numbers in
> ascending order here instead).
>
> To fix this issue, we should simply stop setting up the secondary
> and subordinate config space registers of the bridge in SLOF - since
> this is done by QEMU already! Thus we replace the "pci-bridge-probe"
> function with a board-qemu-specific function "phb-pci-bridge-probe",
> that does not call "pci-bus!" and "pci-bus-subo!" anymore. (And since
> pci-bridge-probe was the only spot that called phb-pci-probe-bus, we
> can get rid of that wrapper now, too, and call phb-pci-walk-bridge
> from phb-pci-bridge-probe directly instead).
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1377083
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

Regards
Nikunj
Alexey Kardashevskiy Oct. 10, 2016, 3:59 a.m. UTC | #2
On 27/09/16 21:52, Thomas Huth wrote:
> SLOF currently fails to correctly initialize the secondary and
> subordinate bus number registers in the config space of PCI
> bridges, so that for example with the following command line,
> none of the PCI devices is usable:
> 
>  qemu-system-ppc64 -nodefaults -nographic -serial mon:stdio \
>     -device pci-bridge,chassis_nr=1,id=bridge0,addr=0x3 \
>     -device pci-bridge,chassis_nr=2,id=bridge1,addr=0x4 \
>     -device virtio-balloon,bus=bridge1,addr=0x1 \
>     -device virtio-net,bus=bridge0,addr=0x2 \
>     -device virtio-rng,bus=bridge0,addr=0x5 \
>     -device pci-bridge,chassis_nr=3,id=br2,addr=0x6,bus=bridge1 \
>     -device e1000,bus=br2,addr=0x1
> 
> This is because SLOF tries to enumerate the PCI bus numbers
> that are reachable via a bridge. In the function pci-bridge-probe,
> it increases the pci-bus-number counter and writes that value into
> the secondary bus number register of the PCI config space, and
> after probing all attached bridges, it fills the number of the
> last enumerated bus number into the subordinate bus number register.
> This works fine if the whole bus enumeration is done by SLOF,
> however on board-qemu, we nowadays rely on the pre-initialized PCI
> device tree from QEMU - and the numbers that SLOF is trying to use
> here do not match with the bus numbers that QEMU already assigned
> to the bus segments (QEMU provides the device tree nodes in
> descending order, but SLOF tries to enumerate the bus numbers in
> ascending order here instead).
> 
> To fix this issue, we should simply stop setting up the secondary
> and subordinate config space registers of the bridge in SLOF - since
> this is done by QEMU already! Thus we replace the "pci-bridge-probe"
> function with a board-qemu-specific function "phb-pci-bridge-probe",
> that does not call "pci-bus!" and "pci-bus-subo!" anymore. (And since
> pci-bridge-probe was the only spot that called phb-pci-probe-bus, we
> can get rid of that wrapper now, too, and call phb-pci-walk-bridge
> from phb-pci-bridge-probe directly instead).
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1377083
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Thanks, applied.

> ---
>  board-qemu/slof/pci-phb.fs | 15 +++++++++++----
>  slof/fs/pci-properties.fs  |  2 +-
>  slof/fs/pci-scan.fs        |  2 ++
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs
> index f79c5b4..667514e 100644
> --- a/board-qemu/slof/pci-phb.fs
> +++ b/board-qemu/slof/pci-phb.fs
> @@ -304,9 +304,16 @@ setup-puid
>      get-parent set-node
>  ;
>  
> -\ Landing routing to probe the popuated device tree
> -: phb-pci-probe-bus ( busnr -- )
> -    drop phb-pci-walk-bridge
> +\ Similar to pci-bridge-probe, but without setting the secondary and
> +\ subordinate bus numbers (since this has been done by QEMU already)
> +: phb-pci-bridge-probe ( addr -- )
> +    dup pci-bridge-set-bases                      \ Set up all Base Registers
> +    dup func-pci-bridge-range-props               \ Set up temporary "range"
> +    pci-device-vec-len 1+ TO pci-device-vec-len   \ increase the device-slot vector depth
> +    pci-enable                                    \ enable mem/IO transactions
> +    phb-pci-walk-bridge                           \ and walk the secondary bus
> +    pci-device-vec-len 1- TO pci-device-vec-len   \ decrease the device-slot vector depth
> +    pci-bridge-set-limits                         \ Set up all Limit Registers
>  ;
>  
>  \ Stub routine, as qemu has enumerated, we already have the device
> @@ -328,7 +335,7 @@ setup-puid
>         1 0 (probe-pci-host-bridge)
>     ELSE
>         2drop
> -       ['] phb-pci-probe-bus TO func-pci-probe-bus
> +       ['] phb-pci-bridge-probe TO func-pci-bridge-probe
>         ['] phb-pci-device-props TO func-pci-device-props
>         phb-pci-walk-bridge          \ PHB device tree is already populated.
>     THEN
> diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
> index 4f13402..7faa714 100644
> --- a/slof/fs/pci-properties.fs
> +++ b/slof/fs/pci-properties.fs
> @@ -643,7 +643,7 @@
>          pci-device-slots >r             \ save the slot array on return stack
>          dup pci-common-props            \ set the common properties before scanning the bus
>          s" pci" device-type             \ the type is allways "pci"
> -        dup pci-bridge-probe            \ find all device connected to it
> +        dup func-pci-bridge-probe       \ find all device connected to it
>          dup assign-all-bridge-bars      \ set up all memory access BARs
>          dup pci-set-irq-line            \ set the interrupt pin
>          dup pci-set-capabilities        \ set up the capabilities
> diff --git a/slof/fs/pci-scan.fs b/slof/fs/pci-scan.fs
> index 2fdf0e8..a528c8e 100644
> --- a/slof/fs/pci-scan.fs
> +++ b/slof/fs/pci-scan.fs
> @@ -221,6 +221,8 @@ DEFER func-pci-bridge-range-props
>          dup pci-bridge-set-limits                       \ SetUp all Limit Registers
>          drop                                            \ forget the config-addr
>  ;
> +DEFER func-pci-bridge-probe
> +' pci-bridge-probe TO func-pci-bridge-probe
>  
>  \ set up the pci-device
>  : pci-device-setup ( addr -- )
>
diff mbox

Patch

diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs
index f79c5b4..667514e 100644
--- a/board-qemu/slof/pci-phb.fs
+++ b/board-qemu/slof/pci-phb.fs
@@ -304,9 +304,16 @@  setup-puid
     get-parent set-node
 ;
 
-\ Landing routing to probe the popuated device tree
-: phb-pci-probe-bus ( busnr -- )
-    drop phb-pci-walk-bridge
+\ Similar to pci-bridge-probe, but without setting the secondary and
+\ subordinate bus numbers (since this has been done by QEMU already)
+: phb-pci-bridge-probe ( addr -- )
+    dup pci-bridge-set-bases                      \ Set up all Base Registers
+    dup func-pci-bridge-range-props               \ Set up temporary "range"
+    pci-device-vec-len 1+ TO pci-device-vec-len   \ increase the device-slot vector depth
+    pci-enable                                    \ enable mem/IO transactions
+    phb-pci-walk-bridge                           \ and walk the secondary bus
+    pci-device-vec-len 1- TO pci-device-vec-len   \ decrease the device-slot vector depth
+    pci-bridge-set-limits                         \ Set up all Limit Registers
 ;
 
 \ Stub routine, as qemu has enumerated, we already have the device
@@ -328,7 +335,7 @@  setup-puid
        1 0 (probe-pci-host-bridge)
    ELSE
        2drop
-       ['] phb-pci-probe-bus TO func-pci-probe-bus
+       ['] phb-pci-bridge-probe TO func-pci-bridge-probe
        ['] phb-pci-device-props TO func-pci-device-props
        phb-pci-walk-bridge          \ PHB device tree is already populated.
    THEN
diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
index 4f13402..7faa714 100644
--- a/slof/fs/pci-properties.fs
+++ b/slof/fs/pci-properties.fs
@@ -643,7 +643,7 @@ 
         pci-device-slots >r             \ save the slot array on return stack
         dup pci-common-props            \ set the common properties before scanning the bus
         s" pci" device-type             \ the type is allways "pci"
-        dup pci-bridge-probe            \ find all device connected to it
+        dup func-pci-bridge-probe       \ find all device connected to it
         dup assign-all-bridge-bars      \ set up all memory access BARs
         dup pci-set-irq-line            \ set the interrupt pin
         dup pci-set-capabilities        \ set up the capabilities
diff --git a/slof/fs/pci-scan.fs b/slof/fs/pci-scan.fs
index 2fdf0e8..a528c8e 100644
--- a/slof/fs/pci-scan.fs
+++ b/slof/fs/pci-scan.fs
@@ -221,6 +221,8 @@  DEFER func-pci-bridge-range-props
         dup pci-bridge-set-limits                       \ SetUp all Limit Registers
         drop                                            \ forget the config-addr
 ;
+DEFER func-pci-bridge-probe
+' pci-bridge-probe TO func-pci-bridge-probe
 
 \ set up the pci-device
 : pci-device-setup ( addr -- )