diff mbox series

[RFC,v2,08/13] vdpa: Negotiate _F_SUSPEND feature

Message ID 20230112172434.760850-9-eperezma@redhat.com (mailing list archive)
State New, archived
Headers show
Series Dinamycally switch to vhost shadow virtqueues at vdpa net migration | expand

Commit Message

Eugenio Perez Martin Jan. 12, 2023, 5:24 p.m. UTC
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(-)

Comments

Jason Wang Jan. 13, 2023, 4:39 a.m. UTC | #1
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
>
Eugenio Perez Martin Jan. 13, 2023, 8:45 a.m. UTC | #2
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
> >
>
Jason Wang Jan. 16, 2023, 6:48 a.m. UTC | #3
在 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
>>>
Eugenio Perez Martin Jan. 16, 2023, 4:17 p.m. UTC | #4
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 mbox series

Patch

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)) {