| Message ID | 20230112172434.760850-9-eperezma@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Dinamycally switch to vhost shadow virtqueues at vdpa net migration | expand |
On Fri, Jan 13, 2023 at 1:25 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > This is needed for qemu to know it can suspend the device to retrieve > its status and enable SVQ with it, so all the process is transparent to > the guest. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> We probably need to add the resume in the future to have a quick recovery from migration failures. Thanks > --- > hw/virtio/vhost-vdpa.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 4296427a69..a61a6b2a74 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -659,7 +659,8 @@ static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev) > uint64_t features; > uint64_t f = 0x1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 | > 0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH | > - 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID; > + 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID | > + 0x1ULL << VHOST_BACKEND_F_SUSPEND; > int r; > > if (vhost_vdpa_call(dev, VHOST_GET_BACKEND_FEATURES, &features)) { > -- > 2.31.1 >
On Fri, Jan 13, 2023 at 5:39 AM Jason Wang <jasowang@redhat.com> wrote: > > On Fri, Jan 13, 2023 at 1:25 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > This is needed for qemu to know it can suspend the device to retrieve > > its status and enable SVQ with it, so all the process is transparent to > > the guest. > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > Acked-by: Jason Wang <jasowang@redhat.com> > > We probably need to add the resume in the future to have a quick > recovery from migration failures. > The capability of a resume can be useful here but only in a small window. During the most time of the migration SVQ is enabled, so in the event of a migration failure we may need to reset the whole device to enable passthrough again. But maybe is it worth giving a quick review and adding some TODOs where it can be useful in this series? Thanks! > Thanks > > > --- > > hw/virtio/vhost-vdpa.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > index 4296427a69..a61a6b2a74 100644 > > --- a/hw/virtio/vhost-vdpa.c > > +++ b/hw/virtio/vhost-vdpa.c > > @@ -659,7 +659,8 @@ static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev) > > uint64_t features; > > uint64_t f = 0x1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 | > > 0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH | > > - 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID; > > + 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID | > > + 0x1ULL << VHOST_BACKEND_F_SUSPEND; > > int r; > > > > if (vhost_vdpa_call(dev, VHOST_GET_BACKEND_FEATURES, &features)) { > > -- > > 2.31.1 > > >
在 2023/1/13 16:45, Eugenio Perez Martin 写道: > On Fri, Jan 13, 2023 at 5:39 AM Jason Wang <jasowang@redhat.com> wrote: >> On Fri, Jan 13, 2023 at 1:25 AM Eugenio Pérez <eperezma@redhat.com> wrote: >>> This is needed for qemu to know it can suspend the device to retrieve >>> its status and enable SVQ with it, so all the process is transparent to >>> the guest. >>> >>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> >> Acked-by: Jason Wang <jasowang@redhat.com> >> >> We probably need to add the resume in the future to have a quick >> recovery from migration failures. >> > The capability of a resume can be useful here but only in a small > window. During the most time of the migration SVQ is enabled, so in > the event of a migration failure we may need to reset the whole device > to enable passthrough again. Yes. > > But maybe is it worth giving a quick review and adding some TODOs > where it can be useful in this series? We can start by having a TODO in this series, and leave resume in for the future. Thanks > > Thanks! > >> Thanks >> >>> --- >>> hw/virtio/vhost-vdpa.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c >>> index 4296427a69..a61a6b2a74 100644 >>> --- a/hw/virtio/vhost-vdpa.c >>> +++ b/hw/virtio/vhost-vdpa.c >>> @@ -659,7 +659,8 @@ static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev) >>> uint64_t features; >>> uint64_t f = 0x1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 | >>> 0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH | >>> - 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID; >>> + 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID | >>> + 0x1ULL << VHOST_BACKEND_F_SUSPEND; >>> int r; >>> >>> if (vhost_vdpa_call(dev, VHOST_GET_BACKEND_FEATURES, &features)) { >>> -- >>> 2.31.1 >>>
On Mon, Jan 16, 2023 at 7:48 AM Jason Wang <jasowang@redhat.com> wrote: > > > 在 2023/1/13 16:45, Eugenio Perez Martin 写道: > > On Fri, Jan 13, 2023 at 5:39 AM Jason Wang <jasowang@redhat.com> wrote: > >> On Fri, Jan 13, 2023 at 1:25 AM Eugenio Pérez <eperezma@redhat.com> wrote: > >>> This is needed for qemu to know it can suspend the device to retrieve > >>> its status and enable SVQ with it, so all the process is transparent to > >>> the guest. > >>> > >>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > >> Acked-by: Jason Wang <jasowang@redhat.com> > >> > >> We probably need to add the resume in the future to have a quick > >> recovery from migration failures. > >> > > The capability of a resume can be useful here but only in a small > > window. During the most time of the migration SVQ is enabled, so in > > the event of a migration failure we may need to reset the whole device > > to enable passthrough again. > > > Yes. > > > > > > But maybe is it worth giving a quick review and adding some TODOs > > where it can be useful in this series? > > > We can start by having a TODO in this series, and leave resume in for > the future. > Got it, I'll add in the next series. Thanks! > Thanks > > > > > > Thanks! > > > >> Thanks > >> > >>> --- > >>> hw/virtio/vhost-vdpa.c | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > >>> index 4296427a69..a61a6b2a74 100644 > >>> --- a/hw/virtio/vhost-vdpa.c > >>> +++ b/hw/virtio/vhost-vdpa.c > >>> @@ -659,7 +659,8 @@ static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev) > >>> uint64_t features; > >>> uint64_t f = 0x1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 | > >>> 0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH | > >>> - 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID; > >>> + 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID | > >>> + 0x1ULL << VHOST_BACKEND_F_SUSPEND; > >>> int r; > >>> > >>> if (vhost_vdpa_call(dev, VHOST_GET_BACKEND_FEATURES, &features)) { > >>> -- > >>> 2.31.1 > >>> >
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 4296427a69..a61a6b2a74 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -659,7 +659,8 @@ static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev) uint64_t features; uint64_t f = 0x1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 | 0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH | - 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID; + 0x1ULL << VHOST_BACKEND_F_IOTLB_ASID | + 0x1ULL << VHOST_BACKEND_F_SUSPEND; int r; if (vhost_vdpa_call(dev, VHOST_GET_BACKEND_FEATURES, &features)) {
This is needed for qemu to know it can suspend the device to retrieve its status and enable SVQ with it, so all the process is transparent to the guest. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/virtio/vhost-vdpa.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)