On Mon, Jul 21, 2025 at 04:05:17PM +0800, Jason Wang wrote:
> On Fri, Jul 18, 2025 at 6:09 PM Jason Wang <jasow...@redhat.com> wrote:
> >
> > On Fri, Jul 18, 2025 at 5:37 PM Michael S. Tsirkin <m...@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
