Hi CMD,
I've went through the series. Some general suggestions on the series.
1) Patch 01, 02, 04 can be sent separately as they are fixes.
2) This series mixed the ATS and PASID capability a bit. Actually,
they don't have dependency. I'd suggest you split the series into
- support ATS for the requests without PASID
- support ATS for requests with PASID
The second part should be an incremental change based on the first
part. If you can make use of the existing translate() callback, then
it is possible to remove the dependency on Zhenzhong's stage-1 series.
3) Some commits do not have commit message. It would be good to have
it.
4) Some helpers look to be used by device model, if possible, it's better
to submit them with a demo device.
5) A design description in the cover-letter would be helpful.
On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote:
From: Clement Mathieu--Drif <cmd...@gmail.com>
This series belongs to a list of series that add SVM support for VT-d.
As a starting point, we use the series called 'intel_iommu: Enable stage-1
translation' (rfc2) by Zhenzhong Duan and Yi Liu.
Here we focus on the implementation of ATS support in the IOMMU and on a
PCI-level
API for ATS to be used by virtual devices.
This work is based on the VT-d specification version 4.1 (March 2023).
Here is a link to a GitHub repository where you can find the following elements
:
- Qemu with all the patches for SVM
- ATS
- PRI
- Device IOTLB invalidations
- Requests with already translated addresses
- A demo device
- A simple driver for the demo device
- A userspace program (for testing and demonstration purposes)
https://github.com/BullSequana/Qemu-in-guest-SVM-demo
v2
- handle huge pages better by detecting the page table level at which the
translation errors occur
- Changes after review by ZhenZhong Duan :
- Set the access bit after checking permissions
- helper for PASID and ATS : make the commit message more accurate
('present' replaced with 'enabled')
- pcie_pasid_init: add PCI_PASID_CAP_WIDTH_SHIFT and use it instead of
PCI_EXT_CAP_PASID_SIZEOF for shifting the pasid width when preparing the
capability register
- pci: do not check pci_bus_bypass_iommu after calling
pci_device_get_iommu_bus_devfn
- do not alter formatting of IOMMUTLBEntry declaration
- vtd_iova_fl_check_canonical : directly use s->aw_bits instead of aw
for the sake of clarity
v3
- rebase on new version of Zhenzhong's flts implementation
- fix the atc lookup operation (check the mask before returning an entry)
- add a unit test for the ATC
- store a user pointer in the iommu notifiers to simplify the
implementation of svm devices
Changes after review by Zhenzhong :
- store the input pasid instead of rid2pasid when returning an entry
after a translation
- split the ATC implementation and its unit tests
v4
Changes after internal review
- Fix the nowrite optimization, an ATS translation without the nowrite
flag should not fail when the write permission is not set
It's strange to list internal review here.
v5
Changes after review by Philippe :
- change the type of 'level' to unsigned in vtd_lookup_iotlb
list change log from latest to the earliest would be nice too. Look forward
to your next version. :)
Regards,
Yi Liu
Clément Mathieu--Drif (22):
intel_iommu: fix FRCD construction macro.
intel_iommu: make types match
intel_iommu: return page walk level even when the translation fails
intel_iommu: do not consider wait_desc as an invalid descriptor
memory: add permissions in IOMMUAccessFlags
pcie: add helper to declare PASID capability for a pcie device
pcie: helper functions to check if PASID and ATS are enabled
intel_iommu: declare supported PASID size
pci: cache the bus mastering status in the device
pci: add IOMMU operations to get address spaces and memory regions
with PASID
memory: store user data pointer in the IOMMU notifiers
pci: add a pci-level initialization function for iommu notifiers
intel_iommu: implement the get_address_space_pasid iommu operation
intel_iommu: implement the get_memory_region_pasid iommu operation
memory: Allow to store the PASID in IOMMUTLBEntry
intel_iommu: fill the PASID field when creating an instance of
IOMMUTLBEntry
atc: generic ATC that can be used by PCIe devices that support SVM
atc: add unit tests
memory: add an API for ATS support
pci: add a pci-level API for ATS
intel_iommu: set the address mask even when a translation fails
intel_iommu: add support for ATS
hw/i386/intel_iommu.c | 146 +++++-
hw/i386/intel_iommu_internal.h | 6 +-
hw/pci/pci.c | 127 +++++-
hw/pci/pcie.c | 42 ++
include/exec/memory.h | 51 ++-
include/hw/i386/intel_iommu.h | 2 +-
include/hw/pci/pci.h | 101 +++++
include/hw/pci/pci_device.h | 1 +
include/hw/pci/pcie.h | 9 +-
include/hw/pci/pcie_regs.h | 3 +
include/standard-headers/linux/pci_regs.h | 1 +
system/memory.c | 20 +
tests/unit/meson.build | 1 +
tests/unit/test-atc.c | 527 ++++++++++++++++++++++
util/atc.c | 211 +++++++++
util/atc.h | 117 +++++
util/meson.build | 1 +
17 files changed, 1332 insertions(+), 34 deletions(-)
create mode 100644 tests/unit/test-atc.c
create mode 100644 util/atc.c
create mode 100644 util/atc.h
