Jason Wang
2021-Jun-01 03:31 UTC
[PATCH V2 RESEND 2/2] vDPA/ifcvf: implement doorbell mapping for ifcvf
? 2021/6/1 ??11:27, Zhu, Lingshan ??:> > > On 5/31/2021 3:56 PM, Jason Wang wrote: >> >> ? 2021/5/31 ??3:33, Zhu Lingshan ??: >>> This commit implements doorbell mapping feature for ifcvf. >>> This feature maps the notify page to userspace, to eliminate >>> vmexit when kick a vq. >>> >>> Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com> >>> --- >>> ? drivers/vdpa/ifcvf/ifcvf_main.c | 17 +++++++++++++++++ >>> ? 1 file changed, 17 insertions(+) >>> >>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>> index ab0ab5cf0f6e..effb0e549135 100644 >>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>> @@ -413,6 +413,22 @@ static int ifcvf_vdpa_get_vq_irq(struct >>> vdpa_device *vdpa_dev, >>> ????? return vf->vring[qid].irq; >>> ? } >>> ? +static struct vdpa_notification_area >>> ifcvf_get_vq_notification(struct vdpa_device *vdpa_dev, >>> +?????????????????????????????????? u16 idx) >>> +{ >>> +??? struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); >>> +??? struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >>> +??? struct pci_dev *pdev = adapter->pdev; >>> +??? struct vdpa_notification_area area; >>> + >>> +??? area.addr = vf->vring[idx].notify_pa; >>> +??? area.size = PAGE_SIZE; >>> +??? if (area.addr % PAGE_SIZE) >>> +??????? IFCVF_DBG(pdev, "vq %u doorbell address is not PAGE_SIZE >>> aligned\n", idx); >> >> >> Let's leave the decision to upper layer by: (see >> vp_vdpa_get_vq_notification) >> >> area.addr = notify_pa; >> area.size = notify_offset_multiplier; >> >> Thanks > > Hi Jason, > > notify_offset_multiplier can be zero, means vqs share the same > doorbell address, distinguished by qid. > and in vdpa.c: > > ??????? if (vma->vm_end - vma->vm_start != notify.size) > ??????????????? return -ENOTSUPP; > > so a zero size would cause this feature failure. > mmap should work on at least a page, so if we really want "area.size = > notify_offset_multiplier;" > I think we should add some code in vdpa.c, like: > > if(!notify.size) > ??? notify.size = PAGE_SIZE; > > sounds good?It's the responsibility of the driver to report a correct one. So I think it's better to tweak it as: area.size = notify_offset_multiplier ?: PAGE_SIZE; Thanks> > Thanks > Zhu Lingshan >> >> >>> + >>> +??? return area; >>> +} >>> + >>> ? /* >>> ?? * IFCVF currently does't have on-chip IOMMU, so not >>> ?? * implemented set_map()/dma_map()/dma_unmap() >>> @@ -440,6 +456,7 @@ static const struct vdpa_config_ops ifc_vdpa_ops >>> = { >>> ????? .get_config??? = ifcvf_vdpa_get_config, >>> ????? .set_config??? = ifcvf_vdpa_set_config, >>> ????? .set_config_cb? = ifcvf_vdpa_set_config_cb, >>> +??? .get_vq_notification = ifcvf_get_vq_notification, >>> ? }; >>> ? ? static int ifcvf_probe(struct pci_dev *pdev, const struct >>> pci_device_id *id) >> >