| Message ID | 20220602023845.2596397-5-lingshan.zhu@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ifcvf/vDPA: support query device config space through netlink | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Jun 2, 2022 at 10:48 AM Zhu Lingshan <lingshan.zhu@intel.com> wrote: > > Users may want to query the config space of a vDPA device, > to choose a appropriate one for a certain guest. This means the > users need to read the config space before FEATURES_OK, and > the existence of config space contents does not depend on > FEATURES_OK. Quotes from the spec: "The device MUST allow reading of any device-specific configuration field before FEATURES_OK is set by the driver. This includes fields which are conditional on feature bits, as long as those feature bits are offered by the device." > > This commit removes FEATURES_OK blocker in vdpa_dev_config_fill() > which calls vdpa_dev_net_config_fill() for virtio-net > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > --- > drivers/vdpa/vdpa.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > index c820dd2b0307..030d96bdeed2 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -863,17 +863,9 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, > { > u32 device_id; > void *hdr; > - u8 status; > int err; > > mutex_lock(&vdev->cf_mutex); > - status = vdev->config->get_status(vdev); > - if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) { > - NL_SET_ERR_MSG_MOD(extack, "Features negotiation not completed"); > - err = -EAGAIN; > - goto out; > - } So we had the following in vdpa_dev_net_config_fill(): features = vdev->config->get_driver_features(vdev); if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_NEGOTIATED_FEATURES, features, VDPA_ATTR_PAD)) return -EMSGSIZE; It looks to me we need to switch to using get_device_features() instead. Thanks > - > hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags, > VDPA_CMD_DEV_CONFIG_GET); > if (!hdr) { > -- > 2.31.1 >
On 6/2/2022 3:36 PM, Jason Wang wrote: > On Thu, Jun 2, 2022 at 10:48 AM Zhu Lingshan <lingshan.zhu@intel.com> wrote: >> Users may want to query the config space of a vDPA device, >> to choose a appropriate one for a certain guest. This means the >> users need to read the config space before FEATURES_OK, and >> the existence of config space contents does not depend on >> FEATURES_OK. > Quotes from the spec: > > "The device MUST allow reading of any device-specific configuration > field before FEATURES_OK is set by the driver. This includes fields > which are conditional on feature bits, as long as those feature bits > are offered by the device." > >> This commit removes FEATURES_OK blocker in vdpa_dev_config_fill() >> which calls vdpa_dev_net_config_fill() for virtio-net >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> drivers/vdpa/vdpa.c | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >> index c820dd2b0307..030d96bdeed2 100644 >> --- a/drivers/vdpa/vdpa.c >> +++ b/drivers/vdpa/vdpa.c >> @@ -863,17 +863,9 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, >> { >> u32 device_id; >> void *hdr; >> - u8 status; >> int err; >> >> mutex_lock(&vdev->cf_mutex); >> - status = vdev->config->get_status(vdev); >> - if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) { >> - NL_SET_ERR_MSG_MOD(extack, "Features negotiation not completed"); >> - err = -EAGAIN; >> - goto out; >> - } > So we had the following in vdpa_dev_net_config_fill(): > > features = vdev->config->get_driver_features(vdev); > if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_NEGOTIATED_FEATURES, features, > VDPA_ATTR_PAD)) > return -EMSGSIZE; > > It looks to me we need to switch to using get_device_features() instead. This is done in the 3/6 patch Thanks, ZHu Lingshan > > Thanks > >> - >> hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags, >> VDPA_CMD_DEV_CONFIG_GET); >> if (!hdr) { >> -- >> 2.31.1 >>
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index c820dd2b0307..030d96bdeed2 100644 --- a/drivers/vdpa/vdpa.c +++ b/drivers/vdpa/vdpa.c @@ -863,17 +863,9 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, { u32 device_id; void *hdr; - u8 status; int err; mutex_lock(&vdev->cf_mutex); - status = vdev->config->get_status(vdev); - if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) { - NL_SET_ERR_MSG_MOD(extack, "Features negotiation not completed"); - err = -EAGAIN; - goto out; - } - hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags, VDPA_CMD_DEV_CONFIG_GET); if (!hdr) {
Users may want to query the config space of a vDPA device, to choose a appropriate one for a certain guest. This means the users need to read the config space before FEATURES_OK, and the existence of config space contents does not depend on FEATURES_OK. This commit removes FEATURES_OK blocker in vdpa_dev_config_fill() which calls vdpa_dev_net_config_fill() for virtio-net Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/vdpa.c | 8 -------- 1 file changed, 8 deletions(-)