Re: [Xen-devel] [PATCH 04/10] vpci/msix: add teardown cleanup

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jun 20, 2018 at 04:42:28PM +0200, Roger Pau Monne wrote:
> So that interrupts are properly freed.
> 
> 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/msix.c | 43 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> index 6132f576b6..cfca1cd43a 100644
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -450,7 +450,48 @@ static int init_msix(struct pci_dev *pdev)
>  
>      return 0;
>  }
> -REGISTER_VPCI_INIT(init_msix, NULL, VPCI_PRIORITY_HIGH);
> +
> +static void teardown_msix(struct pci_dev *pdev)
> +{
> +    struct vpci_msix *msix = pdev->vpci->msix;
> +    unsigned int i;
> +
> +    if ( !msix )
> +        return;
> +
> +    if ( msix->enabled )
> +    {
> +        /* Disable MSIX. */
> +        unsigned int pos = pci_find_cap_offset(pdev->seg, pdev->bus,
> +                                               PCI_SLOT(pdev->devfn),
> +                                               PCI_FUNC(pdev->devfn),
> +                                               PCI_CAP_ID_MSIX);
> +        uint16_t control = pci_conf_read16(pdev->seg, pdev->bus,
> +                                           PCI_SLOT(pdev->devfn),
> +                                           PCI_FUNC(pdev->devfn),
> +                                           msix_control_reg(pos));
> +
> +        pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> +                         PCI_FUNC(pdev->devfn), msix_control_reg(pos),
> +                         (control & ~PCI_MSIX_FLAGS_ENABLE));
> +    }
> +
> +    write_lock(&pdev->domain->arch.hvm_domain.msix_lock);
> +    list_del(&pdev->vpci->msix->next);
> +    write_unlock(&pdev->domain->arch.hvm_domain.msix_lock);
> +
> +    for ( i = 0; i < msix->max_entries && msix->enabled; i++ )

Maybe lift checking msix->enabled outside of the loop? Afaict nothing
in the loop manipulates that flag.

> +    {
> +        int rc = vpci_msix_arch_disable_entry(&msix->entries[i], pdev);
> +
> +        if ( rc && rc != -ENOENT )
> +            gprintk(XENLOG_WARNING,
> +                    "%04x:%02x:%02x.%u: unable to disable MSIX entry %u: 
> %d\n",
> +                    pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> +                    PCI_FUNC(pdev->devfn), i, rc);
> +    }

No freeing msix here?

Wei.

> +}
> +REGISTER_VPCI_INIT(init_msix, teardown_msix, VPCI_PRIORITY_HIGH);
>  
>  /*
>   * Local variables:
> -- 
> 2.17.1
> 

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