Re: [Xen-devel] [PATCH 06/10] vpci/header: add teardown cleanup

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jul 02, 2018 at 10:04:30AM +0200, Roger Pau Monné wrote:
> On Fri, Jun 29, 2018 at 12:15:34PM +0100, Wei Liu wrote:
> > On Wed, Jun 20, 2018 at 04:42:30PM +0200, Roger Pau Monne wrote:
> > > In order to unmap the BARs
> > > 
> > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > ---
> > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > Cc: Julien Grall <julien.grall@xxxxxxx>
> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > Cc: Tim Deegan <tim@xxxxxxx>
> > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > ---
> > >  xen/drivers/vpci/header.c | 30 +++++++++++++++++++++++-------
> > >  1 file changed, 23 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> > > index 4363270a55..686e04e35a 100644
> > > --- a/xen/drivers/vpci/header.c
> > > +++ b/xen/drivers/vpci/header.c
> > > @@ -131,12 +131,15 @@ bool vpci_process_pending(struct vcpu *v)
> > >          if ( rc == -ERESTART )
> > >              return true;
> > >  
> > > -        spin_lock(&v->vpci.pdev->vpci_lock);
> > > -        if ( v->vpci.pdev->vpci )
> > > -            /* Disable memory decoding unconditionally on failure. */
> > > -            modify_decoding(v->vpci.pdev, !rc && v->vpci.map,
> > > -                            !rc && v->vpci.rom_only);
> > > -        spin_unlock(&v->vpci.pdev->vpci_lock);
> > > +        if ( v->vpci.pdev )
> > > +        {
> > > +            spin_lock(&v->vpci.pdev->vpci_lock);
> > > +            if ( v->vpci.pdev->vpci )
> > > +                /* Disable memory decoding unconditionally on failure. */
> > > +                modify_decoding(v->vpci.pdev, !rc && v->vpci.map,
> > > +                                !rc && v->vpci.rom_only);
> > > +            spin_unlock(&v->vpci.pdev->vpci_lock);
> > > +        }
> > >  
> > >          rangeset_destroy(v->vpci.mem);
> > >          v->vpci.mem = NULL;
> > > @@ -560,7 +563,20 @@ static int init_bars(struct pci_dev *pdev)
> > >  
> > >      return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, true, false) : 
> > > 0;
> > >  }
> > > -REGISTER_VPCI_INIT(init_bars, NULL, VPCI_PRIORITY_MIDDLE);
> > > +
> > > +static void teardown_bars(struct pci_dev *pdev)
> > > +{
> > > +    uint16_t cmd = pci_conf_read16(pdev->seg, pdev->bus, 
> > > PCI_SLOT(pdev->devfn),
> > > +                                   PCI_FUNC(pdev->devfn), PCI_COMMAND);
> > > +
> > > +    if ( cmd & PCI_COMMAND_MEMORY )
> > > +    {
> > > +        /* Unmap all BARs from guest p2m. */
> > > +        modify_bars(pdev, false, false);
> > 
> > So modify_bars will eventually call defer_map in most cases (which I
> > believe are the ones your care about here).
> > 
> > But then the following line sets vpci.pdev to NULL, which means the
> > check in vpci_process_pending is false and modify_decoding is skipped.
> > If that what you want?
> 
> Yes, after the call to teardown_bars the pdev can be removed, so the
> deferred part of the unmap shouldn't rely on pdev being available.
> That means that the memory decoding bit is not switched off. I would
> expect that the device will get reset into a proper state (or in the
> SR-IOV just disappear).
> 

Which reset are you referring to? I suppose you mean VF FLR?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel