| Message ID | 20170424030131.32311-1-aik@ozlabs.ru |
|---|---|
| State | Superseded |
| Headers | show |
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. I wonder why this was working when the XHCI controller was connected directly to the PHB? Do the memory regions behave differently here? > 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@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 think this patch is right. Using prefetchable memory here was certainly a bad idea, and if I get the PCI-to-PCI bridge spec right, there is no such thing like a dedicated 64-bit non-prefetchable memory range, so using the non-prefetchable 32-bit memory range is the only option here. Reviewed-by: Thomas Huth <thuth@redhat.com>
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@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 > ; > > Tested-by: Laurent Vivier <lvivier@redhat.com>
On 25/04/2017 12:36, Laurent Vivier 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@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 >> ; >> >> > Tested-by: Laurent Vivier <lvivier@redhat.com> This fixes the problem when the BAR is mem, not when it is io. I have the same problem as with XHCI as with virtio-blk devices behind a pci bridge, and the error is: ... PCI: Cannot allocate resource region 0 of device 0000:01:01.0, will remap ... virtio-pci 0000:01:01.0: can't enable device: BAR 0 [io size 0x0040] not assigned ... And this patch doesn't fix it. XHCI error was: PCI: Cannot allocate resource region 0 of device 0000:01:03.0, will remap ... xhci_hcd 0000:01:03.0: can't enable device: BAR 0 [mem size 0x00004000] not assigned Laurent
On 25/04/17 21:00, Laurent Vivier wrote: > On 25/04/2017 12:36, Laurent Vivier 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@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 >>> ; >>> >>> >> Tested-by: Laurent Vivier <lvivier@redhat.com> > > This fixes the problem when the BAR is mem, not when it is io. > > I have the same problem as with XHCI as with virtio-blk devices behind a > pci bridge, and the error is: > > ... > PCI: Cannot allocate resource region 0 of device 0000:01:01.0, will remap > ... > virtio-pci 0000:01:01.0: can't enable device: BAR 0 [io size 0x0040] > not assigned > ... > > And this patch doesn't fix it. Correct, it did not try to fix it. But afaict virtio uses IO in legacy drivers and also implements mmio BARs which are actually used in guest kernels we care about. Or there is a problem and something does not work? > > XHCI error was: > > PCI: Cannot allocate resource region 0 of device 0000:01:03.0, will remap > ... > xhci_hcd 0000:01:03.0: can't enable device: BAR 0 [mem size 0x00004000] > not assigned > > Laurent >
On 25/04/2017 15:44, Alexey Kardashevskiy wrote: > On 25/04/17 21:00, Laurent Vivier wrote: >> On 25/04/2017 12:36, Laurent Vivier 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@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 >>>> ; >>>> >>>> >>> Tested-by: Laurent Vivier <lvivier@redhat.com> >> >> This fixes the problem when the BAR is mem, not when it is io. >> >> I have the same problem as with XHCI as with virtio-blk devices behind a >> pci bridge, and the error is: >> >> ... >> PCI: Cannot allocate resource region 0 of device 0000:01:01.0, will remap >> ... >> virtio-pci 0000:01:01.0: can't enable device: BAR 0 [io size 0x0040] >> not assigned >> ... >> >> And this patch doesn't fix it. > > > Correct, it did not try to fix it. But afaict virtio uses IO in legacy > drivers and also implements mmio BARs which are actually used in guest > kernels we care about. Or there is a problem and something does not work? You're right, adding "disable-legacy=on" to the virtio-blk-pci device allows to use the disk and remove the errors from the guest kernel logs. What I don't understand is why "disable-legacy=on" fixes the problem even without your patch for SLOF... Thanks, Laurent
On 26/04/17 01:06, Laurent Vivier wrote: > On 25/04/2017 15:44, Alexey Kardashevskiy wrote: >> On 25/04/17 21:00, Laurent Vivier wrote: >>> On 25/04/2017 12:36, Laurent Vivier 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@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 >>>>> ; >>>>> >>>>> >>>> Tested-by: Laurent Vivier <lvivier@redhat.com> >>> >>> This fixes the problem when the BAR is mem, not when it is io. >>> >>> I have the same problem as with XHCI as with virtio-blk devices behind a >>> pci bridge, and the error is: >>> >>> ... >>> PCI: Cannot allocate resource region 0 of device 0000:01:01.0, will remap >>> ... >>> virtio-pci 0000:01:01.0: can't enable device: BAR 0 [io size 0x0040] >>> not assigned >>> ... >>> >>> And this patch doesn't fix it. >> >> >> Correct, it did not try to fix it. But afaict virtio uses IO in legacy >> drivers and also implements mmio BARs which are actually used in guest >> kernels we care about. Or there is a problem and something does not work? > > You're right, adding "disable-legacy=on" to the virtio-blk-pci device > allows to use the disk and remove the errors from the guest kernel logs. > > What I don't understand is why "disable-legacy=on" fixes the problem > even without your patch for SLOF... The patch changes handling of MMIO BARs (up to 64bit of PCI address space) and virtio problem is caused by an IO BAR (65536 IO ports, x86 legacy) - this is what "disable-legacy=on" removes.
On Mon, Apr 24, 2017 at 05:29:52PM +0200, Thomas Huth 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. > > I wonder why this was working when the XHCI controller was connected > directly to the PHB? Do the memory regions behave differently here? > > > 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@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 think this patch is right. Using prefetchable memory here was > certainly a bad idea, and if I get the PCI-to-PCI bridge spec right, > there is no such thing like a dedicated 64-bit non-prefetchable memory > range, so using the non-prefetchable 32-bit memory range is the only > option here. That is my understanding as well. > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > _______________________________________________ > SLOF mailing list > SLOF@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/slof
On Tue, Apr 25, 2017 at 01:00:18PM +0200, Laurent Vivier wrote: > On 25/04/2017 12:36, Laurent Vivier 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@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 > >> ; > >> > >> > > Tested-by: Laurent Vivier <lvivier@redhat.com> > > This fixes the problem when the BAR is mem, not when it is io. > > I have the same problem as with XHCI as with virtio-blk devices behind a > pci bridge, and the error is: > > ... > PCI: Cannot allocate resource region 0 of device 0000:01:01.0, will remap > ... > virtio-pci 0000:01:01.0: can't enable device: BAR 0 [io size 0x0040] > not assigned > ... > > And this patch doesn't fix it. > > XHCI error was: > > PCI: Cannot allocate resource region 0 of device 0000:01:03.0, will remap > ... > xhci_hcd 0000:01:03.0: can't enable device: BAR 0 [mem size 0x00004000] > not assigned I don't think the problem for virtio can be quite analagous. Legacy IO windows don't have either the prefetchable / no-prefetchable or the 32-bit / 64-bit distinctions that memory windows do. So while there certainly could be a bug in assigning IO windows under bridges, I think it's independent from this bug.
On 25/04/17 01:29, Thomas Huth 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. > > I wonder why this was working when the XHCI controller was connected > directly to the PHB? Do the memory regions behave differently here? PCI bridges have IO, 32bit prefetchable, 32bit non-prefetchable, 64bit prefetchable windows; they simply do not have space for storing 64bit non-prefetchable window address in the config space. sPAPR PHB bridges have IO, 32bit non-prefetchable, 64bit non-prefetchable windows - b_p(1) is not set (why is that I am not so sure, it has been always like that, may have something to do with prefetching support on the hardware actual by the time). Prefetchable resources can be allocated within non-prefetchable windows but not vice versa. > >> 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@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 think this patch is right. Using prefetchable memory here was > certainly a bad idea, and if I get the PCI-to-PCI bridge spec right, > there is no such thing like a dedicated 64-bit non-prefetchable memory > range, so using the non-prefetchable 32-bit memory range is the only > option here. > > Reviewed-by: Thomas Huth <thuth@redhat.com> Thanks! I'll push it now.
On 26/04/17 13:28, Alexey Kardashevskiy wrote: > On 25/04/17 01:29, Thomas Huth 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. >> >> I wonder why this was working when the XHCI controller was connected >> directly to the PHB? Do the memory regions behave differently here? > > > PCI bridges have IO, 32bit prefetchable, 32bit non-prefetchable, 64bit > prefetchable windows; they simply do not have space for storing 64bit > non-prefetchable window address in the config space. > > sPAPR PHB bridges have IO, 32bit non-prefetchable, 64bit non-prefetchable > windows - b_p(1) is not set (why is that I am not so sure, it has been > always like that, may have something to do with prefetching support on the > hardware actual by the time). > > Prefetchable resources can be allocated within non-prefetchable windows but > not vice versa. Ah, it is also worth mentioning that BAR allocation done by SLOF and by skiboot is different - on the bare metal, XHCI with 64bit nonprefetchable BARs is put into 32bit MMIO window already. > > > >> >>> 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@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 think this patch is right. Using prefetchable memory here was >> certainly a bad idea, and if I get the PCI-to-PCI bridge spec right, >> there is no such thing like a dedicated 64-bit non-prefetchable memory >> range, so using the non-prefetchable 32-bit memory range is the only >> option here. >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> > > Thanks! I'll push it now. > >
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@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? Thomas
On Wed, 26 Apr 2017 15:23:40 +0200 Thomas Huth <thuth@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@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 :-/ > > Thomas > -- Alexey
On 27.04.2017 08:54, Alexey Kardashevskiy wrote: > On Wed, 26 Apr 2017 15:23:40 +0200 > Thomas Huth <thuth@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@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) Thomas
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > On Wed, 26 Apr 2017 15:23:40 +0200 > Thomas Huth <thuth@redhat.com> wrote: > > > 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 :-/ slof/fs/devices/pci-class_0c.fs::usb-setup-hcidev() ( io-base ) r@ hcd>base ! Regards Nikunj
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@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@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... Thomas
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 ;
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@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(-)