| Message ID | 20210519162903.1172366-16-eperezma@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vDPA software assisted live migration | expand |
在 2021/5/20 上午12:28, Eugenio Pérez 写道: > This operation enable the backend-specific IOTLB entries. > > If a backend support this, it start managing its own entries, and vhost > can disable it through this operation and recover control. > > Every enable/disable operation must also clear all IOTLB device entries. > > At the moment, the only backend that does so is vhost-vdpa. To fully > support these, vdpa needs also to expose a way for vhost subsystem to > map and unmap entries. This will be done in future commits. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> I think there's probably no need to introduce this helper. Instead, we can introduce ops like shadow_vq_start()/stop(). Then the details like this could be hided there. (And hide the backend deatils (avoid calling vhost_vdpa_dma_map()) directly from the vhost.c) Thanks > --- > include/hw/virtio/vhost-backend.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > index bcb112c166..f8eed2ace5 100644 > --- a/include/hw/virtio/vhost-backend.h > +++ b/include/hw/virtio/vhost-backend.h > @@ -128,6 +128,9 @@ typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev); > > typedef int (*vhost_vring_pause_op)(struct vhost_dev *dev); > > +typedef int (*vhost_enable_custom_iommu_op)(struct vhost_dev *dev, > + bool enable); > + > typedef int (*vhost_get_iova_range)(struct vhost_dev *dev, > hwaddr *first, hwaddr *last); > > @@ -177,6 +180,7 @@ typedef struct VhostOps { > vhost_get_device_id_op vhost_get_device_id; > vhost_vring_pause_op vhost_vring_pause; > vhost_force_iommu_op vhost_force_iommu; > + vhost_enable_custom_iommu_op vhost_enable_custom_iommu; > vhost_get_iova_range vhost_get_iova_range; > } VhostOps; >
On Mon, May 31, 2021 at 11:02 AM Jason Wang <jasowang@redhat.com> wrote: > > > 在 2021/5/20 上午12:28, Eugenio Pérez 写道: > > This operation enable the backend-specific IOTLB entries. > > > > If a backend support this, it start managing its own entries, and vhost > > can disable it through this operation and recover control. > > > > Every enable/disable operation must also clear all IOTLB device entries. > > > > At the moment, the only backend that does so is vhost-vdpa. To fully > > support these, vdpa needs also to expose a way for vhost subsystem to > > map and unmap entries. This will be done in future commits. > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > I think there's probably no need to introduce this helper. > > Instead, we can introduce ops like shadow_vq_start()/stop(). Then the > details like this could be hided there. > I'm also fine with your approach, but then the ownership of the shadow virtqueue would be splitted between vhost code and the vhost backend code. With the current code, vhost is in charge for mapping DMA entries, and delegates in the backend when the latter has its own means of map [1]. If we just expose shadow_vq_start/stop, the logic of when to map gets somehow duplicated in vhost and in the backend, and it is not obvious that future code changes in one side need to be duplicated in the backend. I understand that this way needs to expose more vhost operations, but I think each of these are smaller and fit better in "vhost backend implementation of an operation" than just telling the backend that shadow vq is started. > (And hide the backend deatils (avoid calling vhost_vdpa_dma_map()) > directly from the vhost.c) > Sure, the direct call of vhost_vdpa_dma_map is not intended to be that way in the final series, it's just an intermediate step. I could have been more explicit about that, sorry. [1] At the moment it just calls vhost_vdpa_dma_map directly, but this should be changed by a vhost_ops, and that op is optional: If not present, vIOMMU is used. > Thanks > > > > --- > > include/hw/virtio/vhost-backend.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > > index bcb112c166..f8eed2ace5 100644 > > --- a/include/hw/virtio/vhost-backend.h > > +++ b/include/hw/virtio/vhost-backend.h > > @@ -128,6 +128,9 @@ typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev); > > > > typedef int (*vhost_vring_pause_op)(struct vhost_dev *dev); > > > > +typedef int (*vhost_enable_custom_iommu_op)(struct vhost_dev *dev, > > + bool enable); > > + > > typedef int (*vhost_get_iova_range)(struct vhost_dev *dev, > > hwaddr *first, hwaddr *last); > > > > @@ -177,6 +180,7 @@ typedef struct VhostOps { > > vhost_get_device_id_op vhost_get_device_id; > > vhost_vring_pause_op vhost_vring_pause; > > vhost_force_iommu_op vhost_force_iommu; > > + vhost_enable_custom_iommu_op vhost_enable_custom_iommu; > > vhost_get_iova_range vhost_get_iova_range; > > } VhostOps; > > >
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h index bcb112c166..f8eed2ace5 100644 --- a/include/hw/virtio/vhost-backend.h +++ b/include/hw/virtio/vhost-backend.h @@ -128,6 +128,9 @@ typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev); typedef int (*vhost_vring_pause_op)(struct vhost_dev *dev); +typedef int (*vhost_enable_custom_iommu_op)(struct vhost_dev *dev, + bool enable); + typedef int (*vhost_get_iova_range)(struct vhost_dev *dev, hwaddr *first, hwaddr *last); @@ -177,6 +180,7 @@ typedef struct VhostOps { vhost_get_device_id_op vhost_get_device_id; vhost_vring_pause_op vhost_vring_pause; vhost_force_iommu_op vhost_force_iommu; + vhost_enable_custom_iommu_op vhost_enable_custom_iommu; vhost_get_iova_range vhost_get_iova_range; } VhostOps;
This operation enable the backend-specific IOTLB entries. If a backend support this, it start managing its own entries, and vhost can disable it through this operation and recover control. Every enable/disable operation must also clear all IOTLB device entries. At the moment, the only backend that does so is vhost-vdpa. To fully support these, vdpa needs also to expose a way for vhost subsystem to map and unmap entries. This will be done in future commits. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- include/hw/virtio/vhost-backend.h | 4 ++++ 1 file changed, 4 insertions(+)