[SLOF] [PATCH slof] pci: Put non-prefetchable 64bit BARs into 32bit MMIO window

Alexey Kardashevskiy aik at ozlabs.ru
Thu Apr 27 19:21:25 AEST 2017


On Thu, 27 Apr 2017 10:51:12 +0200
Thomas Huth <thuth at redhat.com> wrote:

> On 27.04.2017 10:30, Alexey Kardashevskiy wrote:
> > On Thu, 27 Apr 2017 09:50:36 +0200
> > Thomas Huth <thuth at redhat.com> wrote:
> >   
> >> On 27.04.2017 09:10, Thomas Huth wrote:  
> >>> On 27.04.2017 08:54, Alexey Kardashevskiy wrote:    
> >>>> On Wed, 26 Apr 2017 15:23:40 +0200
> >>>> Thomas Huth <thuth at redhat.com> wrote:
> >>>>    
> >>>>> On 24.04.2017 05:01, Alexey Kardashevskiy wrote:    
> >>>>>> At the moment 64bit non-prefetchable BARs of devices behind PCI
> >>>>>> p2p bridge go to a 64bit prefetchable windows which is not
> >>>>>> correct and causes linux guests to fail to ioremap() such
> >>>>>> resources.
> >>>>>>
> >>>>>> This moves 64bit non-prefetchable BARs 32bit non-prefetchable
> >>>>>> window.
> >>>>>>
> >>>>>> Note that this does not make distinction between P2P and PHB so
> >>>>>> from now on XHCI BARs will be allocated from 32bit MMIO space.
> >>>>>> However since most 64bit-MMIO-capable devices have prefetchable
> >>>>>> BARs, and XHCI BAR is just 4K (so it is unlikely to cause any
> >>>>>> space problems), this should not affect usual behavior.
> >>>>>>
> >>>>>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> >>>>>> ---
> >>>>>>
> >>>>>> This fixes QEMU's XHCI when it is put on a P2P PCI bridge.
> >>>>>>
> >>>>>> There is a little naming confusion as it may look like the
> >>>>>> patch is changing assignment for all 64bit BAR but it does not
> >>>>>> as:
> >>>>>> - "mmio" is used for non-prefetchable memory,
> >>>>>> - "mem" is used for prefetchable memory.
> >>>>>> ---
> >>>>>>  slof/fs/pci-properties.fs | 6 +-----
> >>>>>>  1 file changed, 1 insertion(+), 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/slof/fs/pci-properties.fs
> >>>>>> b/slof/fs/pci-properties.fs index e6fd843..8594e5d 100644
> >>>>>> --- a/slof/fs/pci-properties.fs
> >>>>>> +++ b/slof/fs/pci-properties.fs
> >>>>>> @@ -166,11 +166,7 @@
> >>>>>>  \ Setup a non-prefetchable 64bit BAR and return its size
> >>>>>>  : assign-mmio64-bar ( bar-addr -- 8 )
> >>>>>>          dup pci-bar-size-mem64          \ fetch size
> >>>>>> -        pci-next-mem64 @ 0 = IF          \ Check if we have
> >>>>>> 64-bit memory range
> >>>>>> -	    pci-next-mmio
> >>>>>> -	ELSE
> >>>>>> -	    pci-next-mem64              \ for board-qemu we
> >>>>>> will use same range
> >>>>>> -	THEN
> >>>>>> +        pci-next-mmio
> >>>>>>          assign-bar-value64              \ and set it all
> >>>>>>  ;      
> >>>>>
> >>>>> I'm sorry, but this is now causing trouble with the USB keyboard
> >>>>> in *SLOF* instead. If I start QEMU now like this:
> >>>>>
> >>>>> qemu-system-ppc64 -nodefaults -serial mon:stdio \
> >>>>>   -device pci-bridge,bus=pci.0,id=bridge1,chassis_nr=1 \
> >>>>>   -device nec-usb-xhci,id=xhci1,bus=bridge1,addr=0x3 -vga std \
> >>>>>   -device usb-kbd,bus=xhci1.0 -bios boot_rom.bin
> >>>>>
> >>>>> then SLOF complains: "usb-xhci: failed to initialize XHCI
> >>>>> controller" and the keyboard is not working at the SLOF prompt
> >>>>> anymore.
> >>>>>
> >>>>> Any idea why this is happening now?    
> >>>>
> >>>>
> >>>> XHCI's usb_hcd_dev::base == 0xc0000000 which is a bus address
> >>>> while it is supposed to be a mapped address, i.e.
> >>>> 800000020000000.
> >>>>
> >>>> It used to work before the patch as pci address is the same as
> >>>> mapped address - 210000000000 - and I am not sure if this is not
> >>>> an accident, could be QEMU's PCI MMIO windows/spacing rework
> >>>> David did some time ago.
> >>>>
> >>>> I am trying to figure out now where usb_hcd_dev::base is set in
> >>>> SLOF, it is tricky :-/    
> >>>
> >>> I think the value comes from usb-setup-hcidev in
> >>> pci-class_0c.fs ... so it's likely that translate-my-address is
> >>> failing now.
> >>>
> >>> ... but thinking about this issue again, I now wonder whether your
> >>> original patch was really the right solution here, since QEMU only
> >>> provides three memory regions to the guest:
> >>> 1) I/O
> >>> 2) 32-bit memory
> >>> 3) 64-bit memory
> >>> There is no distinction between prefetchable and non-prefetchable
> >>> here. So I think the original problem might have been caused by
> >>> something else instead... (not sure about that yet, though)    
> >>
> >> Yup, looks like the real culprit is the "ranges" property of the
> >> PCI bridges. This can not deal with 64-bit memory regions yet. I
> >> think we should revert the patch to assign-mmio64-bar and fix
> >> pci-bridge-range-props and friends instead...  
> > 
> > Try this:
> > 
> > diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
> > index 8594e5d..66e364d 100644
> > --- a/slof/fs/pci-properties.fs
> > +++ b/slof/fs/pci-properties.fs
> > @@ -349,7 +349,7 @@
> >                  1 OF gen-io-bar-prop     ENDOF          \ - IO-BAR
> >                  2 OF gen-mem32-bar-prop  ENDOF          \ - MEM32
> >                  3 OF gen-pmem32-bar-prop ENDOF          \ - MEM32
> > prefetchable
> > -                4 OF gen-mem64-bar-prop  ENDOF          \ - MEM64
> > +                4 OF gen-mem32-bar-prop  ENDOF          \ - MEM64
> >                  5 OF gen-pmem64-bar-prop ENDOF          \ - MEM64
> > prefetchable ENDCASE                                         \ ESAC
> > ( paddr plen bsize ) ;
> > 
> > Works for me, for both PHB and P2P bridges.  
> 
> That might work as long as the device has only one BAR. But as soon
> as you've got a device with multiple BARs, you'll run into problems
> since pci-add-assigned-address now returns the wrong BAR size.

Ah, right. This should do it instead:

diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
index 8594e5d..d446473 100644
--- a/slof/fs/pci-properties.fs
+++ b/slof/fs/pci-properties.fs
@@ -254,13 +254,13 @@
 : gen-mem64-bar-prop ( prop-addr prop-len bar-addr -- prop-addr prop-len 8 )
         dup pci-bar-size-mem64                  \ fetch BAR Size        ( paddr plen baddr bsize )
         dup IF                                  \ IF Size > 0
                 >r dup rtas-config-l@           \ | save size and fetch lower 32 bits ( paddr plen baddr val.lo R: size)
                 over 4 + rtas-config-l@         \ | fetch upper 32 bits               ( paddr plen baddr val.lo val.hi R: size)
                 20 lshift + -10 and >r          \ | calc 64 bit value and save it     ( paddr plen baddr R: size val )
-                83000000 or encode-int+         \ | Encode config addr                ( paddr plen R: size val )
+                82000000 or encode-int+         \ | Encode config addr                ( paddr plen R: size val )
                 r> encode-64+                   \ | Encode assigned addr              ( paddr plen R: size )
                 r> encode-64+                   \ | Encode size                       ( paddr plen )
         ELSE                                    \ ELSE
                 2drop                           \ | don't do anything
         THEN                                    \ FI
         8                                       \ sizeof(BAR) = 8 Bytes


> 
> Please try this instead: Revert your original patch to
> assign-mmio64-bar and apply this patch instead:
> 
> diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
> index e6fd843..625692a 100644
> --- a/slof/fs/pci-properties.fs
> +++ b/slof/fs/pci-properties.fs
> @@ -411,12 +411,14 @@
>  : pci-bridge-gen-mem-range ( addr prop-addr prop-len -- addr
> prop-addr prop-len ) 2 pick 24 + rtas-config-l@      \ fetch
> Value           ( addr paddr plen val ) dup 000FFFFF
> or                 \ calc limit Bits 31:0  ( addr paddr plen val
> limit.31:0 )
> -        swap 0000FFF0 and 10 lshift     \ calc base Bits 31:0
> ( addr paddr plen limit.31:0 base.31:0 )
> +        swap dup F and >r               \ R: 64-bit?
> +        0000FFF0 and 10 lshift          \ calc base Bits 31:0
> ( addr paddr plen limit.31:0 base.31:0 ) 4 pick 28 +
> rtas-config-l@      \ fetch upper Basebits  ( addr paddr plen
> limit.31:0 base.31:0 base.63:32 ) 20 lshift or swap               \
> and calc Base         ( addr paddr plen base.63:0 limit.31:0 ) 4 pick
> 2C + rtas-config-l@      \ fetch upper Limitbits ( addr paddr plen
> base.63:0 limit.31:0 limit.63:32 ) 20 lshift or                    \
> and calc Limit        ( addr paddr plen base.63:0 limit.63:0 )
> -        42000000 pci-bridge-gen-range   \ and generate it
> ( addr paddr plen )
> +        r> IF 03000000 ELSE 42000000 THEN  \ 64-bit or 32-bit ?
> +        pci-bridge-gen-range            \ and generate it
> ( addr paddr plen ) ;
> 
> This fixes all the XHCI problems for me.


Yes it does fix XHCI but it also puts 64bit non-prefetchable BAR to
prefetchable window which we decided is not correct.



--
Alexey


More information about the SLOF mailing list