Re: [Xen-devel] [PATCH v7 for-next 12/12] vpci/msix: add MSI-X handlers

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Dec 20, 2017 at 09:13:34AM -0700, Jan Beulich wrote:
> >>> On 18.10.17 at 13:40, <roger.pau@xxxxxxxxxx> wrote:
> > +    if ( new_enabled && !new_masked && (!msix->enabled || msix->masked) )
> > +    {
> > +        for ( i = 0; i < msix->max_entries; i++ )
> > +        {
> > +            if ( msix->entries[i].masked || !msix->entries[i].updated )
> > +                continue;
> > +
> > +            rc = vpci_msix_arch_disable_entry(&msix->entries[i], pdev);
> > +            /* Ignore ENOENT, it means the entry wasn't setup. */
> > +            if ( rc && rc != -ENOENT )
> > +            {
> > +                gprintk(XENLOG_WARNING,
> > +                        "%04x:%02x:%02x.%u: unable to disable entry %u: 
> > %d\n",
> > +                        pdev->seg, pdev->bus, slot, func, i, rc);
> 
> In a log from e.g. a customer, how am I to tell this message from ...

I've replaced this message with:

"%04x:%02x:%02x.%u: unable to disable entry %u for update: %d\n"

> > +                return;
> > +            }
> > +
> > +            rc = vpci_msix_arch_enable_entry(&msix->entries[i], pdev,
> > +                                             VMSIX_TABLE_BASE(pdev->vpci,
> > +                                                              
> > VPCI_MSIX_TABLE));
> > +            if ( rc )
> > +            {
> > +                gprintk(XENLOG_WARNING,
> > +                        "%04x:%02x:%02x.%u: unable to enable entry %u: 
> > %d\n",
> > +                        pdev->seg, pdev->bus, slot, func, i, rc);
> > +                /* Entry is likely not properly configured, skip it. */
> > +                continue;
> > +            }
> > +
> > +            /*
> > +             * At this point the PIRQ is still masked. Unmask it, or else 
> > the
> > +             * guest won't receive interrupts. This is due to the
> > +             * disable/enable sequence performed above.
> > +             */
> > +            vpci_msix_arch_mask_entry(&msix->entries[i], pdev, false);
> > +
> > +            msix->entries[i].updated = false;
> > +        }
> > +    }
> > +    else if ( !new_enabled && msix->enabled )
> > +    {
> > +        /* Guest has disabled MSIX, disable all entries. */
> > +        for ( i = 0; i < msix->max_entries; i++ )
> > +        {
> > +            /*
> > +             * NB: vpci_msix_arch_disable can be called for entries that 
> > are
> > +             * not setup, it will return -ENOENT in that case.
> > +             */
> > +            rc = vpci_msix_arch_disable_entry(&msix->entries[i], pdev);
> > +            switch ( rc )
> > +            {
> > +            case 0:
> > +                /*
> > +                 * Mark the entry successfully disabled as updated, so 
> > that on
> > +                 * the next enable the entry is properly setup. This is 
> > done
> > +                 * so that the following flow works correctly:
> > +                 *
> > +                 * mask entry -> disable MSIX -> enable MSIX -> unmask 
> > entry
> > +                 *
> > +                 * Without setting 'updated', the 'unmask entry' step will 
> > fail
> > +                 * because the entry has not been updated, so it would not 
> > be
> > +                 * mapped/bound at all.
> > +                 */
> > +                msix->entries[i].updated = true;
> > +                break;
> > +            case -ENOENT:
> > +                /* Ignore non-present entry. */
> > +                break;
> > +            default:
> > +                gprintk(XENLOG_WARNING,
> > +                        "%04x:%02x:%02x.%u: unable to disable entry %u: 
> > %d\n",
> > +                        pdev->seg, pdev->bus, slot, func, i, rc);
> 
> ... this one?

So that it doesn't crash with this one.

Thanks, Roger.

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