Hi Jason,
-----Original Message-----
From: Jason Wang <jasow...@redhat.com>
Sent: Wednesday, June 2, 2021 2:18 PM
To: Gautam Dawar <gda...@xilinx.com>; m...@redhat.com; qemu-devel@nongnu.org
Cc: l...@redhat.com; qemu-sta...@nongnu.org
Subject: Re: [PATCH 1/2] vhost-vdpa: don't initialize backend_features
Hi Gautam:
在 2021/6/2 下午3:38, Gautam Dawar 写道:
> Hi Jason,
>
> Pls see my comments inline marked by GD>>
>
> Regards,
> Gautam
>
> -----Original Message-----
> From: Jason Wang <jasow...@redhat.com>
> Sent: Wednesday, June 2, 2021 9:01 AM
> To: m...@redhat.com; qemu-devel@nongnu.org
> Cc: Gautam Dawar <gda...@xilinx.com>; l...@redhat.com; Jason Wang
> <jasow...@redhat.com>; qemu-sta...@nongnu.org
> Subject: [PATCH 1/2] vhost-vdpa: don't initialize backend_features
>
> We used to initialize backend_features during vhost_vdpa_init() regardless
> whether or not it was supported by vhost. This will lead the unsupported
> features like VIRTIO_F_IN_ORDER to be included and set to the vhost-vdpa
> during vhost_dev_start. Because the VIRTIO_F_IN_ORDER is not supported by
> vhost-vdpa so it won't be advertised to guest which will break the datapath.
>
> Fix this by not initializing the backend_features, so the acked_features
> could be built only from guest features via vhost_net_ack_features().
>
> Fixes: 108a64818e69b ("vhost-vdpa: introduce vhost-vdpa backend")
> Cc: qemu-sta...@nongnu.org
> Cc: Gautam Dawar <gda...@xilinx.com>
> Signed-off-by: Jason Wang <jasow...@redhat.com>
> ---
> hw/virtio/vhost-vdpa.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index
> 01d2101d09..5fe43a4eb5 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -275,15 +275,12 @@ static void vhost_vdpa_add_status(struct vhost_dev
> *dev, uint8_t status) static int vhost_vdpa_init(struct vhost_dev *dev, void
> *opaque) {
> struct vhost_vdpa *v;
> - uint64_t features;
> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
> trace_vhost_vdpa_init(dev, opaque);
>
> v = opaque;
> v->dev = dev;
> dev->opaque = opaque ;
> - vhost_vdpa_call(dev, VHOST_GET_FEATURES, &features);
> - dev->backend_features = features;
> [GD>>] Should this be initialized with 0 here? I am not sure if memory
> allocated for struct vhost_dev is initialized with 0.
See vhost_net_init:
struct vhost_net *net = g_new0(struct vhost_net, 1);
vhost_dev is embedded in the vhost_net structure. So I think it should be zero.
[GD>>] That's correct. The embedded vhost_dev structure is indeed getting
cleared to 0 in vhost_net_init().
Thanks
> v->listener = vhost_vdpa_memory_listener;
> v->msg_type = VHOST_IOTLB_MSG_V2;
>
> --
> 2.25.1
>
[GD>>]
Signed-off-by: Gautam Dawar <gda...@xilinx.com>
Acked-by: Gautam Dawar <gda...@xilinx.com>
