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
From: Clement Mathieu--Drif <cmdetu@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 v5 Changes after review by Philippe : - change the type of 'level' to unsigned in vtd_lookup_iotlb 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 -- 2.45.2
On Tue, Jul 02, 2024 at 05:52:29AM +0000, CLEMENT MATHIEU--DRIF wrote: > From: Clement Mathieu--Drif <cmdetu@gmail.com> > > This series belongs to a list of series that add SVM support for VT-d. You don't need ats_vtd as a tag, I think, so if it's helpful for someone, I don't mind. What you do need is "repost" so people know how it's related to your previous v5 of the same patchset.
On 02/07/2024 14:16, Michael S. Tsirkin wrote: > Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe. > > > On Tue, Jul 02, 2024 at 05:52:29AM +0000, CLEMENT MATHIEU--DRIF wrote: >> From: Clement Mathieu--Drif <cmdetu@gmail.com> >> >> This series belongs to a list of series that add SVM support for VT-d. > You don't need ats_vtd as a tag, I think, so if it's helpful > for someone, I don't mind. What you do need is "repost" so > people know how it's related to your previous v5 of the > same patchset. > Ok fine, I will remove it in future versions, sorry
On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote: > From: Clement Mathieu--Drif <cmdetu@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 > > v5 > Changes after review by Philippe : > - change the type of 'level' to unsigned in vtd_lookup_iotlb Hi CMD, I saw two v5 in my inbox, are they the same? :) > 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 > -- Regards, Yi Liu
On 02/07/2024 15:44, Yi Liu wrote: > Caution: External email. Do not open attachments or click links, > unless this email comes from a known sender and you know the content > is safe. > > > On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote: >> From: Clement Mathieu--Drif <cmdetu@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 >> >> v5 >> Changes after review by Philippe : >> - change the type of 'level' to unsigned in vtd_lookup_iotlb > > Hi CMD, > > I saw two v5 in my inbox, are they the same? :) Hi, No, it's a resend following a request by Michael, sorry for that > >> 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 >> > > -- > Regards, > Yi Liu
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 <cmdetu@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 >
On 03/07/2024 14:32, Yi Liu wrote: > Caution: External email. Do not open attachments or click links, > unless this email comes from a known sender and you know the content > is safe. > Hi, thanks for your review! very efficient! > > 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. Will do > 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. The final purpose is to support SVM, consequently, we only add support for ATS with PASID here > 3) Some commits do not have commit message. It would be good to have > it. Ok, I will be more verbose ;) > 4) Some helpers look to be used by device model, if possible, it's better > to submit them with a demo device. The demo device is already in my GitHub repo (https://github.com/BullSequana/qemu/tree/master) It will be sent in a future series that adds the last features required for SVM (splitting the series to make reviews less painful) > 5) A design description in the cover-letter would be helpful. Ok, I will elaborate > > On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote: >> From: Clement Mathieu--Drif <cmdetu@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 >> > >
On 2024/7/4 12:36, CLEMENT MATHIEU--DRIF wrote: > > On 03/07/2024 14:32, Yi Liu wrote: >> Caution: External email. Do not open attachments or click links, >> unless this email comes from a known sender and you know the content >> is safe. >> > Hi, thanks for your review! very efficient! >> >> 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. > Will do >> 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. > The final purpose is to support SVM, consequently, we only add support > for ATS with PASID here yes. but no need to put all of them in one series. Just like you sent the PRI series separately. >> 3) Some commits do not have commit message. It would be good to have >> it. > Ok, I will be more verbose ;) >> 4) Some helpers look to be used by device model, if possible, it's better >> to submit them with a demo device. > The demo device is already in my GitHub repo > (https://github.com/BullSequana/qemu/tree/master) > It will be sent in a future series that adds the last features required > for SVM (splitting the series to make reviews less painful) >> 5) A design description in the cover-letter would be helpful. > Ok, I will elaborate >> >> On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote: >>> From: Clement Mathieu--Drif <cmdetu@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 >>> >> >> -- Regards, Yi Liu
© 2016 - 2024 Red Hat, Inc.