On Tue, Dec 21, 2021 at 2:17 PM Eli Cohen <elic@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > > 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
> > >
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
