Re: [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
在 2021/4/15 下午4:12, Stefano Garzarella 写道: On Wed, Apr 14, 2021 at 05:18:32PM +0800, Zhu Lingshan wrote: get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 9b6a38b798fa..b48b9789b69e 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + return size; I'm not familiar with the ifcvf details, but can it happen that the device is not block or net? Should we set `size` to 0 by default to handle this case or are we sure it's one of the two? Maybe we should add a comment or a warning message in this case, to prevent some analysis tool or compiler from worrying that `size` might be uninitialized. I was thinking something like this: switch(vf->dev_type) { case VIRTIO_ID_NET: size = sizeof(struct virtio_net_config); break; case VIRTIO_ID_BLOCK: size = sizeof(struct virtio_blk_config); break; default: /* or WARN(1, "") if dev_warn() not apply */ dev_warn(... , "virtio ID [0x%x] not supported\n") size = 0; } Yes, I agree. Thanks Thanks, Stefano
Re: [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
On Wed, Apr 14, 2021 at 05:18:32PM +0800, Zhu Lingshan wrote: get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 9b6a38b798fa..b48b9789b69e 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + return size; I'm not familiar with the ifcvf details, but can it happen that the device is not block or net? Should we set `size` to 0 by default to handle this case or are we sure it's one of the two? Maybe we should add a comment or a warning message in this case, to prevent some analysis tool or compiler from worrying that `size` might be uninitialized. I was thinking something like this: switch(vf->dev_type) { case VIRTIO_ID_NET: size = sizeof(struct virtio_net_config); break; case VIRTIO_ID_BLOCK: size = sizeof(struct virtio_blk_config); break; default: /* or WARN(1, "") if dev_warn() not apply */ dev_warn(... , "virtio ID [0x%x] not supported\n") size = 0; } Thanks, Stefano
Re: [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
在 2021/4/14 下午5:18, Zhu Lingshan 写道: get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 9b6a38b798fa..b48b9789b69e 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + return size; } static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev,
[PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 9b6a38b798fa..b48b9789b69e 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + return size; } static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev, -- 2.27.0