[Top][All Lists]

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

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

From: Jason Wang
Subject: Re: [PATCH] vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support
Date: Mon, 8 Feb 2021 11:21:23 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

On 2021/2/5 下午11:31, Peter Xu wrote:
On Fri, Feb 05, 2021 at 09:33:29AM +0100, Auger Eric wrote:

On 2/5/21 4:16 AM, Jason Wang wrote:
On 2021/2/5 上午3:12, Peter Xu wrote:
Previous work on dev-iotlb message broke vhost on either SMMU

Have a quick git grep and it looks to me v3 support ATS and have command
for device iotlb (ATC) invalidation.

Yes I will do that. Should not be a big deal.
Great, thanks.

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.

I think vhost is a good example with device IOTLB? It solves the issue exactly the bottleneck of a centralized IOTLB when everything is cached in the vhost.

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?

I think the answer is yes it virtio-iommu is the only choice for some platform/archs.

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.

Looks like another topic (e.g if we need to expose ATS to guest for assigned device)?

is not yet supported for those archs.

Rethink about this, it looks to me the point is that we should make
vhost work when ATS is disabled like what ATS spec defined:


ATS is enabled through a new Capability and associated configuration
structure.  To enable 15 ATS, software must detect this Capability and
enable the Function to issue ATS TLP.  If a Function is not enabled, the
Function is required not to issue ATS Translation Requests and is
required to issue all DMA Read and Write Requests with the TLP AT field
set to “untranslated.”


Maybe we can add this in the commit log.
I saw Michael was super fast on handling this patch and already got it in a
pull, so I may not directly post a new version.  But I'll add it if I'll post a
new version.



Patch looks good. I wonder whether we should fix intel when ATS is
good point
I'm not sure I remember it right, but we seem to have similar discussion
previously on "what if the user didn't specify ats=on" - I think at that time
the conclusion was that we ignore the failure since that's not a valid
configuration for qemu.

Yes, but I think I was wrong at that time.

But I agree it would be nicer to be able to fallback.

So see my reply quoted from ATS spec. My understanding is that the device should behave correctly if ATS is disabled.

The other issue I'm worried is (I think I mentioned it somewhere, but just to
double confirm): I'd like to make sure SMMU and virtio-iommu are the only IOMMU
platform that will use vhost.

For upstream, it won't be easy :)

   Otherwise IIUC we need to fix those vIOMMUs too.

Right, last time I check AMD IOMMU emulation, it simply trigger device IOTLB invalidation during IOTLB invalidation which looks wrong.



reply via email to

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