Re: [Xen-devel] [PATCH v5 09/11] vpci/msi: add MSI handlers

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

On Thu, Sep 07, 2017 at 09:29:41AM -0600, Jan Beulich wrote:
> >>> On 14.08.17 at 16:28, <roger.pau@xxxxxxxxxx> wrote:
> > +int vpci_msi_arch_enable(struct vpci_arch_msi *arch, struct pci_dev *pdev,
> > +                         uint64_t address, uint32_t data, unsigned int 
> > vectors)
> > +{
> > +    struct msi_info msi_info = {
> > +        .seg = pdev->seg,
> > +        .bus = pdev->bus,
> > +        .devfn = pdev->devfn,
> > +        .entry_nr = vectors,
> > +    };
> > +    unsigned int i;
> > +    int rc;
> > +
> > +    ASSERT(arch->pirq == INVALID_PIRQ);
> > +
> > +    /* Get a PIRQ. */
> > +    rc = allocate_and_map_msi_pirq(pdev->domain, -1, &arch->pirq,
> > +                                   MAP_PIRQ_TYPE_MULTI_MSI, &msi_info);
> > +    if ( rc )
> > +    {
> > +        gdprintk(XENLOG_ERR, "%04x:%02x:%02x.%u: failed to map PIRQ: %d\n",
> > +                 pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> > +                 PCI_FUNC(pdev->devfn), rc);
> > +        return rc;
> > +    }
> > +
> > +    for ( i = 0; i < vectors; i++ )
> > +    {
> > +        xen_domctl_bind_pt_irq_t bind = {
> > +            .machine_irq = arch->pirq + i,
> > +            .irq_type = PT_IRQ_TYPE_MSI,
> > +            .u.msi.gvec = msi_vector(data) + i,
> 
> Isn't that rather msi_vector(data + i), i.e. wouldn't you better
> increment data together with i?

That's true, because the vector is fetched from the last 8bits of the
data, but I find it more confusing (and it requires that the reader
knows this detail). IMHO I would prefer to leave it as-is.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel