On Tue, Sep 20, 2022 at 1:46 PM Zhu, Lingshan [off-list ref] wrote:

On 9/20/2022 10:16 AM, Jason Wang wrote:

quoted

On Fri, Sep 9, 2022 at 5:05 PM Zhu Lingshan [off-list ref] wrote:

quoted

vdpa_dev_net_config_fill() should only report driver features
to userspace after features negotiation is done.

Signed-off-by: Zhu Lingshan <redacted>
---
  drivers/vdpa/vdpa.c | 13 +++++++++----
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 798a02c7aa94..29d7e8858e6f 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c

@@ -819,6 +819,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
         struct virtio_net_config config = {};
         u64 features_device, features_driver;
         u16 val_u16;
+       u8 status;

         vdev->config->get_config(vdev, 0, &config, sizeof(config));

@@ -834,10 +835,14 @@ 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;
+       /* only read driver features after the feature negotiation is done */
+       status = vdev->config->get_status(vdev);
+       if (status & VIRTIO_CONFIG_S_FEATURES_OK) {

Any reason this is not checked in its caller as what it used to do before?

will check the existence of vdev->config->get_status before calling it in V2

Just to clarify, I meant to check FEAUTRES_OK in the caller -
vdpa_dev_config_fill() otherwise each type needs to repeat this in
their specific codes.

Thanks

Thanks,
Zhu Lingshan

quoted

Thanks

quoted

+               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);

--
2.31.1