> From: Jason Gunthorpe <jgg@xxxxxxxxxx> > 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
