| Message ID | 20220613101652.195216-5-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: Monday, June 13, 2022 6:17 AM > > 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. > > The spec says: > 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. > > 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 > 9b0e39b2f022..d76b22b2f7ae 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -851,17 +851,9 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, > struct sk_buff *msg, u32 portid, { > u32 device_id; > void *hdr; > - u8 status; > int err; > > down_read(&vdev->cf_lock); > - 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) { > -- > 2.31.1 Management interface should not mix up the device state machine checks like above. Hence above code removal is better choice. Please add fixes tag to this patch. Reviewed-by: Parav Pandit <parav@nvidia.com>
On 6/14/2022 4:31 AM, Parav Pandit wrote: > >> From: Zhu Lingshan <lingshan.zhu@intel.com> >> Sent: Monday, June 13, 2022 6:17 AM >> >> 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. >> >> The spec says: >> 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. >> >> 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 >> 9b0e39b2f022..d76b22b2f7ae 100644 >> --- a/drivers/vdpa/vdpa.c >> +++ b/drivers/vdpa/vdpa.c >> @@ -851,17 +851,9 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, >> struct sk_buff *msg, u32 portid, { >> u32 device_id; >> void *hdr; >> - u8 status; >> int err; >> >> down_read(&vdev->cf_lock); >> - 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) { >> -- >> 2.31.1 > Management interface should not mix up the device state machine checks like above. > Hence above code removal is better choice. > Please add fixes tag to this patch. will do, thanks for your review Thanks, Zhu Lingshan > > Reviewed-by: Parav Pandit <parav@nvidia.com>
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index 9b0e39b2f022..d76b22b2f7ae 100644 --- a/drivers/vdpa/vdpa.c +++ b/drivers/vdpa/vdpa.c @@ -851,17 +851,9 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, { u32 device_id; void *hdr; - u8 status; int err; down_read(&vdev->cf_lock); - 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. The spec says: 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. Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/vdpa.c | 8 -------- 1 file changed, 8 deletions(-)