Jason Wang
2022-Sep-21 07:43 UTC
[PATCH 2/4] vDPA: only report driver features if FEATURES_OK is set
On Wed, Sep 21, 2022 at 1:39 PM Zhu, Lingshan <lingshan.zhu at intel.com> wrote:> > > > On 9/21/2022 10:14 AM, Jason Wang wrote: > > On Tue, Sep 20, 2022 at 1:46 PM Zhu, Lingshan <lingshan.zhu at intel.com> wrote: > >> > >> > >> On 9/20/2022 10:16 AM, Jason Wang wrote: > >>> On Fri, Sep 9, 2022 at 5:05 PM Zhu Lingshan <lingshan.zhu at intel.com> wrote: > >>>> vdpa_dev_net_config_fill() should only report driver features > >>>> to userspace after features negotiation is done. > >>>> > >>>> Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com> > >>>> --- > >>>> 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. > if we check FEATURES_OK in the caller vdpa_dev_config_fill(), then > !FEATURES_OK will block reporting all attributions, for example > the device features and virtio device config space fields in this series > and device status. > Currently only driver features needs to check FEATURES_OK. > Or did I missed anything?I don't see much difference, we just move the following part to the caller, it is not the config but the VDPA_ATTR_DEV_NEGOTIATED_FEATURES is blocked. Thanks> > Thanks > > > > Thanks > > > >> Thanks, > >> Zhu Lingshan > >>> Thanks > >>> > >>>> + 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 > >>>> >