RE: [PATCH] vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support

[Tian, Kevin]

> From: Peter Xu <peterx@redhat.com>
> Sent: Sunday, February 7, 2021 10:47 PM
> 
> Hi, Kevin,
> 
> On Sun, Feb 07, 2021 at 09:04:55AM +0000, Tian, Kevin wrote:
> > > From: Peter Xu
> > > Sent: Friday, February 5, 2021 11:31 PM
> > >
> > > > >
> > > > >
> > > > >> or virtio-iommu
> > > > >> since dev-iotlb (or PCIe ATS)
> > > > >
> > > > >
> > > > > We may need to add this in the future.
> > > > added Jean-Philippe in CC
> > >
> > > So that's the part I'm unsure about..  Since everybody is cced so maybe
> good
> > > time to ask. :)
> > >
> > > The thing is I'm still not clear on whether dev-iotlb is useful for a full
> > > emulation environment and how that should differ from a normal iotlb,
> since
> > > after all normal iotlb will be attached with device information too.
> >
> > dev-iotlb is useful in two manners. First, it's a functional prerequisite 
> > for
> > supporting I/O page faults.
> 
> Is this also a hard requirement for virtio-iommu, which is not a real hardware
> after all?

Not exactly but why do we want to use a non-standard way in the virtual
platform when PCI ATS is already in place?

> 
> > Second, it has performance benefit as you don't
> > need to contend the lock of global iotlb.
> 
> Hmm.. are you talking about e.g. vt-d driver or virtio-iommu?

It is a general iommu concept.

> 
> Assuming it's about vt-d, qi_flush_dev_iotlb() will still call 
> qi_submit_sync()
> and taking the same global QI lock, as I see it, or I could be wrong
> somewhere.
> I don't see where dev-iotlb has a standalone channel for delivery.

What I referred to is about lookup, instead of invalidation. 

> 
> For virtio-iommu, we haven't defined dev-iotlb, right?  Sorry I missed things
> when I completely didn't follow virtio-iommu recently - let's say if
> virtio-iommu in the future can support per-dev dev-iotlb queue so it doesn't
> need a global lock, what if we make it still per-device but still delivering
> iotlb message?  Again, it's still a bit unclear to me why a full emulation
> iommu would need that definition of "iotlb" and "dev-iotlb".

well, my view of definition of "iotlb" vs. "dev-iotlb" is from guest software
p.o.v. From this angle they have distinct meaning and semantics which
must be properly emulated according to the spec, regardless of whether 
they are maintained in separate or same data structure in the virtual platform.

> 
> >
> > >
> > > For real hardwares, they make sense because they ask for two things:
> iotlb is
> > > for IOMMU, but dev-iotlb is for the device cache.  For emulation
> > > environment
> > > (virtio-iommu is the case) do we really need that complexity?
> > >
> > > Note that even if there're assigned devices under virtio-iommu in the
> future,
> > > we can still isolate that and iiuc we can easily convert an iotlb (from
> > > virtio-iommu) into a hardware IOMMU dev-iotlb no matter what type of
> > > IOMMU is
> > > underneath the vIOMMU.
> > >
> >
> > Didn't get this point. Hardware dev-iotlb is updated by hardware (between
> > the device and the IOMMU). How could software convert a virtual iotlb
> > entry into hardware dev-iotlb?
> 
> I mean if virtio-iommu must be run in a guest, then we can trap that message
> first, right?  If there're assigned device in the guest, we must convert that
> invalidation to whatever message required for the host, that seems to not
> require the virtio-iommu to have dev-iotlb knowledge, still?
> 

It really depends on the definition of dev-iotlb in this context. To me the
fact that virtio-iommu needs to notify the kernel for updating split cache
is already sort of dev-iotlb semantics, regardless of whether it's delivered 
through a iotlb message or dev-iotlb message in a specific implementation. 😊

Thanks
Kevin