| Message ID | 20220701132826.8132-7-lingshan.zhu@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ifcvf/vDPA: support query device config space through netlink | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
> From: Zhu Lingshan <lingshan.zhu@intel.com> > Sent: Friday, July 1, 2022 9:28 AM > > This commit fixes spars warnings: cast to restricted __le16 in function > vdpa_dev_net_config_fill() and > vdpa_fill_stats_rec() > Missing fixes tag. But I fail to understand the warning. config.status is le16, and API used is to convert le16 to cpu. What is the warning about, can you please explain? > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > --- > drivers/vdpa/vdpa.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index > 846dd37f3549..ed49fe46a79e 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct > vdpa_device *vdev, struct sk_buff *ms > config.mac)) > return -EMSGSIZE; > > - val_u16 = le16_to_cpu(config.status); > + val_u16 = __virtio16_to_cpu(true, config.status); > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > return -EMSGSIZE; > > - val_u16 = le16_to_cpu(config.mtu); > + val_u16 = __virtio16_to_cpu(true, config.mtu); > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > return -EMSGSIZE; > > @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device > *vdev, struct sk_buff *msg, > } > vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > > - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, > max_vqp)) > return -EMSGSIZE; > > -- > 2.31.1
On 7/2/2022 6:18 AM, Parav Pandit wrote: >> From: Zhu Lingshan <lingshan.zhu@intel.com> >> Sent: Friday, July 1, 2022 9:28 AM >> >> This commit fixes spars warnings: cast to restricted __le16 in function >> vdpa_dev_net_config_fill() and >> vdpa_fill_stats_rec() >> > Missing fixes tag. I am not sure whether this deserve a fix tag, I will look into it. > > But I fail to understand the warning. > config.status is le16, and API used is to convert le16 to cpu. > What is the warning about, can you please explain? The warnings are: drivers/vdpa/vdpa.c:828:19: warning: cast to restricted __le16 drivers/vdpa/vdpa.c:828:19: warning: cast from restricted __virtio16 > >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> drivers/vdpa/vdpa.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index >> 846dd37f3549..ed49fe46a79e 100644 >> --- a/drivers/vdpa/vdpa.c >> +++ b/drivers/vdpa/vdpa.c >> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct >> vdpa_device *vdev, struct sk_buff *ms >> config.mac)) >> return -EMSGSIZE; >> >> - val_u16 = le16_to_cpu(config.status); >> + val_u16 = __virtio16_to_cpu(true, config.status); >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) >> return -EMSGSIZE; >> >> - val_u16 = le16_to_cpu(config.mtu); >> + val_u16 = __virtio16_to_cpu(true, config.mtu); >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) >> return -EMSGSIZE; >> >> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device >> *vdev, struct sk_buff *msg, >> } >> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); >> >> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); >> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, >> max_vqp)) >> return -EMSGSIZE; >> >> -- >> 2.31.1
> From: Zhu, Lingshan <lingshan.zhu@intel.com> > Sent: Friday, July 8, 2022 2:25 AM > > > On 7/2/2022 6:18 AM, Parav Pandit wrote: > >> From: Zhu Lingshan <lingshan.zhu@intel.com> > >> Sent: Friday, July 1, 2022 9:28 AM > >> > >> This commit fixes spars warnings: cast to restricted __le16 in > >> function > >> vdpa_dev_net_config_fill() and > >> vdpa_fill_stats_rec() > >> > > Missing fixes tag. > I am not sure whether this deserve a fix tag, I will look into it. > > > > But I fail to understand the warning. > > config.status is le16, and API used is to convert le16 to cpu. > > What is the warning about, can you please explain? I see it. Its not le16, its __virtio16. Please add fixes tag. With that Reviewed-by: Parav Pandit <parav@nvidia.com> > The warnings are: > drivers/vdpa/vdpa.c:828:19: warning: cast to restricted __le16 > drivers/vdpa/vdpa.c:828:19: warning: cast from restricted __virtio16 > > > >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > >> --- > >> drivers/vdpa/vdpa.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index > >> 846dd37f3549..ed49fe46a79e 100644 > >> --- a/drivers/vdpa/vdpa.c > >> +++ b/drivers/vdpa/vdpa.c > >> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct > >> vdpa_device *vdev, struct sk_buff *ms > >> config.mac)) > >> return -EMSGSIZE; > >> > >> - val_u16 = le16_to_cpu(config.status); > >> + val_u16 = __virtio16_to_cpu(true, config.status); > >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > >> return -EMSGSIZE; > >> > >> - val_u16 = le16_to_cpu(config.mtu); > >> + val_u16 = __virtio16_to_cpu(true, config.mtu); > >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > >> return -EMSGSIZE; > >> > >> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device > >> *vdev, struct sk_buff *msg, > >> } > >> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > >> > >> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > >> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, > >> max_vqp)) > >> return -EMSGSIZE; > >> > >> -- > >> 2.31.1
On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: > This commit fixes spars warnings: cast to restricted __le16 > in function vdpa_dev_net_config_fill() and > vdpa_fill_stats_rec() > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > --- > drivers/vdpa/vdpa.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > index 846dd37f3549..ed49fe46a79e 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > config.mac)) > return -EMSGSIZE; > > - val_u16 = le16_to_cpu(config.status); > + val_u16 = __virtio16_to_cpu(true, config.status); > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > return -EMSGSIZE; > > - val_u16 = le16_to_cpu(config.mtu); > + val_u16 = __virtio16_to_cpu(true, config.mtu); > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > return -EMSGSIZE; > Wrong on BE platforms with legacy interface, isn't it? We generally don't handle legacy properly in VDPA so it's not a huge deal, but maybe add a comment at least? > @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, > } > vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > > - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) > return -EMSGSIZE; > > -- > 2.31.1
On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: > On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: >> This commit fixes spars warnings: cast to restricted __le16 >> in function vdpa_dev_net_config_fill() and >> vdpa_fill_stats_rec() >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> drivers/vdpa/vdpa.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >> index 846dd37f3549..ed49fe46a79e 100644 >> --- a/drivers/vdpa/vdpa.c >> +++ b/drivers/vdpa/vdpa.c >> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms >> config.mac)) >> return -EMSGSIZE; >> >> - val_u16 = le16_to_cpu(config.status); >> + val_u16 = __virtio16_to_cpu(true, config.status); >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) >> return -EMSGSIZE; >> >> - val_u16 = le16_to_cpu(config.mtu); >> + val_u16 = __virtio16_to_cpu(true, config.mtu); >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) >> return -EMSGSIZE; >> > Wrong on BE platforms with legacy interface, isn't it? > We generally don't handle legacy properly in VDPA so it's > not a huge deal, but maybe add a comment at least? Sure, I can add a comment here: this is for modern devices only. Thanks, Zhu Lingshan > > >> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, >> } >> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); >> >> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); >> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) >> return -EMSGSIZE; >> >> -- >> 2.31.1
On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: > > > On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: > > On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: > > > This commit fixes spars warnings: cast to restricted __le16 > > > in function vdpa_dev_net_config_fill() and > > > vdpa_fill_stats_rec() > > > > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > > > --- > > > drivers/vdpa/vdpa.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > > > index 846dd37f3549..ed49fe46a79e 100644 > > > --- a/drivers/vdpa/vdpa.c > > > +++ b/drivers/vdpa/vdpa.c > > > @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > > > config.mac)) > > > return -EMSGSIZE; > > > - val_u16 = le16_to_cpu(config.status); > > > + val_u16 = __virtio16_to_cpu(true, config.status); > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > > > return -EMSGSIZE; > > > - val_u16 = le16_to_cpu(config.mtu); > > > + val_u16 = __virtio16_to_cpu(true, config.mtu); > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > > > return -EMSGSIZE; > > Wrong on BE platforms with legacy interface, isn't it? > > We generally don't handle legacy properly in VDPA so it's > > not a huge deal, but maybe add a comment at least? > Sure, I can add a comment here: this is for modern devices only. > > Thanks, > Zhu Lingshan Hmm. what "this" is for modern devices only here? > > > > > > > @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, > > > } > > > vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > > > - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > > > + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) > > > return -EMSGSIZE; > > > -- > > > 2.31.1
On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: > On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: >> >> On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: >>> On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: >>>> This commit fixes spars warnings: cast to restricted __le16 >>>> in function vdpa_dev_net_config_fill() and >>>> vdpa_fill_stats_rec() >>>> >>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>> --- >>>> drivers/vdpa/vdpa.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >>>> index 846dd37f3549..ed49fe46a79e 100644 >>>> --- a/drivers/vdpa/vdpa.c >>>> +++ b/drivers/vdpa/vdpa.c >>>> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms >>>> config.mac)) >>>> return -EMSGSIZE; >>>> - val_u16 = le16_to_cpu(config.status); >>>> + val_u16 = __virtio16_to_cpu(true, config.status); >>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) >>>> return -EMSGSIZE; >>>> - val_u16 = le16_to_cpu(config.mtu); >>>> + val_u16 = __virtio16_to_cpu(true, config.mtu); >>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) >>>> return -EMSGSIZE; >>> Wrong on BE platforms with legacy interface, isn't it? >>> We generally don't handle legacy properly in VDPA so it's >>> not a huge deal, but maybe add a comment at least? >> Sure, I can add a comment here: this is for modern devices only. >> >> Thanks, >> Zhu Lingshan > Hmm. what "this" is for modern devices only here? this cast, for LE modern devices. > >>> >>>> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, >>>> } >>>> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); >>>> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); >>>> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); >>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) >>>> return -EMSGSIZE; >>>> -- >>>> 2.31.1
On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: > > > On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: > > On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: > > > > > > On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: > > > > On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: > > > > > This commit fixes spars warnings: cast to restricted __le16 > > > > > in function vdpa_dev_net_config_fill() and > > > > > vdpa_fill_stats_rec() > > > > > > > > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > > > > > --- > > > > > drivers/vdpa/vdpa.c | 6 +++--- > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > > > > > index 846dd37f3549..ed49fe46a79e 100644 > > > > > --- a/drivers/vdpa/vdpa.c > > > > > +++ b/drivers/vdpa/vdpa.c > > > > > @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > > > > > config.mac)) > > > > > return -EMSGSIZE; > > > > > - val_u16 = le16_to_cpu(config.status); > > > > > + val_u16 = __virtio16_to_cpu(true, config.status); > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > > > > > return -EMSGSIZE; > > > > > - val_u16 = le16_to_cpu(config.mtu); > > > > > + val_u16 = __virtio16_to_cpu(true, config.mtu); > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > > > > > return -EMSGSIZE; > > > > Wrong on BE platforms with legacy interface, isn't it? > > > > We generally don't handle legacy properly in VDPA so it's > > > > not a huge deal, but maybe add a comment at least? > > > Sure, I can add a comment here: this is for modern devices only. > > > > > > Thanks, > > > Zhu Lingshan > > Hmm. what "this" is for modern devices only here? > this cast, for LE modern devices. I think status existed in legacy for sure, and it's possible that some legacy devices backported mtu and max_virtqueue_pairs otherwise we would have these fields as __le not as __virtio, right? > > > > > > > > > > > @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, > > > > > } > > > > > vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > > > > > - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > > > > > + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) > > > > > return -EMSGSIZE; > > > > > -- > > > > > 2.31.1
On 7/29/2022 5:23 PM, Michael S. Tsirkin wrote: > On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: >> >> On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: >>> On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: >>>> On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: >>>>> On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: >>>>>> This commit fixes spars warnings: cast to restricted __le16 >>>>>> in function vdpa_dev_net_config_fill() and >>>>>> vdpa_fill_stats_rec() >>>>>> >>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>>>> --- >>>>>> drivers/vdpa/vdpa.c | 6 +++--- >>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >>>>>> index 846dd37f3549..ed49fe46a79e 100644 >>>>>> --- a/drivers/vdpa/vdpa.c >>>>>> +++ b/drivers/vdpa/vdpa.c >>>>>> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms >>>>>> config.mac)) >>>>>> return -EMSGSIZE; >>>>>> - val_u16 = le16_to_cpu(config.status); >>>>>> + val_u16 = __virtio16_to_cpu(true, config.status); >>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) >>>>>> return -EMSGSIZE; >>>>>> - val_u16 = le16_to_cpu(config.mtu); >>>>>> + val_u16 = __virtio16_to_cpu(true, config.mtu); >>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) >>>>>> return -EMSGSIZE; >>>>> Wrong on BE platforms with legacy interface, isn't it? >>>>> We generally don't handle legacy properly in VDPA so it's >>>>> not a huge deal, but maybe add a comment at least? >>>> Sure, I can add a comment here: this is for modern devices only. >>>> >>>> Thanks, >>>> Zhu Lingshan >>> Hmm. what "this" is for modern devices only here? >> this cast, for LE modern devices. > I think status existed in legacy for sure, and it's possible that > some legacy devices backported mtu and max_virtqueue_pairs otherwise > we would have these fields as __le not as __virtio, right? yes, that's the reason why it is virtio_16 than just le16. I may find a better solution to detect whether it is LE, or BE without a virtio_dev structure. Check whether vdpa_device->get_device_features() has VIRTIO_F_VERISON_1. If the device offers _F_VERSION_1, then it is a LE device, or it is a BE device, then we use __virtio16_to_cpu(false, config.status). Does this look good? > >>>>>> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, >>>>>> } >>>>>> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); >>>>>> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); >>>>>> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); >>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) >>>>>> return -EMSGSIZE; >>>>>> -- >>>>>> 2.31.1
On Fri, Jul 29, 2022 at 05:35:09PM +0800, Zhu, Lingshan wrote: > > > On 7/29/2022 5:23 PM, Michael S. Tsirkin wrote: > > On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: > > > > > > On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: > > > > On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: > > > > > On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: > > > > > > On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: > > > > > > > This commit fixes spars warnings: cast to restricted __le16 > > > > > > > in function vdpa_dev_net_config_fill() and > > > > > > > vdpa_fill_stats_rec() > > > > > > > > > > > > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > > > > > > > --- > > > > > > > drivers/vdpa/vdpa.c | 6 +++--- > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > > > > > > > index 846dd37f3549..ed49fe46a79e 100644 > > > > > > > --- a/drivers/vdpa/vdpa.c > > > > > > > +++ b/drivers/vdpa/vdpa.c > > > > > > > @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > > > > > > > config.mac)) > > > > > > > return -EMSGSIZE; > > > > > > > - val_u16 = le16_to_cpu(config.status); > > > > > > > + val_u16 = __virtio16_to_cpu(true, config.status); > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > > > > > > > return -EMSGSIZE; > > > > > > > - val_u16 = le16_to_cpu(config.mtu); > > > > > > > + val_u16 = __virtio16_to_cpu(true, config.mtu); > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > > > > > > > return -EMSGSIZE; > > > > > > Wrong on BE platforms with legacy interface, isn't it? > > > > > > We generally don't handle legacy properly in VDPA so it's > > > > > > not a huge deal, but maybe add a comment at least? > > > > > Sure, I can add a comment here: this is for modern devices only. > > > > > > > > > > Thanks, > > > > > Zhu Lingshan > > > > Hmm. what "this" is for modern devices only here? > > > this cast, for LE modern devices. > > I think status existed in legacy for sure, and it's possible that > > some legacy devices backported mtu and max_virtqueue_pairs otherwise > > we would have these fields as __le not as __virtio, right? > yes, that's the reason why it is virtio_16 than just le16. > > I may find a better solution to detect whether it is LE, or BE without a > virtio_dev structure. > Check whether vdpa_device->get_device_features() has VIRTIO_F_VERISON_1. If > the device offers _F_VERSION_1, then it is a LE device, > or it is a BE device, then we use __virtio16_to_cpu(false, config.status). > > Does this look good? No since the question is can be a legacy driver with a transitional device. I don't have a good idea yet. vhost has VHOST_SET_VRING_ENDIAN and maybe we need something like this for config as well? > > > > > > > > > @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, > > > > > > > } > > > > > > > vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > > > > > > > - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > > > > > > > + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) > > > > > > > return -EMSGSIZE; > > > > > > > -- > > > > > > > 2.31.1
On 7/29/2022 5:39 PM, Michael S. Tsirkin wrote: > On Fri, Jul 29, 2022 at 05:35:09PM +0800, Zhu, Lingshan wrote: >> >> On 7/29/2022 5:23 PM, Michael S. Tsirkin wrote: >>> On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: >>>> On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: >>>>> On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: >>>>>> On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: >>>>>>> On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: >>>>>>>> This commit fixes spars warnings: cast to restricted __le16 >>>>>>>> in function vdpa_dev_net_config_fill() and >>>>>>>> vdpa_fill_stats_rec() >>>>>>>> >>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>>>>>> --- >>>>>>>> drivers/vdpa/vdpa.c | 6 +++--- >>>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >>>>>>>> index 846dd37f3549..ed49fe46a79e 100644 >>>>>>>> --- a/drivers/vdpa/vdpa.c >>>>>>>> +++ b/drivers/vdpa/vdpa.c >>>>>>>> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms >>>>>>>> config.mac)) >>>>>>>> return -EMSGSIZE; >>>>>>>> - val_u16 = le16_to_cpu(config.status); >>>>>>>> + val_u16 = __virtio16_to_cpu(true, config.status); >>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) >>>>>>>> return -EMSGSIZE; >>>>>>>> - val_u16 = le16_to_cpu(config.mtu); >>>>>>>> + val_u16 = __virtio16_to_cpu(true, config.mtu); >>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) >>>>>>>> return -EMSGSIZE; >>>>>>> Wrong on BE platforms with legacy interface, isn't it? >>>>>>> We generally don't handle legacy properly in VDPA so it's >>>>>>> not a huge deal, but maybe add a comment at least? >>>>>> Sure, I can add a comment here: this is for modern devices only. >>>>>> >>>>>> Thanks, >>>>>> Zhu Lingshan >>>>> Hmm. what "this" is for modern devices only here? >>>> this cast, for LE modern devices. >>> I think status existed in legacy for sure, and it's possible that >>> some legacy devices backported mtu and max_virtqueue_pairs otherwise >>> we would have these fields as __le not as __virtio, right? >> yes, that's the reason why it is virtio_16 than just le16. >> >> I may find a better solution to detect whether it is LE, or BE without a >> virtio_dev structure. >> Check whether vdpa_device->get_device_features() has VIRTIO_F_VERISON_1. If >> the device offers _F_VERSION_1, then it is a LE device, >> or it is a BE device, then we use __virtio16_to_cpu(false, config.status). >> >> Does this look good? > No since the question is can be a legacy driver with a transitional > device. I don't have a good idea yet. vhost has VHOST_SET_VRING_ENDIAN > and maybe we need something like this for config as well? Is it a little overkill to implementing vdpa_ops.get_endian()? > >>>>>>>> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, >>>>>>>> } >>>>>>>> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); >>>>>>>> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); >>>>>>>> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); >>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) >>>>>>>> return -EMSGSIZE; >>>>>>>> -- >>>>>>>> 2.31.1
On Fri, Jul 29, 2022 at 06:01:38PM +0800, Zhu, Lingshan wrote: > > > On 7/29/2022 5:39 PM, Michael S. Tsirkin wrote: > > On Fri, Jul 29, 2022 at 05:35:09PM +0800, Zhu, Lingshan wrote: > > > > > > On 7/29/2022 5:23 PM, Michael S. Tsirkin wrote: > > > > On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: > > > > > On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: > > > > > > On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: > > > > > > > On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: > > > > > > > > On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: > > > > > > > > > This commit fixes spars warnings: cast to restricted __le16 > > > > > > > > > in function vdpa_dev_net_config_fill() and > > > > > > > > > vdpa_fill_stats_rec() > > > > > > > > > > > > > > > > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > > > > > > > > > --- > > > > > > > > > drivers/vdpa/vdpa.c | 6 +++--- > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > > > > > > > > > index 846dd37f3549..ed49fe46a79e 100644 > > > > > > > > > --- a/drivers/vdpa/vdpa.c > > > > > > > > > +++ b/drivers/vdpa/vdpa.c > > > > > > > > > @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > > > > > > > > > config.mac)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > > - val_u16 = le16_to_cpu(config.status); > > > > > > > > > + val_u16 = __virtio16_to_cpu(true, config.status); > > > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > > - val_u16 = le16_to_cpu(config.mtu); > > > > > > > > > + val_u16 = __virtio16_to_cpu(true, config.mtu); > > > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > Wrong on BE platforms with legacy interface, isn't it? > > > > > > > > We generally don't handle legacy properly in VDPA so it's > > > > > > > > not a huge deal, but maybe add a comment at least? > > > > > > > Sure, I can add a comment here: this is for modern devices only. > > > > > > > > > > > > > > Thanks, > > > > > > > Zhu Lingshan > > > > > > Hmm. what "this" is for modern devices only here? > > > > > this cast, for LE modern devices. > > > > I think status existed in legacy for sure, and it's possible that > > > > some legacy devices backported mtu and max_virtqueue_pairs otherwise > > > > we would have these fields as __le not as __virtio, right? > > > yes, that's the reason why it is virtio_16 than just le16. > > > > > > I may find a better solution to detect whether it is LE, or BE without a > > > virtio_dev structure. > > > Check whether vdpa_device->get_device_features() has VIRTIO_F_VERISON_1. If > > > the device offers _F_VERSION_1, then it is a LE device, > > > or it is a BE device, then we use __virtio16_to_cpu(false, config.status). > > > > > > Does this look good? > > No since the question is can be a legacy driver with a transitional > > device. I don't have a good idea yet. vhost has VHOST_SET_VRING_ENDIAN > > and maybe we need something like this for config as well? > Is it a little overkill to implementing vdpa_ops.get_endian()? I think the question is driver endian-ness. But another approach is really just to say userspace should tweak config endian itself. Let's just say that in the comment? /* * Assume little endian for now, userspace can tweak this for * legacy guest support. */ ? > > > > > > > > > > > @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, > > > > > > > > > } > > > > > > > > > vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > > > > > > > > > - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > > > > > > > > > + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > > > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > > -- > > > > > > > > > 2.31.1
On 7/29/2022 6:16 PM, Michael S. Tsirkin wrote: > On Fri, Jul 29, 2022 at 06:01:38PM +0800, Zhu, Lingshan wrote: >> >> On 7/29/2022 5:39 PM, Michael S. Tsirkin wrote: >>> On Fri, Jul 29, 2022 at 05:35:09PM +0800, Zhu, Lingshan wrote: >>>> On 7/29/2022 5:23 PM, Michael S. Tsirkin wrote: >>>>> On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: >>>>>> On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: >>>>>>> On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: >>>>>>>> On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: >>>>>>>>> On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: >>>>>>>>>> This commit fixes spars warnings: cast to restricted __le16 >>>>>>>>>> in function vdpa_dev_net_config_fill() and >>>>>>>>>> vdpa_fill_stats_rec() >>>>>>>>>> >>>>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>>>>>>>> --- >>>>>>>>>> drivers/vdpa/vdpa.c | 6 +++--- >>>>>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>>>>> >>>>>>>>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >>>>>>>>>> index 846dd37f3549..ed49fe46a79e 100644 >>>>>>>>>> --- a/drivers/vdpa/vdpa.c >>>>>>>>>> +++ b/drivers/vdpa/vdpa.c >>>>>>>>>> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms >>>>>>>>>> config.mac)) >>>>>>>>>> return -EMSGSIZE; >>>>>>>>>> - val_u16 = le16_to_cpu(config.status); >>>>>>>>>> + val_u16 = __virtio16_to_cpu(true, config.status); >>>>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) >>>>>>>>>> return -EMSGSIZE; >>>>>>>>>> - val_u16 = le16_to_cpu(config.mtu); >>>>>>>>>> + val_u16 = __virtio16_to_cpu(true, config.mtu); >>>>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) >>>>>>>>>> return -EMSGSIZE; >>>>>>>>> Wrong on BE platforms with legacy interface, isn't it? >>>>>>>>> We generally don't handle legacy properly in VDPA so it's >>>>>>>>> not a huge deal, but maybe add a comment at least? >>>>>>>> Sure, I can add a comment here: this is for modern devices only. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Zhu Lingshan >>>>>>> Hmm. what "this" is for modern devices only here? >>>>>> this cast, for LE modern devices. >>>>> I think status existed in legacy for sure, and it's possible that >>>>> some legacy devices backported mtu and max_virtqueue_pairs otherwise >>>>> we would have these fields as __le not as __virtio, right? >>>> yes, that's the reason why it is virtio_16 than just le16. >>>> >>>> I may find a better solution to detect whether it is LE, or BE without a >>>> virtio_dev structure. >>>> Check whether vdpa_device->get_device_features() has VIRTIO_F_VERISON_1. If >>>> the device offers _F_VERSION_1, then it is a LE device, >>>> or it is a BE device, then we use __virtio16_to_cpu(false, config.status). >>>> >>>> Does this look good? >>> No since the question is can be a legacy driver with a transitional >>> device. I don't have a good idea yet. vhost has VHOST_SET_VRING_ENDIAN >>> and maybe we need something like this for config as well? >> Is it a little overkill to implementing vdpa_ops.get_endian()? > I think the question is driver endian-ness. > > But another approach is really just to say userspace should > tweak config endian itself. Let's just say that in the comment? > /* > * Assume little endian for now, userspace can tweak this for > * legacy guest support. > */ > ? Oh, yes, the user space can tweak the value!!! I will add this comment, thanks!!!! >>>>>>>>>> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, >>>>>>>>>> } >>>>>>>>>> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); >>>>>>>>>> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); >>>>>>>>>> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); >>>>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) >>>>>>>>>> return -EMSGSIZE; >>>>>>>>>> -- >>>>>>>>>> 2.31.1
在 2022/7/29 17:39, Michael S. Tsirkin 写道: > On Fri, Jul 29, 2022 at 05:35:09PM +0800, Zhu, Lingshan wrote: >> >> On 7/29/2022 5:23 PM, Michael S. Tsirkin wrote: >>> On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: >>>> On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: >>>>> On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: >>>>>> On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: >>>>>>> On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: >>>>>>>> This commit fixes spars warnings: cast to restricted __le16 >>>>>>>> in function vdpa_dev_net_config_fill() and >>>>>>>> vdpa_fill_stats_rec() >>>>>>>> >>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>>>>>> --- >>>>>>>> drivers/vdpa/vdpa.c | 6 +++--- >>>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >>>>>>>> index 846dd37f3549..ed49fe46a79e 100644 >>>>>>>> --- a/drivers/vdpa/vdpa.c >>>>>>>> +++ b/drivers/vdpa/vdpa.c >>>>>>>> @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms >>>>>>>> config.mac)) >>>>>>>> return -EMSGSIZE; >>>>>>>> - val_u16 = le16_to_cpu(config.status); >>>>>>>> + val_u16 = __virtio16_to_cpu(true, config.status); >>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) >>>>>>>> return -EMSGSIZE; >>>>>>>> - val_u16 = le16_to_cpu(config.mtu); >>>>>>>> + val_u16 = __virtio16_to_cpu(true, config.mtu); >>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) >>>>>>>> return -EMSGSIZE; >>>>>>> Wrong on BE platforms with legacy interface, isn't it? >>>>>>> We generally don't handle legacy properly in VDPA so it's >>>>>>> not a huge deal, but maybe add a comment at least? >>>>>> Sure, I can add a comment here: this is for modern devices only. >>>>>> >>>>>> Thanks, >>>>>> Zhu Lingshan >>>>> Hmm. what "this" is for modern devices only here? >>>> this cast, for LE modern devices. >>> I think status existed in legacy for sure, and it's possible that >>> some legacy devices backported mtu and max_virtqueue_pairs otherwise >>> we would have these fields as __le not as __virtio, right? >> yes, that's the reason why it is virtio_16 than just le16. >> >> I may find a better solution to detect whether it is LE, or BE without a >> virtio_dev structure. >> Check whether vdpa_device->get_device_features() has VIRTIO_F_VERISON_1. If >> the device offers _F_VERSION_1, then it is a LE device, >> or it is a BE device, then we use __virtio16_to_cpu(false, config.status). >> >> Does this look good? > No since the question is can be a legacy driver with a transitional > device. I don't have a good idea yet. vhost has VHOST_SET_VRING_ENDIAN > and maybe we need something like this for config as well? Not sure, and even if we had this, the query could happen before VHOST_SET_VRING_ENDIAN. Actually, the patch should be fine itself, since the issue exist even before the patch (which assumes a le). Thanks > >>>>>>>> @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, >>>>>>>> } >>>>>>>> vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); >>>>>>>> - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); >>>>>>>> + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); >>>>>>>> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) >>>>>>>> return -EMSGSIZE; >>>>>>>> -- >>>>>>>> 2.31.1
On Mon, Aug 01, 2022 at 12:33:44PM +0800, Jason Wang wrote: > > 在 2022/7/29 17:39, Michael S. Tsirkin 写道: > > On Fri, Jul 29, 2022 at 05:35:09PM +0800, Zhu, Lingshan wrote: > > > > > > On 7/29/2022 5:23 PM, Michael S. Tsirkin wrote: > > > > On Fri, Jul 29, 2022 at 05:20:17PM +0800, Zhu, Lingshan wrote: > > > > > On 7/29/2022 5:17 PM, Michael S. Tsirkin wrote: > > > > > > On Fri, Jul 29, 2022 at 05:07:11PM +0800, Zhu, Lingshan wrote: > > > > > > > On 7/29/2022 4:53 PM, Michael S. Tsirkin wrote: > > > > > > > > On Fri, Jul 01, 2022 at 09:28:26PM +0800, Zhu Lingshan wrote: > > > > > > > > > This commit fixes spars warnings: cast to restricted __le16 > > > > > > > > > in function vdpa_dev_net_config_fill() and > > > > > > > > > vdpa_fill_stats_rec() > > > > > > > > > > > > > > > > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > > > > > > > > > --- > > > > > > > > > drivers/vdpa/vdpa.c | 6 +++--- > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > > > > > > > > > index 846dd37f3549..ed49fe46a79e 100644 > > > > > > > > > --- a/drivers/vdpa/vdpa.c > > > > > > > > > +++ b/drivers/vdpa/vdpa.c > > > > > > > > > @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms > > > > > > > > > config.mac)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > > - val_u16 = le16_to_cpu(config.status); > > > > > > > > > + val_u16 = __virtio16_to_cpu(true, config.status); > > > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > > - val_u16 = le16_to_cpu(config.mtu); > > > > > > > > > + val_u16 = __virtio16_to_cpu(true, config.mtu); > > > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > Wrong on BE platforms with legacy interface, isn't it? > > > > > > > > We generally don't handle legacy properly in VDPA so it's > > > > > > > > not a huge deal, but maybe add a comment at least? > > > > > > > Sure, I can add a comment here: this is for modern devices only. > > > > > > > > > > > > > > Thanks, > > > > > > > Zhu Lingshan > > > > > > Hmm. what "this" is for modern devices only here? > > > > > this cast, for LE modern devices. > > > > I think status existed in legacy for sure, and it's possible that > > > > some legacy devices backported mtu and max_virtqueue_pairs otherwise > > > > we would have these fields as __le not as __virtio, right? > > > yes, that's the reason why it is virtio_16 than just le16. > > > > > > I may find a better solution to detect whether it is LE, or BE without a > > > virtio_dev structure. > > > Check whether vdpa_device->get_device_features() has VIRTIO_F_VERISON_1. If > > > the device offers _F_VERSION_1, then it is a LE device, > > > or it is a BE device, then we use __virtio16_to_cpu(false, config.status). > > > > > > Does this look good? > > No since the question is can be a legacy driver with a transitional > > device. I don't have a good idea yet. vhost has VHOST_SET_VRING_ENDIAN > > and maybe we need something like this for config as well? > > > Not sure, and even if we had this, the query could happen before > VHOST_SET_VRING_ENDIAN. > > Actually, the patch should be fine itself, since the issue exist even before > the patch (which assumes a le). > > Thanks I agree, let's just add a TODO comment. > > > > > > > > > > > > @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, > > > > > > > > > } > > > > > > > > > vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); > > > > > > > > > - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); > > > > > > > > > + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); > > > > > > > > > if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) > > > > > > > > > return -EMSGSIZE; > > > > > > > > > -- > > > > > > > > > 2.31.1
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index 846dd37f3549..ed49fe46a79e 100644 --- a/drivers/vdpa/vdpa.c +++ b/drivers/vdpa/vdpa.c @@ -825,11 +825,11 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms config.mac)) return -EMSGSIZE; - val_u16 = le16_to_cpu(config.status); + val_u16 = __virtio16_to_cpu(true, config.status); if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) return -EMSGSIZE; - val_u16 = le16_to_cpu(config.mtu); + val_u16 = __virtio16_to_cpu(true, config.mtu); if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) return -EMSGSIZE; @@ -911,7 +911,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg, } vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config)); - max_vqp = le16_to_cpu(config.max_virtqueue_pairs); + max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs); if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp)) return -EMSGSIZE;
This commit fixes spars warnings: cast to restricted __le16 in function vdpa_dev_net_config_fill() and vdpa_fill_stats_rec() Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/vdpa.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)