[PATCH v2 0/4] PCI: Generic fixes and moving enumeration back to QEMU

[PATCH v2 0/4] PCI: Generic fixes and moving enumeration back to QEMU

[Nikunj A Dadhania]

patch 1:   Fixes a bug in pci-to-pci bridge while programming the limit 
           registers during probe
patch 2,3: Support 64-bit address translation and usb devices can have
      	   64-bit pci memory BARs
patch 4:   Use QEMU create device tree nodes and program the BARs


Nikunj A Dadhania (4):
  pci: program correct bridge limit registers during probe
  pci: Support 64-bit address translation
  usb: support 64-bit pci bars
  pci: Use QEMU created PCI device nodes

 board-qemu/slof/pci-phb.fs      | 44 ++++++++++++++++++++++++++++++++++++++++-
 slof/fs/devices/pci-class_0c.fs | 10 ++++++++--
 slof/fs/pci-properties.fs       |  6 +++++-
 slof/fs/pci-scan.fs             |  6 +++---
 slof/fs/translate.fs            |  6 ++----
 5 files changed, 61 insertions(+), 11 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/4] pci: program correct bridge limit registers during probe

[Nikunj A Dadhania]

According to the pci-to-pci bridge specification, memory/io limit
should be the last address of the window, while currently its set to
last + 1. It broke when the memory range was increased and hit 32-bit
limit. The last address in the window is 0xFFFF.FFFF and max-mmio is
0x1.0000.0000, because of the bug 0x0000 got programmed in the memory
limit register. All the mmio reads across the pci-to-pci bridge
started failing during probe.

All pci-bridge-set-[mmio,mem,io]-base sufferred from similar problem.

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 slof/fs/pci-scan.fs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/slof/fs/pci-scan.fs b/slof/fs/pci-scan.fs
index 15d0c8e..b8b9fe6 100644
--- a/slof/fs/pci-scan.fs
+++ b/slof/fs/pci-scan.fs
@@ -87,7 +87,7 @@ here 100 allot CONSTANT pci-device-vec
         pci-next-mmio @ 100000 #aligned         \ read the current Value and align to 1MB boundary
         dup 100000 + pci-next-mmio !            \ and write back with 1MB for bridge
         10 rshift                               \ mmio-base reg is only the upper 16 bits
-        pci-max-mmio @ FFFF0000 and or          \ and Insert mmio Limit (set it to max)
+        pci-max-mmio @ 1- FFFF0000 and or       \ and Insert mmio Limit (set it to max)
         swap 20 + rtas-config-l!                \ and write it into the bridge
 ;
 
@@ -116,7 +116,7 @@ here 100 allot CONSTANT pci-device-vec
                 2 pick 2C + rtas-config-l!      \ | and set the Limit
         THEN                                    \ FI
         10 rshift                               \ keep upper 16 bits
-        pci-max-mem @ FFFF0000 and or           \ and Insert mmem Limit (set it to max)
+        pci-max-mem @ 1- FFFF0000 and or        \ and Insert mmem Limit (set it to max)
         swap 24 + rtas-config-l!                \ and write it into the bridge
 ;
 
@@ -150,7 +150,7 @@ here 100 allot CONSTANT pci-device-vec
                 swap 30 + rtas-config-l!        \ | and write it into the Base-Upper16-bits
         THEN                                    \ FI
         8 rshift 000000FF and                   \ keep upper 8 bits
-        pci-max-io @ 0000FF00 and or            \ insert upper 8 bits of Max-Limit
+        pci-max-io @ 1- 0000FF00 and or         \ insert upper 8 bits of Max-Limit
         over rtas-config-l@ FFFF0000 and        \ fetch original Value
         or swap 1C + rtas-config-l!             \ and write it into the bridge
 ;
-- 
1.8.3.1

Re: [PATCH v2 1/4] pci: program correct bridge limit registers during probe

[Thomas Huth]

On Mon, 27 Apr 2015 13:32:30 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> According to the pci-to-pci bridge specification, memory/io limit
> should be the last address of the window, while currently its set to
> last + 1. It broke when the memory range was increased and hit 32-bit
> limit. The last address in the window is 0xFFFF.FFFF and max-mmio is
> 0x1.0000.0000, because of the bug 0x0000 got programmed in the memory
> limit register. All the mmio reads across the pci-to-pci bridge
> started failing during probe.
> 
> All pci-bridge-set-[mmio,mem,io]-base sufferred from similar problem.
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  slof/fs/pci-scan.fs | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/slof/fs/pci-scan.fs b/slof/fs/pci-scan.fs
> index 15d0c8e..b8b9fe6 100644
> --- a/slof/fs/pci-scan.fs
> +++ b/slof/fs/pci-scan.fs
> @@ -87,7 +87,7 @@ here 100 allot CONSTANT pci-device-vec
>          pci-next-mmio @ 100000 #aligned         \ read the current Value and align to 1MB boundary
>          dup 100000 + pci-next-mmio !            \ and write back with 1MB for bridge
>          10 rshift                               \ mmio-base reg is only the upper 16 bits
> -        pci-max-mmio @ FFFF0000 and or          \ and Insert mmio Limit (set it to max)
> +        pci-max-mmio @ 1- FFFF0000 and or       \ and Insert mmio Limit (set it to max)
>          swap 20 + rtas-config-l!                \ and write it into the bridge
>  ;
>  
> @@ -116,7 +116,7 @@ here 100 allot CONSTANT pci-device-vec
>                  2 pick 2C + rtas-config-l!      \ | and set the Limit
>          THEN                                    \ FI
>          10 rshift                               \ keep upper 16 bits
> -        pci-max-mem @ FFFF0000 and or           \ and Insert mmem Limit (set it to max)
> +        pci-max-mem @ 1- FFFF0000 and or        \ and Insert mmem Limit (set it to max)
>          swap 24 + rtas-config-l!                \ and write it into the bridge
>  ;
>  
> @@ -150,7 +150,7 @@ here 100 allot CONSTANT pci-device-vec
>                  swap 30 + rtas-config-l!        \ | and write it into the Base-Upper16-bits
>          THEN                                    \ FI
>          8 rshift 000000FF and                   \ keep upper 8 bits
> -        pci-max-io @ 0000FF00 and or            \ insert upper 8 bits of Max-Limit
> +        pci-max-io @ 1- 0000FF00 and or         \ insert upper 8 bits of Max-Limit
>          over rtas-config-l@ FFFF0000 and        \ fetch original Value
>          or swap 1C + rtas-config-l!             \ and write it into the bridge
>  ;

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH v2 2/4] pci: Support 64-bit address translation

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 slof/fs/translate.fs | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/slof/fs/translate.fs b/slof/fs/translate.fs
index e2633e5..9654f24 100644
--- a/slof/fs/translate.fs
+++ b/slof/fs/translate.fs
@@ -33,11 +33,9 @@
       dup l@ FF AND 0<> ( prop_type address data cfgspace_offset? )
       3 pick 0= ( prop_type address data cfgspace_offset? reg_prop? )
       AND NOT IF 
-         2dup 8 + ( prop_type address data address data' )
-         2dup l@ 2 pick 8 + l@ + <= -rot l@  >= and  IF
+         2dup 4 + ( prop_type address data address data' )
+         2dup @ 2 pick 8 + @ + <= -rot @  >= and  IF
             l@ 03000000 and 18 rshift nip
-            \ no 64bit translations supported pretend it is 32bit
-            dup 3 = IF  1-  THEN
             ( prop_type type )
             swap drop ( type )
             UNLOOP EXIT
-- 
1.8.3.1

Re: [PATCH v2 2/4] pci: Support 64-bit address translation

[Thomas Huth]

On Mon, 27 Apr 2015 13:32:31 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  slof/fs/translate.fs | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/slof/fs/translate.fs b/slof/fs/translate.fs
> index e2633e5..9654f24 100644
> --- a/slof/fs/translate.fs
> +++ b/slof/fs/translate.fs
> @@ -33,11 +33,9 @@
>        dup l@ FF AND 0<> ( prop_type address data cfgspace_offset? )
>        3 pick 0= ( prop_type address data cfgspace_offset? reg_prop? )
>        AND NOT IF 
> -         2dup 8 + ( prop_type address data address data' )
> -         2dup l@ 2 pick 8 + l@ + <= -rot l@  >= and  IF
> +         2dup 4 + ( prop_type address data address data' )
> +         2dup @ 2 pick 8 + @ + <= -rot @  >= and  IF
>              l@ 03000000 and 18 rshift nip
> -            \ no 64bit translations supported pretend it is 32bit
> -            dup 3 = IF  1-  THEN
>              ( prop_type type )
>              swap drop ( type )
>              UNLOOP EXIT

Phew, translate-address stuff ... one of my "favorites" ;-)
I wonder whether this will break "legacy" code like board-js2x ...
there might have been a reason for this "pretend it is 32bit" hack
(I've put Adrian Reber on CC:, maybe he still remembers why this was
needed).

Anyway, I think we can fix that up as soon as somebody tries to run the
js2x code again (I still have a YDL PowerStation at home somewhere,
maybe I will have enough spare time to reactivate it one day).

And since your changes itself looks fine to me:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 2/4] pci: Support 64-bit address translation

[Benjamin Herrenschmidt]

On Mon, 2015-04-27 at 11:32 +0200, Thomas Huth wrote:
> Anyway, I think we can fix that up as soon as somebody tries to run
> the
> js2x code again (I still have a YDL PowerStation at home somewhere,
> maybe I will have enough spare time to reactivate it one day).
> 
> And since your changes itself looks fine to me:

Another thing we could do with the new IPMI stack floating around would
be to do a js2x model in qemu :-) Shouldn't be too hard .... I've
promised Alex Graf I would do a proper U4 model one day ... :-)

Cheers,
Ben.

Re: [PATCH v2 2/4] pci: Support 64-bit address translation

[Thomas Huth]

On Mon, 27 Apr 2015 19:57:26 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Mon, 2015-04-27 at 11:32 +0200, Thomas Huth wrote:
> > Anyway, I think we can fix that up as soon as somebody tries to run
> > the
> > js2x code again (I still have a YDL PowerStation at home somewhere,
> > maybe I will have enough spare time to reactivate it one day).
> > 
> > And since your changes itself looks fine to me:
> 
> Another thing we could do with the new IPMI stack floating around would
> be to do a js2x model in qemu :-) Shouldn't be too hard .... I've
> promised Alex Graf I would do a proper U4 model one day ... :-)

That would be very cool, indeed!

 Thomas

[PATCH v2 3/4] usb: support 64-bit pci bars

[Nikunj A Dadhania]

With the addition of 64-bit bar ranges (commit: 9fbfed08 - pci: use
64bit bar ranges), the usb-setup-hcidev needs to handle 64-bit
addresses.

For example, QEMU provided NEC USB XHCI device requests 64-bit memory
address. Added code to check the memory type before address
translation.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 slof/fs/devices/pci-class_0c.fs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/slof/fs/devices/pci-class_0c.fs b/slof/fs/devices/pci-class_0c.fs
index 9c47325..39453fb 100644
--- a/slof/fs/devices/pci-class_0c.fs
+++ b/slof/fs/devices/pci-class_0c.fs
@@ -25,8 +25,14 @@ CONSTANT /hci-dev
 
 : usb-setup-hcidev ( num hci-dev -- )
     >r
-    10 config-l@ translate-my-address
-    3 not AND
+    10 config-l@ F AND case
+	0 OF 10 config-l@ translate-my-address ENDOF       \ 32-bit memory space
+	4 OF                                               \ 64-bit memory space
+	    14 config-l@ 20 lshift                         \ Read two bars
+	    10 config-l@ OR translate-my-address
+	ENDOF
+    ENDCASE
+    F not AND
     ( io-base ) r@ hcd>base !
     08 config-l@ 8 rshift  0000000F0 AND 4 rshift
     ( usb-type ) r@ hcd>type !
-- 
1.8.3.1

Re: [PATCH v2 3/4] usb: support 64-bit pci bars

[Thomas Huth]

On Mon, 27 Apr 2015 13:32:32 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> With the addition of 64-bit bar ranges (commit: 9fbfed08 - pci: use
> 64bit bar ranges), the usb-setup-hcidev needs to handle 64-bit
> addresses.
> 
> For example, QEMU provided NEC USB XHCI device requests 64-bit memory
> address. Added code to check the memory type before address
> translation.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  slof/fs/devices/pci-class_0c.fs | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/slof/fs/devices/pci-class_0c.fs b/slof/fs/devices/pci-class_0c.fs
> index 9c47325..39453fb 100644
> --- a/slof/fs/devices/pci-class_0c.fs
> +++ b/slof/fs/devices/pci-class_0c.fs
> @@ -25,8 +25,14 @@ CONSTANT /hci-dev
>  
>  : usb-setup-hcidev ( num hci-dev -- )
>      >r
> -    10 config-l@ translate-my-address
> -    3 not AND
> +    10 config-l@ F AND case
> +	0 OF 10 config-l@ translate-my-address ENDOF       \ 32-bit memory space
> +	4 OF                                               \ 64-bit memory space
> +	    14 config-l@ 20 lshift                         \ Read two bars
> +	    10 config-l@ OR translate-my-address
> +	ENDOF
> +    ENDCASE
> +    F not AND
>      ( io-base ) r@ hcd>base !
>      08 config-l@ 8 rshift  0000000F0 AND 4 rshift
>      ( usb-type ) r@ hcd>type !

Looks good to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH v2 4/4] pci: Use QEMU created PCI device nodes

[Nikunj A Dadhania]

PCI Enumeration has been part of SLOF. Now with hotplug code addition
in QEMU, it makes more sense to have this code in one place,
i.e. QEMU.

Adding routines to walk through the device nodes created by QEMU. SLOF
will now configure the device/bridges and program the BARs for
communicating with the devices.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 board-qemu/slof/pci-phb.fs | 44 +++++++++++++++++++++++++++++++++++++++++++-
 slof/fs/pci-properties.fs  |  6 +++++-
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs
index 529772f..a8fb7ca 100644
--- a/board-qemu/slof/pci-phb.fs
+++ b/board-qemu/slof/pci-phb.fs
@@ -282,6 +282,41 @@ setup-puid
    THEN
 ;
 
+: phb-pci-walk-bridge ( -- )
+    phb-debug? IF ."   Calling pci-walk-bridge " pwd cr THEN
+
+    get-node child ?dup 0= IF EXIT THEN    \ get and check if we have children
+    0 to pci-device-slots                  \ reset slot array to unpoppulated
+    BEGIN
+        dup                                \ Continue as long as there are children
+    WHILE
+        dup set-node                       \ Set child node as current node
+        my-space pci-set-slot              \ set the slot bit
+        my-space pci-htype@                \ read HEADER-Type
+        7f and                             \ Mask bit 7 - multifunction device
+        CASE
+            0 OF my-space pci-device-setup ENDOF  \ | set up the device
+            1 OF my-space pci-bridge-setup ENDOF  \ | set up the bridge
+            dup OF my-space pci-htype@ pci-out ENDOF
+        ENDCASE
+        peer
+    REPEAT drop
+    get-parent set-node
+;
+
+\ Landing routing to probe the popuated device tree
+: phb-pci-probe-bus ( busnr -- )
+    drop phb-pci-walk-bridge
+;
+
+\ Stub routine, as qemu has enumerated, we already have the device
+\ properties set.
+: phb-pci-device-props ( addr -- )
+    dup pci-class-name device-name
+    dup pci-device-assigned-addresses-prop
+    drop
+;
+
 \ Scan the child nodes of the pci root node to assign bars, fixup
 \ properties etc.
 : phb-setup-children
@@ -289,7 +324,14 @@ setup-puid
    my-puid TO puid                  \ Set current puid
    phb-parse-ranges
    1 TO pci-hotplug-enabled
-   1 0 (probe-pci-host-bridge)
+   s" qemu,phb-enumerated" get-node get-property 0<> IF
+       1 0 (probe-pci-host-bridge)
+   ELSE
+       2drop
+       ['] phb-pci-probe-bus TO func-pci-probe-bus
+       ['] phb-pci-device-props TO func-pci-device-props
+       phb-pci-walk-bridge          \ PHB device tree is already populated.
+   THEN
    r> TO puid                       \ Restore previous puid
 ;
 phb-setup-children
diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
index 9efa87e..4f13402 100644
--- a/slof/fs/pci-properties.fs
+++ b/slof/fs/pci-properties.fs
@@ -651,6 +651,8 @@
         r> TO pci-device-slots          \ and reset the slot array
 ;
 
+DEFER func-pci-device-props
+
 \ used for an gerneric device set up
 \ if a device has no special handling for setup
 \ the device file (pci-device_VENDOR_DEVICE.fs) can call
@@ -659,6 +661,8 @@
         dup assign-all-device-bars      \ calc all BARs
         dup pci-set-irq-line            \ set the interrupt pin
         dup pci-set-capabilities        \ set up the capabilities
-        dup pci-device-props            \ and generate all properties
+        dup func-pci-device-props       \ and generate all properties
         drop                            \ forget the config-addr
 ;
+
+' pci-device-props TO func-pci-device-props
-- 
1.8.3.1

Re: [PATCH v2 4/4] pci: Use QEMU created PCI device nodes

[Thomas Huth]

On Mon, 27 Apr 2015 13:32:33 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> PCI Enumeration has been part of SLOF. Now with hotplug code addition
> in QEMU, it makes more sense to have this code in one place,
> i.e. QEMU.
> 
> Adding routines to walk through the device nodes created by QEMU. SLOF
> will now configure the device/bridges and program the BARs for
> communicating with the devices.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  board-qemu/slof/pci-phb.fs | 44 +++++++++++++++++++++++++++++++++++++++++++-
>  slof/fs/pci-properties.fs  |  6 +++++-
>  2 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs
> index 529772f..a8fb7ca 100644
> --- a/board-qemu/slof/pci-phb.fs
> +++ b/board-qemu/slof/pci-phb.fs
> @@ -282,6 +282,41 @@ setup-puid
>     THEN
>  ;
>  
> +: phb-pci-walk-bridge ( -- )
> +    phb-debug? IF ."   Calling pci-walk-bridge " pwd cr THEN
> +
> +    get-node child ?dup 0= IF EXIT THEN    \ get and check if we have children
> +    0 to pci-device-slots                  \ reset slot array to unpoppulated
> +    BEGIN
> +        dup                                \ Continue as long as there are children
> +    WHILE
> +        dup set-node                       \ Set child node as current node
> +        my-space pci-set-slot              \ set the slot bit
> +        my-space pci-htype@                \ read HEADER-Type
> +        7f and                             \ Mask bit 7 - multifunction device
> +        CASE
> +            0 OF my-space pci-device-setup ENDOF  \ | set up the device
> +            1 OF my-space pci-bridge-setup ENDOF  \ | set up the bridge
> +            dup OF my-space pci-htype@ pci-out ENDOF
> +        ENDCASE
> +        peer
> +    REPEAT drop
> +    get-parent set-node
> +;
> +
> +\ Landing routing to probe the popuated device tree
> +: phb-pci-probe-bus ( busnr -- )
> +    drop phb-pci-walk-bridge
> +;
> +
> +\ Stub routine, as qemu has enumerated, we already have the device
> +\ properties set.
> +: phb-pci-device-props ( addr -- )
> +    dup pci-class-name device-name
> +    dup pci-device-assigned-addresses-prop
> +    drop
> +;
> +
>  \ Scan the child nodes of the pci root node to assign bars, fixup
>  \ properties etc.
>  : phb-setup-children
> @@ -289,7 +324,14 @@ setup-puid
>     my-puid TO puid                  \ Set current puid
>     phb-parse-ranges
>     1 TO pci-hotplug-enabled
> -   1 0 (probe-pci-host-bridge)
> +   s" qemu,phb-enumerated" get-node get-property 0<> IF

I think you could simply omit the "0<>" in the above line (get-property
returns TRUE if the property has not been found and FALSE if it has
been found, so the topmost stack item is already suitable for the IF
statement)

> +       1 0 (probe-pci-host-bridge)
> +   ELSE
> +       2drop
> +       ['] phb-pci-probe-bus TO func-pci-probe-bus
> +       ['] phb-pci-device-props TO func-pci-device-props
> +       phb-pci-walk-bridge          \ PHB device tree is already populated.
> +   THEN
>     r> TO puid                       \ Restore previous puid
>  ;
>  phb-setup-children
> diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
> index 9efa87e..4f13402 100644
> --- a/slof/fs/pci-properties.fs
> +++ b/slof/fs/pci-properties.fs
> @@ -651,6 +651,8 @@
>          r> TO pci-device-slots          \ and reset the slot array
>  ;
>  
> +DEFER func-pci-device-props
> +
>  \ used for an gerneric device set up
>  \ if a device has no special handling for setup
>  \ the device file (pci-device_VENDOR_DEVICE.fs) can call
> @@ -659,6 +661,8 @@
>          dup assign-all-device-bars      \ calc all BARs
>          dup pci-set-irq-line            \ set the interrupt pin
>          dup pci-set-capabilities        \ set up the capabilities
> -        dup pci-device-props            \ and generate all properties
> +        dup func-pci-device-props       \ and generate all properties
>          drop                            \ forget the config-addr
>  ;
> +
> +' pci-device-props TO func-pci-device-props

Apart from the minor nit, looks fine to me now.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 4/4] pci: Use QEMU created PCI device nodes

[Nikunj A Dadhania]

Thomas Huth <thuth@redhat.com> writes:

> On Mon, 27 Apr 2015 13:32:33 +0530
> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
>
>> PCI Enumeration has been part of SLOF. Now with hotplug code addition
>> in QEMU, it makes more sense to have this code in one place,
>> i.e. QEMU.
>> 
>> Adding routines to walk through the device nodes created by QEMU. SLOF
>> will now configure the device/bridges and program the BARs for
>> communicating with the devices.
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  board-qemu/slof/pci-phb.fs | 44 +++++++++++++++++++++++++++++++++++++++++++-
>>  slof/fs/pci-properties.fs  |  6 +++++-
>>  2 files changed, 48 insertions(+), 2 deletions(-)
>> 
>> diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs
>> index 529772f..a8fb7ca 100644
>> --- a/board-qemu/slof/pci-phb.fs
>> +++ b/board-qemu/slof/pci-phb.fs
>> @@ -282,6 +282,41 @@ setup-puid
>>     THEN
>>  ;
>>  
>> +: phb-pci-walk-bridge ( -- )
>> +    phb-debug? IF ."   Calling pci-walk-bridge " pwd cr THEN
>> +
>> +    get-node child ?dup 0= IF EXIT THEN    \ get and check if we have children
>> +    0 to pci-device-slots                  \ reset slot array to unpoppulated
>> +    BEGIN
>> +        dup                                \ Continue as long as there are children
>> +    WHILE
>> +        dup set-node                       \ Set child node as current node
>> +        my-space pci-set-slot              \ set the slot bit
>> +        my-space pci-htype@                \ read HEADER-Type
>> +        7f and                             \ Mask bit 7 - multifunction device
>> +        CASE
>> +            0 OF my-space pci-device-setup ENDOF  \ | set up the device
>> +            1 OF my-space pci-bridge-setup ENDOF  \ | set up the bridge
>> +            dup OF my-space pci-htype@ pci-out ENDOF
>> +        ENDCASE
>> +        peer
>> +    REPEAT drop
>> +    get-parent set-node
>> +;
>> +
>> +\ Landing routing to probe the popuated device tree
>> +: phb-pci-probe-bus ( busnr -- )
>> +    drop phb-pci-walk-bridge
>> +;
>> +
>> +\ Stub routine, as qemu has enumerated, we already have the device
>> +\ properties set.
>> +: phb-pci-device-props ( addr -- )
>> +    dup pci-class-name device-name
>> +    dup pci-device-assigned-addresses-prop
>> +    drop
>> +;
>> +
>>  \ Scan the child nodes of the pci root node to assign bars, fixup
>>  \ properties etc.
>>  : phb-setup-children
>> @@ -289,7 +324,14 @@ setup-puid
>>     my-puid TO puid                  \ Set current puid
>>     phb-parse-ranges
>>     1 TO pci-hotplug-enabled
>> -   1 0 (probe-pci-host-bridge)
>> +   s" qemu,phb-enumerated" get-node get-property 0<> IF
>
> I think you could simply omit the "0<>" in the above line (get-property
> returns TRUE if the property has not been found and FALSE if it has
> been found, so the topmost stack item is already suitable for the IF
> statement)

I find the usage of get-property a bit odd, as it returns TRUE when it
is not found. So have put " != 0"  as the condition, which I felt as more
intuitive.

If you don't feel strongly about it, I will like to have it this way.

>> +       1 0 (probe-pci-host-bridge)
>> +   ELSE
>> +       2drop
>> +       ['] phb-pci-probe-bus TO func-pci-probe-bus
>> +       ['] phb-pci-device-props TO func-pci-device-props
>> +       phb-pci-walk-bridge          \ PHB device tree is already populated.
>> +   THEN
>>     r> TO puid                       \ Restore previous puid
>>  ;
>>  phb-setup-children
>> diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
>> index 9efa87e..4f13402 100644
>> --- a/slof/fs/pci-properties.fs
>> +++ b/slof/fs/pci-properties.fs
>> @@ -651,6 +651,8 @@
>>          r> TO pci-device-slots          \ and reset the slot array
>>  ;
>>  
>> +DEFER func-pci-device-props
>> +
>>  \ used for an gerneric device set up
>>  \ if a device has no special handling for setup
>>  \ the device file (pci-device_VENDOR_DEVICE.fs) can call
>> @@ -659,6 +661,8 @@
>>          dup assign-all-device-bars      \ calc all BARs
>>          dup pci-set-irq-line            \ set the interrupt pin
>>          dup pci-set-capabilities        \ set up the capabilities
>> -        dup pci-device-props            \ and generate all properties
>> +        dup func-pci-device-props       \ and generate all properties
>>          drop                            \ forget the config-addr
>>  ;
>> +
>> +' pci-device-props TO func-pci-device-props
>
> Apart from the minor nit, looks fine to me now.
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks,
Nikunj

Re: [PATCH v2 4/4] pci: Use QEMU created PCI device nodes

[Thomas Huth]

On Mon, 27 Apr 2015 15:26:00 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> Thomas Huth <thuth@redhat.com> writes:
> 
> > On Mon, 27 Apr 2015 13:32:33 +0530
> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
> >
> >> PCI Enumeration has been part of SLOF. Now with hotplug code addition
> >> in QEMU, it makes more sense to have this code in one place,
> >> i.e. QEMU.
> >> 
> >> Adding routines to walk through the device nodes created by QEMU. SLOF
> >> will now configure the device/bridges and program the BARs for
> >> communicating with the devices.
> >> 
> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >> ---
> >>  board-qemu/slof/pci-phb.fs | 44 +++++++++++++++++++++++++++++++++++++++++++-
> >>  slof/fs/pci-properties.fs  |  6 +++++-
> >>  2 files changed, 48 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/board-qemu/slof/pci-phb.fs b/board-qemu/slof/pci-phb.fs
> >> index 529772f..a8fb7ca 100644
> >> --- a/board-qemu/slof/pci-phb.fs
> >> +++ b/board-qemu/slof/pci-phb.fs
> >> @@ -282,6 +282,41 @@ setup-puid
> >>     THEN
> >>  ;
> >>  
> >> +: phb-pci-walk-bridge ( -- )
> >> +    phb-debug? IF ."   Calling pci-walk-bridge " pwd cr THEN
> >> +
> >> +    get-node child ?dup 0= IF EXIT THEN    \ get and check if we have children
> >> +    0 to pci-device-slots                  \ reset slot array to unpoppulated
> >> +    BEGIN
> >> +        dup                                \ Continue as long as there are children
> >> +    WHILE
> >> +        dup set-node                       \ Set child node as current node
> >> +        my-space pci-set-slot              \ set the slot bit
> >> +        my-space pci-htype@                \ read HEADER-Type
> >> +        7f and                             \ Mask bit 7 - multifunction device
> >> +        CASE
> >> +            0 OF my-space pci-device-setup ENDOF  \ | set up the device
> >> +            1 OF my-space pci-bridge-setup ENDOF  \ | set up the bridge
> >> +            dup OF my-space pci-htype@ pci-out ENDOF
> >> +        ENDCASE
> >> +        peer
> >> +    REPEAT drop
> >> +    get-parent set-node
> >> +;
> >> +
> >> +\ Landing routing to probe the popuated device tree
> >> +: phb-pci-probe-bus ( busnr -- )
> >> +    drop phb-pci-walk-bridge
> >> +;
> >> +
> >> +\ Stub routine, as qemu has enumerated, we already have the device
> >> +\ properties set.
> >> +: phb-pci-device-props ( addr -- )
> >> +    dup pci-class-name device-name
> >> +    dup pci-device-assigned-addresses-prop
> >> +    drop
> >> +;
> >> +
> >>  \ Scan the child nodes of the pci root node to assign bars, fixup
> >>  \ properties etc.
> >>  : phb-setup-children
> >> @@ -289,7 +324,14 @@ setup-puid
> >>     my-puid TO puid                  \ Set current puid
> >>     phb-parse-ranges
> >>     1 TO pci-hotplug-enabled
> >> -   1 0 (probe-pci-host-bridge)
> >> +   s" qemu,phb-enumerated" get-node get-property 0<> IF
> >
> > I think you could simply omit the "0<>" in the above line (get-property
> > returns TRUE if the property has not been found and FALSE if it has
> > been found, so the topmost stack item is already suitable for the IF
> > statement)
> 
> I find the usage of get-property a bit odd, as it returns TRUE when it
> is not found. So have put " != 0"  as the condition, which I felt as more
> intuitive.
> 
> If you don't feel strongly about it, I will like to have it this way.

That's fine for me, too.

 Thomas

Re: [PATCH v2 0/4] PCI: Generic fixes and moving enumeration back to QEMU

[Alexey Kardashevskiy]

On 04/27/2015 06:02 PM, Nikunj A Dadhania wrote:
> patch 1:   Fixes a bug in pci-to-pci bridge while programming the limit
>             registers during probe
> patch 2,3: Support 64-bit address translation and usb devices can have
>        	   64-bit pci memory BARs
> patch 4:   Use QEMU create device tree nodes and program the BARs



I taught my "git send-email" wrapper script to add "kernel/qemu/slof" in 
the subject, otherwise I believe it is hard for busy people to tell quickly 
what patchset is for what, I'd recommend you doing the same, especially 
when you mention "QEMU" in a subject  :)



 >
>
>
> Nikunj A Dadhania (4):
>    pci: program correct bridge limit registers during probe
>    pci: Support 64-bit address translation
>    usb: support 64-bit pci bars
>    pci: Use QEMU created PCI device nodes
>
>   board-qemu/slof/pci-phb.fs      | 44 ++++++++++++++++++++++++++++++++++++++++-
>   slof/fs/devices/pci-class_0c.fs | 10 ++++++++--
>   slof/fs/pci-properties.fs       |  6 +++++-
>   slof/fs/pci-scan.fs             |  6 +++---
>   slof/fs/translate.fs            |  6 ++----
>   5 files changed, 61 insertions(+), 11 deletions(-)
>


-- 
Alexey

Re: [PATCH v2 0/4] PCI: Generic fixes and moving enumeration back to QEMU

[Nikunj A Dadhania]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 04/27/2015 06:02 PM, Nikunj A Dadhania wrote:
>> patch 1:   Fixes a bug in pci-to-pci bridge while programming the limit
>>             registers during probe
>> patch 2,3: Support 64-bit address translation and usb devices can have
>>        	   64-bit pci memory BARs
>> patch 4:   Use QEMU create device tree nodes and program the BARs
>
>
>
> I taught my "git send-email" wrapper script to add "kernel/qemu/slof" in 
> the subject, otherwise I believe it is hard for busy people to tell quickly 
> what patchset is for what, I'd recommend you doing the same, especially 
> when you mention "QEMU" in a subject  :)

Adding in the Subject will put "slof" in the commit title, which is what
we do not want. So will add SLOF in the "[ ]" now onwards. Something
like this: "[ PATCH SLOF <version> ]"

Regards
Nikunj