Messages in this thread | | | Subject | Re: [PATCH] ifcvf: move IRQ request/free to status change handlers | From | Francesco Lavra <> | Date | Mon, 11 May 2020 12:18:29 +0200 |
| |
On 5/11/20 11:26 AM, Jason Wang wrote: > > On 2020/5/11 下午3:19, Zhu Lingshan wrote: >> This commit move IRQ request and free operations from probe() >> to VIRTIO status change handler to comply with VIRTIO spec. >> >> VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field >> The device MUST NOT consume buffers or send any used buffer >> notifications to the driver before DRIVER_OK. > > > My previous explanation might be wrong here. It depends on how you > implement your hardware, if you hardware guarantee that no interrupt > will be triggered before DRIVER_OK, then it's fine. > > And the main goal for this patch is to allocate the interrupt on demand. > > >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> drivers/vdpa/ifcvf/ifcvf_main.c | 119 >> ++++++++++++++++++++++++---------------- >> 1 file changed, 73 insertions(+), 46 deletions(-) >> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >> b/drivers/vdpa/ifcvf/ifcvf_main.c >> index abf6a061..4d58bf2 100644 >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >> @@ -28,6 +28,60 @@ static irqreturn_t ifcvf_intr_handler(int irq, void >> *arg) >> return IRQ_HANDLED; >> } >> +static void ifcvf_free_irq_vectors(void *data) >> +{ >> + pci_free_irq_vectors(data); >> +} >> + >> +static void ifcvf_free_irq(struct ifcvf_adapter *adapter, int queues) >> +{ >> + struct pci_dev *pdev = adapter->pdev; >> + struct ifcvf_hw *vf = &adapter->vf; >> + int i; >> + >> + >> + for (i = 0; i < queues; i++) >> + devm_free_irq(&pdev->dev, vf->vring[i].irq, &vf->vring[i]); >> + >> + ifcvf_free_irq_vectors(pdev); >> +} >> + >> +static int ifcvf_request_irq(struct ifcvf_adapter *adapter) >> +{ >> + struct pci_dev *pdev = adapter->pdev; >> + struct ifcvf_hw *vf = &adapter->vf; >> + int vector, i, ret, irq; >> + >> + ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR, >> + IFCVF_MAX_INTR, PCI_IRQ_MSIX); >> + if (ret < 0) { >> + IFCVF_ERR(pdev, "Failed to alloc IRQ vectors\n"); >> + return ret; >> + } >> + >> + for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { >> + snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", >> + pci_name(pdev), i); >> + vector = i + IFCVF_MSI_QUEUE_OFF; >> + irq = pci_irq_vector(pdev, vector); >> + ret = devm_request_irq(&pdev->dev, irq, >> + ifcvf_intr_handler, 0, >> + vf->vring[i].msix_name, >> + &vf->vring[i]); >> + if (ret) { >> + IFCVF_ERR(pdev, >> + "Failed to request irq for vq %d\n", i); >> + ifcvf_free_irq(adapter, i); > > > I'm not sure this unwind is correct. It looks like we should loop and > call devm_free_irq() for virtqueue [0, i);
That's exactly what the code does: ifcvf_free_irq() contains a (i = 0; i < queues; i++) loop, and here the function is called with the `queues` argument set to `i`.
| |