Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Wed, Oct 20, 2021 at 7:57 PM Eugenio Perez Martin
wrote:
>
> On Wed, Oct 20, 2021 at 11:03 AM Jason Wang wrote:
> >
> > On Wed, Oct 20, 2021 at 2:52 PM Eugenio Perez Martin
> > wrote:
> > >
> > > On Wed, Oct 20, 2021 at 4:07 AM Jason Wang wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 10:02 AM Jason Wang wrote:
> > > > >
> > > > > On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
> > > > > wrote:
> > > > > >
> > > > > > On Tue, Oct 19, 2021 at 11:25 AM Jason Wang
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > > > > > Use translations added in VhostIOVATree in SVQ.
> > > > > > > >
> > > > > > > > Now every element needs to store the previous address also, so
> > > > > > > > VirtQueue
> > > > > > > > can consume the elements properly. This adds a little overhead
> > > > > > > > per VQ
> > > > > > > > element, having to allocate more memory to stash them. As a
> > > > > > > > possible
> > > > > > > > optimization, this allocation could be avoided if the
> > > > > > > > descriptor is not
> > > > > > > > a chain but a single one, but this is left undone.
> > > > > > > >
> > > > > > > > TODO: iova range should be queried before, and add logic to
> > > > > > > > fail when
> > > > > > > > GPA is outside of its range and memory listener or svq add it.
> > > > > > > >
> > > > > > > > Signed-off-by: Eugenio Pérez
> > > > > > > > ---
> > > > > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > > > > > > > hw/virtio/vhost-shadow-virtqueue.c | 130
> > > > > > > > -
> > > > > > > > hw/virtio/vhost-vdpa.c | 40 -
> > > > > > > > hw/virtio/trace-events | 1 +
> > > > > > > > 4 files changed, 152 insertions(+), 23 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > index b7baa424a7..a0e6b5267a 100644
> > > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > @@ -11,6 +11,7 @@
> > > > > > > > #define VHOST_SHADOW_VIRTQUEUE_H
> > > > > > > >
> > > > > > > > #include "hw/virtio/vhost.h"
> > > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > > >
> > > > > > > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > > > > > > >
> > > > > > > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev,
> > > > > > > > unsigned idx,
> > > > > > > > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > > > > > > VhostShadowVirtqueue *svq);
> > > > > > > >
> > > > > > > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int
> > > > > > > > idx);
> > > > > > > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int
> > > > > > > > idx,
> > > > > > > > +VhostIOVATree *iova_map);
> > > > > > > >
> > > > > > > > void vhost_svq_free(VhostShadowVirtqueue *vq);
> > > > > > > >
> > > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > index 2fd0bab75d..9db538547e 100644
> > > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > @@ -11,12 +11,19 @@
> > > > > > > > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > > > > > > #include "hw/virtio/vhost.h"
> > > > > > > > #include "hw/virtio/virtio-access.h"
> > > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > > >
> > > > > > > > #include "standard-headers/linux/vhost_types.h"
> > > > > > > >
> > > > > > > > #include "qemu/error-report.h"
> > > > > > > > #include "qemu/main-loop.h"
> > > > > > > >
> > > > > > > > +typedef struct SVQElement {
> > > > > > > > +VirtQueueElement elem;
> > > > > > > > +void **in_sg_stash;
> > > > > > > > +void **out_sg_stash;
> > > > > > > > +} SVQElement;
> > > > > > > > +
> > > > > > > > /* Shadow virtqueue to relay notifications */
> > > > > > > > typedef struct VhostShadowVirtqueue {
> > > > > > > > /* Shadow vring */
> > > > > > > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > > > > > > /* Virtio device */
> > > > > > > > VirtIODevice *vdev;
> > > > > > > >
> > > > > > > > +/* IOVA mapping if used */
> > > > > > > > +VhostIOVATree *iova_map;
> > > > > > > > +
> > > > > > > > /* Map for returning guest's descriptors */
> > > > > > > > -VirtQueueElement **ring_id_maps;
> > > > > > > > +SVQElement **ring_id_maps;
> > > > > > > >
> > > > > > > > /* Next head to expose to device */
> > > > > > > > uint16_t avail_idx_shadow;
> > > > > > > > @@ -79,13 +89,6 @@ bool
> > > > > > > > vhost_svq_valid_device_features(uint64_t *dev_features)
> > > > > > > > continue;
> > > > > > > >
> > > > > > > > case VIRTIO_F_ACCESS_PLATFORM:
> > > > > >
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Wed, Oct 20, 2021 at 7:57 PM Eugenio Perez Martin
wrote:
>
> On Wed, Oct 20, 2021 at 11:03 AM Jason Wang wrote:
> >
> > On Wed, Oct 20, 2021 at 2:52 PM Eugenio Perez Martin
> > wrote:
> > >
> > > On Wed, Oct 20, 2021 at 4:07 AM Jason Wang wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 10:02 AM Jason Wang wrote:
> > > > >
> > > > > On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
> > > > > wrote:
> > > > > >
> > > > > > On Tue, Oct 19, 2021 at 11:25 AM Jason Wang
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > > > > > Use translations added in VhostIOVATree in SVQ.
> > > > > > > >
> > > > > > > > Now every element needs to store the previous address also, so
> > > > > > > > VirtQueue
> > > > > > > > can consume the elements properly. This adds a little overhead
> > > > > > > > per VQ
> > > > > > > > element, having to allocate more memory to stash them. As a
> > > > > > > > possible
> > > > > > > > optimization, this allocation could be avoided if the
> > > > > > > > descriptor is not
> > > > > > > > a chain but a single one, but this is left undone.
> > > > > > > >
> > > > > > > > TODO: iova range should be queried before, and add logic to
> > > > > > > > fail when
> > > > > > > > GPA is outside of its range and memory listener or svq add it.
> > > > > > > >
> > > > > > > > Signed-off-by: Eugenio Pérez
> > > > > > > > ---
> > > > > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > > > > > > > hw/virtio/vhost-shadow-virtqueue.c | 130
> > > > > > > > -
> > > > > > > > hw/virtio/vhost-vdpa.c | 40 -
> > > > > > > > hw/virtio/trace-events | 1 +
> > > > > > > > 4 files changed, 152 insertions(+), 23 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > index b7baa424a7..a0e6b5267a 100644
> > > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > @@ -11,6 +11,7 @@
> > > > > > > > #define VHOST_SHADOW_VIRTQUEUE_H
> > > > > > > >
> > > > > > > > #include "hw/virtio/vhost.h"
> > > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > > >
> > > > > > > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > > > > > > >
> > > > > > > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev,
> > > > > > > > unsigned idx,
> > > > > > > > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > > > > > > VhostShadowVirtqueue *svq);
> > > > > > > >
> > > > > > > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int
> > > > > > > > idx);
> > > > > > > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int
> > > > > > > > idx,
> > > > > > > > +VhostIOVATree *iova_map);
> > > > > > > >
> > > > > > > > void vhost_svq_free(VhostShadowVirtqueue *vq);
> > > > > > > >
> > > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > index 2fd0bab75d..9db538547e 100644
> > > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > @@ -11,12 +11,19 @@
> > > > > > > > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > > > > > > #include "hw/virtio/vhost.h"
> > > > > > > > #include "hw/virtio/virtio-access.h"
> > > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > > >
> > > > > > > > #include "standard-headers/linux/vhost_types.h"
> > > > > > > >
> > > > > > > > #include "qemu/error-report.h"
> > > > > > > > #include "qemu/main-loop.h"
> > > > > > > >
> > > > > > > > +typedef struct SVQElement {
> > > > > > > > +VirtQueueElement elem;
> > > > > > > > +void **in_sg_stash;
> > > > > > > > +void **out_sg_stash;
> > > > > > > > +} SVQElement;
> > > > > > > > +
> > > > > > > > /* Shadow virtqueue to relay notifications */
> > > > > > > > typedef struct VhostShadowVirtqueue {
> > > > > > > > /* Shadow vring */
> > > > > > > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > > > > > > /* Virtio device */
> > > > > > > > VirtIODevice *vdev;
> > > > > > > >
> > > > > > > > +/* IOVA mapping if used */
> > > > > > > > +VhostIOVATree *iova_map;
> > > > > > > > +
> > > > > > > > /* Map for returning guest's descriptors */
> > > > > > > > -VirtQueueElement **ring_id_maps;
> > > > > > > > +SVQElement **ring_id_maps;
> > > > > > > >
> > > > > > > > /* Next head to expose to device */
> > > > > > > > uint16_t avail_idx_shadow;
> > > > > > > > @@ -79,13 +89,6 @@ bool
> > > > > > > > vhost_svq_valid_device_features(uint64_t *dev_features)
> > > > > > > > continue;
> > > > > > > >
> > > > > > > > case VIRTIO_F_ACCESS_PLATFORM:
> > > > > >
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Wed, Oct 20, 2021 at 11:03 AM Jason Wang wrote:
>
> On Wed, Oct 20, 2021 at 2:52 PM Eugenio Perez Martin
> wrote:
> >
> > On Wed, Oct 20, 2021 at 4:07 AM Jason Wang wrote:
> > >
> > > On Wed, Oct 20, 2021 at 10:02 AM Jason Wang wrote:
> > > >
> > > > On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
> > > > wrote:
> > > > >
> > > > > On Tue, Oct 19, 2021 at 11:25 AM Jason Wang
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > > > > Use translations added in VhostIOVATree in SVQ.
> > > > > > >
> > > > > > > Now every element needs to store the previous address also, so
> > > > > > > VirtQueue
> > > > > > > can consume the elements properly. This adds a little overhead
> > > > > > > per VQ
> > > > > > > element, having to allocate more memory to stash them. As a
> > > > > > > possible
> > > > > > > optimization, this allocation could be avoided if the descriptor
> > > > > > > is not
> > > > > > > a chain but a single one, but this is left undone.
> > > > > > >
> > > > > > > TODO: iova range should be queried before, and add logic to fail
> > > > > > > when
> > > > > > > GPA is outside of its range and memory listener or svq add it.
> > > > > > >
> > > > > > > Signed-off-by: Eugenio Pérez
> > > > > > > ---
> > > > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > > > > > > hw/virtio/vhost-shadow-virtqueue.c | 130
> > > > > > > -
> > > > > > > hw/virtio/vhost-vdpa.c | 40 -
> > > > > > > hw/virtio/trace-events | 1 +
> > > > > > > 4 files changed, 152 insertions(+), 23 deletions(-)
> > > > > > >
> > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > index b7baa424a7..a0e6b5267a 100644
> > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > @@ -11,6 +11,7 @@
> > > > > > > #define VHOST_SHADOW_VIRTQUEUE_H
> > > > > > >
> > > > > > > #include "hw/virtio/vhost.h"
> > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > >
> > > > > > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > > > > > >
> > > > > > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev,
> > > > > > > unsigned idx,
> > > > > > > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > > > > > VhostShadowVirtqueue *svq);
> > > > > > >
> > > > > > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int
> > > > > > > idx);
> > > > > > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int
> > > > > > > idx,
> > > > > > > +VhostIOVATree *iova_map);
> > > > > > >
> > > > > > > void vhost_svq_free(VhostShadowVirtqueue *vq);
> > > > > > >
> > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > index 2fd0bab75d..9db538547e 100644
> > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > @@ -11,12 +11,19 @@
> > > > > > > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > > > > > #include "hw/virtio/vhost.h"
> > > > > > > #include "hw/virtio/virtio-access.h"
> > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > >
> > > > > > > #include "standard-headers/linux/vhost_types.h"
> > > > > > >
> > > > > > > #include "qemu/error-report.h"
> > > > > > > #include "qemu/main-loop.h"
> > > > > > >
> > > > > > > +typedef struct SVQElement {
> > > > > > > +VirtQueueElement elem;
> > > > > > > +void **in_sg_stash;
> > > > > > > +void **out_sg_stash;
> > > > > > > +} SVQElement;
> > > > > > > +
> > > > > > > /* Shadow virtqueue to relay notifications */
> > > > > > > typedef struct VhostShadowVirtqueue {
> > > > > > > /* Shadow vring */
> > > > > > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > > > > > /* Virtio device */
> > > > > > > VirtIODevice *vdev;
> > > > > > >
> > > > > > > +/* IOVA mapping if used */
> > > > > > > +VhostIOVATree *iova_map;
> > > > > > > +
> > > > > > > /* Map for returning guest's descriptors */
> > > > > > > -VirtQueueElement **ring_id_maps;
> > > > > > > +SVQElement **ring_id_maps;
> > > > > > >
> > > > > > > /* Next head to expose to device */
> > > > > > > uint16_t avail_idx_shadow;
> > > > > > > @@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t
> > > > > > > *dev_features)
> > > > > > > continue;
> > > > > > >
> > > > > > > case VIRTIO_F_ACCESS_PLATFORM:
> > > > > > > -/* SVQ needs this feature disabled. Can't continue */
> > > > > > > -if (*dev_features & BIT_ULL(b)) {
> > > > > > > -clear_bit(b, dev_features);
> > > > > > > -r = false;
> > > > > > > -}
> > > > > > > -
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Wed, Oct 20, 2021 at 2:52 PM Eugenio Perez Martin
wrote:
>
> On Wed, Oct 20, 2021 at 4:07 AM Jason Wang wrote:
> >
> > On Wed, Oct 20, 2021 at 10:02 AM Jason Wang wrote:
> > >
> > > On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
> > > wrote:
> > > >
> > > > On Tue, Oct 19, 2021 at 11:25 AM Jason Wang wrote:
> > > > >
> > > > >
> > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > > > Use translations added in VhostIOVATree in SVQ.
> > > > > >
> > > > > > Now every element needs to store the previous address also, so
> > > > > > VirtQueue
> > > > > > can consume the elements properly. This adds a little overhead per
> > > > > > VQ
> > > > > > element, having to allocate more memory to stash them. As a possible
> > > > > > optimization, this allocation could be avoided if the descriptor is
> > > > > > not
> > > > > > a chain but a single one, but this is left undone.
> > > > > >
> > > > > > TODO: iova range should be queried before, and add logic to fail
> > > > > > when
> > > > > > GPA is outside of its range and memory listener or svq add it.
> > > > > >
> > > > > > Signed-off-by: Eugenio Pérez
> > > > > > ---
> > > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > > > > > hw/virtio/vhost-shadow-virtqueue.c | 130
> > > > > > -
> > > > > > hw/virtio/vhost-vdpa.c | 40 -
> > > > > > hw/virtio/trace-events | 1 +
> > > > > > 4 files changed, 152 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > index b7baa424a7..a0e6b5267a 100644
> > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > @@ -11,6 +11,7 @@
> > > > > > #define VHOST_SHADOW_VIRTQUEUE_H
> > > > > >
> > > > > > #include "hw/virtio/vhost.h"
> > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > >
> > > > > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > > > > >
> > > > > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev,
> > > > > > unsigned idx,
> > > > > > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > > > > VhostShadowVirtqueue *svq);
> > > > > >
> > > > > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int
> > > > > > idx);
> > > > > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx,
> > > > > > +VhostIOVATree *iova_map);
> > > > > >
> > > > > > void vhost_svq_free(VhostShadowVirtqueue *vq);
> > > > > >
> > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > index 2fd0bab75d..9db538547e 100644
> > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > @@ -11,12 +11,19 @@
> > > > > > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > > > > #include "hw/virtio/vhost.h"
> > > > > > #include "hw/virtio/virtio-access.h"
> > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > >
> > > > > > #include "standard-headers/linux/vhost_types.h"
> > > > > >
> > > > > > #include "qemu/error-report.h"
> > > > > > #include "qemu/main-loop.h"
> > > > > >
> > > > > > +typedef struct SVQElement {
> > > > > > +VirtQueueElement elem;
> > > > > > +void **in_sg_stash;
> > > > > > +void **out_sg_stash;
> > > > > > +} SVQElement;
> > > > > > +
> > > > > > /* Shadow virtqueue to relay notifications */
> > > > > > typedef struct VhostShadowVirtqueue {
> > > > > > /* Shadow vring */
> > > > > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > > > > /* Virtio device */
> > > > > > VirtIODevice *vdev;
> > > > > >
> > > > > > +/* IOVA mapping if used */
> > > > > > +VhostIOVATree *iova_map;
> > > > > > +
> > > > > > /* Map for returning guest's descriptors */
> > > > > > -VirtQueueElement **ring_id_maps;
> > > > > > +SVQElement **ring_id_maps;
> > > > > >
> > > > > > /* Next head to expose to device */
> > > > > > uint16_t avail_idx_shadow;
> > > > > > @@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t
> > > > > > *dev_features)
> > > > > > continue;
> > > > > >
> > > > > > case VIRTIO_F_ACCESS_PLATFORM:
> > > > > > -/* SVQ needs this feature disabled. Can't continue */
> > > > > > -if (*dev_features & BIT_ULL(b)) {
> > > > > > -clear_bit(b, dev_features);
> > > > > > -r = false;
> > > > > > -}
> > > > > > -break;
> > > > > > -
> > > > > > case VIRTIO_F_VERSION_1:
> > > > > > /* SVQ needs this feature, so can't continue */
> > > > > > if (!(*dev_features & BIT_ULL(b))) {
> > > > > > @@ -126,6 +129,64 @@ static void
> > > > > > vhost_svq_set_notification(VhostShadowVirtqueue *svq,
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Wed, Oct 20, 2021 at 4:07 AM Jason Wang wrote:
>
> On Wed, Oct 20, 2021 at 10:02 AM Jason Wang wrote:
> >
> > On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
> > wrote:
> > >
> > > On Tue, Oct 19, 2021 at 11:25 AM Jason Wang wrote:
> > > >
> > > >
> > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > > Use translations added in VhostIOVATree in SVQ.
> > > > >
> > > > > Now every element needs to store the previous address also, so
> > > > > VirtQueue
> > > > > can consume the elements properly. This adds a little overhead per VQ
> > > > > element, having to allocate more memory to stash them. As a possible
> > > > > optimization, this allocation could be avoided if the descriptor is
> > > > > not
> > > > > a chain but a single one, but this is left undone.
> > > > >
> > > > > TODO: iova range should be queried before, and add logic to fail when
> > > > > GPA is outside of its range and memory listener or svq add it.
> > > > >
> > > > > Signed-off-by: Eugenio Pérez
> > > > > ---
> > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > > > > hw/virtio/vhost-shadow-virtqueue.c | 130
> > > > > -
> > > > > hw/virtio/vhost-vdpa.c | 40 -
> > > > > hw/virtio/trace-events | 1 +
> > > > > 4 files changed, 152 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > index b7baa424a7..a0e6b5267a 100644
> > > > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > @@ -11,6 +11,7 @@
> > > > > #define VHOST_SHADOW_VIRTQUEUE_H
> > > > >
> > > > > #include "hw/virtio/vhost.h"
> > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > >
> > > > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > > > >
> > > > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev,
> > > > > unsigned idx,
> > > > > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > > > VhostShadowVirtqueue *svq);
> > > > >
> > > > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx);
> > > > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx,
> > > > > +VhostIOVATree *iova_map);
> > > > >
> > > > > void vhost_svq_free(VhostShadowVirtqueue *vq);
> > > > >
> > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > index 2fd0bab75d..9db538547e 100644
> > > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > @@ -11,12 +11,19 @@
> > > > > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > > > #include "hw/virtio/vhost.h"
> > > > > #include "hw/virtio/virtio-access.h"
> > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > >
> > > > > #include "standard-headers/linux/vhost_types.h"
> > > > >
> > > > > #include "qemu/error-report.h"
> > > > > #include "qemu/main-loop.h"
> > > > >
> > > > > +typedef struct SVQElement {
> > > > > +VirtQueueElement elem;
> > > > > +void **in_sg_stash;
> > > > > +void **out_sg_stash;
> > > > > +} SVQElement;
> > > > > +
> > > > > /* Shadow virtqueue to relay notifications */
> > > > > typedef struct VhostShadowVirtqueue {
> > > > > /* Shadow vring */
> > > > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > > > /* Virtio device */
> > > > > VirtIODevice *vdev;
> > > > >
> > > > > +/* IOVA mapping if used */
> > > > > +VhostIOVATree *iova_map;
> > > > > +
> > > > > /* Map for returning guest's descriptors */
> > > > > -VirtQueueElement **ring_id_maps;
> > > > > +SVQElement **ring_id_maps;
> > > > >
> > > > > /* Next head to expose to device */
> > > > > uint16_t avail_idx_shadow;
> > > > > @@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t
> > > > > *dev_features)
> > > > > continue;
> > > > >
> > > > > case VIRTIO_F_ACCESS_PLATFORM:
> > > > > -/* SVQ needs this feature disabled. Can't continue */
> > > > > -if (*dev_features & BIT_ULL(b)) {
> > > > > -clear_bit(b, dev_features);
> > > > > -r = false;
> > > > > -}
> > > > > -break;
> > > > > -
> > > > > case VIRTIO_F_VERSION_1:
> > > > > /* SVQ needs this feature, so can't continue */
> > > > > if (!(*dev_features & BIT_ULL(b))) {
> > > > > @@ -126,6 +129,64 @@ static void
> > > > > vhost_svq_set_notification(VhostShadowVirtqueue *svq, bool enable)
> > > > > }
> > > > > }
> > > > >
> > > > > +static void vhost_svq_stash_addr(void ***stash, const struct iovec
> > > > > *iov,
> > > > > + size_t num)
> > > > > +{
> > > > > +size_t i;
> > > > > +
> > > > > +if (num == 0) {
> > > > > +return;
> > > > > +
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Wed, Oct 20, 2021 at 10:02 AM Jason Wang wrote:
>
> On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
> wrote:
> >
> > On Tue, Oct 19, 2021 at 11:25 AM Jason Wang wrote:
> > >
> > >
> > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > Use translations added in VhostIOVATree in SVQ.
> > > >
> > > > Now every element needs to store the previous address also, so VirtQueue
> > > > can consume the elements properly. This adds a little overhead per VQ
> > > > element, having to allocate more memory to stash them. As a possible
> > > > optimization, this allocation could be avoided if the descriptor is not
> > > > a chain but a single one, but this is left undone.
> > > >
> > > > TODO: iova range should be queried before, and add logic to fail when
> > > > GPA is outside of its range and memory listener or svq add it.
> > > >
> > > > Signed-off-by: Eugenio Pérez
> > > > ---
> > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > > > hw/virtio/vhost-shadow-virtqueue.c | 130 -
> > > > hw/virtio/vhost-vdpa.c | 40 -
> > > > hw/virtio/trace-events | 1 +
> > > > 4 files changed, 152 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > > index b7baa424a7..a0e6b5267a 100644
> > > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > > @@ -11,6 +11,7 @@
> > > > #define VHOST_SHADOW_VIRTQUEUE_H
> > > >
> > > > #include "hw/virtio/vhost.h"
> > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > >
> > > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > > >
> > > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev, unsigned
> > > > idx,
> > > > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > > VhostShadowVirtqueue *svq);
> > > >
> > > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx);
> > > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx,
> > > > +VhostIOVATree *iova_map);
> > > >
> > > > void vhost_svq_free(VhostShadowVirtqueue *vq);
> > > >
> > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > > index 2fd0bab75d..9db538547e 100644
> > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > @@ -11,12 +11,19 @@
> > > > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > > #include "hw/virtio/vhost.h"
> > > > #include "hw/virtio/virtio-access.h"
> > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > >
> > > > #include "standard-headers/linux/vhost_types.h"
> > > >
> > > > #include "qemu/error-report.h"
> > > > #include "qemu/main-loop.h"
> > > >
> > > > +typedef struct SVQElement {
> > > > +VirtQueueElement elem;
> > > > +void **in_sg_stash;
> > > > +void **out_sg_stash;
> > > > +} SVQElement;
> > > > +
> > > > /* Shadow virtqueue to relay notifications */
> > > > typedef struct VhostShadowVirtqueue {
> > > > /* Shadow vring */
> > > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > > /* Virtio device */
> > > > VirtIODevice *vdev;
> > > >
> > > > +/* IOVA mapping if used */
> > > > +VhostIOVATree *iova_map;
> > > > +
> > > > /* Map for returning guest's descriptors */
> > > > -VirtQueueElement **ring_id_maps;
> > > > +SVQElement **ring_id_maps;
> > > >
> > > > /* Next head to expose to device */
> > > > uint16_t avail_idx_shadow;
> > > > @@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t
> > > > *dev_features)
> > > > continue;
> > > >
> > > > case VIRTIO_F_ACCESS_PLATFORM:
> > > > -/* SVQ needs this feature disabled. Can't continue */
> > > > -if (*dev_features & BIT_ULL(b)) {
> > > > -clear_bit(b, dev_features);
> > > > -r = false;
> > > > -}
> > > > -break;
> > > > -
> > > > case VIRTIO_F_VERSION_1:
> > > > /* SVQ needs this feature, so can't continue */
> > > > if (!(*dev_features & BIT_ULL(b))) {
> > > > @@ -126,6 +129,64 @@ static void
> > > > vhost_svq_set_notification(VhostShadowVirtqueue *svq, bool enable)
> > > > }
> > > > }
> > > >
> > > > +static void vhost_svq_stash_addr(void ***stash, const struct iovec
> > > > *iov,
> > > > + size_t num)
> > > > +{
> > > > +size_t i;
> > > > +
> > > > +if (num == 0) {
> > > > +return;
> > > > +}
> > > > +
> > > > +*stash = g_new(void *, num);
> > > > +for (i = 0; i < num; ++i) {
> > > > +(*stash)[i] = iov[i].iov_base;
> > > > +}
> > > > +}
> > > > +
> > > > +static void vhost_svq_unstash_addr(void **stash, struct iovec *iov,
> > > > size_t num)
> > > > +{
> > > > +size_t i;
> > > > +
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
wrote:
>
> On Tue, Oct 19, 2021 at 11:25 AM Jason Wang wrote:
> >
> >
> > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > Use translations added in VhostIOVATree in SVQ.
> > >
> > > Now every element needs to store the previous address also, so VirtQueue
> > > can consume the elements properly. This adds a little overhead per VQ
> > > element, having to allocate more memory to stash them. As a possible
> > > optimization, this allocation could be avoided if the descriptor is not
> > > a chain but a single one, but this is left undone.
> > >
> > > TODO: iova range should be queried before, and add logic to fail when
> > > GPA is outside of its range and memory listener or svq add it.
> > >
> > > Signed-off-by: Eugenio Pérez
> > > ---
> > > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > > hw/virtio/vhost-shadow-virtqueue.c | 130 -
> > > hw/virtio/vhost-vdpa.c | 40 -
> > > hw/virtio/trace-events | 1 +
> > > 4 files changed, 152 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > index b7baa424a7..a0e6b5267a 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > @@ -11,6 +11,7 @@
> > > #define VHOST_SHADOW_VIRTQUEUE_H
> > >
> > > #include "hw/virtio/vhost.h"
> > > +#include "hw/virtio/vhost-iova-tree.h"
> > >
> > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > >
> > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev, unsigned
> > > idx,
> > > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > VhostShadowVirtqueue *svq);
> > >
> > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx);
> > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx,
> > > +VhostIOVATree *iova_map);
> > >
> > > void vhost_svq_free(VhostShadowVirtqueue *vq);
> > >
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > index 2fd0bab75d..9db538547e 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > @@ -11,12 +11,19 @@
> > > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > #include "hw/virtio/vhost.h"
> > > #include "hw/virtio/virtio-access.h"
> > > +#include "hw/virtio/vhost-iova-tree.h"
> > >
> > > #include "standard-headers/linux/vhost_types.h"
> > >
> > > #include "qemu/error-report.h"
> > > #include "qemu/main-loop.h"
> > >
> > > +typedef struct SVQElement {
> > > +VirtQueueElement elem;
> > > +void **in_sg_stash;
> > > +void **out_sg_stash;
> > > +} SVQElement;
> > > +
> > > /* Shadow virtqueue to relay notifications */
> > > typedef struct VhostShadowVirtqueue {
> > > /* Shadow vring */
> > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > /* Virtio device */
> > > VirtIODevice *vdev;
> > >
> > > +/* IOVA mapping if used */
> > > +VhostIOVATree *iova_map;
> > > +
> > > /* Map for returning guest's descriptors */
> > > -VirtQueueElement **ring_id_maps;
> > > +SVQElement **ring_id_maps;
> > >
> > > /* Next head to expose to device */
> > > uint16_t avail_idx_shadow;
> > > @@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t
> > > *dev_features)
> > > continue;
> > >
> > > case VIRTIO_F_ACCESS_PLATFORM:
> > > -/* SVQ needs this feature disabled. Can't continue */
> > > -if (*dev_features & BIT_ULL(b)) {
> > > -clear_bit(b, dev_features);
> > > -r = false;
> > > -}
> > > -break;
> > > -
> > > case VIRTIO_F_VERSION_1:
> > > /* SVQ needs this feature, so can't continue */
> > > if (!(*dev_features & BIT_ULL(b))) {
> > > @@ -126,6 +129,64 @@ static void
> > > vhost_svq_set_notification(VhostShadowVirtqueue *svq, bool enable)
> > > }
> > > }
> > >
> > > +static void vhost_svq_stash_addr(void ***stash, const struct iovec *iov,
> > > + size_t num)
> > > +{
> > > +size_t i;
> > > +
> > > +if (num == 0) {
> > > +return;
> > > +}
> > > +
> > > +*stash = g_new(void *, num);
> > > +for (i = 0; i < num; ++i) {
> > > +(*stash)[i] = iov[i].iov_base;
> > > +}
> > > +}
> > > +
> > > +static void vhost_svq_unstash_addr(void **stash, struct iovec *iov,
> > > size_t num)
> > > +{
> > > +size_t i;
> > > +
> > > +if (num == 0) {
> > > +return;
> > > +}
> > > +
> > > +for (i = 0; i < num; ++i) {
> > > +iov[i].iov_base = stash[i];
> > > +}
> > > +g_free(stash);
> > > +}
> > > +
> > > +static void vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
> > > +
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Tue, Oct 19, 2021 at 11:25 AM Jason Wang wrote:
>
>
> 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > Use translations added in VhostIOVATree in SVQ.
> >
> > Now every element needs to store the previous address also, so VirtQueue
> > can consume the elements properly. This adds a little overhead per VQ
> > element, having to allocate more memory to stash them. As a possible
> > optimization, this allocation could be avoided if the descriptor is not
> > a chain but a single one, but this is left undone.
> >
> > TODO: iova range should be queried before, and add logic to fail when
> > GPA is outside of its range and memory listener or svq add it.
> >
> > Signed-off-by: Eugenio Pérez
> > ---
> > hw/virtio/vhost-shadow-virtqueue.h | 4 +-
> > hw/virtio/vhost-shadow-virtqueue.c | 130 -
> > hw/virtio/vhost-vdpa.c | 40 -
> > hw/virtio/trace-events | 1 +
> > 4 files changed, 152 insertions(+), 23 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > b/hw/virtio/vhost-shadow-virtqueue.h
> > index b7baa424a7..a0e6b5267a 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > @@ -11,6 +11,7 @@
> > #define VHOST_SHADOW_VIRTQUEUE_H
> >
> > #include "hw/virtio/vhost.h"
> > +#include "hw/virtio/vhost-iova-tree.h"
> >
> > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> >
> > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev, unsigned idx,
> > void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > VhostShadowVirtqueue *svq);
> >
> > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx);
> > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx,
> > +VhostIOVATree *iova_map);
> >
> > void vhost_svq_free(VhostShadowVirtqueue *vq);
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > b/hw/virtio/vhost-shadow-virtqueue.c
> > index 2fd0bab75d..9db538547e 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -11,12 +11,19 @@
> > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > #include "hw/virtio/vhost.h"
> > #include "hw/virtio/virtio-access.h"
> > +#include "hw/virtio/vhost-iova-tree.h"
> >
> > #include "standard-headers/linux/vhost_types.h"
> >
> > #include "qemu/error-report.h"
> > #include "qemu/main-loop.h"
> >
> > +typedef struct SVQElement {
> > +VirtQueueElement elem;
> > +void **in_sg_stash;
> > +void **out_sg_stash;
> > +} SVQElement;
> > +
> > /* Shadow virtqueue to relay notifications */
> > typedef struct VhostShadowVirtqueue {
> > /* Shadow vring */
> > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > /* Virtio device */
> > VirtIODevice *vdev;
> >
> > +/* IOVA mapping if used */
> > +VhostIOVATree *iova_map;
> > +
> > /* Map for returning guest's descriptors */
> > -VirtQueueElement **ring_id_maps;
> > +SVQElement **ring_id_maps;
> >
> > /* Next head to expose to device */
> > uint16_t avail_idx_shadow;
> > @@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t
> > *dev_features)
> > continue;
> >
> > case VIRTIO_F_ACCESS_PLATFORM:
> > -/* SVQ needs this feature disabled. Can't continue */
> > -if (*dev_features & BIT_ULL(b)) {
> > -clear_bit(b, dev_features);
> > -r = false;
> > -}
> > -break;
> > -
> > case VIRTIO_F_VERSION_1:
> > /* SVQ needs this feature, so can't continue */
> > if (!(*dev_features & BIT_ULL(b))) {
> > @@ -126,6 +129,64 @@ static void
> > vhost_svq_set_notification(VhostShadowVirtqueue *svq, bool enable)
> > }
> > }
> >
> > +static void vhost_svq_stash_addr(void ***stash, const struct iovec *iov,
> > + size_t num)
> > +{
> > +size_t i;
> > +
> > +if (num == 0) {
> > +return;
> > +}
> > +
> > +*stash = g_new(void *, num);
> > +for (i = 0; i < num; ++i) {
> > +(*stash)[i] = iov[i].iov_base;
> > +}
> > +}
> > +
> > +static void vhost_svq_unstash_addr(void **stash, struct iovec *iov, size_t
> > num)
> > +{
> > +size_t i;
> > +
> > +if (num == 0) {
> > +return;
> > +}
> > +
> > +for (i = 0; i < num; ++i) {
> > +iov[i].iov_base = stash[i];
> > +}
> > +g_free(stash);
> > +}
> > +
> > +static void vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
> > + struct iovec *iovec, size_t num)
> > +{
> > +size_t i;
> > +
> > +for (i = 0; i < num; ++i) {
> > +VhostDMAMap needle = {
> > +.translated_addr = iovec[i].iov_base,
> > +.size = iovec[i].iov_len,
> > +};
> > +size_t off;
> > +
> > +const VhostDMAMap *map =
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
在 2021/10/1 下午3:06, Eugenio Pérez 写道:
Use translations added in VhostIOVATree in SVQ.
Now every element needs to store the previous address also, so VirtQueue
can consume the elements properly. This adds a little overhead per VQ
element, having to allocate more memory to stash them. As a possible
optimization, this allocation could be avoided if the descriptor is not
a chain but a single one, but this is left undone.
TODO: iova range should be queried before, and add logic to fail when
GPA is outside of its range and memory listener or svq add it.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.h | 4 +-
hw/virtio/vhost-shadow-virtqueue.c | 130 -
hw/virtio/vhost-vdpa.c | 40 -
hw/virtio/trace-events | 1 +
4 files changed, 152 insertions(+), 23 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
index b7baa424a7..a0e6b5267a 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -11,6 +11,7 @@
#define VHOST_SHADOW_VIRTQUEUE_H
#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-iova-tree.h"
typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
@@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev, unsigned idx,
void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
VhostShadowVirtqueue *svq);
-VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx);
+VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx,
+VhostIOVATree *iova_map);
void vhost_svq_free(VhostShadowVirtqueue *vq);
diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 2fd0bab75d..9db538547e 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -11,12 +11,19 @@
#include "hw/virtio/vhost-shadow-virtqueue.h"
#include "hw/virtio/vhost.h"
#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/vhost-iova-tree.h"
#include "standard-headers/linux/vhost_types.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
+typedef struct SVQElement {
+VirtQueueElement elem;
+void **in_sg_stash;
+void **out_sg_stash;
+} SVQElement;
+
/* Shadow virtqueue to relay notifications */
typedef struct VhostShadowVirtqueue {
/* Shadow vring */
@@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
/* Virtio device */
VirtIODevice *vdev;
+/* IOVA mapping if used */
+VhostIOVATree *iova_map;
+
/* Map for returning guest's descriptors */
-VirtQueueElement **ring_id_maps;
+SVQElement **ring_id_maps;
/* Next head to expose to device */
uint16_t avail_idx_shadow;
@@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t *dev_features)
continue;
case VIRTIO_F_ACCESS_PLATFORM:
-/* SVQ needs this feature disabled. Can't continue */
-if (*dev_features & BIT_ULL(b)) {
-clear_bit(b, dev_features);
-r = false;
-}
-break;
-
case VIRTIO_F_VERSION_1:
/* SVQ needs this feature, so can't continue */
if (!(*dev_features & BIT_ULL(b))) {
@@ -126,6 +129,64 @@ static void
vhost_svq_set_notification(VhostShadowVirtqueue *svq, bool enable)
}
}
+static void vhost_svq_stash_addr(void ***stash, const struct iovec *iov,
+ size_t num)
+{
+size_t i;
+
+if (num == 0) {
+return;
+}
+
+*stash = g_new(void *, num);
+for (i = 0; i < num; ++i) {
+(*stash)[i] = iov[i].iov_base;
+}
+}
+
+static void vhost_svq_unstash_addr(void **stash, struct iovec *iov, size_t num)
+{
+size_t i;
+
+if (num == 0) {
+return;
+}
+
+for (i = 0; i < num; ++i) {
+iov[i].iov_base = stash[i];
+}
+g_free(stash);
+}
+
+static void vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
+ struct iovec *iovec, size_t num)
+{
+size_t i;
+
+for (i = 0; i < num; ++i) {
+VhostDMAMap needle = {
+.translated_addr = iovec[i].iov_base,
+.size = iovec[i].iov_len,
+};
+size_t off;
+
+const VhostDMAMap *map = vhost_iova_tree_find_iova(svq->iova_map,
+ );
Is it possible that we end up with more than one maps here?
+/*
+ * Map cannot be NULL since iova map contains all guest space and
+ * qemu already has a physical address mapped
+ */
+assert(map);
+
+/*
+ * Map->iova chunk size is ignored. What to do if descriptor
+ * (addr, size) does not fit is delegated to the device.
+ */
+off = needle.translated_addr - map->translated_addr;
+
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Fri, Oct 15, 2021 at 10:20 AM Eugenio Perez Martin wrote: > > On Fri, Oct 15, 2021 at 9:37 AM Jason Wang wrote: > > > > On Fri, Oct 15, 2021 at 3:28 PM Eugenio Perez Martin > > wrote: > > > > > > On Wed, Oct 13, 2021 at 7:34 AM Jason Wang wrote: > > > > > > > > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道: > > > > > Use translations added in VhostIOVATree in SVQ. > > > > > > > > > > Now every element needs to store the previous address also, so > > > > > VirtQueue > > > > > can consume the elements properly. This adds a little overhead per VQ > > > > > element, having to allocate more memory to stash them. As a possible > > > > > optimization, this allocation could be avoided if the descriptor is > > > > > not > > > > > a chain but a single one, but this is left undone. > > > > > > > > > > TODO: iova range should be queried before, and add logic to fail when > > > > > GPA is outside of its range and memory listener or svq add it. > > > > > > > > > > Signed-off-by: Eugenio Pérez > > > > > --- > > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +- > > > > > hw/virtio/vhost-shadow-virtqueue.c | 130 > > > > > - > > > > > hw/virtio/vhost-vdpa.c | 40 - > > > > > hw/virtio/trace-events | 1 + > > > > > 4 files changed, 152 insertions(+), 23 deletions(-) > > > > > > > > > > > > Think hard about the whole logic. This is safe since qemu memory map > > > > will fail if guest submits a invalidate IOVA. > > > > > > > > > > Can you expand on this? What you mean is that VirtQueue already > > > protects SVQ code if the guest sets an invalid buffer address (GPA), > > > isn't it? > > > > Yes. > > > > > > > > > Then I wonder if we do something much more simpler: > > > > > > > > 1) Using qemu VA as IOVA but only maps the VA that belongs to guest > > > > 2) Then we don't need any IOVA tree here, what we need is to just map > > > > vring and use qemu VA without any translation > > > > > > > > > > That would be great, but either qemu's SVQ vring or guest translated > > > buffers address (in qemu VA form) were already in high addresses, > > > outside of the device's iova range (in my test). > > > > You're right. I miss that and that's why we need e.g iova tree and > > allocator. > > > > What I proposed only makes sense when shared virtual memory (SVA) is > > implemented. In the case of SVA, the valid iova range should be the > > full VA range. > > > > > > > > I didn't try remapping tricks to make them fit in the range, but I > > > think it does complicate the solution relatively fast if there was > > > already memory in that range owned by qemu before enabling SVQ: > > > > > > * Guest memory must be contiguous in VA address space, but it "must" > > > support hotplug/unplug (although vDPA currently pins it). Hotplug > > > memory could always overlap with SVQ vring, so we would need to move > > > it. > > > * Duplicating mapped memory for writing? (Not sure if guest memory is > > > actually movable in qemu). > > > * Indirect descriptors will need to allocate and free memory more or > > > less frequently, increasing the possibility of overlapping. > > > > I'm not sure I get the problem, but overlapping is not an issue since > > we're using VA. > > > > It's basically the same (potential) problem of DPDK's SVQ: IOVA Range > goes from 0 to X. That means that both GPA and SVQ must be in IOVA > range. As an example, we put GPA at the beginning of the range, that > grows upwards when memory is hot plugged, and SVQ vrings that grows > downwards when devices are added or set in SVQ mode. > > Even without both space fragmentation problems, we could reach a point > where both will take the same address, and we would need to go to the > tree. > > But since we are able to detect those situations, I can see how we can > work in two modes as an optimization: 1:1 when they don't overlap, and > fragmented tree where it does. But I don't think it's a good idea to > include it from the beginning, and I'm not sure if that is worth it > without measuring the tree translation cost first. > > > > > > > If we can move guest memory, > > > > I'm not sure we can do this or it looks very tricky. > > > > Just thinking out loud here, but maybe we could map all memory and > play with remap_file_pages [1] a little bit for that. > > > > however, I can see how we can track it in > > > a tree *but* mark when the tree is 1:1 with qemu's VA, so buffers > > > forwarding does not take the translation penalty. When guest memory > > > cannot be map 1:1, we can resort to tree, and come back to 1:1 > > > translation if the offending tree node(s) get deleted. > > > > > > However I think this puts the solution a little bit farther than > > > "starting simple" :). > > > > > > Does it make sense? > > > > Yes. So I think I will review the IOVA tree codes and get back to you. > > > > Looking forward to it :). > PS: Actually, they still use the GArray solution as the previous series. I'm currently
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Fri, Oct 15, 2021 at 9:37 AM Jason Wang wrote: > > On Fri, Oct 15, 2021 at 3:28 PM Eugenio Perez Martin > wrote: > > > > On Wed, Oct 13, 2021 at 7:34 AM Jason Wang wrote: > > > > > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道: > > > > Use translations added in VhostIOVATree in SVQ. > > > > > > > > Now every element needs to store the previous address also, so VirtQueue > > > > can consume the elements properly. This adds a little overhead per VQ > > > > element, having to allocate more memory to stash them. As a possible > > > > optimization, this allocation could be avoided if the descriptor is not > > > > a chain but a single one, but this is left undone. > > > > > > > > TODO: iova range should be queried before, and add logic to fail when > > > > GPA is outside of its range and memory listener or svq add it. > > > > > > > > Signed-off-by: Eugenio Pérez > > > > --- > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +- > > > > hw/virtio/vhost-shadow-virtqueue.c | 130 - > > > > hw/virtio/vhost-vdpa.c | 40 - > > > > hw/virtio/trace-events | 1 + > > > > 4 files changed, 152 insertions(+), 23 deletions(-) > > > > > > > > > Think hard about the whole logic. This is safe since qemu memory map > > > will fail if guest submits a invalidate IOVA. > > > > > > > Can you expand on this? What you mean is that VirtQueue already > > protects SVQ code if the guest sets an invalid buffer address (GPA), > > isn't it? > > Yes. > > > > > > Then I wonder if we do something much more simpler: > > > > > > 1) Using qemu VA as IOVA but only maps the VA that belongs to guest > > > 2) Then we don't need any IOVA tree here, what we need is to just map > > > vring and use qemu VA without any translation > > > > > > > That would be great, but either qemu's SVQ vring or guest translated > > buffers address (in qemu VA form) were already in high addresses, > > outside of the device's iova range (in my test). > > You're right. I miss that and that's why we need e.g iova tree and allocator. > > What I proposed only makes sense when shared virtual memory (SVA) is > implemented. In the case of SVA, the valid iova range should be the > full VA range. > > > > > I didn't try remapping tricks to make them fit in the range, but I > > think it does complicate the solution relatively fast if there was > > already memory in that range owned by qemu before enabling SVQ: > > > > * Guest memory must be contiguous in VA address space, but it "must" > > support hotplug/unplug (although vDPA currently pins it). Hotplug > > memory could always overlap with SVQ vring, so we would need to move > > it. > > * Duplicating mapped memory for writing? (Not sure if guest memory is > > actually movable in qemu). > > * Indirect descriptors will need to allocate and free memory more or > > less frequently, increasing the possibility of overlapping. > > I'm not sure I get the problem, but overlapping is not an issue since > we're using VA. > It's basically the same (potential) problem of DPDK's SVQ: IOVA Range goes from 0 to X. That means that both GPA and SVQ must be in IOVA range. As an example, we put GPA at the beginning of the range, that grows upwards when memory is hot plugged, and SVQ vrings that grows downwards when devices are added or set in SVQ mode. Even without both space fragmentation problems, we could reach a point where both will take the same address, and we would need to go to the tree. But since we are able to detect those situations, I can see how we can work in two modes as an optimization: 1:1 when they don't overlap, and fragmented tree where it does. But I don't think it's a good idea to include it from the beginning, and I'm not sure if that is worth it without measuring the tree translation cost first. > > > > If we can move guest memory, > > I'm not sure we can do this or it looks very tricky. > Just thinking out loud here, but maybe we could map all memory and play with remap_file_pages [1] a little bit for that. > > however, I can see how we can track it in > > a tree *but* mark when the tree is 1:1 with qemu's VA, so buffers > > forwarding does not take the translation penalty. When guest memory > > cannot be map 1:1, we can resort to tree, and come back to 1:1 > > translation if the offending tree node(s) get deleted. > > > > However I think this puts the solution a little bit farther than > > "starting simple" :). > > > > Does it make sense? > > Yes. So I think I will review the IOVA tree codes and get back to you. > Looking forward to it :). Thanks! [1] https://linux.die.net/man/2/remap_file_pages > THanks > > > > > Thanks! > > > > > Thanks > > > > > >
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Fri, Oct 15, 2021 at 4:21 PM Eugenio Perez Martin wrote: > > On Fri, Oct 15, 2021 at 9:37 AM Jason Wang wrote: > > > > On Fri, Oct 15, 2021 at 3:28 PM Eugenio Perez Martin > > wrote: > > > > > > On Wed, Oct 13, 2021 at 7:34 AM Jason Wang wrote: > > > > > > > > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道: > > > > > Use translations added in VhostIOVATree in SVQ. > > > > > > > > > > Now every element needs to store the previous address also, so > > > > > VirtQueue > > > > > can consume the elements properly. This adds a little overhead per VQ > > > > > element, having to allocate more memory to stash them. As a possible > > > > > optimization, this allocation could be avoided if the descriptor is > > > > > not > > > > > a chain but a single one, but this is left undone. > > > > > > > > > > TODO: iova range should be queried before, and add logic to fail when > > > > > GPA is outside of its range and memory listener or svq add it. > > > > > > > > > > Signed-off-by: Eugenio Pérez > > > > > --- > > > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +- > > > > > hw/virtio/vhost-shadow-virtqueue.c | 130 > > > > > - > > > > > hw/virtio/vhost-vdpa.c | 40 - > > > > > hw/virtio/trace-events | 1 + > > > > > 4 files changed, 152 insertions(+), 23 deletions(-) > > > > > > > > > > > > Think hard about the whole logic. This is safe since qemu memory map > > > > will fail if guest submits a invalidate IOVA. > > > > > > > > > > Can you expand on this? What you mean is that VirtQueue already > > > protects SVQ code if the guest sets an invalid buffer address (GPA), > > > isn't it? > > > > Yes. > > > > > > > > > Then I wonder if we do something much more simpler: > > > > > > > > 1) Using qemu VA as IOVA but only maps the VA that belongs to guest > > > > 2) Then we don't need any IOVA tree here, what we need is to just map > > > > vring and use qemu VA without any translation > > > > > > > > > > That would be great, but either qemu's SVQ vring or guest translated > > > buffers address (in qemu VA form) were already in high addresses, > > > outside of the device's iova range (in my test). > > > > You're right. I miss that and that's why we need e.g iova tree and > > allocator. > > > > What I proposed only makes sense when shared virtual memory (SVA) is > > implemented. In the case of SVA, the valid iova range should be the > > full VA range. > > > > > > > > I didn't try remapping tricks to make them fit in the range, but I > > > think it does complicate the solution relatively fast if there was > > > already memory in that range owned by qemu before enabling SVQ: > > > > > > * Guest memory must be contiguous in VA address space, but it "must" > > > support hotplug/unplug (although vDPA currently pins it). Hotplug > > > memory could always overlap with SVQ vring, so we would need to move > > > it. > > > * Duplicating mapped memory for writing? (Not sure if guest memory is > > > actually movable in qemu). > > > * Indirect descriptors will need to allocate and free memory more or > > > less frequently, increasing the possibility of overlapping. > > > > I'm not sure I get the problem, but overlapping is not an issue since > > we're using VA. > > > > It's basically the same (potential) problem of DPDK's SVQ: IOVA Range > goes from 0 to X. That means that both GPA and SVQ must be in IOVA > range. As an example, we put GPA at the beginning of the range, that > grows upwards when memory is hot plugged, and SVQ vrings that grows > downwards when devices are added or set in SVQ mode. Yes, but this is not the case if we're using VA. > > Even without both space fragmentation problems, we could reach a point > where both will take the same address, and we would need to go to the > tree. > > But since we are able to detect those situations, I can see how we can > work in two modes as an optimization: 1:1 when they don't overlap, and > fragmented tree where it does. But I don't think it's a good idea to > include it from the beginning, and I'm not sure if that is worth it > without measuring the tree translation cost first. > > > > > > > If we can move guest memory, > > > > I'm not sure we can do this or it looks very tricky. > > > > Just thinking out loud here, but maybe we could map all memory and > play with remap_file_pages [1] a little bit for that. The problem is that there's no guarantee that it will always succeed. So let's start with the current dedicated IOVA address space. We can do optimization on top anyhow. > > > > however, I can see how we can track it in > > > a tree *but* mark when the tree is 1:1 with qemu's VA, so buffers > > > forwarding does not take the translation penalty. When guest memory > > > cannot be map 1:1, we can resort to tree, and come back to 1:1 > > > translation if the offending tree node(s) get deleted. > > > > > > However I think this puts the solution a little bit farther than > > > "starting simple" :). > > > > > >
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Fri, Oct 15, 2021 at 3:28 PM Eugenio Perez Martin wrote: > > On Wed, Oct 13, 2021 at 7:34 AM Jason Wang wrote: > > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道: > > > Use translations added in VhostIOVATree in SVQ. > > > > > > Now every element needs to store the previous address also, so VirtQueue > > > can consume the elements properly. This adds a little overhead per VQ > > > element, having to allocate more memory to stash them. As a possible > > > optimization, this allocation could be avoided if the descriptor is not > > > a chain but a single one, but this is left undone. > > > > > > TODO: iova range should be queried before, and add logic to fail when > > > GPA is outside of its range and memory listener or svq add it. > > > > > > Signed-off-by: Eugenio Pérez > > > --- > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +- > > > hw/virtio/vhost-shadow-virtqueue.c | 130 - > > > hw/virtio/vhost-vdpa.c | 40 - > > > hw/virtio/trace-events | 1 + > > > 4 files changed, 152 insertions(+), 23 deletions(-) > > > > > > Think hard about the whole logic. This is safe since qemu memory map > > will fail if guest submits a invalidate IOVA. > > > > Can you expand on this? What you mean is that VirtQueue already > protects SVQ code if the guest sets an invalid buffer address (GPA), > isn't it? Yes. > > > Then I wonder if we do something much more simpler: > > > > 1) Using qemu VA as IOVA but only maps the VA that belongs to guest > > 2) Then we don't need any IOVA tree here, what we need is to just map > > vring and use qemu VA without any translation > > > > That would be great, but either qemu's SVQ vring or guest translated > buffers address (in qemu VA form) were already in high addresses, > outside of the device's iova range (in my test). You're right. I miss that and that's why we need e.g iova tree and allocator. What I proposed only makes sense when shared virtual memory (SVA) is implemented. In the case of SVA, the valid iova range should be the full VA range. > > I didn't try remapping tricks to make them fit in the range, but I > think it does complicate the solution relatively fast if there was > already memory in that range owned by qemu before enabling SVQ: > > * Guest memory must be contiguous in VA address space, but it "must" > support hotplug/unplug (although vDPA currently pins it). Hotplug > memory could always overlap with SVQ vring, so we would need to move > it. > * Duplicating mapped memory for writing? (Not sure if guest memory is > actually movable in qemu). > * Indirect descriptors will need to allocate and free memory more or > less frequently, increasing the possibility of overlapping. I'm not sure I get the problem, but overlapping is not an issue since we're using VA. > > If we can move guest memory, I'm not sure we can do this or it looks very tricky. > however, I can see how we can track it in > a tree *but* mark when the tree is 1:1 with qemu's VA, so buffers > forwarding does not take the translation penalty. When guest memory > cannot be map 1:1, we can resort to tree, and come back to 1:1 > translation if the offending tree node(s) get deleted. > > However I think this puts the solution a little bit farther than > "starting simple" :). > > Does it make sense? Yes. So I think I will review the IOVA tree codes and get back to you. THanks > > Thanks! > > > Thanks > > >
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Wed, Oct 13, 2021 at 7:34 AM Jason Wang wrote: > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道: > > Use translations added in VhostIOVATree in SVQ. > > > > Now every element needs to store the previous address also, so VirtQueue > > can consume the elements properly. This adds a little overhead per VQ > > element, having to allocate more memory to stash them. As a possible > > optimization, this allocation could be avoided if the descriptor is not > > a chain but a single one, but this is left undone. > > > > TODO: iova range should be queried before, and add logic to fail when > > GPA is outside of its range and memory listener or svq add it. > > > > Signed-off-by: Eugenio Pérez > > --- > > hw/virtio/vhost-shadow-virtqueue.h | 4 +- > > hw/virtio/vhost-shadow-virtqueue.c | 130 - > > hw/virtio/vhost-vdpa.c | 40 - > > hw/virtio/trace-events | 1 + > > 4 files changed, 152 insertions(+), 23 deletions(-) > > > Think hard about the whole logic. This is safe since qemu memory map > will fail if guest submits a invalidate IOVA. > Can you expand on this? What you mean is that VirtQueue already protects SVQ code if the guest sets an invalid buffer address (GPA), isn't it? > Then I wonder if we do something much more simpler: > > 1) Using qemu VA as IOVA but only maps the VA that belongs to guest > 2) Then we don't need any IOVA tree here, what we need is to just map > vring and use qemu VA without any translation > That would be great, but either qemu's SVQ vring or guest translated buffers address (in qemu VA form) were already in high addresses, outside of the device's iova range (in my test). I didn't try remapping tricks to make them fit in the range, but I think it does complicate the solution relatively fast if there was already memory in that range owned by qemu before enabling SVQ: * Guest memory must be contiguous in VA address space, but it "must" support hotplug/unplug (although vDPA currently pins it). Hotplug memory could always overlap with SVQ vring, so we would need to move it. * Duplicating mapped memory for writing? (Not sure if guest memory is actually movable in qemu). * Indirect descriptors will need to allocate and free memory more or less frequently, increasing the possibility of overlapping. If we can move guest memory, however, I can see how we can track it in a tree *but* mark when the tree is 1:1 with qemu's VA, so buffers forwarding does not take the translation penalty. When guest memory cannot be map 1:1, we can resort to tree, and come back to 1:1 translation if the offending tree node(s) get deleted. However I think this puts the solution a little bit farther than "starting simple" :). Does it make sense? Thanks! > Thanks >
Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
在 2021/10/1 下午3:06, Eugenio Pérez 写道: Use translations added in VhostIOVATree in SVQ. Now every element needs to store the previous address also, so VirtQueue can consume the elements properly. This adds a little overhead per VQ element, having to allocate more memory to stash them. As a possible optimization, this allocation could be avoided if the descriptor is not a chain but a single one, but this is left undone. TODO: iova range should be queried before, and add logic to fail when GPA is outside of its range and memory listener or svq add it. Signed-off-by: Eugenio Pérez --- hw/virtio/vhost-shadow-virtqueue.h | 4 +- hw/virtio/vhost-shadow-virtqueue.c | 130 - hw/virtio/vhost-vdpa.c | 40 - hw/virtio/trace-events | 1 + 4 files changed, 152 insertions(+), 23 deletions(-) Think hard about the whole logic. This is safe since qemu memory map will fail if guest submits a invalidate IOVA. Then I wonder if we do something much more simpler: 1) Using qemu VA as IOVA but only maps the VA that belongs to guest 2) Then we don't need any IOVA tree here, what we need is to just map vring and use qemu VA without any translation Thanks
[RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
Use translations added in VhostIOVATree in SVQ.
Now every element needs to store the previous address also, so VirtQueue
can consume the elements properly. This adds a little overhead per VQ
element, having to allocate more memory to stash them. As a possible
optimization, this allocation could be avoided if the descriptor is not
a chain but a single one, but this is left undone.
TODO: iova range should be queried before, and add logic to fail when
GPA is outside of its range and memory listener or svq add it.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.h | 4 +-
hw/virtio/vhost-shadow-virtqueue.c | 130 -
hw/virtio/vhost-vdpa.c | 40 -
hw/virtio/trace-events | 1 +
4 files changed, 152 insertions(+), 23 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
index b7baa424a7..a0e6b5267a 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -11,6 +11,7 @@
#define VHOST_SHADOW_VIRTQUEUE_H
#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-iova-tree.h"
typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
@@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev, unsigned idx,
void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
VhostShadowVirtqueue *svq);
-VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx);
+VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int idx,
+VhostIOVATree *iova_map);
void vhost_svq_free(VhostShadowVirtqueue *vq);
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index 2fd0bab75d..9db538547e 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -11,12 +11,19 @@
#include "hw/virtio/vhost-shadow-virtqueue.h"
#include "hw/virtio/vhost.h"
#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/vhost-iova-tree.h"
#include "standard-headers/linux/vhost_types.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
+typedef struct SVQElement {
+VirtQueueElement elem;
+void **in_sg_stash;
+void **out_sg_stash;
+} SVQElement;
+
/* Shadow virtqueue to relay notifications */
typedef struct VhostShadowVirtqueue {
/* Shadow vring */
@@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
/* Virtio device */
VirtIODevice *vdev;
+/* IOVA mapping if used */
+VhostIOVATree *iova_map;
+
/* Map for returning guest's descriptors */
-VirtQueueElement **ring_id_maps;
+SVQElement **ring_id_maps;
/* Next head to expose to device */
uint16_t avail_idx_shadow;
@@ -79,13 +89,6 @@ bool vhost_svq_valid_device_features(uint64_t *dev_features)
continue;
case VIRTIO_F_ACCESS_PLATFORM:
-/* SVQ needs this feature disabled. Can't continue */
-if (*dev_features & BIT_ULL(b)) {
-clear_bit(b, dev_features);
-r = false;
-}
-break;
-
case VIRTIO_F_VERSION_1:
/* SVQ needs this feature, so can't continue */
if (!(*dev_features & BIT_ULL(b))) {
@@ -126,6 +129,64 @@ static void
vhost_svq_set_notification(VhostShadowVirtqueue *svq, bool enable)
}
}
+static void vhost_svq_stash_addr(void ***stash, const struct iovec *iov,
+ size_t num)
+{
+size_t i;
+
+if (num == 0) {
+return;
+}
+
+*stash = g_new(void *, num);
+for (i = 0; i < num; ++i) {
+(*stash)[i] = iov[i].iov_base;
+}
+}
+
+static void vhost_svq_unstash_addr(void **stash, struct iovec *iov, size_t num)
+{
+size_t i;
+
+if (num == 0) {
+return;
+}
+
+for (i = 0; i < num; ++i) {
+iov[i].iov_base = stash[i];
+}
+g_free(stash);
+}
+
+static void vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
+ struct iovec *iovec, size_t num)
+{
+size_t i;
+
+for (i = 0; i < num; ++i) {
+VhostDMAMap needle = {
+.translated_addr = iovec[i].iov_base,
+.size = iovec[i].iov_len,
+};
+size_t off;
+
+const VhostDMAMap *map = vhost_iova_tree_find_iova(svq->iova_map,
+ );
+/*
+ * Map cannot be NULL since iova map contains all guest space and
+ * qemu already has a physical address mapped
+ */
+assert(map);
+
+/*
+ * Map->iova chunk size is ignored. What to do if descriptor
+ * (addr, size) does not fit is delegated to the device.
+ */
+off = needle.translated_addr - map->translated_addr;
+iovec[i].iov_base = (void *)(map->iova + off);
+}
+}
+
static void vhost_vring_write_descs(VhostShadowVirtqueue *svq,
const
