qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/1] virtio: fix feature negotiation for ACCESS_PLATFORM


From: Halil Pasic
Subject: Re: [PATCH 1/1] virtio: fix feature negotiation for ACCESS_PLATFORM
Date: Fri, 4 Mar 2022 12:08:09 +0100

On Fri, 4 Mar 2022 03:12:03 -0500
"Michael S. Tsirkin" <mst@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 <cohuck@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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]