Shouldn't VFIO virtualize the ATS capability?

Shouldn't VFIO virtualize the ATS capability?

[Ilya Lesokhin]

Hi 
I've noticed that VFIO doesn't virtualize the ATS capability.
It seems to me that translation caching and Smallest Translation Unit is something you would want to control on the host. Am I wrong?

Thanks,
Ilya

Re: Shouldn't VFIO virtualize the ATS capability?

[Alex Williamson]

On Sun, 6 Nov 2016 11:13:09 +0000
Ilya Lesokhin <ilyal@mellanox.com> wrote:

> Hi 
> I've noticed that VFIO doesn't virtualize the ATS capability.
> It seems to me that translation caching and Smallest Translation Unit is something you would want to control on the host. Am I wrong?

What about those fields would we virtualize?  Why does the host need to
be an intermediary?  Can the user induce poor behavior with direct
access to them?  Thanks,

Alex

RE: Shouldn't VFIO virtualize the ATS capability?

[Ilya Lesokhin]

I would virtualize the "ATS Control Register".

Regarding poor behavior, I couldn't really find what happens when ATS is misconfigured, but I would assume it can cause problems.
The scenarios I'm concerned about are:
	1. The guest enables translation caching, while the hypervisor thinks there are disabled -> Hypervisor won't issue invalidations.
	2. Smallest Translation Unit misconfiguration. Not sure if it will cause invalid access or only poor caching behavior.

Thanks,
Ilya

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Sunday, November 06, 2016 7:09 PM
> To: Ilya Lesokhin <ilyal@mellanox.com>
> Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com
> Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> 
> On Sun, 6 Nov 2016 11:13:09 +0000
> Ilya Lesokhin <ilyal@mellanox.com> wrote:
> 
> > Hi
> > I've noticed that VFIO doesn't virtualize the ATS capability.
> > It seems to me that translation caching and Smallest Translation Unit is
> something you would want to control on the host. Am I wrong?
> 
> What about those fields would we virtualize?  Why does the host need to be
> an intermediary?  Can the user induce poor behavior with direct access to
> them?  Thanks,
> 
> Alex

Re: Shouldn't VFIO virtualize the ATS capability?

[Alex Williamson]

On Wed, 9 Nov 2016 14:49:02 +0000
Ilya Lesokhin <ilyal@mellanox.com> wrote:

> I would virtualize the "ATS Control Register".

And do what?

> Regarding poor behavior, I couldn't really find what happens when ATS is misconfigured, but I would assume it can cause problems.
> The scenarios I'm concerned about are:
> 	1. The guest enables translation caching, while the hypervisor thinks there are disabled -> Hypervisor won't issue invalidations.

Aren't invalidations issued by the iommu, why does the hypervisor need
to participate?  How would a software entity induce an invalidation?

> 	2. Smallest Translation Unit misconfiguration. Not sure if it will cause invalid access or only poor caching behavior.
> 
> Thanks,
> Ilya
> 
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Sunday, November 06, 2016 7:09 PM
> > To: Ilya Lesokhin <ilyal@mellanox.com>
> > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com
> > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > 
> > On Sun, 6 Nov 2016 11:13:09 +0000
> > Ilya Lesokhin <ilyal@mellanox.com> wrote:
> >   
> > > Hi
> > > I've noticed that VFIO doesn't virtualize the ATS capability.
> > > It seems to me that translation caching and Smallest Translation Unit is  
> > something you would want to control on the host. Am I wrong?
> > 
> > What about those fields would we virtualize?  Why does the host need to be
> > an intermediary?  Can the user induce poor behavior with direct access to
> > them?  Thanks,
> > 
> > Alex  

RE: Shouldn't VFIO virtualize the ATS capability?

[Ilya Lesokhin]

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Wednesday, November 09, 2016 5:08 PM
> To: Ilya Lesokhin <ilyal@mellanox.com>
> Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com;
> Adi Menachem <adim@mellanox.com>
> Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> 
> On Wed, 9 Nov 2016 14:49:02 +0000
> Ilya Lesokhin <ilyal@mellanox.com> wrote:
> 
> > I would virtualize the "ATS Control Register".
> 
> And do what?
I think it should be read only.
> 
> > Regarding poor behavior, I couldn't really find what happens when ATS is
> misconfigured, but I would assume it can cause problems.
> > The scenarios I'm concerned about are:
> > 	1. The guest enables translation caching, while the hypervisor thinks
> there are disabled -> Hypervisor won't issue invalidations.
> 
> Aren't invalidations issued by the iommu, why does the hypervisor need to
> participate?  How would a software entity induce an invalidation?
That's what one might expect from a sane design, but
http://lxr.free-electrons.com/source/drivers/iommu/intel-iommu.c?v=4.8#L1549
seems to imply otherwise :(
> 
> > 	2. Smallest Translation Unit misconfiguration. Not sure if it will cause
> invalid access or only poor caching behavior.
> >
> > Thanks,
> > Ilya
> >
> > > -----Original Message-----
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Sunday, November 06, 2016 7:09 PM
> > > To: Ilya Lesokhin <ilyal@mellanox.com>
> > > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org;
> > > bhelgaas@google.com
> > > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > >
> > > On Sun, 6 Nov 2016 11:13:09 +0000
> > > Ilya Lesokhin <ilyal@mellanox.com> wrote:
> > >
> > > > Hi
> > > > I've noticed that VFIO doesn't virtualize the ATS capability.
> > > > It seems to me that translation caching and Smallest Translation
> > > > Unit is
> > > something you would want to control on the host. Am I wrong?
> > >
> > > What about those fields would we virtualize?  Why does the host need
> > > to be an intermediary?  Can the user induce poor behavior with
> > > direct access to them?  Thanks,
> > >
> > > Alex

Re: Shouldn't VFIO virtualize the ATS capability?

[Alex Williamson]

On Wed, 9 Nov 2016 15:25:16 +0000
Ilya Lesokhin <ilyal@mellanox.com> wrote:

> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Wednesday, November 09, 2016 5:08 PM
> > To: Ilya Lesokhin <ilyal@mellanox.com>
> > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com;
> > Adi Menachem <adim@mellanox.com>
> > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > 
> > On Wed, 9 Nov 2016 14:49:02 +0000
> > Ilya Lesokhin <ilyal@mellanox.com> wrote:
> >   
> > > I would virtualize the "ATS Control Register".  
> > 
> > And do what?  
> I think it should be read only.

That would violate the spec, in which case it shouldn't be virtualized,
the capability should be hidden.
 
> > > Regarding poor behavior, I couldn't really find what happens when ATS is  
> > misconfigured, but I would assume it can cause problems.  
> > > The scenarios I'm concerned about are:
> > > 	1. The guest enables translation caching, while the hypervisor thinks  
> > there are disabled -> Hypervisor won't issue invalidations.
> > 
> > Aren't invalidations issued by the iommu, why does the hypervisor need to
> > participate?  How would a software entity induce an invalidation?  
> That's what one might expect from a sane design, but
> http://lxr.free-electrons.com/source/drivers/iommu/intel-iommu.c?v=4.8#L1549
> seems to imply otherwise :(
> >   
> > > 	2. Smallest Translation Unit misconfiguration. Not sure if it will cause  
> > invalid access or only poor caching behavior.  
> > >
> > > Thanks,
> > > Ilya
> > >  
> > > > -----Original Message-----
> > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > Sent: Sunday, November 06, 2016 7:09 PM
> > > > To: Ilya Lesokhin <ilyal@mellanox.com>
> > > > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org;
> > > > bhelgaas@google.com
> > > > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > > >
> > > > On Sun, 6 Nov 2016 11:13:09 +0000
> > > > Ilya Lesokhin <ilyal@mellanox.com> wrote:
> > > >  
> > > > > Hi
> > > > > I've noticed that VFIO doesn't virtualize the ATS capability.
> > > > > It seems to me that translation caching and Smallest Translation
> > > > > Unit is  
> > > > something you would want to control on the host. Am I wrong?
> > > >
> > > > What about those fields would we virtualize?  Why does the host need
> > > > to be an intermediary?  Can the user induce poor behavior with
> > > > direct access to them?  Thanks,
> > > >
> > > > Alex  
> 

RE: Shouldn't VFIO virtualize the ATS capability?

[Ilya Lesokhin]

I guess hiding it is even better.

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Wednesday, November 09, 2016 5:53 PM
> To: Ilya Lesokhin <ilyal@mellanox.com>
> Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com;
> Adi Menachem <adim@mellanox.com>
> Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> 
> On Wed, 9 Nov 2016 15:25:16 +0000
> Ilya Lesokhin <ilyal@mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Wednesday, November 09, 2016 5:08 PM
> > > To: Ilya Lesokhin <ilyal@mellanox.com>
> > > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org;
> > > bhelgaas@google.com; Adi Menachem <adim@mellanox.com>
> > > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > >
> > > On Wed, 9 Nov 2016 14:49:02 +0000
> > > Ilya Lesokhin <ilyal@mellanox.com> wrote:
> > >
> > > > I would virtualize the "ATS Control Register".
> > >
> > > And do what?
> > I think it should be read only.
> 
> That would violate the spec, in which case it shouldn't be virtualized, the
> capability should be hidden.
> 
> > > > Regarding poor behavior, I couldn't really find what happens when
> > > > ATS is
> > > misconfigured, but I would assume it can cause problems.
> > > > The scenarios I'm concerned about are:
> > > > 	1. The guest enables translation caching, while the hypervisor
> > > > thinks
> > > there are disabled -> Hypervisor won't issue invalidations.
> > >
> > > Aren't invalidations issued by the iommu, why does the hypervisor
> > > need to participate?  How would a software entity induce an
> invalidation?
> > That's what one might expect from a sane design, but
> > http://lxr.free-electrons.com/source/drivers/iommu/intel-iommu.c?v=4.8
> > #L1549
> > seems to imply otherwise :(
> > >
> > > > 	2. Smallest Translation Unit misconfiguration. Not sure if it
> > > > will cause
> > > invalid access or only poor caching behavior.
> > > >
> > > > Thanks,
> > > > Ilya
> > > >
> > > > > -----Original Message-----
> > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > Sent: Sunday, November 06, 2016 7:09 PM
> > > > > To: Ilya Lesokhin <ilyal@mellanox.com>
> > > > > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org;
> > > > > bhelgaas@google.com
> > > > > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > > > >
> > > > > On Sun, 6 Nov 2016 11:13:09 +0000 Ilya Lesokhin
> > > > > <ilyal@mellanox.com> wrote:
> > > > >
> > > > > > Hi
> > > > > > I've noticed that VFIO doesn't virtualize the ATS capability.
> > > > > > It seems to me that translation caching and Smallest
> > > > > > Translation Unit is
> > > > > something you would want to control on the host. Am I wrong?
> > > > >
> > > > > What about those fields would we virtualize?  Why does the host
> > > > > need to be an intermediary?  Can the user induce poor behavior
> > > > > with direct access to them?  Thanks,
> > > > >
> > > > > Alex
> >

Re: Shouldn't VFIO virtualize the ATS capability?

[Alex Williamson]

Revisiting this...

On Wed, 9 Nov 2016 15:55:55 +0000
Ilya Lesokhin <ilyal@mellanox.com> wrote:

> I guess hiding it is even better.

AIUI, a device supporting ATS can help offload some of the iotlb
pressure from the IOMMU, so a high performance device implementing a
device iotlb may experience less of an impact if it can perform much
of the iotlb work on its own.  But I suppose the question is, does the
guest driver or even the guest OS need to be aware of ATS to get that
benefit?  More below...

> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Wednesday, November 09, 2016 5:53 PM
> > To: Ilya Lesokhin <ilyal@mellanox.com>
> > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com;
> > Adi Menachem <adim@mellanox.com>
> > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > 
> > On Wed, 9 Nov 2016 15:25:16 +0000
> > Ilya Lesokhin <ilyal@mellanox.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > Sent: Wednesday, November 09, 2016 5:08 PM
> > > > To: Ilya Lesokhin <ilyal@mellanox.com>
> > > > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org;
> > > > bhelgaas@google.com; Adi Menachem <adim@mellanox.com>
> > > > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > > >
> > > > On Wed, 9 Nov 2016 14:49:02 +0000
> > > > Ilya Lesokhin <ilyal@mellanox.com> wrote:
> > > >  
> > > > > I would virtualize the "ATS Control Register".  
> > > >
> > > > And do what?  
> > > I think it should be read only.  
> > 
> > That would violate the spec, in which case it shouldn't be virtualized, the
> > capability should be hidden.
> >   
> > > > > Regarding poor behavior, I couldn't really find what happens when
> > > > > ATS is  
> > > > misconfigured, but I would assume it can cause problems.  
> > > > > The scenarios I'm concerned about are:
> > > > > 	1. The guest enables translation caching, while the hypervisor
> > > > > thinks  
> > > > there are disabled -> Hypervisor won't issue invalidations.
> > > >
> > > > Aren't invalidations issued by the iommu, why does the hypervisor
> > > > need to participate?  How would a software entity induce an  
> > invalidation?  
> > > That's what one might expect from a sane design, but
> > > http://lxr.free-electrons.com/source/drivers/iommu/intel-iommu.c?v=4.8
> > > #L1549
> > > seems to imply otherwise :(  

This seems correct though, the device iotlb would interact with the
physical IOMMU, so this is happening on the host.  The call path would
be:

ioctl(container, VFIO_IOMMU_UNMAP_DMA, ...)
  vfio_fops_unl_ioctl
     vfio_iommu_type1_ioctl
       vfio_dma_do_unmap
         vfio_remove_dma
           vfio_unmap_unpin
             iommu_unmap
               intel_iommu_unmap
                 iommu_flush_iotlb_psi
                   iommu_flush_dev_iotlb

For a non-iommu VM, mappings will be mostly static, so this will be
rare.  If we had VT-d emulation support in the VM, the iommu domain
used by the VM would map to an iommu domain in the host and any
invalidations within that domain would trigger an unmap through to the
host domain.
 
> > > > > 	2. Smallest Translation Unit misconfiguration. Not sure if it
> > > > > will cause  
> > > > invalid access or only poor caching behavior.  

I'm not sure about this either.  I think that ATS is enabled on the
device prior to the guest having access to it, but could the guest
interfere or cause poor behavior by further interaction with the ATS
capability.  I guess my question would be whether the guest needs
visibility or access to the ATS capability to still make use of it.  We
certainly want to take advantage of an iotlb where available.  For a
Linux guest we only seem to manipulate ATS enable through the iommu
code, so I expect a non-iommu VM to leave ATS alone.  What's the best
solution then, to hide the ATS capability, assuming that it works
transparently on the host level?  Expose it to the guest, perhaps
virtualizing the STU field to the VM, giving the VM enable/disable
control?  How can we test any of this?  Thanks,

Alex

RE: Shouldn't VFIO virtualize the ATS capability?

[Ilya Lesokhin]

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Friday, December 2, 2016 1:23 AM
> To: Ilya Lesokhin <ilyal@mellanox.com>
> Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com;
> Adi Menachem <adim@mellanox.com>
> Subject: Re: Shouldn't VFIO virtualize the ATS capability?
... 
> > > > > Aren't invalidations issued by the iommu, why does the
> > > > > hypervisor need to participate?  How would a software entity
> > > > > induce an
> > > invalidation?
> > > > That's what one might expect from a sane design, but
> > > > http://lxr.free-electrons.com/source/drivers/iommu/intel-iommu.c?v
> > > > =4.8
> > > > #L1549
> > > > seems to imply otherwise :(
> 
> This seems correct though, the device iotlb would interact with the physical
> IOMMU, so this is happening on the host.  The call path would
> be:
> 
> ioctl(container, VFIO_IOMMU_UNMAP_DMA, ...)
>   vfio_fops_unl_ioctl
>      vfio_iommu_type1_ioctl
>        vfio_dma_do_unmap
>          vfio_remove_dma
>            vfio_unmap_unpin
>              iommu_unmap
>                intel_iommu_unmap
>                  iommu_flush_iotlb_psi
>                    iommu_flush_dev_iotlb
> 
> For a non-iommu VM, mappings will be mostly static, so this will be rare.  If
> we had VT-d emulation support in the VM, the iommu domain used by the
> VM would map to an iommu domain in the host and any invalidations within
> that domain would trigger an unmap through to the host domain.

My concern was for the case where the host is not aware of ATS or decides not to use it for some reason.
In that case the guest might enable ATS and abuse the fact that the host doesn't know it needs to 
issue invalidations to the device

> 
> > > > > > 	2. Smallest Translation Unit misconfiguration. Not sure if it
> > > > > > will cause
> > > > > invalid access or only poor caching behavior.
> 
> I'm not sure about this either.  I think that ATS is enabled on the
> device prior to the guest having access to it, but could the guest
> interfere or cause poor behavior by further interaction with the ATS
> capability.  I guess my question would be whether the guest needs
> visibility or access to the ATS capability to still make use of it.  We
> certainly want to take advantage of an iotlb where available.  For a
> Linux guest we only seem to manipulate ATS enable through the iommu
> code, so I expect a non-iommu VM to leave ATS alone.  What's the best
> solution then, to hide the ATS capability, assuming that it works
> transparently on the host level?  Expose it to the guest, perhaps
> virtualizing the STU field to the VM, giving the VM enable/disable
> control?  How can we test any of this?  Thanks,
> 
I don't see the benefit of exposing the capability to the guest.
If the Host enables ATS, the guest doesn't need to take any further action to benefit from the improved caching.
If the host doesn't enable it, he won't issue invalidation either, so allowing the guest to enable it is unsafe.

Thanks,
Ilya

Re: Shouldn't VFIO virtualize the ATS capability?

[Alex Williamson]

On Fri, 2 Dec 2016 07:45:16 +0000
Ilya Lesokhin <ilyal@mellanox.com> wrote:

> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Friday, December 2, 2016 1:23 AM
> > To: Ilya Lesokhin <ilyal@mellanox.com>
> > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com;
> > Adi Menachem <adim@mellanox.com>
> > Subject: Re: Shouldn't VFIO virtualize the ATS capability?  
> ... 
> > > > > > Aren't invalidations issued by the iommu, why does the
> > > > > > hypervisor need to participate?  How would a software entity
> > > > > > induce an  
> > > > invalidation?  
> > > > > That's what one might expect from a sane design, but
> > > > > http://lxr.free-electrons.com/source/drivers/iommu/intel-iommu.c?v
> > > > > =4.8
> > > > > #L1549
> > > > > seems to imply otherwise :(  
> > 
> > This seems correct though, the device iotlb would interact with the physical
> > IOMMU, so this is happening on the host.  The call path would
> > be:
> > 
> > ioctl(container, VFIO_IOMMU_UNMAP_DMA, ...)
> >   vfio_fops_unl_ioctl
> >      vfio_iommu_type1_ioctl
> >        vfio_dma_do_unmap
> >          vfio_remove_dma
> >            vfio_unmap_unpin
> >              iommu_unmap
> >                intel_iommu_unmap
> >                  iommu_flush_iotlb_psi
> >                    iommu_flush_dev_iotlb
> > 
> > For a non-iommu VM, mappings will be mostly static, so this will be rare.  If
> > we had VT-d emulation support in the VM, the iommu domain used by the
> > VM would map to an iommu domain in the host and any invalidations within
> > that domain would trigger an unmap through to the host domain.  
> 
> My concern was for the case where the host is not aware of ATS or decides not to use it for some reason.
> In that case the guest might enable ATS and abuse the fact that the host doesn't know it needs to 
> issue invalidations to the device

Is there any valid reason that a driver would enable ATS without a
visible IOMMU?  I think we want to be careful that we're not policing
guest drivers simply because they might do something incorrectly,
especially if the incorrect behavior only affects the device.  We
really only want to hide ATS at the host level if it cannot be used
correctly or if using it incorrectly can affect devices or system
behavior outside of the realm of that user instance.  For instance, it
seems valid that a user could enable or disable ATS, but perhaps
manipulating the page size should be virtualized (we need to be careful
about not violating the spec though).  We also have the option to hide
capabilities at the QEMU level, which is a bit softer and suggests that
there are valid uses of the capability, but they may not be compatible
or necessary with the current VM instance.

Is it true that a guest driver has no business enabling ATS without an
IOMMU visible in the VM?  Preventing that case seems like the type of
scenario that vfio should not be policing, the driver is doing
something arguably wrong.  When an IOMMU is exposed to the VM, then
perhaps we have the case where the guest enabling ATS if the host has
not is a scenario where the guest cannot behave correctly.  Perhaps
we can therefore derive that vfio in the host should only expose the ATS
capability iff ATS is enabled on the host, in which case STU should
likely be virtualized.  QEMU vfio would then still have the option
whether to expose ATS to the VM, but I think the worst that could
happen would be that the guest can gratuitously disable ATS.

   
> > > > > > > 	2. Smallest Translation Unit misconfiguration. Not sure if it
> > > > > > > will cause  
> > > > > > invalid access or only poor caching behavior.  
> > 
> > I'm not sure about this either.  I think that ATS is enabled on the
> > device prior to the guest having access to it, but could the guest
> > interfere or cause poor behavior by further interaction with the ATS
> > capability.  I guess my question would be whether the guest needs
> > visibility or access to the ATS capability to still make use of it.  We
> > certainly want to take advantage of an iotlb where available.  For a
> > Linux guest we only seem to manipulate ATS enable through the iommu
> > code, so I expect a non-iommu VM to leave ATS alone.  What's the best
> > solution then, to hide the ATS capability, assuming that it works
> > transparently on the host level?  Expose it to the guest, perhaps
> > virtualizing the STU field to the VM, giving the VM enable/disable
> > control?  How can we test any of this?  Thanks,
> >   
> I don't see the benefit of exposing the capability to the guest.
> If the Host enables ATS, the guest doesn't need to take any further action to benefit from the improved caching.
> If the host doesn't enable it, he won't issue invalidation either, so allowing the guest to enable it is unsafe.

So perhaps the right answer is that host vfio should hide the ATS
capability unless ATS is enabled by the host and virtualize the STU to
prevent the user programming conflicting values.  The assumption being
that it's never valid for the user to enable ATS if the host has not,
but it is valid for the user to be able to disable/re-enable (perhaps
for performance testing or debug).  With such a change, is there any
additional value in QEMU further hiding ATS from the guest?  We could
prevent gratuitous disables, but we don't know that there's any need
to do so.  Thanks,

Alex 

RE: Shouldn't VFIO virtualize the ATS capability?

[Ilya Lesokhin]

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Friday, December 02, 2016 5:58 PM
> To: Ilya Lesokhin <ilyal@mellanox.com>
> Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org; bhelgaas@google.com;
> Adi Menachem <adim@mellanox.com>; David Gibson
> <david@gibson.dropbear.id.au>; Eric Auger <eric.auger@linaro.org>
> Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> 
> On Fri, 2 Dec 2016 07:45:16 +0000
> Ilya Lesokhin <ilyal@mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Friday, December 2, 2016 1:23 AM
> > > To: Ilya Lesokhin <ilyal@mellanox.com>
> > > Cc: linux-pci@vger.kernel.org; kvm@vger.kernel.org;
> > > bhelgaas@google.com; Adi Menachem <adim@mellanox.com>
> > > Subject: Re: Shouldn't VFIO virtualize the ATS capability?
> > ...
> > > > > > > Aren't invalidations issued by the iommu, why does the
> > > > > > > hypervisor need to participate?  How would a software entity
> > > > > > > induce an
> > > > > invalidation?
> > > > > > That's what one might expect from a sane design, but
> > > > > > http://lxr.free-electrons.com/source/drivers/iommu/intel-iommu
> > > > > > .c?v
> > > > > > =4.8
> > > > > > #L1549
> > > > > > seems to imply otherwise :(
> > >
> > > This seems correct though, the device iotlb would interact with the
> > > physical IOMMU, so this is happening on the host.  The call path
> > > would
> > > be:
> > >
> > > ioctl(container, VFIO_IOMMU_UNMAP_DMA, ...)
> > >   vfio_fops_unl_ioctl
> > >      vfio_iommu_type1_ioctl
> > >        vfio_dma_do_unmap
> > >          vfio_remove_dma
> > >            vfio_unmap_unpin
> > >              iommu_unmap
> > >                intel_iommu_unmap
> > >                  iommu_flush_iotlb_psi
> > >                    iommu_flush_dev_iotlb
> > >
> > > For a non-iommu VM, mappings will be mostly static, so this will be
> > > rare.  If we had VT-d emulation support in the VM, the iommu domain
> > > used by the VM would map to an iommu domain in the host and any
> > > invalidations within that domain would trigger an unmap through to the
> host domain.
> >
> > My concern was for the case where the host is not aware of ATS or decides
> not to use it for some reason.
> > In that case the guest might enable ATS and abuse the fact that the
> > host doesn't know it needs to issue invalidations to the device
> 
> Is there any valid reason that a driver would enable ATS without a visible
> IOMMU?  I think we want to be careful that we're not policing guest drivers
> simply because they might do something incorrectly, especially if the
> incorrect behavior only affects the device.  We really only want to hide ATS
> at the host level if it cannot be used correctly or if using it incorrectly can
> affect devices or system behavior outside of the realm of that user instance.

I guess as long as the guest memory is pinned everything is fine. My concern was 
about the case where hypervisor removes a page from the guest memory and assumes
It's no longer accessible by the guest. While in fact it is accessible by the guest due to ATS
Caching.
Since today device assignment implies pinning, at least on x86 I guess it's not really an issue.


> For instance, it seems valid that a user could enable or disable ATS, but
> perhaps manipulating the page size should be virtualized (we need to be
> careful about not violating the spec though).  We also have the option to
> hide capabilities at the QEMU level, which is a bit softer and suggests that
> there are valid uses of the capability, but they may not be compatible or
> necessary with the current VM instance.
> 
I have no objection to hiding it in QEMU, in fact I haven't checked whether that's the case or not.
I was just surprised that VFIO exposes ATS as is and I was hoping to get an explanation from
Someone who understand it better than me.

> Is it true that a guest driver has no business enabling ATS without an IOMMU
> visible in the VM? 
I've actually take back what I said earlier. It can be useful, even without guest IOMMU.
If the host doesn't enable ATS for the guest,
The guest can do it on its own to improve the performance, and it is actually safe as long
As the host never changes the mapping.
However it seems a bit strange, I think a solution where the Host enable ATS on the behalf of the
Guest is cleaner.

> Preventing that case seems like the type of scenario that
> vfio should not be policing, the driver is doing something arguably wrong.
> When an IOMMU is exposed to the VM, then perhaps we have the case
> where the guest enabling ATS if the host has not is a scenario where the
> guest cannot behave correctly.  Perhaps we can therefore derive that vfio in
> the host should only expose the ATS capability iff ATS is enabled on the host,
> in which case STU should likely be virtualized.  QEMU vfio would then still
> have the option whether to expose ATS to the VM, but I think the worst that
> could happen would be that the guest can gratuitously disable ATS.
> 
Regarding STU,
The SR-IOV spec section 3.7.4 says that the Smallest Translation Unit and
the Invalidate Queue Depth fields in the Virtual Function's ATS capability
are hard-wired to 0.
I think this behavior makes sense for any assigned device.


> 
> > > > > > > > 	2. Smallest Translation Unit misconfiguration. Not sure
> > > > > > > > if it will cause
> > > > > > > invalid access or only poor caching behavior.
> > >
> > > I'm not sure about this either.  I think that ATS is enabled on the
> > > device prior to the guest having access to it, but could the guest
> > > interfere or cause poor behavior by further interaction with the ATS
> > > capability.  I guess my question would be whether the guest needs
> > > visibility or access to the ATS capability to still make use of it.
> > > We certainly want to take advantage of an iotlb where available.
> > > For a Linux guest we only seem to manipulate ATS enable through the
> > > iommu code, so I expect a non-iommu VM to leave ATS alone.  What's
> > > the best solution then, to hide the ATS capability, assuming that it
> > > works transparently on the host level?  Expose it to the guest,
> > > perhaps virtualizing the STU field to the VM, giving the VM
> > > enable/disable control?  How can we test any of this?  Thanks,
> > >
> > I don't see the benefit of exposing the capability to the guest.
> > If the Host enables ATS, the guest doesn't need to take any further action
> to benefit from the improved caching.
> > If the host doesn't enable it, he won't issue invalidation either, so allowing
> the guest to enable it is unsafe.
> 
> So perhaps the right answer is that host vfio should hide the ATS capability
> unless ATS is enabled by the host and virtualize the STU to prevent the user
> programming conflicting values.  The assumption being that it's never valid
> for the user to enable ATS if the host has not, but it is valid for the user to be
> able to disable/re-enable (perhaps for performance testing or debug).  With
> such a change, is there any additional value in QEMU further hiding ATS
> from the guest?  We could prevent gratuitous disables, but we don't know
> that there's any need to do so.  Thanks,
> 
> Alex

Thanks,
Ilya