? 2022/2/1 ??7:45, Eugenio Perez Martin ??:> On Sun, Jan 30, 2022 at 7:50 AM Jason Wang <jasowang at redhat.com>
wrote:
>>
>> ? 2022/1/22 ??4:27, Eugenio P?rez ??:
>>> SVQ is able to log the dirty bits by itself, so let's use it to
not
>>> block migration.
>>>
>>> Also, ignore set and clear of VHOST_F_LOG_ALL on set_features if
SVQ is
>>> enabled. Even if the device supports it, the reports would be
nonsense
>>> because SVQ memory is in the qemu region.
>>>
>>> The log region is still allocated. Future changes might skip that,
but
>>> this series is already long enough.
>>>
>>> Signed-off-by: Eugenio P?rez <eperezma at redhat.com>
>>> ---
>>> hw/virtio/vhost-vdpa.c | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>>
>>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>>> index fb0a338baa..75090d65e8 100644
>>> --- a/hw/virtio/vhost-vdpa.c
>>> +++ b/hw/virtio/vhost-vdpa.c
>>> @@ -1022,6 +1022,9 @@ static int vhost_vdpa_get_features(struct
vhost_dev *dev, uint64_t *features)
>>> if (ret == 0 && v->shadow_vqs_enabled) {
>>> /* Filter only features that SVQ can offer to guest */
>>> vhost_svq_valid_guest_features(features);
>>> +
>>> + /* Add SVQ logging capabilities */
>>> + *features |= BIT_ULL(VHOST_F_LOG_ALL);
>>> }
>>>
>>> return ret;
>>> @@ -1039,8 +1042,25 @@ static int vhost_vdpa_set_features(struct
vhost_dev *dev,
>>>
>>> if (v->shadow_vqs_enabled) {
>>> uint64_t dev_features, svq_features, acked_features;
>>> + uint8_t status = 0;
>>> bool ok;
>>>
>>> + ret = vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS,
&status);
>>> + if (unlikely(ret)) {
>>> + return ret;
>>> + }
>>> +
>>> + if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
>>> + /*
>>> + * vhost is trying to enable or disable _F_LOG, and
the device
>>> + * would report wrong dirty pages. SVQ handles it.
>>> + */
>>
>> I fail to understand this comment, I'd think there's no way to
disable
>> dirty page tracking for SVQ.
>>
> vhost_log_global_{start,stop} are called at the beginning and end of
> migration. To inform the device that it should start logging, they set
> or clean VHOST_F_LOG_ALL at vhost_dev_set_log.
Yes, but for SVQ, we can't disable dirty page tracking, isn't it? The
only thing is to ignore or filter out the F_LOG_ALL and pretend to be
enabled and disabled.
>
> While SVQ does not use VHOST_F_LOG_ALL, it exports the feature bit so
> vhost does not block migration. Maybe we need to look for another way
> to do this?
I'm fine with filtering since it's much more simpler, but I fail to
understand why we need to check DRIVER_OK.
Thanks
>
> Thanks!
>
>> Thanks
>>
>>
>>> + return 0;
>>> + }
>>> +
>>> + /* We must not ack _F_LOG if SVQ is enabled */
>>> + features &= ~BIT_ULL(VHOST_F_LOG_ALL);
>>> +
>>> ret = vhost_vdpa_get_dev_features(dev,
&dev_features);
>>> if (ret != 0) {
>>> error_report("Can't get vdpa device
features, got (%d)", ret);