Re: [PATCH V2 RESEND 2/6] vDPA: only report driver features if FEATURES_OK is set

"Zhu, Lingshan" <lingshan.zhu@xxxxxxxxx> · Tue, 27 Sep 2022 18:02:53 +0800

On 9/27/2022 12:37 PM, Jason Wang wrote:
On Tue, Sep 27, 2022 at 11:09 AM Zhu Lingshan <lingshan.zhu@xxxxxxxxx> wrote:
This commit reports driver features to user space
only after FEATURES_OK is features negotiation is done.

Signed-off-by: Zhu Lingshan <lingshan.zhu@xxxxxxxxx>
---
  drivers/vdpa/vdpa.c | 22 ++++++++++++++++------
  1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 2035700d6fc8..e7765953307f 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -816,7 +816,7 @@ static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
  static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *msg)
  {
         struct virtio_net_config config = {};
-       u64 features_device, features_driver;
+       u64 features_device;
         u16 val_u16;

         vdev->config->get_config(vdev, 0, &config, sizeof(config));
@@ -833,11 +833,6 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
         if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16))
                 return -EMSGSIZE;

-       features_driver = vdev->config->get_driver_features(vdev);
-       if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_NEGOTIATED_FEATURES, features_driver,
-                             VDPA_ATTR_PAD))
-               return -EMSGSIZE;
-
         features_device = vdev->config->get_device_features(vdev);

         if (nla_put_u64_64bit(msg, VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, features_device,
@@ -851,6 +846,8 @@ static int
  vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, u32 seq,
                      int flags, struct netlink_ext_ack *extack)
  {
+       u64 features_driver;
+       u8 status = 0;
         u32 device_id;
         void *hdr;
         int err;
@@ -874,6 +871,19 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid,
                 goto msg_err;
         }

+       /* only read driver features after the feature negotiation is done */
+       if (vdev->config->get_status)
+               status = vdev->config->get_status(vdev);
get_status is mandatory, so I think we can remove this check.

Or if you want a strict check on the config operations, we need to do
that in __vdpa_alloc_device().
I will remove it

Thanks!

Thanks

+
+       if (status & VIRTIO_CONFIG_S_FEATURES_OK) {
+               features_driver = vdev->config->get_driver_features(vdev);
+               if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_NEGOTIATED_FEATURES, features_driver,
+                                     VDPA_ATTR_PAD)) {
+                       err = -EMSGSIZE;
+                       goto msg_err;
+               }
+       }
+
         switch (device_id) {
         case VIRTIO_ID_NET:
                 err = vdpa_dev_net_config_fill(vdev, msg);
--
2.31.1








