* [PATCH v3 0/2] vdpasim: support doorbell mapping
@ 2023-01-29 2:50 Longpeng(Mike)
2023-01-29 2:50 ` [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area Longpeng(Mike)
2023-01-29 2:50 ` [PATCH v3 2/2] vdpasim: support doorbell mapping Longpeng(Mike)
0 siblings, 2 replies; 10+ messages in thread
From: Longpeng(Mike) @ 2023-01-29 2:50 UTC (permalink / raw)
To: mst, jasowang
Cc: stefanha, sgarzare, arei.gonglei, yechuan, huangzhichao,
virtualization, linux-kernel, eperezma, Longpeng
From: Longpeng <longpeng2@huawei.com>
This patchset supports doorbell mapping for vdpasim devices.
v2: https://lore.kernel.org/lkml/CACGkMEtdT5fG=ffbpQadkGmzHf6Ax-+L50LsriYqJaW++natMg@mail.gmail.com/T/
Changes v2->v3:
- add a new callback named get_vq_notification_pgprot to vdpa_config_ops [Jason]
- remove the new added module parameter 'parameter' [Jason]
- opencode the schedule/cancel_delayed() [Jason]
Changes v1->v2:
- support both kick mode and passthrough mode. [Jason]
- poll the notify register first. [Jason, Michael]
Longpeng (2):
vdpa: support specify the pgprot of vq notification area
vdpasim: support doorbell mapping
drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 ++++++++++++++++++++++++++++++++
drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++
drivers/vhost/vdpa.c | 4 +-
include/linux/vdpa.h | 9 +++++
4 files changed, 80 insertions(+), 1 deletion(-)
--
2.23.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area 2023-01-29 2:50 [PATCH v3 0/2] vdpasim: support doorbell mapping Longpeng(Mike) @ 2023-01-29 2:50 ` Longpeng(Mike) 2023-01-29 6:03 ` Jason Wang 2023-01-29 2:50 ` [PATCH v3 2/2] vdpasim: support doorbell mapping Longpeng(Mike) 1 sibling, 1 reply; 10+ messages in thread From: Longpeng(Mike) @ 2023-01-29 2:50 UTC (permalink / raw) To: mst, jasowang Cc: stefanha, sgarzare, arei.gonglei, yechuan, huangzhichao, virtualization, linux-kernel, eperezma, Longpeng From: Longpeng <longpeng2@huawei.com> Adds get_vq_notification_pgprot operation to vdpa_config_ops to support specify the pgprot of vq norification area. It's an optional operation, the vdpa framework will treat the pgprot of vq notification area as noncached as default as usual. --- drivers/vhost/vdpa.c | 4 +++- include/linux/vdpa.h | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 166044642fd5..036fe88425c8 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -1263,7 +1263,9 @@ static vm_fault_t vhost_vdpa_fault(struct vm_fault *vmf) notify = ops->get_vq_notification(vdpa, index); - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); + vma->vm_page_prot = ops->get_vq_notification_pgprot ? + ops->get_vq_notification_pgprot(vdpa, index, vma->vm_page_prot) : + pgprot_noncached(vma->vm_page_prot); if (remap_pfn_range(vma, vmf->address & PAGE_MASK, PFN_DOWN(notify.addr), PAGE_SIZE, vma->vm_page_prot)) diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index 6d0f5e4e82c2..07fcf5e6abc8 100644 --- a/include/linux/vdpa.h +++ b/include/linux/vdpa.h @@ -169,6 +169,12 @@ struct vdpa_map_file { * @vdev: vdpa device * @idx: virtqueue index * Returns the notifcation area + * @get_vq_notification_pgprot: Get the pgprot of the vq's notification area (optional) + * @vdev: vdpa device + * @idx: virtqueue index + * @prot: original page protection value of the + * notification area + * Returns pgprot_t: the pgprot of the notification area * @get_vq_irq: Get the irq number of a virtqueue (optional, * but must implemented if require vq irq offloading) * @vdev: vdpa device @@ -305,6 +311,9 @@ struct vdpa_config_ops { struct netlink_ext_ack *extack); struct vdpa_notification_area (*get_vq_notification)(struct vdpa_device *vdev, u16 idx); + pgprot_t (*get_vq_notification_pgprot)(struct vdpa_device *vdev, + u16 idx, + pgprot_t prot); /* vq irq is not expected to be changed once DRIVER_OK is set */ int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx); -- 2.23.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area 2023-01-29 2:50 ` [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area Longpeng(Mike) @ 2023-01-29 6:03 ` Jason Wang 0 siblings, 0 replies; 10+ messages in thread From: Jason Wang @ 2023-01-29 6:03 UTC (permalink / raw) To: Longpeng(Mike) Cc: mst, linux-kernel, yechuan, eperezma, huangzhichao, stefanha, virtualization On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) <longpeng2@huawei.com> wrote: > > From: Longpeng <longpeng2@huawei.com> > > Adds get_vq_notification_pgprot operation to vdpa_config_ops to support > specify the pgprot of vq norification area. It's an optional operation, > the vdpa framework will treat the pgprot of vq notification area as > noncached as default as usual. Missing sob. Other than this. Acked-by: Jason Wang <jasowang@redhat.com> Thanks > --- > drivers/vhost/vdpa.c | 4 +++- > include/linux/vdpa.h | 9 +++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index 166044642fd5..036fe88425c8 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -1263,7 +1263,9 @@ static vm_fault_t vhost_vdpa_fault(struct vm_fault *vmf) > > notify = ops->get_vq_notification(vdpa, index); > > - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > + vma->vm_page_prot = ops->get_vq_notification_pgprot ? > + ops->get_vq_notification_pgprot(vdpa, index, vma->vm_page_prot) : > + pgprot_noncached(vma->vm_page_prot); > if (remap_pfn_range(vma, vmf->address & PAGE_MASK, > PFN_DOWN(notify.addr), PAGE_SIZE, > vma->vm_page_prot)) > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index 6d0f5e4e82c2..07fcf5e6abc8 100644 > --- a/include/linux/vdpa.h > +++ b/include/linux/vdpa.h > @@ -169,6 +169,12 @@ struct vdpa_map_file { > * @vdev: vdpa device > * @idx: virtqueue index > * Returns the notifcation area > + * @get_vq_notification_pgprot: Get the pgprot of the vq's notification area (optional) > + * @vdev: vdpa device > + * @idx: virtqueue index > + * @prot: original page protection value of the > + * notification area > + * Returns pgprot_t: the pgprot of the notification area > * @get_vq_irq: Get the irq number of a virtqueue (optional, > * but must implemented if require vq irq offloading) > * @vdev: vdpa device > @@ -305,6 +311,9 @@ struct vdpa_config_ops { > struct netlink_ext_ack *extack); > struct vdpa_notification_area > (*get_vq_notification)(struct vdpa_device *vdev, u16 idx); > + pgprot_t (*get_vq_notification_pgprot)(struct vdpa_device *vdev, > + u16 idx, > + pgprot_t prot); > /* vq irq is not expected to be changed once DRIVER_OK is set */ > int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx); > > -- > 2.23.0 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area @ 2023-01-29 6:03 ` Jason Wang 0 siblings, 0 replies; 10+ messages in thread From: Jason Wang @ 2023-01-29 6:03 UTC (permalink / raw) To: Longpeng(Mike) Cc: mst, stefanha, sgarzare, arei.gonglei, yechuan, huangzhichao, virtualization, linux-kernel, eperezma On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) <longpeng2@huawei.com> wrote: > > From: Longpeng <longpeng2@huawei.com> > > Adds get_vq_notification_pgprot operation to vdpa_config_ops to support > specify the pgprot of vq norification area. It's an optional operation, > the vdpa framework will treat the pgprot of vq notification area as > noncached as default as usual. Missing sob. Other than this. Acked-by: Jason Wang <jasowang@redhat.com> Thanks > --- > drivers/vhost/vdpa.c | 4 +++- > include/linux/vdpa.h | 9 +++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index 166044642fd5..036fe88425c8 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -1263,7 +1263,9 @@ static vm_fault_t vhost_vdpa_fault(struct vm_fault *vmf) > > notify = ops->get_vq_notification(vdpa, index); > > - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > + vma->vm_page_prot = ops->get_vq_notification_pgprot ? > + ops->get_vq_notification_pgprot(vdpa, index, vma->vm_page_prot) : > + pgprot_noncached(vma->vm_page_prot); > if (remap_pfn_range(vma, vmf->address & PAGE_MASK, > PFN_DOWN(notify.addr), PAGE_SIZE, > vma->vm_page_prot)) > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index 6d0f5e4e82c2..07fcf5e6abc8 100644 > --- a/include/linux/vdpa.h > +++ b/include/linux/vdpa.h > @@ -169,6 +169,12 @@ struct vdpa_map_file { > * @vdev: vdpa device > * @idx: virtqueue index > * Returns the notifcation area > + * @get_vq_notification_pgprot: Get the pgprot of the vq's notification area (optional) > + * @vdev: vdpa device > + * @idx: virtqueue index > + * @prot: original page protection value of the > + * notification area > + * Returns pgprot_t: the pgprot of the notification area > * @get_vq_irq: Get the irq number of a virtqueue (optional, > * but must implemented if require vq irq offloading) > * @vdev: vdpa device > @@ -305,6 +311,9 @@ struct vdpa_config_ops { > struct netlink_ext_ack *extack); > struct vdpa_notification_area > (*get_vq_notification)(struct vdpa_device *vdev, u16 idx); > + pgprot_t (*get_vq_notification_pgprot)(struct vdpa_device *vdev, > + u16 idx, > + pgprot_t prot); > /* vq irq is not expected to be changed once DRIVER_OK is set */ > int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx); > > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] vdpasim: support doorbell mapping 2023-01-29 2:50 [PATCH v3 0/2] vdpasim: support doorbell mapping Longpeng(Mike) 2023-01-29 2:50 ` [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area Longpeng(Mike) @ 2023-01-29 2:50 ` Longpeng(Mike) 2023-01-29 6:19 ` Jason Wang 1 sibling, 1 reply; 10+ messages in thread From: Longpeng(Mike) @ 2023-01-29 2:50 UTC (permalink / raw) To: mst, jasowang Cc: stefanha, sgarzare, arei.gonglei, yechuan, huangzhichao, virtualization, linux-kernel, eperezma, Longpeng From: Longpeng <longpeng2@huawei.com> Support doorbell mapping for vdpasim devices, then we can test the notify passthrough feature even if there's no real hardware on hand. Allocates a dummy page which is used to emulate the notify page of the device, all VQs share the same notify register that initiated to 0xffff. A periodic work will check whether there're requests need to process ( the value of the notify register is 0xffff or not ). --- drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 ++++++++++++++++++++++++++++++++ drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++ 2 files changed, 68 insertions(+) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c index b071f0d842fb..4fcfeb6e2fb8 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries, #define VDPASIM_QUEUE_ALIGN PAGE_SIZE #define VDPASIM_QUEUE_MAX 256 #define VDPASIM_VENDOR_ID 0 +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */ +#define VDPASIM_NOTIFY_DEFVAL 0xffff static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa) { @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = { static const struct vdpa_config_ops vdpasim_config_ops; static const struct vdpa_config_ops vdpasim_batch_config_ops; +static void vdpasim_notify_work(struct work_struct *work) +{ + struct vdpasim *vdpasim; + u16 *val; + + vdpasim = container_of(work, struct vdpasim, notify_work.work); + + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) + goto out; + + if (!vdpasim->running) + goto out; + + val = (u16 *)vdpasim->notify; + if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL) + schedule_work(&vdpasim->work); + +out: + schedule_delayed_work(&vdpasim->notify_work, + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); +} + struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, const struct vdpa_dev_set_config *config) { @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, set_dma_ops(dev, &vdpasim_dma_ops); vdpasim->vdpa.mdev = dev_attr->mgmt_dev; + INIT_DELAYED_WORK(&vdpasim->notify_work, vdpasim_notify_work); + + vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO); + if (!vdpasim->notify) + goto err_iommu; + *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL; + vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL); if (!vdpasim->config) goto err_iommu; @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa) static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status) { struct vdpasim *vdpasim = vdpa_to_sim(vdpa); + bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK; spin_lock(&vdpasim->lock); vdpasim->status = status; spin_unlock(&vdpasim->lock); + if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK)) + schedule_delayed_work(&vdpasim->notify_work, + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); } static int vdpasim_reset(struct vdpa_device *vdpa) { struct vdpasim *vdpasim = vdpa_to_sim(vdpa); + cancel_delayed_work_sync(&vdpasim->notify_work); spin_lock(&vdpasim->lock); vdpasim->status = 0; vdpasim_do_reset(vdpasim); @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid, return 0; } +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device *vdpa, + u16 qid, pgprot_t prot) +{ + /* + * We use normal RAM pages to emulate the vq notification area, so + * just keep the pgprot as it mmaped. + */ + return prot; +} + +static struct vdpa_notification_area +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid) +{ + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); + struct vdpa_notification_area notify; + + notify.addr = virt_to_phys((void *)vdpasim->notify); + notify.size = PAGE_SIZE; + + return notify; +} + static void vdpasim_free(struct vdpa_device *vdpa) { struct vdpasim *vdpasim = vdpa_to_sim(vdpa); int i; + cancel_delayed_work_sync(&vdpasim->notify_work); cancel_work_sync(&vdpasim->work); for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { @@ -693,6 +752,8 @@ static void vdpasim_free(struct vdpa_device *vdpa) vhost_iotlb_free(vdpasim->iommu); kfree(vdpasim->vqs); kfree(vdpasim->config); + if (vdpasim->notify) + free_page(vdpasim->notify); } static const struct vdpa_config_ops vdpasim_config_ops = { @@ -704,6 +765,8 @@ static const struct vdpa_config_ops vdpasim_config_ops = { .get_vq_ready = vdpasim_get_vq_ready, .set_vq_state = vdpasim_set_vq_state, .get_vq_state = vdpasim_get_vq_state, + .get_vq_notification = vdpasim_get_vq_notification, + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, .get_vq_align = vdpasim_get_vq_align, .get_vq_group = vdpasim_get_vq_group, .get_device_features = vdpasim_get_device_features, @@ -737,6 +800,8 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { .get_vq_ready = vdpasim_get_vq_ready, .set_vq_state = vdpasim_set_vq_state, .get_vq_state = vdpasim_get_vq_state, + .get_vq_notification = vdpasim_get_vq_notification, + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, .get_vq_align = vdpasim_get_vq_align, .get_vq_group = vdpasim_get_vq_group, .get_device_features = vdpasim_get_device_features, diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h index 0e78737dcc16..0769ccbd3911 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h @@ -69,6 +69,9 @@ struct vdpasim { bool running; /* spinlock to synchronize iommu table */ spinlock_t iommu_lock; + /* dummy notify page */ + unsigned long notify; + struct delayed_work notify_work; }; struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, -- 2.23.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vdpasim: support doorbell mapping 2023-01-29 2:50 ` [PATCH v3 2/2] vdpasim: support doorbell mapping Longpeng(Mike) @ 2023-01-29 6:19 ` Jason Wang 0 siblings, 0 replies; 10+ messages in thread From: Jason Wang @ 2023-01-29 6:19 UTC (permalink / raw) To: Longpeng(Mike) Cc: mst, linux-kernel, yechuan, eperezma, huangzhichao, stefanha, virtualization On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) <longpeng2@huawei.com> wrote: > > From: Longpeng <longpeng2@huawei.com> > > Support doorbell mapping for vdpasim devices, then we can test the notify > passthrough feature even if there's no real hardware on hand. > > Allocates a dummy page which is used to emulate the notify page of the device, > all VQs share the same notify register that initiated to 0xffff. A periodic > work will check whether there're requests need to process ( the value of the > notify register is 0xffff or not ). > --- > drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 ++++++++++++++++++++++++++++++++ > drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++ > 2 files changed, 68 insertions(+) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c > index b071f0d842fb..4fcfeb6e2fb8 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c > @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries, > #define VDPASIM_QUEUE_ALIGN PAGE_SIZE > #define VDPASIM_QUEUE_MAX 256 > #define VDPASIM_VENDOR_ID 0 > +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */ > +#define VDPASIM_NOTIFY_DEFVAL 0xffff > > static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa) > { > @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = { > static const struct vdpa_config_ops vdpasim_config_ops; > static const struct vdpa_config_ops vdpasim_batch_config_ops; > > +static void vdpasim_notify_work(struct work_struct *work) > +{ > + struct vdpasim *vdpasim; > + u16 *val; > + > + vdpasim = container_of(work, struct vdpasim, notify_work.work); > + > + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > + goto out; > + > + if (!vdpasim->running) > + goto out; > + > + val = (u16 *)vdpasim->notify; > + if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL) > + schedule_work(&vdpasim->work); > + > +out: > + schedule_delayed_work(&vdpasim->notify_work, > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > +} > + > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > const struct vdpa_dev_set_config *config) > { > @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > set_dma_ops(dev, &vdpasim_dma_ops); > vdpasim->vdpa.mdev = dev_attr->mgmt_dev; > > + INIT_DELAYED_WORK(&vdpasim->notify_work, vdpasim_notify_work); > + > + vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO); > + if (!vdpasim->notify) > + goto err_iommu; We can simply avoid the advertising notification area in this case. > + *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL; WRITE_ONCE()? > + > vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL); > if (!vdpasim->config) > goto err_iommu; > @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa) > static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status) > { > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > + bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK; Do we need to do the check under the vdpasim->lock? > > spin_lock(&vdpasim->lock); > vdpasim->status = status; > spin_unlock(&vdpasim->lock); > + if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK)) > + schedule_delayed_work(&vdpasim->notify_work, > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > } > > static int vdpasim_reset(struct vdpa_device *vdpa) > { > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > + cancel_delayed_work_sync(&vdpasim->notify_work); Do we need to do this after setting running to zero? Otherwise it's racy. Thanks > spin_lock(&vdpasim->lock); > vdpasim->status = 0; > vdpasim_do_reset(vdpasim); > @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid, > return 0; > } > > +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device *vdpa, > + u16 qid, pgprot_t prot) > +{ > + /* > + * We use normal RAM pages to emulate the vq notification area, so > + * just keep the pgprot as it mmaped. > + */ > + return prot; > +} > + > +static struct vdpa_notification_area > +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid) > +{ > + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > + struct vdpa_notification_area notify; > + > + notify.addr = virt_to_phys((void *)vdpasim->notify); > + notify.size = PAGE_SIZE; > + > + return notify; > +} > + > static void vdpasim_free(struct vdpa_device *vdpa) > { > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > int i; > > + cancel_delayed_work_sync(&vdpasim->notify_work); > cancel_work_sync(&vdpasim->work); > > for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { > @@ -693,6 +752,8 @@ static void vdpasim_free(struct vdpa_device *vdpa) > vhost_iotlb_free(vdpasim->iommu); > kfree(vdpasim->vqs); > kfree(vdpasim->config); > + if (vdpasim->notify) > + free_page(vdpasim->notify); > } > > static const struct vdpa_config_ops vdpasim_config_ops = { > @@ -704,6 +765,8 @@ static const struct vdpa_config_ops vdpasim_config_ops = { > .get_vq_ready = vdpasim_get_vq_ready, > .set_vq_state = vdpasim_set_vq_state, > .get_vq_state = vdpasim_get_vq_state, > + .get_vq_notification = vdpasim_get_vq_notification, > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > .get_vq_align = vdpasim_get_vq_align, > .get_vq_group = vdpasim_get_vq_group, > .get_device_features = vdpasim_get_device_features, > @@ -737,6 +800,8 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { > .get_vq_ready = vdpasim_get_vq_ready, > .set_vq_state = vdpasim_set_vq_state, > .get_vq_state = vdpasim_get_vq_state, > + .get_vq_notification = vdpasim_get_vq_notification, > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > .get_vq_align = vdpasim_get_vq_align, > .get_vq_group = vdpasim_get_vq_group, > .get_device_features = vdpasim_get_device_features, > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h > index 0e78737dcc16..0769ccbd3911 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h > @@ -69,6 +69,9 @@ struct vdpasim { > bool running; > /* spinlock to synchronize iommu table */ > spinlock_t iommu_lock; > + /* dummy notify page */ > + unsigned long notify; > + struct delayed_work notify_work; > }; > > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, > -- > 2.23.0 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vdpasim: support doorbell mapping @ 2023-01-29 6:19 ` Jason Wang 0 siblings, 0 replies; 10+ messages in thread From: Jason Wang @ 2023-01-29 6:19 UTC (permalink / raw) To: Longpeng(Mike) Cc: mst, stefanha, sgarzare, arei.gonglei, yechuan, huangzhichao, virtualization, linux-kernel, eperezma On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) <longpeng2@huawei.com> wrote: > > From: Longpeng <longpeng2@huawei.com> > > Support doorbell mapping for vdpasim devices, then we can test the notify > passthrough feature even if there's no real hardware on hand. > > Allocates a dummy page which is used to emulate the notify page of the device, > all VQs share the same notify register that initiated to 0xffff. A periodic > work will check whether there're requests need to process ( the value of the > notify register is 0xffff or not ). > --- > drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 ++++++++++++++++++++++++++++++++ > drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++ > 2 files changed, 68 insertions(+) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c > index b071f0d842fb..4fcfeb6e2fb8 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c > @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries, > #define VDPASIM_QUEUE_ALIGN PAGE_SIZE > #define VDPASIM_QUEUE_MAX 256 > #define VDPASIM_VENDOR_ID 0 > +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */ > +#define VDPASIM_NOTIFY_DEFVAL 0xffff > > static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa) > { > @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = { > static const struct vdpa_config_ops vdpasim_config_ops; > static const struct vdpa_config_ops vdpasim_batch_config_ops; > > +static void vdpasim_notify_work(struct work_struct *work) > +{ > + struct vdpasim *vdpasim; > + u16 *val; > + > + vdpasim = container_of(work, struct vdpasim, notify_work.work); > + > + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > + goto out; > + > + if (!vdpasim->running) > + goto out; > + > + val = (u16 *)vdpasim->notify; > + if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL) > + schedule_work(&vdpasim->work); > + > +out: > + schedule_delayed_work(&vdpasim->notify_work, > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > +} > + > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > const struct vdpa_dev_set_config *config) > { > @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > set_dma_ops(dev, &vdpasim_dma_ops); > vdpasim->vdpa.mdev = dev_attr->mgmt_dev; > > + INIT_DELAYED_WORK(&vdpasim->notify_work, vdpasim_notify_work); > + > + vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO); > + if (!vdpasim->notify) > + goto err_iommu; We can simply avoid the advertising notification area in this case. > + *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL; WRITE_ONCE()? > + > vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL); > if (!vdpasim->config) > goto err_iommu; > @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa) > static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status) > { > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > + bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK; Do we need to do the check under the vdpasim->lock? > > spin_lock(&vdpasim->lock); > vdpasim->status = status; > spin_unlock(&vdpasim->lock); > + if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK)) > + schedule_delayed_work(&vdpasim->notify_work, > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > } > > static int vdpasim_reset(struct vdpa_device *vdpa) > { > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > + cancel_delayed_work_sync(&vdpasim->notify_work); Do we need to do this after setting running to zero? Otherwise it's racy. Thanks > spin_lock(&vdpasim->lock); > vdpasim->status = 0; > vdpasim_do_reset(vdpasim); > @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid, > return 0; > } > > +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device *vdpa, > + u16 qid, pgprot_t prot) > +{ > + /* > + * We use normal RAM pages to emulate the vq notification area, so > + * just keep the pgprot as it mmaped. > + */ > + return prot; > +} > + > +static struct vdpa_notification_area > +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid) > +{ > + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > + struct vdpa_notification_area notify; > + > + notify.addr = virt_to_phys((void *)vdpasim->notify); > + notify.size = PAGE_SIZE; > + > + return notify; > +} > + > static void vdpasim_free(struct vdpa_device *vdpa) > { > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > int i; > > + cancel_delayed_work_sync(&vdpasim->notify_work); > cancel_work_sync(&vdpasim->work); > > for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { > @@ -693,6 +752,8 @@ static void vdpasim_free(struct vdpa_device *vdpa) > vhost_iotlb_free(vdpasim->iommu); > kfree(vdpasim->vqs); > kfree(vdpasim->config); > + if (vdpasim->notify) > + free_page(vdpasim->notify); > } > > static const struct vdpa_config_ops vdpasim_config_ops = { > @@ -704,6 +765,8 @@ static const struct vdpa_config_ops vdpasim_config_ops = { > .get_vq_ready = vdpasim_get_vq_ready, > .set_vq_state = vdpasim_set_vq_state, > .get_vq_state = vdpasim_get_vq_state, > + .get_vq_notification = vdpasim_get_vq_notification, > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > .get_vq_align = vdpasim_get_vq_align, > .get_vq_group = vdpasim_get_vq_group, > .get_device_features = vdpasim_get_device_features, > @@ -737,6 +800,8 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { > .get_vq_ready = vdpasim_get_vq_ready, > .set_vq_state = vdpasim_set_vq_state, > .get_vq_state = vdpasim_get_vq_state, > + .get_vq_notification = vdpasim_get_vq_notification, > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > .get_vq_align = vdpasim_get_vq_align, > .get_vq_group = vdpasim_get_vq_group, > .get_device_features = vdpasim_get_device_features, > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h > index 0e78737dcc16..0769ccbd3911 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h > @@ -69,6 +69,9 @@ struct vdpasim { > bool running; > /* spinlock to synchronize iommu table */ > spinlock_t iommu_lock; > + /* dummy notify page */ > + unsigned long notify; > + struct delayed_work notify_work; > }; > > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vdpasim: support doorbell mapping 2023-01-29 6:19 ` Jason Wang @ 2023-02-13 12:05 ` Michael S. Tsirkin -1 siblings, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2023-02-13 12:05 UTC (permalink / raw) To: Jason Wang Cc: linux-kernel, yechuan, eperezma, huangzhichao, stefanha, Longpeng(Mike), virtualization On Sun, Jan 29, 2023 at 02:19:51PM +0800, Jason Wang wrote: > On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) <longpeng2@huawei.com> wrote: > > > > From: Longpeng <longpeng2@huawei.com> > > > > Support doorbell mapping for vdpasim devices, then we can test the notify > > passthrough feature even if there's no real hardware on hand. > > > > Allocates a dummy page which is used to emulate the notify page of the device, > > all VQs share the same notify register that initiated to 0xffff. A periodic > > work will check whether there're requests need to process ( the value of the > > notify register is 0xffff or not ). > > --- > > drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 ++++++++++++++++++++++++++++++++ > > drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++ > > 2 files changed, 68 insertions(+) > > > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c > > index b071f0d842fb..4fcfeb6e2fb8 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c > > @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries, > > #define VDPASIM_QUEUE_ALIGN PAGE_SIZE > > #define VDPASIM_QUEUE_MAX 256 > > #define VDPASIM_VENDOR_ID 0 > > +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */ > > +#define VDPASIM_NOTIFY_DEFVAL 0xffff > > > > static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa) > > { > > @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = { > > static const struct vdpa_config_ops vdpasim_config_ops; > > static const struct vdpa_config_ops vdpasim_batch_config_ops; > > > > +static void vdpasim_notify_work(struct work_struct *work) > > +{ > > + struct vdpasim *vdpasim; > > + u16 *val; > > + > > + vdpasim = container_of(work, struct vdpasim, notify_work.work); > > + > > + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > > + goto out; > > + > > + if (!vdpasim->running) > > + goto out; > > + > > + val = (u16 *)vdpasim->notify; > > + if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL) > > + schedule_work(&vdpasim->work); > > + > > +out: > > + schedule_delayed_work(&vdpasim->notify_work, > > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > > +} > > + > > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > > const struct vdpa_dev_set_config *config) > > { > > @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > > set_dma_ops(dev, &vdpasim_dma_ops); > > vdpasim->vdpa.mdev = dev_attr->mgmt_dev; > > > > + INIT_DELAYED_WORK(&vdpasim->notify_work, vdpasim_notify_work); > > + > > + vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO); > > + if (!vdpasim->notify) > > + goto err_iommu; > > We can simply avoid the advertising notification area in this case. > > > + *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL; > > WRITE_ONCE()? it is just initialization so it should not matter. > > + > > vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL); > > if (!vdpasim->config) > > goto err_iommu; > > @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa) > > static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status) > > { > > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > + bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK; > > Do we need to do the check under the vdpasim->lock? > > > > > spin_lock(&vdpasim->lock); > > vdpasim->status = status; > > spin_unlock(&vdpasim->lock); > > + if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK)) > > + schedule_delayed_work(&vdpasim->notify_work, > > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > > } > > > > static int vdpasim_reset(struct vdpa_device *vdpa) > > { > > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > > > + cancel_delayed_work_sync(&vdpasim->notify_work); > > Do we need to do this after setting running to zero? Otherwise it's racy. > > Thanks > > > spin_lock(&vdpasim->lock); > > vdpasim->status = 0; > > vdpasim_do_reset(vdpasim); > > @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid, > > return 0; > > } > > > > +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device *vdpa, > > + u16 qid, pgprot_t prot) > > +{ > > + /* > > + * We use normal RAM pages to emulate the vq notification area, so > > + * just keep the pgprot as it mmaped. > > + */ > > + return prot; > > +} > > + > > +static struct vdpa_notification_area > > +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid) > > +{ > > + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > + struct vdpa_notification_area notify; > > + > > + notify.addr = virt_to_phys((void *)vdpasim->notify); > > + notify.size = PAGE_SIZE; > > + > > + return notify; > > +} > > + > > static void vdpasim_free(struct vdpa_device *vdpa) > > { > > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > int i; > > > > + cancel_delayed_work_sync(&vdpasim->notify_work); > > cancel_work_sync(&vdpasim->work); > > > > for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { > > @@ -693,6 +752,8 @@ static void vdpasim_free(struct vdpa_device *vdpa) > > vhost_iotlb_free(vdpasim->iommu); > > kfree(vdpasim->vqs); > > kfree(vdpasim->config); > > + if (vdpasim->notify) > > + free_page(vdpasim->notify); > > } > > > > static const struct vdpa_config_ops vdpasim_config_ops = { > > @@ -704,6 +765,8 @@ static const struct vdpa_config_ops vdpasim_config_ops = { > > .get_vq_ready = vdpasim_get_vq_ready, > > .set_vq_state = vdpasim_set_vq_state, > > .get_vq_state = vdpasim_get_vq_state, > > + .get_vq_notification = vdpasim_get_vq_notification, > > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > > .get_vq_align = vdpasim_get_vq_align, > > .get_vq_group = vdpasim_get_vq_group, > > .get_device_features = vdpasim_get_device_features, > > @@ -737,6 +800,8 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { > > .get_vq_ready = vdpasim_get_vq_ready, > > .set_vq_state = vdpasim_set_vq_state, > > .get_vq_state = vdpasim_get_vq_state, > > + .get_vq_notification = vdpasim_get_vq_notification, > > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > > .get_vq_align = vdpasim_get_vq_align, > > .get_vq_group = vdpasim_get_vq_group, > > .get_device_features = vdpasim_get_device_features, > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h > > index 0e78737dcc16..0769ccbd3911 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h > > @@ -69,6 +69,9 @@ struct vdpasim { > > bool running; > > /* spinlock to synchronize iommu table */ > > spinlock_t iommu_lock; > > + /* dummy notify page */ > > + unsigned long notify; > > + struct delayed_work notify_work; > > }; > > > > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, > > -- > > 2.23.0 > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vdpasim: support doorbell mapping @ 2023-02-13 12:05 ` Michael S. Tsirkin 0 siblings, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2023-02-13 12:05 UTC (permalink / raw) To: Jason Wang Cc: Longpeng(Mike), stefanha, sgarzare, arei.gonglei, yechuan, huangzhichao, virtualization, linux-kernel, eperezma On Sun, Jan 29, 2023 at 02:19:51PM +0800, Jason Wang wrote: > On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) <longpeng2@huawei.com> wrote: > > > > From: Longpeng <longpeng2@huawei.com> > > > > Support doorbell mapping for vdpasim devices, then we can test the notify > > passthrough feature even if there's no real hardware on hand. > > > > Allocates a dummy page which is used to emulate the notify page of the device, > > all VQs share the same notify register that initiated to 0xffff. A periodic > > work will check whether there're requests need to process ( the value of the > > notify register is 0xffff or not ). > > --- > > drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 ++++++++++++++++++++++++++++++++ > > drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++ > > 2 files changed, 68 insertions(+) > > > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c > > index b071f0d842fb..4fcfeb6e2fb8 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c > > @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries, > > #define VDPASIM_QUEUE_ALIGN PAGE_SIZE > > #define VDPASIM_QUEUE_MAX 256 > > #define VDPASIM_VENDOR_ID 0 > > +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */ > > +#define VDPASIM_NOTIFY_DEFVAL 0xffff > > > > static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa) > > { > > @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = { > > static const struct vdpa_config_ops vdpasim_config_ops; > > static const struct vdpa_config_ops vdpasim_batch_config_ops; > > > > +static void vdpasim_notify_work(struct work_struct *work) > > +{ > > + struct vdpasim *vdpasim; > > + u16 *val; > > + > > + vdpasim = container_of(work, struct vdpasim, notify_work.work); > > + > > + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > > + goto out; > > + > > + if (!vdpasim->running) > > + goto out; > > + > > + val = (u16 *)vdpasim->notify; > > + if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL) > > + schedule_work(&vdpasim->work); > > + > > +out: > > + schedule_delayed_work(&vdpasim->notify_work, > > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > > +} > > + > > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > > const struct vdpa_dev_set_config *config) > > { > > @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, > > set_dma_ops(dev, &vdpasim_dma_ops); > > vdpasim->vdpa.mdev = dev_attr->mgmt_dev; > > > > + INIT_DELAYED_WORK(&vdpasim->notify_work, vdpasim_notify_work); > > + > > + vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO); > > + if (!vdpasim->notify) > > + goto err_iommu; > > We can simply avoid the advertising notification area in this case. > > > + *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL; > > WRITE_ONCE()? it is just initialization so it should not matter. > > + > > vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL); > > if (!vdpasim->config) > > goto err_iommu; > > @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa) > > static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status) > > { > > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > + bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK; > > Do we need to do the check under the vdpasim->lock? > > > > > spin_lock(&vdpasim->lock); > > vdpasim->status = status; > > spin_unlock(&vdpasim->lock); > > + if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK)) > > + schedule_delayed_work(&vdpasim->notify_work, > > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); > > } > > > > static int vdpasim_reset(struct vdpa_device *vdpa) > > { > > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > > > + cancel_delayed_work_sync(&vdpasim->notify_work); > > Do we need to do this after setting running to zero? Otherwise it's racy. > > Thanks > > > spin_lock(&vdpasim->lock); > > vdpasim->status = 0; > > vdpasim_do_reset(vdpasim); > > @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid, > > return 0; > > } > > > > +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device *vdpa, > > + u16 qid, pgprot_t prot) > > +{ > > + /* > > + * We use normal RAM pages to emulate the vq notification area, so > > + * just keep the pgprot as it mmaped. > > + */ > > + return prot; > > +} > > + > > +static struct vdpa_notification_area > > +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid) > > +{ > > + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > + struct vdpa_notification_area notify; > > + > > + notify.addr = virt_to_phys((void *)vdpasim->notify); > > + notify.size = PAGE_SIZE; > > + > > + return notify; > > +} > > + > > static void vdpasim_free(struct vdpa_device *vdpa) > > { > > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > int i; > > > > + cancel_delayed_work_sync(&vdpasim->notify_work); > > cancel_work_sync(&vdpasim->work); > > > > for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { > > @@ -693,6 +752,8 @@ static void vdpasim_free(struct vdpa_device *vdpa) > > vhost_iotlb_free(vdpasim->iommu); > > kfree(vdpasim->vqs); > > kfree(vdpasim->config); > > + if (vdpasim->notify) > > + free_page(vdpasim->notify); > > } > > > > static const struct vdpa_config_ops vdpasim_config_ops = { > > @@ -704,6 +765,8 @@ static const struct vdpa_config_ops vdpasim_config_ops = { > > .get_vq_ready = vdpasim_get_vq_ready, > > .set_vq_state = vdpasim_set_vq_state, > > .get_vq_state = vdpasim_get_vq_state, > > + .get_vq_notification = vdpasim_get_vq_notification, > > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > > .get_vq_align = vdpasim_get_vq_align, > > .get_vq_group = vdpasim_get_vq_group, > > .get_device_features = vdpasim_get_device_features, > > @@ -737,6 +800,8 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { > > .get_vq_ready = vdpasim_get_vq_ready, > > .set_vq_state = vdpasim_set_vq_state, > > .get_vq_state = vdpasim_get_vq_state, > > + .get_vq_notification = vdpasim_get_vq_notification, > > + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, > > .get_vq_align = vdpasim_get_vq_align, > > .get_vq_group = vdpasim_get_vq_group, > > .get_device_features = vdpasim_get_device_features, > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h > > index 0e78737dcc16..0769ccbd3911 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h > > @@ -69,6 +69,9 @@ struct vdpasim { > > bool running; > > /* spinlock to synchronize iommu table */ > > spinlock_t iommu_lock; > > + /* dummy notify page */ > > + unsigned long notify; > > + struct delayed_work notify_work; > > }; > > > > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, > > -- > > 2.23.0 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] vdpasim: support doorbell mapping 2023-02-13 12:05 ` Michael S. Tsirkin (?) @ 2023-02-14 2:05 ` Longpeng (Mike, Cloud Infrastructure Service Product Dept.) -1 siblings, 0 replies; 10+ messages in thread From: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) @ 2023-02-14 2:05 UTC (permalink / raw) To: Michael S. Tsirkin, Jason Wang Cc: stefanha, sgarzare, arei.gonglei, yechuan, huangzhichao, virtualization, linux-kernel, eperezma 在 2023/2/13 20:05, Michael S. Tsirkin 写道: > On Sun, Jan 29, 2023 at 02:19:51PM +0800, Jason Wang wrote: >> On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) <longpeng2@huawei.com> wrote: >>> >>> From: Longpeng <longpeng2@huawei.com> >>> >>> Support doorbell mapping for vdpasim devices, then we can test the notify >>> passthrough feature even if there's no real hardware on hand. >>> >>> Allocates a dummy page which is used to emulate the notify page of the device, >>> all VQs share the same notify register that initiated to 0xffff. A periodic >>> work will check whether there're requests need to process ( the value of the >>> notify register is 0xffff or not ). >>> --- >>> drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 ++++++++++++++++++++++++++++++++ >>> drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++ >>> 2 files changed, 68 insertions(+) >>> >>> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c >>> index b071f0d842fb..4fcfeb6e2fb8 100644 >>> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c >>> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c >>> @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries, >>> #define VDPASIM_QUEUE_ALIGN PAGE_SIZE >>> #define VDPASIM_QUEUE_MAX 256 >>> #define VDPASIM_VENDOR_ID 0 >>> +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */ >>> +#define VDPASIM_NOTIFY_DEFVAL 0xffff >>> >>> static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa) >>> { >>> @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = { >>> static const struct vdpa_config_ops vdpasim_config_ops; >>> static const struct vdpa_config_ops vdpasim_batch_config_ops; >>> >>> +static void vdpasim_notify_work(struct work_struct *work) >>> +{ >>> + struct vdpasim *vdpasim; >>> + u16 *val; >>> + >>> + vdpasim = container_of(work, struct vdpasim, notify_work.work); >>> + >>> + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) >>> + goto out; >>> + >>> + if (!vdpasim->running) >>> + goto out; >>> + >>> + val = (u16 *)vdpasim->notify; >>> + if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL) >>> + schedule_work(&vdpasim->work); >>> + >>> +out: >>> + schedule_delayed_work(&vdpasim->notify_work, >>> + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); >>> +} >>> + >>> struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, >>> const struct vdpa_dev_set_config *config) >>> { >>> @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, >>> set_dma_ops(dev, &vdpasim_dma_ops); >>> vdpasim->vdpa.mdev = dev_attr->mgmt_dev; >>> >>> + INIT_DELAYED_WORK(&vdpasim->notify_work, vdpasim_notify_work); >>> + >>> + vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO); >>> + if (!vdpasim->notify) >>> + goto err_iommu; >> >> We can simply avoid the advertising notification area in this case. >> >>> + *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL; >> >> WRITE_ONCE()? > > it is just initialization so it should not matter. > I think so. And WRITE_ONCE seems operate on a variable, but we want write value into a specific memory location here. >>> + >>> vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL); >>> if (!vdpasim->config) >>> goto err_iommu; >>> @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa) >>> static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status) >>> { >>> struct vdpasim *vdpasim = vdpa_to_sim(vdpa); >>> + bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK; >> >> Do we need to do the check under the vdpasim->lock? >> >>> >>> spin_lock(&vdpasim->lock); >>> vdpasim->status = status; >>> spin_unlock(&vdpasim->lock); >>> + if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK)) >>> + schedule_delayed_work(&vdpasim->notify_work, >>> + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD)); >>> } >>> >>> static int vdpasim_reset(struct vdpa_device *vdpa) >>> { >>> struct vdpasim *vdpasim = vdpa_to_sim(vdpa); >>> >>> + cancel_delayed_work_sync(&vdpasim->notify_work); >> >> Do we need to do this after setting running to zero? Otherwise it's racy. >> >> Thanks >> >>> spin_lock(&vdpasim->lock); >>> vdpasim->status = 0; >>> vdpasim_do_reset(vdpasim); >>> @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid, >>> return 0; >>> } >>> >>> +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device *vdpa, >>> + u16 qid, pgprot_t prot) >>> +{ >>> + /* >>> + * We use normal RAM pages to emulate the vq notification area, so >>> + * just keep the pgprot as it mmaped. >>> + */ >>> + return prot; >>> +} >>> + >>> +static struct vdpa_notification_area >>> +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid) >>> +{ >>> + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); >>> + struct vdpa_notification_area notify; >>> + >>> + notify.addr = virt_to_phys((void *)vdpasim->notify); >>> + notify.size = PAGE_SIZE; >>> + >>> + return notify; >>> +} >>> + >>> static void vdpasim_free(struct vdpa_device *vdpa) >>> { >>> struct vdpasim *vdpasim = vdpa_to_sim(vdpa); >>> int i; >>> >>> + cancel_delayed_work_sync(&vdpasim->notify_work); >>> cancel_work_sync(&vdpasim->work); >>> >>> for (i = 0; i < vdpasim->dev_attr.nvqs; i++) { >>> @@ -693,6 +752,8 @@ static void vdpasim_free(struct vdpa_device *vdpa) >>> vhost_iotlb_free(vdpasim->iommu); >>> kfree(vdpasim->vqs); >>> kfree(vdpasim->config); >>> + if (vdpasim->notify) >>> + free_page(vdpasim->notify); >>> } >>> >>> static const struct vdpa_config_ops vdpasim_config_ops = { >>> @@ -704,6 +765,8 @@ static const struct vdpa_config_ops vdpasim_config_ops = { >>> .get_vq_ready = vdpasim_get_vq_ready, >>> .set_vq_state = vdpasim_set_vq_state, >>> .get_vq_state = vdpasim_get_vq_state, >>> + .get_vq_notification = vdpasim_get_vq_notification, >>> + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, >>> .get_vq_align = vdpasim_get_vq_align, >>> .get_vq_group = vdpasim_get_vq_group, >>> .get_device_features = vdpasim_get_device_features, >>> @@ -737,6 +800,8 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { >>> .get_vq_ready = vdpasim_get_vq_ready, >>> .set_vq_state = vdpasim_set_vq_state, >>> .get_vq_state = vdpasim_get_vq_state, >>> + .get_vq_notification = vdpasim_get_vq_notification, >>> + .get_vq_notification_pgprot = vdpasim_get_vq_notification_pgprot, >>> .get_vq_align = vdpasim_get_vq_align, >>> .get_vq_group = vdpasim_get_vq_group, >>> .get_device_features = vdpasim_get_device_features, >>> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h >>> index 0e78737dcc16..0769ccbd3911 100644 >>> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h >>> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h >>> @@ -69,6 +69,9 @@ struct vdpasim { >>> bool running; >>> /* spinlock to synchronize iommu table */ >>> spinlock_t iommu_lock; >>> + /* dummy notify page */ >>> + unsigned long notify; >>> + struct delayed_work notify_work; >>> }; >>> >>> struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, >>> -- >>> 2.23.0 >>> > > . ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-02-14 2:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-29 2:50 [PATCH v3 0/2] vdpasim: support doorbell mapping Longpeng(Mike) 2023-01-29 2:50 ` [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area Longpeng(Mike) 2023-01-29 6:03 ` Jason Wang 2023-01-29 6:03 ` Jason Wang 2023-01-29 2:50 ` [PATCH v3 2/2] vdpasim: support doorbell mapping Longpeng(Mike) 2023-01-29 6:19 ` Jason Wang 2023-01-29 6:19 ` Jason Wang 2023-02-13 12:05 ` Michael S. Tsirkin 2023-02-13 12:05 ` Michael S. Tsirkin 2023-02-14 2:05 ` Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.