Jason Wang
2021-Apr-28 10:03 UTC
[PATCH 2/2] vDPA/ifcvf: implement doorbell mapping for ifcvf
? 2021/4/28 ??5:56, Zhu, Lingshan ??:> > > On 4/28/2021 5:21 PM, Jason Wang wrote: >> >> ? 2021/4/28 ??4:59, Zhu, Lingshan ??: >>> >>> >>> On 4/28/2021 4:42 PM, Jason Wang wrote: >>>> >>>> ? 2021/4/28 ??4:21, 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 | 18 ++++++++++++++++++ >>>>> ? 1 file changed, 18 insertions(+) >>>>> >>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> index e48e6b74fe2e..afcb71bc0f51 100644 >>>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> @@ -413,6 +413,23 @@ 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_hw *vf = vdpa_to_vf(vdpa_dev); >>>>> +??? struct vdpa_notification_area area; >>>>> + >>>>> +??? if (vf->notify_pa % PAGE_SIZE) { >>>>> +??????? area.addr = 0; >>>>> +??????? area.size = 0; >>>> >>>> >>>> We don't need this since: >>>> >>>> 1) there's a check in the vhost vDPA >>> I think you mean this code block in vdpa.c >>> ??????? notify = ops->get_vq_notification(vdpa, index); >>> ??????? if (notify.addr & (PAGE_SIZE - 1)) >>> ??????????????? return -EINVAL; >>> >>> This should work, however, I think the parent driver should ensure >>> it passes a PAGE_SIZE aligned address to userspace, to be robust, to >>> be reliable. >> >> >> The point is parent is unaware of whether or not there's a userspace. > when calling this, I think it targets a usersapce program, why kernel > space need it, so IMHO no harm if we check this to keep the parent > driver robust.Again, vDPA device is unaware of what driver that is bound. It could be virtio-vpda, vhost-vdpa or other in the future. It's only the vDPA bus driver know how it is actually used.>> >> >>>> 2) device is unaware of the bound driver, non page aligned doorbell >>>> doesn't necessarily meant it can be used >>> Yes, non page aligned doorbell can not be used, so there is a check. >> >> >> Typo, what I meant is "it can't be used". That is to say, we should >> let the vDPA bus driver to decide whether or not it can be used. > If it is not page aligned, there would be extra complexities for > vhost/qemu, I see it as a hardware defect,It is allowed by the virtio spec, isn't it? Thanks> why adapt to this kind of defects? > > Thanks > Zhu Lingshan >> >> Thanks >> >> >>> >>> Thanks >>> Zhu Lingshan >>>> >>>> Let's leave those polices to the driver. >>>> >>>> Thanks >>>> >>>> >>>>> +??? } else { >>>>> +??????? area.addr = vf->notify_pa; >>>>> +??????? area.size = PAGE_SIZE; >>>>> +??? } >>>>> + >>>>> +??? return area; >>>>> +} >>>>> + >>>>> ? /* >>>>> ?? * IFCVF currently does't have on-chip IOMMU, so not >>>>> ?? * implemented set_map()/dma_map()/dma_unmap() >>>>> @@ -440,6 +457,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) >>>> >>> >> >