Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-30 Thread Alex Williamson 
On Mon, 29 May 2017 23:30:52 -0400
Jon Masters  wrote:

> Following up on this thread...
> 
> On 05/23/2017 06:15 PM, Alex Williamson wrote:
> > On Tue, 23 May 2017 14:22:01 -0700
> > David Daney  wrote:
> >   
> >> On 05/23/2017 02:04 PM, Alex Williamson wrote:  
> >>> On Tue, 23 May 2017 15:47:50 -0500
> >>> Bjorn Helgaas  wrote:
> >>> 
>  On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> > With the recent improvements in arm64 and vfio-pci, we are seeing
> > failures like this (on cn8890 based systems):
> >
> > [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> > at 0xfc00c1000100
> > [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> > .
> > .
> > .
> > [  236.208820] [] pci_generic_config_read+0x38/0x9c
> > [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> > [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> > [  236.227596] [] 
> > pci_find_next_ext_capability.part.15+0x40/0xb8
> > [  236.234896] [] pci_find_ext_capability+0x20/0x30
> > [  236.241068] [] pci_restore_vc_state+0x34/0x88
> > [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> > [  236.253410] [] pci_dev_restore+0x4c/0x50
> > [  236.258887] [] pci_bus_restore+0x24/0x4c
> > [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> > [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c 
> > [vfio_pci]
> > [  236.276547] [] 
> > vfio_device_fops_unl_ioctl+0x20/0x30 [vfio]
> > [  236.283587] [] do_vfs_ioctl+0xac/0x744
> > [  236.288890] [] SyS_ioctl+0x84/0x98
> > [  236.293846] [] __sys_trace_return+0x0/0x4
> >
> > These are caused by the inability of the PCIe root port and Intel
> > e1000e to sucessfully do a bus reset.  
> 
> Right now, we have a whole bunch of systems in our labs and across the
> industry where people are testing VFIO on Cavium ThunderX platforms. It
> is amazing how many people have e1000 cards lying around (this is even
> more popular on ARM servers because things like Tianocore usually "just
> work" with an e1000 card installed...). I know I have many more e1000s
> than I do x86 systems. So we need a solution to not crash on use.
> 
> > The proposed fix is to not do a bus reset on these systems.  
> 
> It's not the best solution, but it's a solution, and there are plenty of
> other quirks. I would like to see us figure out a sensible path to
> perhaps take this patch now and continue researching to determine
> whether there's an even better option later.
> 
> The reasons I say this are:
> 
> 1). It will take some time for various teams with protocol analyzers to
> determine which end is not fully spec compliant, while in the interim
> everyone has to carry this patch or some other nasty hack anyway.
> 
> 2). I have an even worse patch in my test kernels (just disable reset
> for every device, which is nasty) and I'm sure I'm not alone. It would
> be better (IMO) to confine the lack of reset to a subset of devices.
> 
> 
> 
> > My hope would be that such analysis would help us understand what's
> > really happening and perhaps present a less drastic workaround.  Is the
> > point at which we crash simply the first read from the device?  Is the
> > link state shown as stable at this point?  Is there any chance that we
> > have a timing issue and an additional delay could resolve it?  Is there
> > some manipulation of the link state before or after the bus reset that
> > produces different results?  If such options have been exhausted, then
> > I guess I have no objection, or at least no better solution, and I'll
> > keep an eye out for any fallout.  Thanks,  
> 
> My understanding is that various teams have already spent man months of
> time with protocol analyzer equipment trying to figure out who is at
> "fault". I have no reason not to believe that they don't intend to
> continue that work, since they certainly want to ensure future root port
> implementations are event more robust in the presence of both fully
> compliant, and also somewhat compliant endpoints. If David affirms that
> the work will continue, can we take this for now?

I'd feel slightly more inclined to agree on that course if we gave
hardware vendors some incentive to resolve the problem and users some
indication that a problem exists, by adding a dev_warn when we apply
PCI_DEV_FLAGS_NO_BUS_RESET to the device.  I'd also entertain the idea
of only printing a warning when an affected devices is opened by the
user through vfio.  Thanks,

Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-30 Thread Alex Williamson 
On Mon, 29 May 2017 23:30:52 -0400
Jon Masters  wrote:

> Following up on this thread...
> 
> On 05/23/2017 06:15 PM, Alex Williamson wrote:
> > On Tue, 23 May 2017 14:22:01 -0700
> > David Daney  wrote:
> >   
> >> On 05/23/2017 02:04 PM, Alex Williamson wrote:  
> >>> On Tue, 23 May 2017 15:47:50 -0500
> >>> Bjorn Helgaas  wrote:
> >>> 
>  On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> > With the recent improvements in arm64 and vfio-pci, we are seeing
> > failures like this (on cn8890 based systems):
> >
> > [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> > at 0xfc00c1000100
> > [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> > .
> > .
> > .
> > [  236.208820] [] pci_generic_config_read+0x38/0x9c
> > [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> > [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> > [  236.227596] [] 
> > pci_find_next_ext_capability.part.15+0x40/0xb8
> > [  236.234896] [] pci_find_ext_capability+0x20/0x30
> > [  236.241068] [] pci_restore_vc_state+0x34/0x88
> > [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> > [  236.253410] [] pci_dev_restore+0x4c/0x50
> > [  236.258887] [] pci_bus_restore+0x24/0x4c
> > [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> > [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c 
> > [vfio_pci]
> > [  236.276547] [] 
> > vfio_device_fops_unl_ioctl+0x20/0x30 [vfio]
> > [  236.283587] [] do_vfs_ioctl+0xac/0x744
> > [  236.288890] [] SyS_ioctl+0x84/0x98
> > [  236.293846] [] __sys_trace_return+0x0/0x4
> >
> > These are caused by the inability of the PCIe root port and Intel
> > e1000e to sucessfully do a bus reset.  
> 
> Right now, we have a whole bunch of systems in our labs and across the
> industry where people are testing VFIO on Cavium ThunderX platforms. It
> is amazing how many people have e1000 cards lying around (this is even
> more popular on ARM servers because things like Tianocore usually "just
> work" with an e1000 card installed...). I know I have many more e1000s
> than I do x86 systems. So we need a solution to not crash on use.
> 
> > The proposed fix is to not do a bus reset on these systems.  
> 
> It's not the best solution, but it's a solution, and there are plenty of
> other quirks. I would like to see us figure out a sensible path to
> perhaps take this patch now and continue researching to determine
> whether there's an even better option later.
> 
> The reasons I say this are:
> 
> 1). It will take some time for various teams with protocol analyzers to
> determine which end is not fully spec compliant, while in the interim
> everyone has to carry this patch or some other nasty hack anyway.
> 
> 2). I have an even worse patch in my test kernels (just disable reset
> for every device, which is nasty) and I'm sure I'm not alone. It would
> be better (IMO) to confine the lack of reset to a subset of devices.
> 
> 
> 
> > My hope would be that such analysis would help us understand what's
> > really happening and perhaps present a less drastic workaround.  Is the
> > point at which we crash simply the first read from the device?  Is the
> > link state shown as stable at this point?  Is there any chance that we
> > have a timing issue and an additional delay could resolve it?  Is there
> > some manipulation of the link state before or after the bus reset that
> > produces different results?  If such options have been exhausted, then
> > I guess I have no objection, or at least no better solution, and I'll
> > keep an eye out for any fallout.  Thanks,  
> 
> My understanding is that various teams have already spent man months of
> time with protocol analyzer equipment trying to figure out who is at
> "fault". I have no reason not to believe that they don't intend to
> continue that work, since they certainly want to ensure future root port
> implementations are event more robust in the presence of both fully
> compliant, and also somewhat compliant endpoints. If David affirms that
> the work will continue, can we take this for now?

I'd feel slightly more inclined to agree on that course if we gave
hardware vendors some incentive to resolve the problem and users some
indication that a problem exists, by adding a dev_warn when we apply
PCI_DEV_FLAGS_NO_BUS_RESET to the device.  I'd also entertain the idea
of only printing a warning when an affected devices is opened by the
user through vfio.  Thanks,

Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-29 Thread Jon Masters 
Following up on this thread...

On 05/23/2017 06:15 PM, Alex Williamson wrote:
> On Tue, 23 May 2017 14:22:01 -0700
> David Daney  wrote:
> 
>> On 05/23/2017 02:04 PM, Alex Williamson wrote:
>>> On Tue, 23 May 2017 15:47:50 -0500
>>> Bjorn Helgaas  wrote:
>>>   
 On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:  
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
>
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> at 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
>
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.

Right now, we have a whole bunch of systems in our labs and across the
industry where people are testing VFIO on Cavium ThunderX platforms. It
is amazing how many people have e1000 cards lying around (this is even
more popular on ARM servers because things like Tianocore usually "just
work" with an e1000 card installed...). I know I have many more e1000s
than I do x86 systems. So we need a solution to not crash on use.

> The proposed fix is to not do a bus reset on these systems.

It's not the best solution, but it's a solution, and there are plenty of
other quirks. I would like to see us figure out a sensible path to
perhaps take this patch now and continue researching to determine
whether there's an even better option later.

The reasons I say this are:

1). It will take some time for various teams with protocol analyzers to
determine which end is not fully spec compliant, while in the interim
everyone has to carry this patch or some other nasty hack anyway.

2). I have an even worse patch in my test kernels (just disable reset
for every device, which is nasty) and I'm sure I'm not alone. It would
be better (IMO) to confine the lack of reset to a subset of devices.



> My hope would be that such analysis would help us understand what's
> really happening and perhaps present a less drastic workaround.  Is the
> point at which we crash simply the first read from the device?  Is the
> link state shown as stable at this point?  Is there any chance that we
> have a timing issue and an additional delay could resolve it?  Is there
> some manipulation of the link state before or after the bus reset that
> produces different results?  If such options have been exhausted, then
> I guess I have no objection, or at least no better solution, and I'll
> keep an eye out for any fallout.  Thanks,

My understanding is that various teams have already spent man months of
time with protocol analyzer equipment trying to figure out who is at
"fault". I have no reason not to believe that they don't intend to
continue that work, since they certainly want to ensure future root port
implementations are event more robust in the presence of both fully
compliant, and also somewhat compliant endpoints. If David affirms that
the work will continue, can we take this for now?

Jon.

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-29 Thread Jon Masters 
Following up on this thread...

On 05/23/2017 06:15 PM, Alex Williamson wrote:
> On Tue, 23 May 2017 14:22:01 -0700
> David Daney  wrote:
> 
>> On 05/23/2017 02:04 PM, Alex Williamson wrote:
>>> On Tue, 23 May 2017 15:47:50 -0500
>>> Bjorn Helgaas  wrote:
>>>   
 On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:  
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
>
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> at 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
>
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.

Right now, we have a whole bunch of systems in our labs and across the
industry where people are testing VFIO on Cavium ThunderX platforms. It
is amazing how many people have e1000 cards lying around (this is even
more popular on ARM servers because things like Tianocore usually "just
work" with an e1000 card installed...). I know I have many more e1000s
than I do x86 systems. So we need a solution to not crash on use.

> The proposed fix is to not do a bus reset on these systems.

It's not the best solution, but it's a solution, and there are plenty of
other quirks. I would like to see us figure out a sensible path to
perhaps take this patch now and continue researching to determine
whether there's an even better option later.

The reasons I say this are:

1). It will take some time for various teams with protocol analyzers to
determine which end is not fully spec compliant, while in the interim
everyone has to carry this patch or some other nasty hack anyway.

2). I have an even worse patch in my test kernels (just disable reset
for every device, which is nasty) and I'm sure I'm not alone. It would
be better (IMO) to confine the lack of reset to a subset of devices.



> My hope would be that such analysis would help us understand what's
> really happening and perhaps present a less drastic workaround.  Is the
> point at which we crash simply the first read from the device?  Is the
> link state shown as stable at this point?  Is there any chance that we
> have a timing issue and an additional delay could resolve it?  Is there
> some manipulation of the link state before or after the bus reset that
> produces different results?  If such options have been exhausted, then
> I guess I have no objection, or at least no better solution, and I'll
> keep an eye out for any fallout.  Thanks,

My understanding is that various teams have already spent man months of
time with protocol analyzer equipment trying to figure out who is at
"fault". I have no reason not to believe that they don't intend to
continue that work, since they certainly want to ensure future root port
implementations are event more robust in the presence of both fully
compliant, and also somewhat compliant endpoints. If David affirms that
the work will continue, can we take this for now?

Jon.

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Alex Williamson 
On Tue, 23 May 2017 14:22:01 -0700
David Daney  wrote:

> On 05/23/2017 02:04 PM, Alex Williamson wrote:
> > On Tue, 23 May 2017 15:47:50 -0500
> > Bjorn Helgaas  wrote:
> >   
> >> On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:  
> >>> With the recent improvements in arm64 and vfio-pci, we are seeing
> >>> failures like this (on cn8890 based systems):
> >>>
> >>> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> >>> at 0xfc00c1000100
> >>> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> >>> .
> >>> .
> >>> .
> >>> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> >>> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> >>> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> >>> [  236.227596] [] 
> >>> pci_find_next_ext_capability.part.15+0x40/0xb8
> >>> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> >>> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> >>> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> >>> [  236.253410] [] pci_dev_restore+0x4c/0x50
> >>> [  236.258887] [] pci_bus_restore+0x24/0x4c
> >>> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> >>> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> >>> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> >>> [vfio]
> >>> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> >>> [  236.288890] [] SyS_ioctl+0x84/0x98
> >>> [  236.293846] [] __sys_trace_return+0x0/0x4
> >>>
> >>> These are caused by the inability of the PCIe root port and Intel
> >>> e1000e to sucessfully do a bus reset.
> >>>
> >>> The proposed fix is to not do a bus reset on these systems.
> >>>
> >>> David Daney (2):
> >>>PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
> >>>PCI: Avoid bus reset for Cavium cn8xxx root ports.
> >>>
> >>>   drivers/pci/pci.c| 4 
> >>>   drivers/pci/quirks.c | 8 
> >>>   2 files changed, 12 insertions(+)  
> >>
> >> Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
> >> v4.13, thanks!  
> > 
> > Hmm, well let me again express my concerns that I'm really not sure how
> > to support this since it removes our last opportunity to reset devices
> > that may otherwise have no reset mechanism.  Certain classes of devices
> > are entirely unsupportable for the code path indicated above without a
> > bus reset.  If we have an endpoint device that goes bonkers at a bus
> > reset, at least we know it's going to behave just as poorly no matter
> > what the host platform.  This series allows endpoints that work
> > perfectly well on one host to be handled differently on another.  
> 
> Yes that is correct.  We choose not to crash the system.  I'm not sure 
> what you are suggesting as an alternative.

It's a valid solution, but my concern is that it's not without
consequences and I didn't get the impression that was fully recognized,
so I wonder if there are better options.

> If a PCI device doesn't work with vfio-pci in such a system, my 
> suggestion would be not to use vfio-pci with the device in that system.

And I can use this as the official support statement from Cavium when
that occurs?
 
> >  It
> > certainly suggests something non-spec compliant about the root port
> > implementation and I wish there was more analysis about exactly what
> > that problem is since this is coming from the hardware vendor.  
> 
> There are two main possibilities here:
> 
> 1) Some (but not all) Intel e1000e and LSI HBA devices are non-spec 
> compliant.
> 
> 2) Cavium root port is non-spec compliant.
> 
> If #1 turns out to be true, would you suggest blacklisting e1000e on all 
> systems, including Intel based servers?

Since we haven't heard anything to the contrary, I think we can also
assume that AMD root ports are compatible.
 
> If #2 turns out to be true would you still object to the patch?

My hope would be that such analysis would help us understand what's
really happening and perhaps present a less drastic workaround.  Is the
point at which we crash simply the first read from the device?  Is the
link state shown as stable at this point?  Is there any chance that we
have a timing issue and an additional delay could resolve it?  Is there
some manipulation of the link state before or after the bus reset that
produces different results?  If such options have been exhausted, then
I guess I have no objection, or at least no better solution, and I'll
keep an eye out for any fallout.  Thanks,

Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Alex Williamson 
On Tue, 23 May 2017 14:22:01 -0700
David Daney  wrote:

> On 05/23/2017 02:04 PM, Alex Williamson wrote:
> > On Tue, 23 May 2017 15:47:50 -0500
> > Bjorn Helgaas  wrote:
> >   
> >> On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:  
> >>> With the recent improvements in arm64 and vfio-pci, we are seeing
> >>> failures like this (on cn8890 based systems):
> >>>
> >>> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> >>> at 0xfc00c1000100
> >>> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> >>> .
> >>> .
> >>> .
> >>> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> >>> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> >>> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> >>> [  236.227596] [] 
> >>> pci_find_next_ext_capability.part.15+0x40/0xb8
> >>> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> >>> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> >>> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> >>> [  236.253410] [] pci_dev_restore+0x4c/0x50
> >>> [  236.258887] [] pci_bus_restore+0x24/0x4c
> >>> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> >>> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> >>> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> >>> [vfio]
> >>> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> >>> [  236.288890] [] SyS_ioctl+0x84/0x98
> >>> [  236.293846] [] __sys_trace_return+0x0/0x4
> >>>
> >>> These are caused by the inability of the PCIe root port and Intel
> >>> e1000e to sucessfully do a bus reset.
> >>>
> >>> The proposed fix is to not do a bus reset on these systems.
> >>>
> >>> David Daney (2):
> >>>PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
> >>>PCI: Avoid bus reset for Cavium cn8xxx root ports.
> >>>
> >>>   drivers/pci/pci.c| 4 
> >>>   drivers/pci/quirks.c | 8 
> >>>   2 files changed, 12 insertions(+)  
> >>
> >> Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
> >> v4.13, thanks!  
> > 
> > Hmm, well let me again express my concerns that I'm really not sure how
> > to support this since it removes our last opportunity to reset devices
> > that may otherwise have no reset mechanism.  Certain classes of devices
> > are entirely unsupportable for the code path indicated above without a
> > bus reset.  If we have an endpoint device that goes bonkers at a bus
> > reset, at least we know it's going to behave just as poorly no matter
> > what the host platform.  This series allows endpoints that work
> > perfectly well on one host to be handled differently on another.  
> 
> Yes that is correct.  We choose not to crash the system.  I'm not sure 
> what you are suggesting as an alternative.

It's a valid solution, but my concern is that it's not without
consequences and I didn't get the impression that was fully recognized,
so I wonder if there are better options.

> If a PCI device doesn't work with vfio-pci in such a system, my 
> suggestion would be not to use vfio-pci with the device in that system.

And I can use this as the official support statement from Cavium when
that occurs?
 
> >  It
> > certainly suggests something non-spec compliant about the root port
> > implementation and I wish there was more analysis about exactly what
> > that problem is since this is coming from the hardware vendor.  
> 
> There are two main possibilities here:
> 
> 1) Some (but not all) Intel e1000e and LSI HBA devices are non-spec 
> compliant.
> 
> 2) Cavium root port is non-spec compliant.
> 
> If #1 turns out to be true, would you suggest blacklisting e1000e on all 
> systems, including Intel based servers?

Since we haven't heard anything to the contrary, I think we can also
assume that AMD root ports are compatible.
 
> If #2 turns out to be true would you still object to the patch?

My hope would be that such analysis would help us understand what's
really happening and perhaps present a less drastic workaround.  Is the
point at which we crash simply the first read from the device?  Is the
link state shown as stable at this point?  Is there any chance that we
have a timing issue and an additional delay could resolve it?  Is there
some manipulation of the link state before or after the bus reset that
produces different results?  If such options have been exhausted, then
I guess I have no objection, or at least no better solution, and I'll
keep an eye out for any fallout.  Thanks,

Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread David Daney 

On 05/23/2017 02:04 PM, Alex Williamson wrote:

On Tue, 23 May 2017 15:47:50 -0500
Bjorn Helgaas  wrote:


On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:

With the recent improvements in arm64 and vfio-pci, we are seeing
failures like this (on cn8890 based systems):

[  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
0xfc00c1000100
[  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
.
.
.
[  236.208820] [] pci_generic_config_read+0x38/0x9c
[  236.214992] [] thunder_pem_config_read+0x54/0x1e8
[  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
[  236.227596] [] 
pci_find_next_ext_capability.part.15+0x40/0xb8
[  236.234896] [] pci_find_ext_capability+0x20/0x30
[  236.241068] [] pci_restore_vc_state+0x34/0x88
[  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
[  236.253410] [] pci_dev_restore+0x4c/0x50
[  236.258887] [] pci_bus_restore+0x24/0x4c
[  236.264362] [] pci_try_reset_bus+0x7c/0xa0
[  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
[  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 [vfio]
[  236.283587] [] do_vfs_ioctl+0xac/0x744
[  236.288890] [] SyS_ioctl+0x84/0x98
[  236.293846] [] __sys_trace_return+0x0/0x4

These are caused by the inability of the PCIe root port and Intel
e1000e to sucessfully do a bus reset.

The proposed fix is to not do a bus reset on these systems.

David Daney (2):
   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
   PCI: Avoid bus reset for Cavium cn8xxx root ports.

  drivers/pci/pci.c| 4 
  drivers/pci/quirks.c | 8 
  2 files changed, 12 insertions(+)


Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
v4.13, thanks!


Hmm, well let me again express my concerns that I'm really not sure how
to support this since it removes our last opportunity to reset devices
that may otherwise have no reset mechanism.  Certain classes of devices
are entirely unsupportable for the code path indicated above without a
bus reset.  If we have an endpoint device that goes bonkers at a bus
reset, at least we know it's going to behave just as poorly no matter
what the host platform.  This series allows endpoints that work
perfectly well on one host to be handled differently on another.


Yes that is correct.  We choose not to crash the system.  I'm not sure 
what you are suggesting as an alternative.


If a PCI device doesn't work with vfio-pci in such a system, my 
suggestion would be not to use vfio-pci with the device in that system.



 It
certainly suggests something non-spec compliant about the root port
implementation and I wish there was more analysis about exactly what
that problem is since this is coming from the hardware vendor.


There are two main possibilities here:

1) Some (but not all) Intel e1000e and LSI HBA devices are non-spec 
compliant.


2) Cavium root port is non-spec compliant.

If #1 turns out to be true, would you suggest blacklisting e1000e on all 
systems, including Intel based servers?


If #2 turns out to be true would you still object to the patch?





https://lkml.org/lkml/2017/5/16/662

Thanks,
Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread David Daney 

On 05/23/2017 02:04 PM, Alex Williamson wrote:

On Tue, 23 May 2017 15:47:50 -0500
Bjorn Helgaas  wrote:


On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:

With the recent improvements in arm64 and vfio-pci, we are seeing
failures like this (on cn8890 based systems):

[  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
0xfc00c1000100
[  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
.
.
.
[  236.208820] [] pci_generic_config_read+0x38/0x9c
[  236.214992] [] thunder_pem_config_read+0x54/0x1e8
[  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
[  236.227596] [] 
pci_find_next_ext_capability.part.15+0x40/0xb8
[  236.234896] [] pci_find_ext_capability+0x20/0x30
[  236.241068] [] pci_restore_vc_state+0x34/0x88
[  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
[  236.253410] [] pci_dev_restore+0x4c/0x50
[  236.258887] [] pci_bus_restore+0x24/0x4c
[  236.264362] [] pci_try_reset_bus+0x7c/0xa0
[  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
[  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 [vfio]
[  236.283587] [] do_vfs_ioctl+0xac/0x744
[  236.288890] [] SyS_ioctl+0x84/0x98
[  236.293846] [] __sys_trace_return+0x0/0x4

These are caused by the inability of the PCIe root port and Intel
e1000e to sucessfully do a bus reset.

The proposed fix is to not do a bus reset on these systems.

David Daney (2):
   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
   PCI: Avoid bus reset for Cavium cn8xxx root ports.

  drivers/pci/pci.c| 4 
  drivers/pci/quirks.c | 8 
  2 files changed, 12 insertions(+)


Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
v4.13, thanks!


Hmm, well let me again express my concerns that I'm really not sure how
to support this since it removes our last opportunity to reset devices
that may otherwise have no reset mechanism.  Certain classes of devices
are entirely unsupportable for the code path indicated above without a
bus reset.  If we have an endpoint device that goes bonkers at a bus
reset, at least we know it's going to behave just as poorly no matter
what the host platform.  This series allows endpoints that work
perfectly well on one host to be handled differently on another.


Yes that is correct.  We choose not to crash the system.  I'm not sure 
what you are suggesting as an alternative.


If a PCI device doesn't work with vfio-pci in such a system, my 
suggestion would be not to use vfio-pci with the device in that system.



 It
certainly suggests something non-spec compliant about the root port
implementation and I wish there was more analysis about exactly what
that problem is since this is coming from the hardware vendor.


There are two main possibilities here:

1) Some (but not all) Intel e1000e and LSI HBA devices are non-spec 
compliant.


2) Cavium root port is non-spec compliant.

If #1 turns out to be true, would you suggest blacklisting e1000e on all 
systems, including Intel based servers?


If #2 turns out to be true would you still object to the patch?





https://lkml.org/lkml/2017/5/16/662

Thanks,
Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Bjorn Helgaas 
On Tue, May 23, 2017 at 03:04:04PM -0600, Alex Williamson wrote:
> On Tue, 23 May 2017 15:47:50 -0500
> Bjorn Helgaas  wrote:
> 
> > On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> > > With the recent improvements in arm64 and vfio-pci, we are seeing
> > > failures like this (on cn8890 based systems):
> > > 
> > > [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> > > at 0xfc00c1000100
> > > [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> > > .
> > > .
> > > .
> > > [  236.208820] [] pci_generic_config_read+0x38/0x9c
> > > [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> > > [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> > > [  236.227596] [] 
> > > pci_find_next_ext_capability.part.15+0x40/0xb8
> > > [  236.234896] [] pci_find_ext_capability+0x20/0x30
> > > [  236.241068] [] pci_restore_vc_state+0x34/0x88
> > > [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> > > [  236.253410] [] pci_dev_restore+0x4c/0x50
> > > [  236.258887] [] pci_bus_restore+0x24/0x4c
> > > [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> > > [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> > > [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> > > [vfio]
> > > [  236.283587] [] do_vfs_ioctl+0xac/0x744
> > > [  236.288890] [] SyS_ioctl+0x84/0x98
> > > [  236.293846] [] __sys_trace_return+0x0/0x4
> > > 
> > > These are caused by the inability of the PCIe root port and Intel
> > > e1000e to sucessfully do a bus reset.
> > > 
> > > The proposed fix is to not do a bus reset on these systems.
> > > 
> > > David Daney (2):
> > >   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
> > >   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> > > 
> > >  drivers/pci/pci.c| 4 
> > >  drivers/pci/quirks.c | 8 
> > >  2 files changed, 12 insertions(+)  
> > 
> > Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
> > v4.13, thanks!
> 
> Hmm, well let me again express my concerns that I'm really not sure how
> to support this since it removes our last opportunity to reset devices
> that may otherwise have no reset mechanism.  Certain classes of devices
> are entirely unsupportable for the code path indicated above without a
> bus reset.  If we have an endpoint device that goes bonkers at a bus
> reset, at least we know it's going to behave just as poorly no matter
> what the host platform.  This series allows endpoints that work
> perfectly well on one host to be handled differently on another.  It
> certainly suggests something non-spec compliant about the root port
> implementation and I wish there was more analysis about exactly what
> that problem is since this is coming from the hardware vendor.
> 
> https://lkml.org/lkml/2017/5/16/662

I almost poked you about this on IRC; guess I should have :)

Is it better to leave it as-is, and just take the aborts David
reported?

I agree, it would be nice to know what's really going on.  I assume
Cavium is interested in that as well to make sure future parts don't
have the issue.

Bjorn

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Bjorn Helgaas 
On Tue, May 23, 2017 at 03:04:04PM -0600, Alex Williamson wrote:
> On Tue, 23 May 2017 15:47:50 -0500
> Bjorn Helgaas  wrote:
> 
> > On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> > > With the recent improvements in arm64 and vfio-pci, we are seeing
> > > failures like this (on cn8890 based systems):
> > > 
> > > [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> > > at 0xfc00c1000100
> > > [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> > > .
> > > .
> > > .
> > > [  236.208820] [] pci_generic_config_read+0x38/0x9c
> > > [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> > > [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> > > [  236.227596] [] 
> > > pci_find_next_ext_capability.part.15+0x40/0xb8
> > > [  236.234896] [] pci_find_ext_capability+0x20/0x30
> > > [  236.241068] [] pci_restore_vc_state+0x34/0x88
> > > [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> > > [  236.253410] [] pci_dev_restore+0x4c/0x50
> > > [  236.258887] [] pci_bus_restore+0x24/0x4c
> > > [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> > > [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> > > [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> > > [vfio]
> > > [  236.283587] [] do_vfs_ioctl+0xac/0x744
> > > [  236.288890] [] SyS_ioctl+0x84/0x98
> > > [  236.293846] [] __sys_trace_return+0x0/0x4
> > > 
> > > These are caused by the inability of the PCIe root port and Intel
> > > e1000e to sucessfully do a bus reset.
> > > 
> > > The proposed fix is to not do a bus reset on these systems.
> > > 
> > > David Daney (2):
> > >   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
> > >   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> > > 
> > >  drivers/pci/pci.c| 4 
> > >  drivers/pci/quirks.c | 8 
> > >  2 files changed, 12 insertions(+)  
> > 
> > Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
> > v4.13, thanks!
> 
> Hmm, well let me again express my concerns that I'm really not sure how
> to support this since it removes our last opportunity to reset devices
> that may otherwise have no reset mechanism.  Certain classes of devices
> are entirely unsupportable for the code path indicated above without a
> bus reset.  If we have an endpoint device that goes bonkers at a bus
> reset, at least we know it's going to behave just as poorly no matter
> what the host platform.  This series allows endpoints that work
> perfectly well on one host to be handled differently on another.  It
> certainly suggests something non-spec compliant about the root port
> implementation and I wish there was more analysis about exactly what
> that problem is since this is coming from the hardware vendor.
> 
> https://lkml.org/lkml/2017/5/16/662

I almost poked you about this on IRC; guess I should have :)

Is it better to leave it as-is, and just take the aborts David
reported?

I agree, it would be nice to know what's really going on.  I assume
Cavium is interested in that as well to make sure future parts don't
have the issue.

Bjorn

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Alex Williamson 
On Tue, 23 May 2017 15:47:50 -0500
Bjorn Helgaas  wrote:

> On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> > With the recent improvements in arm64 and vfio-pci, we are seeing
> > failures like this (on cn8890 based systems):
> > 
> > [  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
> > 0xfc00c1000100
> > [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> > .
> > .
> > .
> > [  236.208820] [] pci_generic_config_read+0x38/0x9c
> > [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> > [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> > [  236.227596] [] 
> > pci_find_next_ext_capability.part.15+0x40/0xb8
> > [  236.234896] [] pci_find_ext_capability+0x20/0x30
> > [  236.241068] [] pci_restore_vc_state+0x34/0x88
> > [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> > [  236.253410] [] pci_dev_restore+0x4c/0x50
> > [  236.258887] [] pci_bus_restore+0x24/0x4c
> > [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> > [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> > [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> > [vfio]
> > [  236.283587] [] do_vfs_ioctl+0xac/0x744
> > [  236.288890] [] SyS_ioctl+0x84/0x98
> > [  236.293846] [] __sys_trace_return+0x0/0x4
> > 
> > These are caused by the inability of the PCIe root port and Intel
> > e1000e to sucessfully do a bus reset.
> > 
> > The proposed fix is to not do a bus reset on these systems.
> > 
> > David Daney (2):
> >   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
> >   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> > 
> >  drivers/pci/pci.c| 4 
> >  drivers/pci/quirks.c | 8 
> >  2 files changed, 12 insertions(+)  
> 
> Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
> v4.13, thanks!

Hmm, well let me again express my concerns that I'm really not sure how
to support this since it removes our last opportunity to reset devices
that may otherwise have no reset mechanism.  Certain classes of devices
are entirely unsupportable for the code path indicated above without a
bus reset.  If we have an endpoint device that goes bonkers at a bus
reset, at least we know it's going to behave just as poorly no matter
what the host platform.  This series allows endpoints that work
perfectly well on one host to be handled differently on another.  It
certainly suggests something non-spec compliant about the root port
implementation and I wish there was more analysis about exactly what
that problem is since this is coming from the hardware vendor.

https://lkml.org/lkml/2017/5/16/662

Thanks,
Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Alex Williamson 
On Tue, 23 May 2017 15:47:50 -0500
Bjorn Helgaas  wrote:

> On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> > With the recent improvements in arm64 and vfio-pci, we are seeing
> > failures like this (on cn8890 based systems):
> > 
> > [  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
> > 0xfc00c1000100
> > [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> > .
> > .
> > .
> > [  236.208820] [] pci_generic_config_read+0x38/0x9c
> > [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> > [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> > [  236.227596] [] 
> > pci_find_next_ext_capability.part.15+0x40/0xb8
> > [  236.234896] [] pci_find_ext_capability+0x20/0x30
> > [  236.241068] [] pci_restore_vc_state+0x34/0x88
> > [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> > [  236.253410] [] pci_dev_restore+0x4c/0x50
> > [  236.258887] [] pci_bus_restore+0x24/0x4c
> > [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> > [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> > [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> > [vfio]
> > [  236.283587] [] do_vfs_ioctl+0xac/0x744
> > [  236.288890] [] SyS_ioctl+0x84/0x98
> > [  236.293846] [] __sys_trace_return+0x0/0x4
> > 
> > These are caused by the inability of the PCIe root port and Intel
> > e1000e to sucessfully do a bus reset.
> > 
> > The proposed fix is to not do a bus reset on these systems.
> > 
> > David Daney (2):
> >   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
> >   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> > 
> >  drivers/pci/pci.c| 4 
> >  drivers/pci/quirks.c | 8 
> >  2 files changed, 12 insertions(+)  
> 
> Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
> v4.13, thanks!

Hmm, well let me again express my concerns that I'm really not sure how
to support this since it removes our last opportunity to reset devices
that may otherwise have no reset mechanism.  Certain classes of devices
are entirely unsupportable for the code path indicated above without a
bus reset.  If we have an endpoint device that goes bonkers at a bus
reset, at least we know it's going to behave just as poorly no matter
what the host platform.  This series allows endpoints that work
perfectly well on one host to be handled differently on another.  It
certainly suggests something non-spec compliant about the root port
implementation and I wish there was more analysis about exactly what
that problem is since this is coming from the hardware vendor.

https://lkml.org/lkml/2017/5/16/662

Thanks,
Alex

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Bjorn Helgaas 
On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
> 
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
> 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
> 
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.
> 
> The proposed fix is to not do a bus reset on these systems.
> 
> David Daney (2):
>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
>   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> 
>  drivers/pci/pci.c| 4 
>  drivers/pci/quirks.c | 8 
>  2 files changed, 12 insertions(+)

Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
v4.13, thanks!

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-23 Thread Bjorn Helgaas 
On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
> 
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
> 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
> 
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.
> 
> The proposed fix is to not do a bus reset on these systems.
> 
> David Daney (2):
>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
>   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> 
>  drivers/pci/pci.c| 4 
>  drivers/pci/quirks.c | 8 
>  2 files changed, 12 insertions(+)

Applied with Eric's reviewed-by and typo fixes to pci/virtualization for
v4.13, thanks!

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-16 Thread Auger Eric 
Hi David,

On 16/05/2017 02:17, David Daney wrote:
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
> 
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
> 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
> 
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.

I tested the series on Cavium ThunderX with an e1000e device and it
fixes the above issue.

Feel free to add
Reviewed-by: Eric Auger 

Thanks

Eric


> 
> The proposed fix is to not do a bus reset on these systems.
> 
> David Daney (2):
>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
>   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> 
>  drivers/pci/pci.c| 4 
>  drivers/pci/quirks.c | 8 
>  2 files changed, 12 insertions(+)
>

Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-16 Thread Auger Eric 
Hi David,

On 16/05/2017 02:17, David Daney wrote:
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
> 
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
> 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
> 
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.

I tested the series on Cavium ThunderX with an e1000e device and it
fixes the above issue.

Feel free to add
Reviewed-by: Eric Auger 

Thanks

Eric


> 
> The proposed fix is to not do a bus reset on these systems.
> 
> David Daney (2):
>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
>   PCI: Avoid bus reset for Cavium cn8xxx root ports.
> 
>  drivers/pci/pci.c| 4 
>  drivers/pci/quirks.c | 8 
>  2 files changed, 12 insertions(+)
>

[PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-15 Thread David Daney 
With the recent improvements in arm64 and vfio-pci, we are seeing
failures like this (on cn8890 based systems):

[  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
0xfc00c1000100
[  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
.
.
.
[  236.208820] [] pci_generic_config_read+0x38/0x9c
[  236.214992] [] thunder_pem_config_read+0x54/0x1e8
[  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
[  236.227596] [] 
pci_find_next_ext_capability.part.15+0x40/0xb8
[  236.234896] [] pci_find_ext_capability+0x20/0x30
[  236.241068] [] pci_restore_vc_state+0x34/0x88
[  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
[  236.253410] [] pci_dev_restore+0x4c/0x50
[  236.258887] [] pci_bus_restore+0x24/0x4c
[  236.264362] [] pci_try_reset_bus+0x7c/0xa0
[  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
[  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 [vfio]
[  236.283587] [] do_vfs_ioctl+0xac/0x744
[  236.288890] [] SyS_ioctl+0x84/0x98
[  236.293846] [] __sys_trace_return+0x0/0x4

These are caused by the inability of the PCIe root port and Intel
e1000e to sucessfully do a bus reset.

The proposed fix is to not do a bus reset on these systems.

David Daney (2):
  PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
  PCI: Avoid bus reset for Cavium cn8xxx root ports.

 drivers/pci/pci.c| 4 
 drivers/pci/quirks.c | 8 
 2 files changed, 12 insertions(+)

-- 
2.9.4

[PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-15 Thread David Daney 
With the recent improvements in arm64 and vfio-pci, we are seeing
failures like this (on cn8890 based systems):

[  235.622361] Unhandled fault: synchronous external abort (0x96000210) at 
0xfc00c1000100
[  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
.
.
.
[  236.208820] [] pci_generic_config_read+0x38/0x9c
[  236.214992] [] thunder_pem_config_read+0x54/0x1e8
[  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
[  236.227596] [] 
pci_find_next_ext_capability.part.15+0x40/0xb8
[  236.234896] [] pci_find_ext_capability+0x20/0x30
[  236.241068] [] pci_restore_vc_state+0x34/0x88
[  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
[  236.253410] [] pci_dev_restore+0x4c/0x50
[  236.258887] [] pci_bus_restore+0x24/0x4c
[  236.264362] [] pci_try_reset_bus+0x7c/0xa0
[  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
[  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 [vfio]
[  236.283587] [] do_vfs_ioctl+0xac/0x744
[  236.288890] [] SyS_ioctl+0x84/0x98
[  236.293846] [] __sys_trace_return+0x0/0x4

These are caused by the inability of the PCIe root port and Intel
e1000e to sucessfully do a bus reset.

The proposed fix is to not do a bus reset on these systems.

David Daney (2):
  PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device.
  PCI: Avoid bus reset for Cavium cn8xxx root ports.

 drivers/pci/pci.c| 4 
 drivers/pci/quirks.c | 8 
 2 files changed, 12 insertions(+)

-- 
2.9.4