> -----Original Message----- > From: Jiang Liu [mailto:jiang....@linux.intel.com] > Sent: Wednesday, December 2, 2015 7:12 PM > To: Jake Oshins <ja...@microsoft.com>; gre...@linuxfoundation.org; KY > Srinivasan <k...@microsoft.com>; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; t...@redhat.com; Haiyang Zhang > <haiya...@microsoft.com>; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH v6 7/7] PCI: hv: New paravirtual PCI front-end for Hyper- > V VMs > > On 2015/11/3 5:33, ja...@microsoft.com wrote: > > From: Jake Oshins <ja...@microsoft.com> > > [...] > > + > > +/** > > + * hv_irq_unmask() - "Unmask" the IRQ by setting its current > > + * affinity. > > + * @data: Describes the IRQ > > + * > > + * Build new a destination for the MSI and make a hypercall to > > + * update the Interrupt Redirection Table. "Device Logical ID" > > + * is built out of this PCI bus's instance GUID and the function > > + * number of the device. > > + */ > > +void hv_irq_unmask(struct irq_data *data) > > +{ > > + struct msi_desc *msi_desc = irq_data_get_msi_desc(data); > > + struct irq_cfg *cfg = irqd_cfg(data); > > + struct retarget_msi_interrupt params; > > + struct hv_pcibus_device *hbus; > > + struct cpumask *dest; > > + struct pci_bus *pbus; > > + struct pci_dev *pdev; > > + int cpu; > > + > > + dest = irq_data_get_affinity_mask(data); > > + pdev = msi_desc_to_pci_dev(msi_desc); > > + pbus = pdev->bus; > > + hbus = container_of(pbus->sysdata, struct hv_pcibus_device, > sysdata); > > + > > + memset(¶ms, 0, sizeof(params)); > > + params.partition_id = HV_PARTITION_ID_SELF; > > + params.source = 1; /* MSI(-X) */ > > + params.address = msi_desc->msg.address_lo; > > + params.data = msi_desc->msg.data; > > + params.device_id = (hbus->hdev->dev_instance.b[5] << 24) | > > + (hbus->hdev->dev_instance.b[4] << 16) | > > + (hbus->hdev->dev_instance.b[7] << 8) | > > + (hbus->hdev->dev_instance.b[6] & 0xf8) | > > + PCI_FUNC(pdev->devfn); > > + params.vector = cfg->vector; > > + > > + for_each_cpu_and(cpu, dest, cpu_online_mask) > > + params.vp_mask |= (1 << > vmbus_cpu_number_to_vp_number(cpu)); > No knowledge about the HV implementation details, but feel some chances > of race here between hv_irq_unmask(), hv_set_affinity() and > cpu_up()/cpu_down() when accessing 'dest' and cpu_online_mask. > Thanks. Is there any architectural contract here? I tried implementing this by doing this work in the set_affinity() callback, but the vector was often wrong when that callback was invoked. (It seems to get changed just after set_affinity().) Can you suggest a durable strategy?
I'll respond to all the other comments you sent (and this one, once I understand the right response) and resend. Thanks for your review, Jake Oshins _______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
