Re: [PATCH V4 7/9] vdpa: rename dma_dev to map_token

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jul 21, 2025 at 04:05:17PM +0800, Jason Wang wrote:
> On Fri, Jul 18, 2025 at 6:09 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, Jul 18, 2025 at 5:37 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Fri, Jul 18, 2025 at 05:16:14PM +0800, Jason Wang wrote:
> > > > Virtio core switches from DMA device to mapping token, let's do that
> > > > as well for vDPA.
> > >
> > > Pls switch to imperative mood.
> > >
> > > And add explanation about what is going on and why please.
> >
> > I think this has been explained in patch [PATCH V4 6/9] virtio:
> > introduce map ops in virtio core?
> >
> > """
> > For the device or transport that doesn't do DMA,
> > they can implement their own mapping logic without the need to trick
> > DMA core. In this case the map_token is opaque to the virtio core that
> > will be passed back to the transport or device specific map
> > operations. For other devices, DMA API will still be used, so map
> > token will still be the dma device to minimize the changeset and
> > performance impact.
> > """
> >
> > >
> > > At least, document what are the actual types stored here.
> >
> > It is opaque to the virtio core and will be used as a token to be
> > passed back when the virtio core wants to map/unmap a buffer. So it
> > doesn't have a type.
> >
> > > I checked and it looks like vduse actually returns struct device * here, too?
> >
> > It's just a token, vduse can return anything that can be used to
> > identify an iova domain. For example, it can return a pointer to
> > iova_domain which will also work. If you prefer to use iova domain, I
> > can change to use that in the next version.
> >
> > > So why do we need this, why lose all type safety?
> >
> > If you worry about the case that assumes map_token as dma device in
> > virtio core. I can keep both map_token and dma_device and make the
> > mutually exclusive, then:
> >
> > - when transport reports dma device, use DMA API
> > - when transport reports map token, use map ops
> >
> > Does this work for you?
> 
> This seems to result in a lot of unnecessary code. I wonder if the
> following would work:
> 
> Just document the requirement in vring_create_virtqueue_map, say
> something like when a virtio device doesn't have a map operation,
> map_token must be a pointer to struct device.
> 
> Please give me some advice so I can work on a new version..
> 
> Thanks
> 
> >
> > Thanks


How about something like this?

typedef union {
	struct device *dma_dev;
	struct vduse_iova_domain *iova_domain;
	void *ptr;
} virtio_dma_dev_t;



Now, just pass this around.
You can also add an enum with the pointer:

typedef struct {
	union {
		struct device *dma_dev;
		struct vduse_iova_domain *iova_domain;
		void *ptr;
	};
	enum {
		VIRTIO_DMA,
		VIRTIO_IOVA
	} type;
} virtio_dma_dev_t;

(possibly only on debug builds, if you find it's costly?)

This allows encapsulation and asserts that the type is right.


-- 
MST