Re: [PATCH v4 2/6] x86/hvm: Allow access to registers on the same page as MSI-X table

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Nov 27, 2023 at 06:00:57PM +0100, Jan Beulich wrote:
> On 24.11.2023 02:47, Marek Marczykowski-Górecki wrote:
> > GCC gets confused about 'desc' variable:
> > 
> >     arch/x86/hvm/vmsi.c: In function ‘msixtbl_range’:
> >     arch/x86/hvm/vmsi.c:553:8: error: ‘desc’ may be used uninitialized 
> > [-Werror=maybe-uninitialized]
> >       553 |     if ( desc )
> >           |        ^
> >     arch/x86/hvm/vmsi.c:537:28: note: ‘desc’ was declared here
> >       537 |     const struct msi_desc *desc;
> >           |                            ^~~~
> 
> This could do with also indicating the gcc version. Issues like this
> tend to get fixed over time.

Sure, I'll add it's GCC 12.2.1.
And indeed, GCC 13.2.1 does not complain anymore.

> > +
> > +    if ( !msix->adj_access_idx[adj_type] )
> > +    {
> > +        gprintk(XENLOG_WARNING,
> > +                "Page for adjacent(%d) MSI-X table access not initialized 
> > for %pp (addr %#lx, gtable %#lx\n",
> > +                adj_type, &entry->pdev->sbdf, addr, entry->gtable);
> > +
> > +        return ADJACENT_DONT_HANDLE;
> > +    }
> > +
> > +    /* If PBA lives on the same page too, discard writes. */
> > +    if ( write &&
> > +         ((adj_type == ADJ_IDX_LAST &&
> > +           msix->table.last == msix->pba.first) ||
> > +          (adj_type == ADJ_IDX_FIRST &&
> > +           msix->table.first == msix->pba.last)) )
> > +    {
> > +        gprintk(XENLOG_WARNING,
> > +                "MSI-X table and PBA of %pp live on the same page, "
> > +                "writing to other registers there is not implemented\n",
> > +                &entry->pdev->sbdf);
> 
> Here and above I think verbosity needs limiting to the first instance per
> device per domain.

Is there some clever API for that already, or do I need to introduce
extra variable in some of those structures (msixtbl_entry? pci_dev?) ?

(other requested changes ok)

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature