Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1530866 > unrolled thread

[PATCH] vfio/pci: Support error recovery

Started byCao jin <caoj.fnst@cn.fujitsu.com>
First post2016-11-27 12:40 +0100
Last post2016-12-07 04:00 +0100
Articles 8 on this page of 28 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-11-27 12:40 +0100
    Re: [PATCH] vfio/pci: Support error recovery "Michael S. Tsirkin" <mst@redhat.com> - 2016-11-28 04:10 +0100
      Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-11-28 10:30 +0100
        Re: [PATCH] vfio/pci: Support error recovery "Michael S. Tsirkin" <mst@redhat.com> - 2016-11-30 02:50 +0100
          Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-01 14:40 +0100
    Re: [PATCH] vfio/pci: Support error recovery Alex Williamson <alex.williamson@redhat.com> - 2016-12-01 05:10 +0100
      Re: [PATCH] vfio/pci: Support error recovery "Michael S. Tsirkin" <mst@redhat.com> - 2016-12-01 06:10 +0100
        Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-01 14:40 +0100
          Re: [PATCH] vfio/pci: Support error recovery "Michael S. Tsirkin" <mst@redhat.com> - 2016-12-06 04:50 +0100
            Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-06 07:50 +0100
      Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-01 14:40 +0100
        Re: [PATCH] vfio/pci: Support error recovery Alex Williamson <alex.williamson@redhat.com> - 2016-12-01 16:00 +0100
          Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-04 13:20 +0100
            Re: [PATCH] vfio/pci: Support error recovery Alex Williamson <alex.williamson@redhat.com> - 2016-12-04 16:40 +0100
              Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-05 06:50 +0100
                Re: [PATCH] vfio/pci: Support error recovery Alex Williamson <alex.williamson@redhat.com> - 2016-12-05 17:20 +0100
                  Re: [PATCH] vfio/pci: Support error recovery "Michael S. Tsirkin" <mst@redhat.com> - 2016-12-06 05:00 +0100
                    Re: [PATCH] vfio/pci: Support error recovery Alex Williamson <alex.williamson@redhat.com> - 2016-12-06 06:10 +0100
                      Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-06 11:50 +0100
                        Re: [PATCH] vfio/pci: Support error recovery Alex Williamson <alex.williamson@redhat.com> - 2016-12-06 16:40 +0100
                          Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-07 03:50 +0100
                          Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-08 15:50 +0100
                            Re: [PATCH] vfio/pci: Support error recovery "Michael S. Tsirkin" <mst@redhat.com> - 2016-12-08 17:40 +0100
                              Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-09 08:50 +0100
                            Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-09 08:50 +0100
                  Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-06 07:10 +0100
                    Re: [PATCH] vfio/pci: Support error recovery Alex Williamson <alex.williamson@redhat.com> - 2016-12-06 16:30 +0100
                      Re: [PATCH] vfio/pci: Support error recovery Cao jin <caoj.fnst@cn.fujitsu.com> - 2016-12-07 04:00 +0100

Page 2 of 2 — ← Prev page 1 [2]


#1537436

FromCao jin <caoj.fnst@cn.fujitsu.com>
Date2016-12-07 03:50 +0100
Message-ID<sLAsV-22T-19@gated-at.bofh.it>
In reply to#1537049

On 12/06/2016 11:35 PM, Alex Williamson wrote:
> On Tue, 6 Dec 2016 18:46:04 +0800
> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> 
>> On 12/06/2016 12:59 PM, Alex Williamson wrote:
>>> On Tue, 6 Dec 2016 05:55:28 +0200
>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>   
>>>> On Mon, Dec 05, 2016 at 09:17:30AM -0700, Alex Williamson wrote:  
>>>>> If you're going to take the lead for these AER patches, I would
>>>>> certainly suggest that understanding the reasoning behind the bus reset
>>>>> behavior is a central aspect to this series.  This effort has dragged
>>>>> out for nearly two years and I apologize, but I don't really have a lot
>>>>> of patience for rehashing some of these issues if you're not going to
>>>>> read the previous discussions or consult with your colleagues to
>>>>> understand how we got to this point.  If you want to challenge some of
>>>>> the design points, that's great, it could use some new eyes, but please
>>>>> understand how we got here first.    
>>>>
>>>> Well I'm guessing Cao jin here isn't the only one not
>>>> willing to plough through all historical versions of the patchset
>>>> just to figure out the motivation for some code.
>>>>
>>>> Including a summary of a high level architecture couldn't hurt.
>>>>
>>>> Any chance of writing such?  Alternatively, we can try to build it as
>>>> part of this thread.  Shouldn't be hard as it seems somewhat
>>>> straight-forward on the surface:
>>>>
>>>> - detect link error on the host, don't reset link as we would normally do  
>>>
>>> This is actually a new approach that I'm not sure I agree with.  By
>>> skipping the host directed link reset, vfio is taking responsibility
>>> for doing this, but then we just assume the user will do it.  I have
>>> issues with this.
>>>
>>> The previous approach was to use the error detected notifier to block
>>> access to the device, allowing the host to perform the link reset.  A
>>> subsequent notification in the AER process released the user access
>>> which allowed the user AER process to proceed.  This did result in both
>>> a host directed and a guest directed link reset, but other than
>>> coordinating the blocking of the user process during host reset, that
>>> hasn't been brought up as an issue previously.
>>>   
>>
>> Tests on previous versions didn't bring up issues as I find, I think
>> that is because we didn't test it completely. As I know, before August
>> of this year, we didn't have cable connected to NIC, let alone
>> connecting NIC to gateway.
> 
> Lack of testing has been a significant issue throughout the development
> of this series.
> 
>> Even if I fixed the guest oops issue in igb driver that Alex found in
>> v9, v9 still cannot work in my test. And in my test, disable link
>> reset(in host) in aer core for vfio-pci is the most significant step to
>> get my test passed.
> 
> But is it the correct step?  I'm not convinced.  Why did blocking guest
> access not work?  How do you plan to manage vfio taking the
> responsibility to perform a bus reset when you don't know whether QEMU
> is the user of the device or whether the user supports AER recovery?
>  
>>>> - report link error to guest
>>>> - detect link reset request from guest
>>>> - reset link on host
>>>>
>>>> Since link reset will reset all devices behind it, for this to work we
>>>> need same set of devices behind the link in host and guest.  Enforcing
>>>> this would be nice to have.  
>>>
>>> This is a pretty significant complication and I think it's a
>>> requirement.  This is the heart of why we have an AER vfio-pci device
>>> option and why we require that QEMU should fail to initialize the
>>> device if AER is enabled in an incompatible configuration.  If a user
>>> cannot be sure that AER is enabled on a device, it's pointless to
>>> bother implementing it, though honestly I question the value of it in
>>> the VM altogether given configuration requirements (ie. are users
>>> going to accept the reason that all the ports of a device need to be
>>> assigned to a single guest for guest-based AER recovery when they were
>>> previously able to assign each port to a separate VM?).
>>>    
>>>> - as link now might end up in bad state, reset
>>>>   it when device is unassigned  
>>>
>>> This is also a new aspect for the approach here, previously we allowed
>>> the host directed link reset so we could assume the device was
>>> sufficiently recovered.  In the proposal here, the AER core skips any
>>> devices bound to vfio-pci, but vfio can't guarantee that we can do a
>>> bus reset on them.  PCI bus isolation is not accounted for in DMA
>>> isolation, which is the basis for IOMMU groups.  A bus can host
>>> multiple IOMMU groups, each of which may have a different user.  Only
>>> in a very specific configuration can vfio do a bus reset.
>>>    
>>>> Any details I missed?  
>>>
>>> AIUI, the critical feature is that the guest needs to be able to reset
>>> the device link, all the other design elements play out from the
>>> logical expansion of that feature.  It means that a guest bus reset
>>> needs to translate to a host bus reset, which means that all of the
>>> affected host devices need to be accounted for and those that are
>>> assigned to the guest need to be affected in the guest in the same
>>> way.  QEMU must enforce this configuration or else a user cannot know
>>> the result of a AER fault, ie. will it cause a VMSTOP condition or is
>>> the fault forwarded to the guest.  The required configuration
>>> restrictions are quite involved, therefore we can't simply require this
>>> of all configurations, so a vfio-pci device option is added.  The
>>> coordination of the host directed reset with the guest directed reset
>>> was only a complications discovered within the last few revisions of
>>> the series.  As noted above, the previous solution to this was to
>>> attempt to block access to the device while the host reset proceeds.
>>>
>>> Clearly it's a little disconcerting if we throw all of that away and
>>> simply assume that an FLR is sufficient to reset the device when it
>>> seems like link issues might be a nontrivial source of AER faults.  If
>>> FLR is sufficient, then why does the core AER handling code in the
>>> kernel not simply do this?  Thanks,
>>>   
>>
>> I agree with the points here.  Now I understand why  translate a guest
>> link reset to host link reset[*], and FLR shouldn't be equivalent to
>> link reset, but the PITY is, we can't trigger a real fatal error to see
>> if a FLR is sufficient to reset the device.
> 
> In this case, it's not a matter of finding a scenario where it works.
> Using FLR to recover from a fatal error would need to be supported by
> verbiage in a specification.  I'm not interested in cases where it
> happens to work on single device for a certain type of error.  The link
> reset is the bare metal mechanism for recovering from a fatal error and
> it should be our mechanism as well unless there's spec wording to
> support another approach.  Thanks,
> 

Ok, I understand your points here, will drag that dropped patch back and
test.

-- 
Sincerely,
Cao jin

[toc] | [prev] | [next] | [standalone]


#1538581

FromCao jin <caoj.fnst@cn.fujitsu.com>
Date2016-12-08 15:50 +0100
Message-ID<sM8bf-7j1-9@gated-at.bofh.it>
In reply to#1537049

On 12/06/2016 11:35 PM, Alex Williamson wrote:
> On Tue, 6 Dec 2016 18:46:04 +0800
> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> 
>> On 12/06/2016 12:59 PM, Alex Williamson wrote:
>>> On Tue, 6 Dec 2016 05:55:28 +0200
>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>   
>>>> On Mon, Dec 05, 2016 at 09:17:30AM -0700, Alex Williamson wrote:  
>>>>> If you're going to take the lead for these AER patches, I would
>>>>> certainly suggest that understanding the reasoning behind the bus reset
>>>>> behavior is a central aspect to this series.  This effort has dragged
>>>>> out for nearly two years and I apologize, but I don't really have a lot
>>>>> of patience for rehashing some of these issues if you're not going to
>>>>> read the previous discussions or consult with your colleagues to
>>>>> understand how we got to this point.  If you want to challenge some of
>>>>> the design points, that's great, it could use some new eyes, but please
>>>>> understand how we got here first.    
>>>>
>>>> Well I'm guessing Cao jin here isn't the only one not
>>>> willing to plough through all historical versions of the patchset
>>>> just to figure out the motivation for some code.
>>>>
>>>> Including a summary of a high level architecture couldn't hurt.
>>>>
>>>> Any chance of writing such?  Alternatively, we can try to build it as
>>>> part of this thread.  Shouldn't be hard as it seems somewhat
>>>> straight-forward on the surface:
>>>>
>>>> - detect link error on the host, don't reset link as we would normally do  
>>>
>>> This is actually a new approach that I'm not sure I agree with.  By
>>> skipping the host directed link reset, vfio is taking responsibility
>>> for doing this, but then we just assume the user will do it.  I have
>>> issues with this.
>>>
>>> The previous approach was to use the error detected notifier to block
>>> access to the device, allowing the host to perform the link reset.  A
>>> subsequent notification in the AER process released the user access
>>> which allowed the user AER process to proceed.  This did result in both
>>> a host directed and a guest directed link reset, but other than
>>> coordinating the blocking of the user process during host reset, that
>>> hasn't been brought up as an issue previously.
>>>   
>>
>> Tests on previous versions didn't bring up issues as I find, I think
>> that is because we didn't test it completely. As I know, before August
>> of this year, we didn't have cable connected to NIC, let alone
>> connecting NIC to gateway.
> 
> Lack of testing has been a significant issue throughout the development
> of this series.
> 
>> Even if I fixed the guest oops issue in igb driver that Alex found in
>> v9, v9 still cannot work in my test. And in my test, disable link
>> reset(in host) in aer core for vfio-pci is the most significant step to
>> get my test passed.
> 
> But is it the correct step?  I'm not convinced.  Why did blocking guest
> access not work?  How do you plan to manage vfio taking the
> responsibility to perform a bus reset when you don't know whether QEMU
> is the user of the device or whether the user supports AER recovery?
>  

Maybe currently we don't have enough proof to prove the correctness, but
I think I did find some facts to prove that link reset in host is a big
trouble, and can answer part of questions above.

1st, some thoughts:
In pci-error-recovery.txt and do_recovery() of kernel tree, we can see,
a recovery consists of several steps(callbacks), link reset is one of
them, and except link reset, the others are seems kind of device
specific. In our case, both host & guest will do recovery, I think the
host recovery actually is some kind of fake recovery, see vfio-pci
driver's error_detected & resume callback, they don't do anything
special, mainly signal error to user, but the link reset in host "fake
reset" does some serious work, in other words, I think host does the
recovery incompletely, so I was thinking, why not just drop incompletely
host recovery(drop link reset) for vfio-pci, and let the guest take care
of the whole serious recovery.  This is part of the reason of why my
version looks like this.  But yes, I admit the issue Alex mentioned,
vfio can't guarantee that user will do a bus reset, this is an issue I
will keep looking for a solution.

2nd, some facts and analyzation from test:
In host, the relationship between time and behviour in each component
roughly looks as following:

     +   HW    +  host kernel   +     qemu      + guest kernel  +
     |         |(error recovery)|               |               |
     |         |                |               |               |
     |         | vfio-pci's     |               |               |
     |         | error_detected |               |               |
     |         | +              |               |               |
     |         | |              |               |               |
     |         | | error notify |               |               |
     |         | | via eventfd  |               |               |
     |         | +---------------> +----------+ |               |
     |         |                |  +vfio_err_ | |               |
     |         |                |  |notifier_ | |               |
     | +---- +<---+link reset   |  |handler   | |               |
     | | HW  | |                |  |          | |               |       
  | |     | |                |             | |               |
     | |r    | | vfio-pci's     |  |pass aer  | |               |
     | |e..  | | resume         |  |to guest  | |               |
     | |s.   | | (blocking end) |  |          | |               |
     | |e    | |                |  |   *2*    | |               |
     | |t    | |                |  +----+-----+ |               |
     | |i    | |                |       |       |               |
     | |n    | |                |       +--------> +----------+ |
     | |g    | |                |               |  | guest    | |
     | |     | |                |               |  | recovery | |
     | |     | |                |               |  | process  | |
     | |     | |                |               |  |(include  | |
     | |     | |                |               |  |register  | |
     | | *1* | |                |               |  |access)   | |
     | |     | |                |               |  |          | |
     | |     | |                |               |  |   *3*    | |
     |         |                |               |  +----------+ |
Time |         |                |               |               |
     |         |                |               |               |
     |         |                |               |               |
     |         |                |               |               |
     v         |                |               |               |

Now let me try to answer: Why did blocking guest access not work?
Some important factor:
1. host recovery doesn't do anything special except error notifying, so
it may be executed very fast.
2. Hardware resetting time is not sure, from pcie spec 6.6.1, guessing
it need many ms, pretty long?

some facts found in v9(block config write, not read, during host
recovery) test:
1. reading uncor error register in vfio_err_notifier_handler sometimes
returns correct value, sometimes return invalid value(All F's)

So, I am thinking, if host blocking on host end early, and *2*, *3* is
parallel with *1*, the way used in v9 to blocking guest access, may not
work.

-- 
Sincerely,
Cao jin

[toc] | [prev] | [next] | [standalone]


#1538714

From"Michael S. Tsirkin" <mst@redhat.com>
Date2016-12-08 17:40 +0100
Message-ID<sM9TI-8om-13@gated-at.bofh.it>
In reply to#1538581
On Thu, Dec 08, 2016 at 10:46:59PM +0800, Cao jin wrote:
> 
> 
> On 12/06/2016 11:35 PM, Alex Williamson wrote:
> > On Tue, 6 Dec 2016 18:46:04 +0800
> > Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> > 
> >> On 12/06/2016 12:59 PM, Alex Williamson wrote:
> >>> On Tue, 6 Dec 2016 05:55:28 +0200
> >>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>   
> >>>> On Mon, Dec 05, 2016 at 09:17:30AM -0700, Alex Williamson wrote:  
> >>>>> If you're going to take the lead for these AER patches, I would
> >>>>> certainly suggest that understanding the reasoning behind the bus reset
> >>>>> behavior is a central aspect to this series.  This effort has dragged
> >>>>> out for nearly two years and I apologize, but I don't really have a lot
> >>>>> of patience for rehashing some of these issues if you're not going to
> >>>>> read the previous discussions or consult with your colleagues to
> >>>>> understand how we got to this point.  If you want to challenge some of
> >>>>> the design points, that's great, it could use some new eyes, but please
> >>>>> understand how we got here first.    
> >>>>
> >>>> Well I'm guessing Cao jin here isn't the only one not
> >>>> willing to plough through all historical versions of the patchset
> >>>> just to figure out the motivation for some code.
> >>>>
> >>>> Including a summary of a high level architecture couldn't hurt.
> >>>>
> >>>> Any chance of writing such?  Alternatively, we can try to build it as
> >>>> part of this thread.  Shouldn't be hard as it seems somewhat
> >>>> straight-forward on the surface:
> >>>>
> >>>> - detect link error on the host, don't reset link as we would normally do  
> >>>
> >>> This is actually a new approach that I'm not sure I agree with.  By
> >>> skipping the host directed link reset, vfio is taking responsibility
> >>> for doing this, but then we just assume the user will do it.  I have
> >>> issues with this.
> >>>
> >>> The previous approach was to use the error detected notifier to block
> >>> access to the device, allowing the host to perform the link reset.  A
> >>> subsequent notification in the AER process released the user access
> >>> which allowed the user AER process to proceed.  This did result in both
> >>> a host directed and a guest directed link reset, but other than
> >>> coordinating the blocking of the user process during host reset, that
> >>> hasn't been brought up as an issue previously.
> >>>   
> >>
> >> Tests on previous versions didn't bring up issues as I find, I think
> >> that is because we didn't test it completely. As I know, before August
> >> of this year, we didn't have cable connected to NIC, let alone
> >> connecting NIC to gateway.
> > 
> > Lack of testing has been a significant issue throughout the development
> > of this series.
> > 
> >> Even if I fixed the guest oops issue in igb driver that Alex found in
> >> v9, v9 still cannot work in my test. And in my test, disable link
> >> reset(in host) in aer core for vfio-pci is the most significant step to
> >> get my test passed.
> > 
> > But is it the correct step?  I'm not convinced.  Why did blocking guest
> > access not work?  How do you plan to manage vfio taking the
> > responsibility to perform a bus reset when you don't know whether QEMU
> > is the user of the device or whether the user supports AER recovery?
> >  
> 
> Maybe currently we don't have enough proof to prove the correctness, but
> I think I did find some facts to prove that link reset in host is a big
> trouble, and can answer part of questions above.
> 
> 1st, some thoughts:
> In pci-error-recovery.txt and do_recovery() of kernel tree, we can see,
> a recovery consists of several steps(callbacks), link reset is one of
> them, and except link reset, the others are seems kind of device
> specific. In our case, both host & guest will do recovery, I think the
> host recovery actually is some kind of fake recovery, see vfio-pci
> driver's error_detected & resume callback, they don't do anything
> special, mainly signal error to user, but the link reset in host "fake
> reset" does some serious work, in other words, I think host does the
> recovery incompletely, so I was thinking, why not just drop incompletely
> host recovery(drop link reset) for vfio-pci, and let the guest take care
> of the whole serious recovery.  This is part of the reason of why my
> version looks like this.  But yes, I admit the issue Alex mentioned,
> vfio can't guarantee that user will do a bus reset, this is an issue I
> will keep looking for a solution.
> 
> 2nd, some facts and analyzation from test:
> In host, the relationship between time and behviour in each component
> roughly looks as following:
> 
>      +   HW    +  host kernel   +     qemu      + guest kernel  +
>      |         |(error recovery)|               |               |
>      |         |                |               |               |
>      |         | vfio-pci's     |               |               |
>      |         | error_detected |               |               |
>      |         | +              |               |               |
>      |         | |              |               |               |
>      |         | | error notify |               |               |
>      |         | | via eventfd  |               |               |
>      |         | +---------------> +----------+ |               |
>      |         |                |  +vfio_err_ | |               |
>      |         |                |  |notifier_ | |               |
>      | +---- +<---+link reset   |  |handler   | |               |
>      | | HW  | |                |  |          | |               |       
>   | |     | |                |             | |               |
>      | |r    | | vfio-pci's     |  |pass aer  | |               |
>      | |e..  | | resume         |  |to guest  | |               |
>      | |s.   | | (blocking end) |  |          | |               |
>      | |e    | |                |  |   *2*    | |               |
>      | |t    | |                |  +----+-----+ |               |
>      | |i    | |                |       |       |               |
>      | |n    | |                |       +--------> +----------+ |
>      | |g    | |                |               |  | guest    | |
>      | |     | |                |               |  | recovery | |
>      | |     | |                |               |  | process  | |
>      | |     | |                |               |  |(include  | |
>      | |     | |                |               |  |register  | |
>      | | *1* | |                |               |  |access)   | |
>      | |     | |                |               |  |          | |
>      | |     | |                |               |  |   *3*    | |
>      |         |                |               |  +----------+ |
> Time |         |                |               |               |
>      |         |                |               |               |
>      |         |                |               |               |
>      |         |                |               |               |
>      v         |                |               |               |
> 
> Now let me try to answer: Why did blocking guest access not work?
> Some important factor:
> 1. host recovery doesn't do anything special except error notifying, so
> it may be executed very fast.
> 2. Hardware resetting time is not sure, from pcie spec 6.6.1, guessing
> it need many ms, pretty long?
> 
> some facts found in v9(block config write, not read, during host
> recovery) test:
> 1. reading uncor error register in vfio_err_notifier_handler sometimes
> returns correct value, sometimes return invalid value(All F's)
> 
> So, I am thinking, if host blocking on host end early, and *2*, *3* is
> parallel with *1*, the way used in v9 to blocking guest access, may not
> work.

I don't think this answers Alex's question. He is simply asking
which kind of errors are we recovering from, and why is FLR
sufficient. All this info is completely lacking in
current linux code, too. Why does it reset the link?
Express spec certainly does not say we should.
Link reset is the strongest kind of reset we can do though,
so maybe the thinking goes, let's just do it.

You would like to weaken this, but it's hard to
be sure it's enough, we are dealing with misbehaving
hardware after all.

So what would help, is a list:

- error type
- recovery required
- how to do it in a VM

As for blocking guest access, I'm not sure why it's important
to do. Guests can put devices in bad states, that's a given.
Why isn't it enough to tell guest that device is in a bad state?
I think it will try to avoid accesses then, if it does not,
it is not supposed to do anything too bad.

> -- 
> Sincerely,
> Cao jin
> 

[toc] | [prev] | [next] | [standalone]


#1539157

FromCao jin <caoj.fnst@cn.fujitsu.com>
Date2016-12-09 08:50 +0100
Message-ID<sMo6l-ug-1@gated-at.bofh.it>
In reply to#1538714

On 12/09/2016 12:30 AM, Michael S. Tsirkin wrote:
> On Thu, Dec 08, 2016 at 10:46:59PM +0800, Cao jin wrote:
>>
>>
>> On 12/06/2016 11:35 PM, Alex Williamson wrote:
>>> On Tue, 6 Dec 2016 18:46:04 +0800
>>> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
>>>
>>>> On 12/06/2016 12:59 PM, Alex Williamson wrote:
>>>>> On Tue, 6 Dec 2016 05:55:28 +0200
>>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>>   
>>>>>> On Mon, Dec 05, 2016 at 09:17:30AM -0700, Alex Williamson wrote:  
>>>>>>> If you're going to take the lead for these AER patches, I would
>>>>>>> certainly suggest that understanding the reasoning behind the bus reset
>>>>>>> behavior is a central aspect to this series.  This effort has dragged
>>>>>>> out for nearly two years and I apologize, but I don't really have a lot
>>>>>>> of patience for rehashing some of these issues if you're not going to
>>>>>>> read the previous discussions or consult with your colleagues to
>>>>>>> understand how we got to this point.  If you want to challenge some of
>>>>>>> the design points, that's great, it could use some new eyes, but please
>>>>>>> understand how we got here first.    
>>>>>>
>>>>>> Well I'm guessing Cao jin here isn't the only one not
>>>>>> willing to plough through all historical versions of the patchset
>>>>>> just to figure out the motivation for some code.
>>>>>>
>>>>>> Including a summary of a high level architecture couldn't hurt.
>>>>>>
>>>>>> Any chance of writing such?  Alternatively, we can try to build it as
>>>>>> part of this thread.  Shouldn't be hard as it seems somewhat
>>>>>> straight-forward on the surface:
>>>>>>
>>>>>> - detect link error on the host, don't reset link as we would normally do  
>>>>>
>>>>> This is actually a new approach that I'm not sure I agree with.  By
>>>>> skipping the host directed link reset, vfio is taking responsibility
>>>>> for doing this, but then we just assume the user will do it.  I have
>>>>> issues with this.
>>>>>
>>>>> The previous approach was to use the error detected notifier to block
>>>>> access to the device, allowing the host to perform the link reset.  A
>>>>> subsequent notification in the AER process released the user access
>>>>> which allowed the user AER process to proceed.  This did result in both
>>>>> a host directed and a guest directed link reset, but other than
>>>>> coordinating the blocking of the user process during host reset, that
>>>>> hasn't been brought up as an issue previously.
>>>>>   
>>>>
>>>> Tests on previous versions didn't bring up issues as I find, I think
>>>> that is because we didn't test it completely. As I know, before August
>>>> of this year, we didn't have cable connected to NIC, let alone
>>>> connecting NIC to gateway.
>>>
>>> Lack of testing has been a significant issue throughout the development
>>> of this series.
>>>
>>>> Even if I fixed the guest oops issue in igb driver that Alex found in
>>>> v9, v9 still cannot work in my test. And in my test, disable link
>>>> reset(in host) in aer core for vfio-pci is the most significant step to
>>>> get my test passed.
>>>
>>> But is it the correct step?  I'm not convinced.  Why did blocking guest
>>> access not work?  How do you plan to manage vfio taking the
>>> responsibility to perform a bus reset when you don't know whether QEMU
>>> is the user of the device or whether the user supports AER recovery?
>>>  
>>
>> Maybe currently we don't have enough proof to prove the correctness, but
>> I think I did find some facts to prove that link reset in host is a big
>> trouble, and can answer part of questions above.
>>
>> 1st, some thoughts:
>> In pci-error-recovery.txt and do_recovery() of kernel tree, we can see,
>> a recovery consists of several steps(callbacks), link reset is one of
>> them, and except link reset, the others are seems kind of device
>> specific. In our case, both host & guest will do recovery, I think the
>> host recovery actually is some kind of fake recovery, see vfio-pci
>> driver's error_detected & resume callback, they don't do anything
>> special, mainly signal error to user, but the link reset in host "fake
>> reset" does some serious work, in other words, I think host does the
>> recovery incompletely, so I was thinking, why not just drop incompletely
>> host recovery(drop link reset) for vfio-pci, and let the guest take care
>> of the whole serious recovery.  This is part of the reason of why my
>> version looks like this.  But yes, I admit the issue Alex mentioned,
>> vfio can't guarantee that user will do a bus reset, this is an issue I
>> will keep looking for a solution.
>>
>> 2nd, some facts and analyzation from test:
>> In host, the relationship between time and behviour in each component
>> roughly looks as following:
>>
>>      +   HW    +  host kernel   +     qemu      + guest kernel  +
>>      |         |(error recovery)|               |               |
>>      |         |                |               |               |
>>      |         | vfio-pci's     |               |               |
>>      |         | error_detected |               |               |
>>      |         | +              |               |               |
>>      |         | |              |               |               |
>>      |         | | error notify |               |               |
>>      |         | | via eventfd  |               |               |
>>      |         | +---------------> +----------+ |               |
>>      |         |                |  +vfio_err_ | |               |
>>      |         |                |  |notifier_ | |               |
>>      | +---- +<---+link reset   |  |handler   | |               |
>>      | | HW  | |                |  |          | |               |       
>>      | |     | |                |             | |               |
>>      | |r    | | vfio-pci's     |  |pass aer  | |               |
>>      | |e..  | | resume         |  |to guest  | |               |
>>      | |s.   | | (blocking end) |  |          | |               |
>>      | |e    | |                |  |   *2*    | |               |
>>      | |t    | |                |  +----+-----+ |               |
>>      | |i    | |                |       |       |               |
>>      | |n    | |                |       +--------> +----------+ |
>>      | |g    | |                |               |  | guest    | |
>>      | |     | |                |               |  | recovery | |
>>      | |     | |                |               |  | process  | |
>>      | |     | |                |               |  |(include  | |
>>      | |     | |                |               |  |register  | |
>>      | | *1* | |                |               |  |access)   | |
>>      | |     | |                |               |  |          | |
>>      | |     | |                |               |  |   *3*    | |
>>      |         |                |               |  +----------+ |
>> Time |         |                |               |               |
>>      |         |                |               |               |
>>      |         |                |               |               |
>>      |         |                |               |               |
>>      v         |                |               |               |
>>
>> Now let me try to answer: Why did blocking guest access not work?
>> Some important factor:
>> 1. host recovery doesn't do anything special except error notifying, so
>> it may be executed very fast.
>> 2. Hardware resetting time is not sure, from pcie spec 6.6.1, guessing
>> it need many ms, pretty long?
>>
>> some facts found in v9(block config write, not read, during host
>> recovery) test:
>> 1. reading uncor error register in vfio_err_notifier_handler sometimes
>> returns correct value, sometimes return invalid value(All F's)
>>
>> So, I am thinking, if host blocking on host end early, and *2*, *3* is
>> parallel with *1*, the way used in v9 to blocking guest access, may not
>> work.
> 
> I don't think this answers Alex's question. He is simply asking
> which kind of errors are we recovering from, and why is FLR
> sufficient. All this info is completely lacking in
> current linux code, too. Why does it reset the link?
> Express spec certainly does not say we should.
> Link reset is the strongest kind of reset we can do though,

I feel there is a mistake here. I have said several times that I
understand and agree with Alex's points: if I disable host link reset,
then "guest bus reset should induce a host bus reset" on fatal error,
and FLR is not sufficient.  This is a fault in this version, I will
address it in next version.

Alex's question "Why did blocking guest access not work?", make me feel
he is talking the way used in last version kernel patch[*], to block
config write, but not read, in host recovery. I guess it is also a
result of long discussion. But it[*] doesn't work well, that's why I
draw a figure to explain.

[*]http://lists.nongnu.org/archive/html/qemu-devel/2016-08/msg00155.html

I am aware of "Link reset is the strongest kind of reset we can do
though", I even spent a whole day on "what the link reset exactly means"
digging in express spec, and induced some unrestrained thoughts:

(I don't have electrical engineering background, and the following
thoughts don't have relationship with our discussion)
set secondary bus reset bit direct physical layer to send TS1(or TS2)
twice(express spec 4.2.5.11) on the link, device received these TS, then
reset itself(my inference, didn't see exact words). So if data(TS) can
still be transferred correctly on physical layer, is link unreliable? or
the *unreliable* is targeting data link layer and transaction Layer?

> so maybe the thinking goes, let's just do it.
> 

Sure I will. But I still have thoughts unsolved on the restrictions of
configuration, there may be another mail on this topic later.

> You would like to weaken this, but it's hard to
> be sure it's enough, we are dealing with misbehaving
> hardware after all.
> 
> So what would help, is a list:
> 
> - error type
> - recovery required
> - how to do it in a VM
> 
> As for blocking guest access, I'm not sure why it's important
> to do. Guests can put devices in bad states, that's a given.
> Why isn't it enough to tell guest that device is in a bad state?
> I think it will try to avoid accesses then, if it does not,
> it is not supposed to do anything too bad.
> 
>> -- 
>> Sincerely,
>> Cao jin
>>
> 
> 
> .
> 

-- 
Sincerely,
Cao jin

[toc] | [prev] | [next] | [standalone]


#1539164

FromCao jin <caoj.fnst@cn.fujitsu.com>
Date2016-12-09 08:50 +0100
Message-ID<sMo6m-ug-25@gated-at.bofh.it>
In reply to#1538581

On 12/08/2016 10:46 PM, Cao jin wrote:
> 
> 
> On 12/06/2016 11:35 PM, Alex Williamson wrote:
>> On Tue, 6 Dec 2016 18:46:04 +0800
>> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
>>
>>> On 12/06/2016 12:59 PM, Alex Williamson wrote:
>>>> On Tue, 6 Dec 2016 05:55:28 +0200
>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>   
>>>>> On Mon, Dec 05, 2016 at 09:17:30AM -0700, Alex Williamson wrote:  
>>>>>> If you're going to take the lead for these AER patches, I would
>>>>>> certainly suggest that understanding the reasoning behind the bus reset
>>>>>> behavior is a central aspect to this series.  This effort has dragged
>>>>>> out for nearly two years and I apologize, but I don't really have a lot
>>>>>> of patience for rehashing some of these issues if you're not going to
>>>>>> read the previous discussions or consult with your colleagues to
>>>>>> understand how we got to this point.  If you want to challenge some of
>>>>>> the design points, that's great, it could use some new eyes, but please
>>>>>> understand how we got here first.    
>>>>>
>>>>> Well I'm guessing Cao jin here isn't the only one not
>>>>> willing to plough through all historical versions of the patchset
>>>>> just to figure out the motivation for some code.
>>>>>
>>>>> Including a summary of a high level architecture couldn't hurt.
>>>>>
>>>>> Any chance of writing such?  Alternatively, we can try to build it as
>>>>> part of this thread.  Shouldn't be hard as it seems somewhat
>>>>> straight-forward on the surface:
>>>>>
>>>>> - detect link error on the host, don't reset link as we would normally do  
>>>>
>>>> This is actually a new approach that I'm not sure I agree with.  By
>>>> skipping the host directed link reset, vfio is taking responsibility
>>>> for doing this, but then we just assume the user will do it.  I have
>>>> issues with this.
>>>>
>>>> The previous approach was to use the error detected notifier to block
>>>> access to the device, allowing the host to perform the link reset.  A
>>>> subsequent notification in the AER process released the user access
>>>> which allowed the user AER process to proceed.  This did result in both
>>>> a host directed and a guest directed link reset, but other than
>>>> coordinating the blocking of the user process during host reset, that
>>>> hasn't been brought up as an issue previously.
>>>>   
>>>
>>> Tests on previous versions didn't bring up issues as I find, I think
>>> that is because we didn't test it completely. As I know, before August
>>> of this year, we didn't have cable connected to NIC, let alone
>>> connecting NIC to gateway.
>>
>> Lack of testing has been a significant issue throughout the development
>> of this series.
>>
>>> Even if I fixed the guest oops issue in igb driver that Alex found in
>>> v9, v9 still cannot work in my test. And in my test, disable link
>>> reset(in host) in aer core for vfio-pci is the most significant step to
>>> get my test passed.
>>
>> But is it the correct step?  I'm not convinced.  Why did blocking guest
>> access not work?  How do you plan to manage vfio taking the
>> responsibility to perform a bus reset when you don't know whether QEMU
>> is the user of the device or whether the user supports AER recovery?
>>  
> 
> Maybe currently we don't have enough proof to prove the correctness, but
> I think I did find some facts to prove that link reset in host is a big
> trouble, and can answer part of questions above.
> 
> 1st, some thoughts:
> In pci-error-recovery.txt and do_recovery() of kernel tree, we can see,
> a recovery consists of several steps(callbacks), link reset is one of
> them, and except link reset, the others are seems kind of device
> specific. In our case, both host & guest will do recovery, I think the
> host recovery actually is some kind of fake recovery, see vfio-pci
> driver's error_detected & resume callback, they don't do anything
> special, mainly signal error to user, but the link reset in host "fake
> reset" does some serious work, in other words, I think host does the
> recovery incompletely, so I was thinking, why not just drop incompletely
> host recovery(drop link reset) for vfio-pci, and let the guest take care
> of the whole serious recovery.  This is part of the reason of why my
> version looks like this.  But yes, I admit the issue Alex mentioned,
> vfio can't guarantee that user will do a bus reset, this is an issue I
> will keep looking for a solution.
> 
> 2nd, some facts and analyzation from test:
> In host, the relationship between time and behviour in each component
> roughly looks as following:
> 
>      +   HW    +  host kernel   +     qemu      + guest kernel  +
>      |         |(error recovery)|               |               |
>      |         |                |               |               |
>      |         | vfio-pci's     |               |               |
>      |         | error_detected |               |               |
>      |         | +              |               |               |
>      |         | |              |               |               |
>      |         | | error notify |               |               |
>      |         | | via eventfd  |               |               |
>      |         | +---------------> +----------+ |               |
>      |         |                |  +vfio_err_ | |               |
>      |         |                |  |notifier_ | |               |
>      | +---- +<---+link reset   |  |handler   | |               |
>      | | HW  | |                |  |          | |               |       
>   | |     | |                |             | |               |
>      | |r    | | vfio-pci's     |  |pass aer  | |               |
>      | |e..  | | resume         |  |to guest  | |               |
>      | |s.   | | (blocking end) |  |          | |               |
>      | |e    | |                |  |   *2*    | |               |
>      | |t    | |                |  +----+-----+ |               |
>      | |i    | |                |       |       |               |
>      | |n    | |                |       +--------> +----------+ |
>      | |g    | |                |               |  | guest    | |
>      | |     | |                |               |  | recovery | |
>      | |     | |                |               |  | process  | |
>      | |     | |                |               |  |(include  | |
>      | |     | |                |               |  |register  | |
>      | | *1* | |                |               |  |access)   | |
>      | |     | |                |               |  |          | |
>      | |     | |                |               |  |   *3*    | |
>      |         |                |               |  +----------+ |
> Time |         |                |               |               |
>      |         |                |               |               |
>      |         |                |               |               |
>      |         |                |               |               |
>      v         |                |               |               |
> 
> Now let me try to answer: Why did blocking guest access not work?
> Some important factor:
> 1. host recovery doesn't do anything special except error notifying, so
> it may be executed very fast.
> 2. Hardware resetting time is not sure, from pcie spec 6.6.1, guessing
> it need many ms, pretty long?
> 
> some facts found in v9(block config write, not read, during host
> recovery) test:
> 1. reading uncor error register in vfio_err_notifier_handler sometimes
> returns correct value, sometimes return invalid value(All F's)
> 

I forget another facts:
2. the igb bug[*] our patch v9 triggered also is the same kind things.
error_detected() of igb in guest read config space, and return invalid
all F's, then igb NULLed its hardware address, then oops shows.

I think it is also a concurrent issue as described below.

[*] http://patchwork.ozlabs.org/patch/692171

> So, I am thinking, if host blocking on host end early, and *2*, *3* is
> parallel with *1*, the way used in v9 to blocking guest access, may not
> work.
> 

-- 
Sincerely,
Cao jin

[toc] | [prev] | [next] | [standalone]


#1536678

FromCao jin <caoj.fnst@cn.fujitsu.com>
Date2016-12-06 07:10 +0100
Message-ID<sLh6W-6Hb-5@gated-at.bofh.it>
In reply to#1536203

On 12/06/2016 12:17 AM, Alex Williamson wrote:
> On Mon, 5 Dec 2016 13:52:03 +0800
> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> 
>> On 12/04/2016 11:30 PM, Alex Williamson wrote:
>>> On Sun, 4 Dec 2016 20:16:42 +0800
>>> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
>>>   
>>>> On 12/01/2016 10:55 PM, Alex Williamson wrote:  
>>>>> On Thu, 1 Dec 2016 21:40:00 +0800    
>>>>  
>>>>>>> If an AER fault occurs and the user doesn't do a reset, what
>>>>>>> happens when that device is released and a host driver tries to make
>>>>>>> use of it?  The user makes no commitment to do a reset and there are
>>>>>>> only limited configurations where we even allow the user to perform a
>>>>>>> reset.
>>>>>>>       
>>>>>>
>>>>>> Limited? Do you mean the things __pci_dev_reset() can do?    
>>>>>
>>>>> I mean that there are significant device and guest configuration
>>>>> restrictions in order to support AER.  For instance, all the functions
>>>>> of the slot need to appear in a PCI-e topology in the guest with all
>>>>> the functions in the right place such that a guest bus reset translates
>>>>> into a host bus reset.  The physical functions cannot be split between
>>>>> guests even if IOMMU isolation would otherwise allow it.  The user
>>>>> needs to explicitly enable AER support for the devices.  A VM need to
>>>>> be specifically configured for AER support in order to set any sort of
>>>>> expectations of a guest directed bus reset, let alone a guarantee that
>>>>> it will happen.  So all the existing VMs, where functions are split
>>>>> between guests, or the topology isn't exactly right, or AER isn't
>>>>> enabled see a regression from the above change as the device is no
>>>>> longer reset.
>>>>>     
>>>>
>>>> I am not clear why set these restrictions in the current design. I take
>>>> a glance at older versions of qemu's patchset, their thoughts is:
>>>> translate a guest bus reset into a host bus reset(Which is
>>>> unreasonable[*] to me). And I guess, that's the *cause* of these
>>>> restrictions?  Is there any other stories behind these restrictions?
>>>>
>>>> [*] In physical world, set bridge's secondary bus reset would send
>>>> hot-reset TLP to all functions below, trigger every device's reset
>>>> separately. Emulated device should behave the same, means just using
>>>> each device's DeviceClass->reset method.  
>>>
>>> Are you trying to say that an FLR is equivalent to a link reset?  
>>
>> No.  Look at old versions patchset, there is one names "vote the
>> function 0 to do host bus reset when aer occurred"[1], that is what I
>> called "translate guest link reset to host link reset", and what I think
>> unreasonable(and I think it also does it wrongly).  So in v10 version of
>> mine, I dropped it.
>>
>> [1]https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg02987.html
>>
>> If "translate guest link reset to host link reset" is right, I can
>> understand these restrictions[2][3].
>>
>> [2]. All physical functions in a single card must be assigned to the VM
>>      with AER enabled on each and configured on the same virtual bus.
>> [3]. Don't place other devices under the virtual bus in [2], no matter
>>      physical, emulated, or paravirtual, even if other device
>>      supporting AER signaling
>>
>> Certain device's FLR calls its DeviceClass->reset method; link reset
>> calls DeviceClass->reset of each device which on the bus. So, apparently
>> they have difference.  But if there is only 1 vfio-pci device under the
>> virtual pci bus,  I think FLR can be equivalent to a link reset, right?
> 
> No.  An FLR resets the device while a secondary bus reset does a reset
> of the link and the device.  AER errors are sometimes issues with the
> link, not the device.  If we were to perform only an FLR, we're not
> performing the same reset as would be done on bare metal.
>  

Thanks for you explanation, it does helps, except the last sentence, I
think I understand it now: fatal error implies there may be link issue
exists(pci express spec: 6.2.2.2.1), so, should do link reset for fatal
error(that is what and why aer core does). And so, in patch[1] above,
qemu does a link reset when seeing secondary bus reset bit of virtual
bus got set. is it right?

-- 
Sincerely,
Cao jin

[toc] | [prev] | [next] | [standalone]


#1537037

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-12-06 16:30 +0100
Message-ID<sLpQS-3Md-13@gated-at.bofh.it>
In reply to#1536678
On Tue, 6 Dec 2016 14:11:03 +0800
Cao jin <caoj.fnst@cn.fujitsu.com> wrote:

> On 12/06/2016 12:17 AM, Alex Williamson wrote:
> > On Mon, 5 Dec 2016 13:52:03 +0800
> > Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> >   
> >> On 12/04/2016 11:30 PM, Alex Williamson wrote:  
> >>> On Sun, 4 Dec 2016 20:16:42 +0800
> >>> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> >>>     
> >>>> On 12/01/2016 10:55 PM, Alex Williamson wrote:    
> >>>>> On Thu, 1 Dec 2016 21:40:00 +0800      
> >>>>    
> >>>>>>> If an AER fault occurs and the user doesn't do a reset, what
> >>>>>>> happens when that device is released and a host driver tries to make
> >>>>>>> use of it?  The user makes no commitment to do a reset and there are
> >>>>>>> only limited configurations where we even allow the user to perform a
> >>>>>>> reset.
> >>>>>>>         
> >>>>>>
> >>>>>> Limited? Do you mean the things __pci_dev_reset() can do?      
> >>>>>
> >>>>> I mean that there are significant device and guest configuration
> >>>>> restrictions in order to support AER.  For instance, all the functions
> >>>>> of the slot need to appear in a PCI-e topology in the guest with all
> >>>>> the functions in the right place such that a guest bus reset translates
> >>>>> into a host bus reset.  The physical functions cannot be split between
> >>>>> guests even if IOMMU isolation would otherwise allow it.  The user
> >>>>> needs to explicitly enable AER support for the devices.  A VM need to
> >>>>> be specifically configured for AER support in order to set any sort of
> >>>>> expectations of a guest directed bus reset, let alone a guarantee that
> >>>>> it will happen.  So all the existing VMs, where functions are split
> >>>>> between guests, or the topology isn't exactly right, or AER isn't
> >>>>> enabled see a regression from the above change as the device is no
> >>>>> longer reset.
> >>>>>       
> >>>>
> >>>> I am not clear why set these restrictions in the current design. I take
> >>>> a glance at older versions of qemu's patchset, their thoughts is:
> >>>> translate a guest bus reset into a host bus reset(Which is
> >>>> unreasonable[*] to me). And I guess, that's the *cause* of these
> >>>> restrictions?  Is there any other stories behind these restrictions?
> >>>>
> >>>> [*] In physical world, set bridge's secondary bus reset would send
> >>>> hot-reset TLP to all functions below, trigger every device's reset
> >>>> separately. Emulated device should behave the same, means just using
> >>>> each device's DeviceClass->reset method.    
> >>>
> >>> Are you trying to say that an FLR is equivalent to a link reset?    
> >>
> >> No.  Look at old versions patchset, there is one names "vote the
> >> function 0 to do host bus reset when aer occurred"[1], that is what I
> >> called "translate guest link reset to host link reset", and what I think
> >> unreasonable(and I think it also does it wrongly).  So in v10 version of
> >> mine, I dropped it.
> >>
> >> [1]https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg02987.html
> >>
> >> If "translate guest link reset to host link reset" is right, I can
> >> understand these restrictions[2][3].
> >>
> >> [2]. All physical functions in a single card must be assigned to the VM
> >>      with AER enabled on each and configured on the same virtual bus.
> >> [3]. Don't place other devices under the virtual bus in [2], no matter
> >>      physical, emulated, or paravirtual, even if other device
> >>      supporting AER signaling
> >>
> >> Certain device's FLR calls its DeviceClass->reset method; link reset
> >> calls DeviceClass->reset of each device which on the bus. So, apparently
> >> they have difference.  But if there is only 1 vfio-pci device under the
> >> virtual pci bus,  I think FLR can be equivalent to a link reset, right?  
> > 
> > No.  An FLR resets the device while a secondary bus reset does a reset
> > of the link and the device.  AER errors are sometimes issues with the
> > link, not the device.  If we were to perform only an FLR, we're not
> > performing the same reset as would be done on bare metal.
> >    
> 
> Thanks for you explanation, it does helps, except the last sentence, I
> think I understand it now: fatal error implies there may be link issue
> exists(pci express spec: 6.2.2.2.1), so, should do link reset for fatal
> error(that is what and why aer core does). And so, in patch[1] above,
> qemu does a link reset when seeing secondary bus reset bit of virtual
> bus got set. is it right?

QEMU is only going to do a bus reset if the guest is participating in
AER recovery AND QEMU supports AER recovery AND the guest topology
configuration allows the guest bus reset to induce a host bus reset.
vfio does not know that QEMU is the user and cannot assume the user
will perform a bus reset.  We need to give the user the ability to
recover from an AER, but not rely on the user to do so.  We cannot
assume the user's intention or capabilities.  Thanks,

Alex

[toc] | [prev] | [next] | [standalone]


#1537437

FromCao jin <caoj.fnst@cn.fujitsu.com>
Date2016-12-07 04:00 +0100
Message-ID<sLACB-265-1@gated-at.bofh.it>
In reply to#1537037

On 12/06/2016 11:25 PM, Alex Williamson wrote:
> On Tue, 6 Dec 2016 14:11:03 +0800
> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> 
>> On 12/06/2016 12:17 AM, Alex Williamson wrote:
>>> On Mon, 5 Dec 2016 13:52:03 +0800
>>> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
>>>   
>>>> On 12/04/2016 11:30 PM, Alex Williamson wrote:  
>>>>> On Sun, 4 Dec 2016 20:16:42 +0800
>>>>> Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
>>>>>     
>>>>>> On 12/01/2016 10:55 PM, Alex Williamson wrote:    
>>>>>>> On Thu, 1 Dec 2016 21:40:00 +0800      
>>>>>>    
>>>>>>>>> If an AER fault occurs and the user doesn't do a reset, what
>>>>>>>>> happens when that device is released and a host driver tries to make
>>>>>>>>> use of it?  The user makes no commitment to do a reset and there are
>>>>>>>>> only limited configurations where we even allow the user to perform a
>>>>>>>>> reset.
>>>>>>>>>         
>>>>>>>>
>>>>>>>> Limited? Do you mean the things __pci_dev_reset() can do?      
>>>>>>>
>>>>>>> I mean that there are significant device and guest configuration
>>>>>>> restrictions in order to support AER.  For instance, all the functions
>>>>>>> of the slot need to appear in a PCI-e topology in the guest with all
>>>>>>> the functions in the right place such that a guest bus reset translates
>>>>>>> into a host bus reset.  The physical functions cannot be split between
>>>>>>> guests even if IOMMU isolation would otherwise allow it.  The user
>>>>>>> needs to explicitly enable AER support for the devices.  A VM need to
>>>>>>> be specifically configured for AER support in order to set any sort of
>>>>>>> expectations of a guest directed bus reset, let alone a guarantee that
>>>>>>> it will happen.  So all the existing VMs, where functions are split
>>>>>>> between guests, or the topology isn't exactly right, or AER isn't
>>>>>>> enabled see a regression from the above change as the device is no
>>>>>>> longer reset.
>>>>>>>       
>>>>>>
>>>>>> I am not clear why set these restrictions in the current design. I take
>>>>>> a glance at older versions of qemu's patchset, their thoughts is:
>>>>>> translate a guest bus reset into a host bus reset(Which is
>>>>>> unreasonable[*] to me). And I guess, that's the *cause* of these
>>>>>> restrictions?  Is there any other stories behind these restrictions?
>>>>>>
>>>>>> [*] In physical world, set bridge's secondary bus reset would send
>>>>>> hot-reset TLP to all functions below, trigger every device's reset
>>>>>> separately. Emulated device should behave the same, means just using
>>>>>> each device's DeviceClass->reset method.    
>>>>>
>>>>> Are you trying to say that an FLR is equivalent to a link reset?    
>>>>
>>>> No.  Look at old versions patchset, there is one names "vote the
>>>> function 0 to do host bus reset when aer occurred"[1], that is what I
>>>> called "translate guest link reset to host link reset", and what I think
>>>> unreasonable(and I think it also does it wrongly).  So in v10 version of
>>>> mine, I dropped it.
>>>>
>>>> [1]https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg02987.html
>>>>
>>>> If "translate guest link reset to host link reset" is right, I can
>>>> understand these restrictions[2][3].
>>>>
>>>> [2]. All physical functions in a single card must be assigned to the VM
>>>>      with AER enabled on each and configured on the same virtual bus.
>>>> [3]. Don't place other devices under the virtual bus in [2], no matter
>>>>      physical, emulated, or paravirtual, even if other device
>>>>      supporting AER signaling
>>>>
>>>> Certain device's FLR calls its DeviceClass->reset method; link reset
>>>> calls DeviceClass->reset of each device which on the bus. So, apparently
>>>> they have difference.  But if there is only 1 vfio-pci device under the
>>>> virtual pci bus,  I think FLR can be equivalent to a link reset, right?  
>>>
>>> No.  An FLR resets the device while a secondary bus reset does a reset
>>> of the link and the device.  AER errors are sometimes issues with the
>>> link, not the device.  If we were to perform only an FLR, we're not
>>> performing the same reset as would be done on bare metal.
>>>    
>>
>> Thanks for you explanation, it does helps, except the last sentence, I
>> think I understand it now: fatal error implies there may be link issue
>> exists(pci express spec: 6.2.2.2.1), so, should do link reset for fatal
>> error(that is what and why aer core does). And so, in patch[1] above,
>> qemu does a link reset when seeing secondary bus reset bit of virtual
>> bus got set. is it right?
> 
> QEMU is only going to do a bus reset if the guest is participating in
> AER recovery AND QEMU supports AER recovery AND the guest topology
> configuration allows the guest bus reset to induce a host bus reset.
> vfio does not know that QEMU is the user and cannot assume the user
> will perform a bus reset.  We need to give the user the ability to
> recover from an AER, but not rely on the user to do so.  We cannot
> assume the user's intention or capabilities.  Thanks,
> 

Got your points, thanks a lot, will consider it more,

-- 
Sincerely,
Cao jin

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web