On Fri, 4 Mar 2022 03:12:03 -0500 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Thu, Feb 10, 2022 at 02:29:29PM +0100, Halil Pasic wrote: > > On Thu, 10 Feb 2022 12:19:25 +0100 > > Cornelia Huck <coh...@redhat.com> wrote: > > > > [..] > > > > > > Nope, that's not my problem. We make sure that the bit is persistent, we > > > fail realization if the bit got removed by the callback when required, > > > and we fail feature validation if the driver removes the bit, which is > > > in a different code path. We should not talk about FEATURES_OK in this > > > code. > > > > I agree. I changed my mind. Expanation follows... > > > > > > > > We force-add the bit, and then still might fail realization. The > > > important condition is the has_iommu one, not the checks later on. I > > > find it very confusing to talk about what a potential driver might do in > > > that context. > > > > > > > I assumed stuff like virtiofs+SE regressed with commit 04ceb61a40 > > ("virtio: Fail if iommu_platform is requested, but unsupported") but I > > think, I was wrong. It didn't work before that, because we did not > > present ACCESS_PLATFORM to the guest. I operated under the assumption > > that virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM) > > does not impact the set of features offered to the driver by the device, > > but it does. > > > > So we need both this patch and "[PATCH v5 1/1] virtio: fix the condition > > for iommu_platform not supported" to get virtiofs to work with > > SE/SEV/Secure VM. > > > > I have to admit I only tested for the error message, and not with a full > > SE setup. > > > > I agree my comment was inadequate. Can we use > > > > /* make sure that the device did not drop a required IOMMU_PLATFORM */ > > > > I will think some more though. This is again about the dual nature > > of ACCESS_PLATFORM... > > Were you going to post a new version of this patch? > Sorry I got sidetracked. I will spin a new version today! Regards, Halil