RE: [PATCH 07/10] vfio: Move vfio_external_check_extension() to vfio_file_ops

Tian, Kevin
2022-04-20T03:05:57Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, April 20, 2022 3:24 AM
>
> On Fri, Apr 15, 2022 at 04:07:51AM +0000, Tian, Kevin wrote:
>
> > > + struct vfio_group *group = filep->private_data;
> > > + bool ret;
> > > +
> > > + /*
> > > + * Since the coherency state is determined only once a container is
> > > + * attached the user must do so before they can prove they have
> > > + * permission.
> > > + */
> > > + if (vfio_group_add_container_user(group))
> > > + return true;
> >
> > I wonder whether it's better to return error here and let KVM to
> > decide whether it wants to allow wbinvd in such case (though
> > likely the conclusion is same) or simply rejects adding the group.
>
> Since the new model is to present proof at add it is OK - it just
> means the user doesn't have a proof to enable wbinvd.

it is OK if this interface is called vfio_file_is_wbinvd_required().

But now it is called vfio_file_enforced_coherent() then whether
it is a proof for wbinvd is a caller-side knowledge. It is clearer
to stick to the fact about cache coherency and if it cannot be
determined at this point then simply return error to the caller
to handle the situation.

Also it sounds more reasonable for KVM to fail adding group
if a container hasn't been attached instead of moving forward
assuming an incorrect coherency and then getting an error
later.

Thanks
Kevin