On Thu, Sep 14, 2017 at 04:19:44AM -0600, Jan Beulich wrote:
> >>> On 14.09.17 at 12:08, <roger....@citrix.com> wrote:
> > On Thu, Sep 07, 2017 at 09:29:41AM -0600, Jan Beulich wrote:
> >> >>> On 14.08.17 at 16:28, <roger....@citrix.com> 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.
>
> No, the problem is the wrap-around case, which your code
> doesn't handle. Iirc hardware behaves along the lines of what
> I've suggested to change to, with potentially the vector
> increment carrying into other parts of the value. Hence you
> either need an early check for there not being any wrapping,
> or other places may need similar adjustment (in which case it
> might be better to really just increment "data" once in the
> loop.
Oh, so the vector increment carries over to the delivery mode, then I
will switch it.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
