On 2020/7/1 下午4:09, Jason Wang wrote:
On 2020/6/30 下午11:39, Peter Xu wrote:
On Tue, Jun 30, 2020 at 10:41:10AM +0800, Jason Wang wrote:
Yes. I think invalidating the whole range is always fine. It's
/* According to ATS spec table 2.4:
* S = 0, bits 15:12 = xxxx range size: 4K
* S = 1, bits 15:12 = xxx0 range size: 8K
* S = 1, bits 15:12 = xx01 range size: 16K
* S = 1, bits 15:12 = x011 range size: 32K
* S = 1, bits 15:12 = 0111 range size: 64K
Right, but the comment is probably misleading here, since it's for
transaction between IOMMU and device not for the device IOTLB
For device IOTLB invalidation descriptor, spec allows a [0, ~0ULL]
220.127.116.11 Device-TLB Invalidate Descriptor
Size (S): The size field indicates the number of consecutive pages
by this invalidation
request. If S field is zero, a single page at page address specified by
Address [63:12] is requested
to be invalidated. If S field is Set, the least significant bit in the
Address field with value 0b
indicates the invalidation address range. For example, if S field is
Address is Clear, it
indicates an 8KB invalidation address range with base address in
[63:13]. If S field and
Address is Set and bit 13 is Clear, it indicates a 16KB
address range with base
address in Address [63:14], etc.
So if we receive an address whose  is 0 and the rest is all 1,
a [0, ~0ULL] invalidation.
arbitrary, right? E.g., we can't even invalidate (0x1000, 0x3000) with
device-iotlb because of the address mask, not to say sub-pages.
Or we can also make a new flag for device iotlb just like current
we replace the vhost type from UNMAP to DEVICE_IOTLB. But IMHO
ARBITRARY_LENGTH flag would work in a similar way. DEVICE_IOTLB
also allow virtio/vhost to only receive one invalidation (now IIUC
receive both iotlb and device-iotlb for unmapping a page when
ats=on), but then
ats=on will be a must and it could break some old (misconfiged)
afaict previously virtio/vhost could even work with vIOMMU
How about just convert to use a range [start, end] for any
notifier and move
the checks (e.g the assert) into the actual notifier implemented
IOMMUTLBEntry itself is the abstraction layer of TLB entry.
Hardware TLB entry
is definitely not arbitrary range either (because AFAICT the
only cache PFN rather than address, so at least PAGE_SIZE aligned).
Introducing this flag will already make this trickier just to
another similar struct to IOMMUTLBEntry, but I really don't want
to make it a
default option... Not to mention I probably have no reason to
urge the rest
iommu notifier users (tcg, vfio) to change their existing good
code to suite
any of the backend who can cooperate with arbitrary address
Ok, so it looks like we need a dedicated notifiers to device IOTLB.
That's a bug and I don't think we need to workaround
IMHO it depends on the strictness we want on the qemu cmdline API. :)
We should at least check libvirt to make sure it's using ats=on
always, then I
agree maybe we can avoid considering the rest...
Cc libvirt list, but I think we should fix libvirt if they don't
Libvirt looks fine, according to the domain XML documentation:
QEMU's virtio devices have some attributes related to the virtio
transport under the driver element: The iommu attribute enables the use
of emulated IOMMU by the device. The attribute ats controls the Address
Translation Service support for PCIe devices. This is needed to make use
of IOTLB support (see IOMMU device). Possible values are on or off.
So I think we agree that a new notifier is needed?