[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] vhost: Warn if DEVIOTLB_UNMAP is not supported and ats is se
From: |
Peter Xu |
Subject: |
Re: [PATCH] vhost: Warn if DEVIOTLB_UNMAP is not supported and ats is set |
Date: |
Tue, 18 Oct 2022 10:25:09 -0400 |
Hi, Eric,
On Tue, Oct 18, 2022 at 02:28:52PM +0200, Eric Auger wrote:
> Since b68ba1ca5767 ("memory: Add IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> IOMMUTLBNotificationType"), vhost attempts to register DEVIOTLB_UNMAP
> notifier. This latter is supported by the intel-iommu which supports
> device-iotlb if the corresponding option is set. Then 958ec334bca3
> ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support") allowed
> silent fallback to the legacy UNMAP notifier if the viommu does not
> support device iotlb.
>
> Initially vhost/viommu integration was introduced with intel iommu
> assuming ats=on was set on virtio-pci device and device-iotlb was set
> on the intel iommu. vhost acts as an ATS capable device since it
> implements an IOTLB on kernel side. However translated transactions
> that hit the device IOTLB do not transit through the vIOMMU. So this
> requires a limited ATS support on viommu side.
>
> However, in theory, if ats=on is set on a pci device, the
> viommu should support ATS for that device to work.
Pure question: what will happen if one ATS supported PCI device got plugged
into a system whose physical IOMMU does not support ATS? Will ATS just be
ignored and the device keep working simply without ATS?
[1]
[...]
> @@ -760,8 +771,16 @@ static void vhost_iommu_region_add(MemoryListener
> *listener,
> iommu->iommu_offset = section->offset_within_address_space -
> section->offset_within_region;
> iommu->hdev = dev;
> - ret = memory_region_register_iommu_notifier(section->mr, &iommu->n,
> NULL);
> + ret = memory_region_register_iommu_notifier(section->mr, &iommu->n,
> &err);
> if (ret) {
> + if (vhost_dev_ats_enabled(dev)) {
> + error_reportf_err(err,
> + "vhost cannot register DEVIOTLB_UNMAP "
> + "although ATS is enabled, "
> + "fall back to legacy UNMAP notifier: ");
We want to use the warning message to either remind the user to (1) add the
dev-iotlb=on parameter for vIOMMU, or (2) drop the ats=on on device. Am I
right?
As we've discussed - I remember Jason used to test with/without dev-iotlb
on vhost on Intel and dev-iotlb is faster on vt-d guest driver than without
it. So that can make sense to me for (1). I don't know whether it helps
for (2) because fundamentally it's the same question as [1] above, and
whether that's a legal configuration.
Thanks,
--
Peter Xu