Jason Wang
2021-Dec-21 07:51 UTC
[PATCH v3 03/10] vdpa: Read device configuration only if FEATURES_OK
On Tue, Dec 21, 2021 at 2:17 PM Eli Cohen <elic at nvidia.com> wrote:> > On Tue, Dec 21, 2021 at 01:51:32PM +0800, Jason Wang wrote: > > On Sun, Dec 19, 2021 at 10:03 PM Eli Cohen <elic at nvidia.com> wrote: > > > > > > Avoid reading device configuration during feature negotiation. Read > > > device status and verify that VIRTIO_CONFIG_S_FEATURES_OK is set. > > > Otherwise, return -EAGAIN. > > > > > > Signed-off-by: Eli Cohen <elic at nvidia.com> > > > --- > > > drivers/vdpa/vdpa.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > > > index 42d71d60d5dc..5749cf0a1500 100644 > > > --- a/drivers/vdpa/vdpa.c > > > +++ b/drivers/vdpa/vdpa.c > > > @@ -819,8 +819,15 @@ vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, > > > { > > > u32 device_id; > > > void *hdr; > > > + u8 status; > > > int err; > > > > > > + status = vdev->config->get_status(vdev); > > > + if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) { > > > + NL_SET_ERR_MSG_MOD(extack, "Features negotiation not completed"); > > > + return -EAGAIN; > > > + } > > > + > > > > I wonder how we synchronize this with set_status(), set/get_config()? > > > > Not sure why set/get_config() is related to this.I think guest may trigger set_config() while we are at vdpa_dev_config_fill(), this will lead to inconsistent values.> How about each upstream driver will take care to have a lock that > serializes set_status() with get_driver_features() to guarantee that we > always provide valid features?Yes and it looks like we've already had a per device cf_mutex, how about simply use this? Thanks> > > Thanks > > > > > hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags, > > > VDPA_CMD_DEV_CONFIG_GET); > > > if (!hdr) > > > -- > > > 2.34.1 > > > > > >