RE: [virtio-dev] Re: [PATCH v3] Add virtio-iommu device specification

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Jean-Philippe Brucker 
> Sent: Wednesday, May 15, 2019 12:08 AM
> 
> On 10/05/2019 17:52, Michael S. Tsirkin wrote:
> >>>> +/* Flags are: */
> >>>> +#define VIRTIO_IOMMU_MAP_F_READ   (1 << 0)
> >>>> +#define VIRTIO_IOMMU_MAP_F_WRITE  (1 << 1)
> >>>> +#define VIRTIO_IOMMU_MAP_F_EXEC   (1 << 2)
> >>>
> >>> what is exec? pls add some comments near all flags.
> >>
> >> Exec means instruction fetch. Some systems have different transaction
> >> flags for read and exec (PCI only supports that in the PASID TLP prefix)
> >> and some page tables have a "no exec" bit.
> >
> > So let's say I don't set EXEC on such a system. Is EXEC allowed or not?
> >
> > Maybe we can ask user to always set EXEC is exec is needed,
> > but then say we can not promise instruction fetch will fail
> > if exec is not set.
> 
> So the semantics could be:
> * device sets EXEC feature bit
>   - driver sets EXEC flag in MAP, instruction fetch succeeds
>   - driver doesn't set EXEC flag in MAP, instructions fetch will fail
> * device doesn't set EXEC feature bit, then driver cannot set EXEC flag
> but instruction fetch may not fail.
> 
> But I'm tempted to just remove the EXEC flag for now, I don't think
> anyone needs it at the moment (and VFIO doesn't support it). I'd add it
> back as NO_EXEC which is closer to what IOMMUs offer.

Agree. It's more for PASID-based usage e.g. sharing CPU page table
with device in SVA. Now this version is purely about MAP/UNMAP.
On Intel VT-d, EXEC is ignored for request-without-PASID.

> 
> 
> >>>> +\begin{description}
> >>>> +  \item[VIRTIO_IOMMU_RESV_MEM_T_RESERVED (0)]
> >>>> +    Accesses to virtual addresses in this region have undefined behavior.
> >>>
> >>> undefined?  how can platforms with such a property coexist with
> untrusted
> >>> devices?
> >>
> >> It is "undefined" because I can't enumerate all cases (don't know about
> >> all of them). This would contain for example the RMRR regions of VT-d
> >> (the BIOS reserving some DMA regions for itself), regions allocated for
> >> host-managed MSIs on arm platforms, and some PCI bridge windows
> >> (although they removed those from the Linux resv regions recently, not
> >> certain why). Ideally we wouldn't have any, but some special cases make
> >> it necessary.
> >>
> >> In general having reserved regions is incompatible with untrusted
> >> devices, and in some cases incompatible with untrusted guests. But
> >> choosing the policy about which devices to present to guest is up to the
> >> platform. The host knows what the resv regions are used for, and if they
> >> are safe enough. The guest just need to knows what to avoid.
> >
> > Yes but ... if you trust driver and device then what's the
> > point of the iommu?
> 
> Improving memory allocation. The guest can do scatter-gather for large
> buffers, instead of allocating big contiguous chunks of guest-physical
> addresses. And with device pass-through, any memory used for DMA has to
> be pinned (since devices generally don't support page faults). So when
> assigning an endpoint to a guest, without a vIOMMU all of the guest
> memory has to be pinned upfront. With an IOMMU only the memory that will
> actually be used for DMA is pinned.

I'd not vote using current vIOMMU just for avoiding pin instead of for
isolation purpose... the map/unmap overhead is high except you only 
talking about static mapping e.g. in guest user space driver (e.g. DPDK) 
(but then it's actually for isolation purpose between user and kernel).
 btw we are working on a lighter-weight approach (aim <5% perf drop
for most cases) while allowing host to fully introspect guest DMA activities. 
Likely we'll introduce a new virtio-iommu capability for this purpose, 
instead of using current MAP/UNMAP primitives. Stay tuned and we'll 
send out RFC soon once getting good result. :-)

And I don't think having reserved regions is incompatible with untrusted
devices. Favoring reserved region is a functional requirement so guest
driver can operate assigned device in a good shape. Violating reserved
region requirement (e.g. map guest memory page to reserved region) just
means device access to this page doesn't provide desired behavior, since
host will always guarantee safe behavior in physical IOMMU on those
regions:

- it is either mapped to fixed resource (e.g ARM MSI doorbell)
which is associated with this device and thus safe
- or map to nothing

I don't think architecturally we break any isolation implication on
IOMMU here.

Thanks
Kevin