On Tue, Dec 14, 2021 at 3:14 PM Eli Cohen <elic@xxxxxxxxxx> wrote:
>
> On Tue, Dec 14, 2021 at 11:44:06AM +0800, Jason Wang wrote:
> > On Mon, Dec 13, 2021 at 10:43 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;
> > > + }
> > > +
> > > hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags,
> > > VDPA_CMD_DEV_CONFIG_GET);
> > > if (!hdr)
> >
> > It looks to me we need to synchronize this with set_status() and
> > cf_mutex (set_config).
>
> Makes sense.
>
> >
> > Or simply return all the config space (the maximum set of features)
>
> No sure I follow you on this. Are you suggesting to return in the
> netlink message all the fields of struct virtio_net_config as individual fields?
>
> How is this related to the need to sync with cf_mutex?
I meant the reason for the synchronization is because some config
fields depend on the feature negotiation. I wonder if it would be
easier we just return all the possible config fields, then there's
probably no need for the synchronization since we don't need to check
the driver feature but the device features.
Thanks
>
> >
> > Thanks
> >
> > > --
> > > 2.33.1
> > >
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
