mbox series

[ats_vtd,v5,00/22] ATS support for VT-d

Message ID 20240603055917.18735-1-clement.mathieu--drif@eviden.com (mailing list archive)
Headers show
Series ATS support for VT-d | expand

Message

CLEMENT MATHIEU--DRIF June 3, 2024, 5:59 a.m. UTC 
From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.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                     | 142 +++++-
 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, 1330 insertions(+), 32 deletions(-)
 create mode 100644 tests/unit/test-atc.c
 create mode 100644 util/atc.c
 create mode 100644 util/atc.h

Comments

Michael S. Tsirkin July 1, 2024, 8:02 p.m. UTC | #1 
On Mon, Jun 03, 2024 at 05:59:38AM +0000, CLEMENT MATHIEU--DRIF wrote:
> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.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

I will merge, but could you please resend this using git format-patch
for formatting?  The patches have trailing CRs and don't show which sha1
they are for, which makes re-applying them after each change painful.


> 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                     | 142 +++++-
>  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, 1330 insertions(+), 32 deletions(-)
>  create mode 100644 tests/unit/test-atc.c
>  create mode 100644 util/atc.c
>  create mode 100644 util/atc.h
> 
> -- 
> 2.45.1
Michael S. Tsirkin July 2, 2024, 12:15 p.m. UTC | #2 
On Tue, Jul 02, 2024 at 05:57:57AM +0000, CLEMENT MATHIEU--DRIF wrote:
> 
> 
> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: 01 July 2024 22:02
> To: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
> Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; jasowang@redhat.com
> <jasowang@redhat.com>; zhenzhong.duan@intel.com <zhenzhong.duan@intel.com>;
> kevin.tian@intel.com <kevin.tian@intel.com>; yi.l.liu@intel.com
> <yi.l.liu@intel.com>; joao.m.martins@oracle.com <joao.m.martins@oracle.com>;
> peterx@redhat.com <peterx@redhat.com>
> Subject: Re: [PATCH ats_vtd v5 00/22] ATS support for VT-d
>  
> 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 Mon, Jun 03, 2024 at 05:59:38AM +0000, CLEMENT MATHIEU--DRIF wrote:
> > From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.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://eur06.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fgithub.com%2FBullSequana%2FQemu-in-guest-SVM-demo&data=
> 05%7C02%7Cclement.mathieu--drif%40eviden.com%7Cf5759aefcc5f4e7d4e6c08dc9a08d29a%7C7d1c77852d8a437db8421ed5d8fbe00a%7C0%7C0%7C638554609882544195%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C
> &sdata=2Gza1VD7hKr1Sx3fOLoRh6tk3taSPKTn5nfimhPLz70%3D&reserved=0
> 
> I will merge, but could you please resend this using git format-patch
> for formatting?  The patches have trailing CRs and don't show which sha1
> they are for, which makes re-applying them after each change painful.
> 
> 
> 
> Hi Michael,
> I sent the series again without the trailing new line.
> Tell me if it's better.
> 
> Is Zhenzhong's FLTS series merged? If not, it might the cause of the sha1
> problem you are facing

I don't think I have FLTS in any queue.

If your series has a dependency please specify this in
the cover letter.

Alternatively just include the dependency in the posting.





> Thanks
> >cmd
> 
> 
> > 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                     | 142 +++++-
> >  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, 1330 insertions(+), 32 deletions(-)
> >  create mode 100644 tests/unit/test-atc.c
> >  create mode 100644 util/atc.c
> >  create mode 100644 util/atc.h
> >
> > --
> > 2.45.1
>
Yi Liu July 2, 2024, 1:42 p.m. UTC | #3 
On 2024/7/2 20:15, Michael S. Tsirkin wrote:
> On Tue, Jul 02, 2024 at 05:57:57AM +0000, CLEMENT MATHIEU--DRIF wrote:
>>
>>
>> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
>> From: Michael S. Tsirkin <mst@redhat.com>
>> Sent: 01 July 2024 22:02
>> To: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>> Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; jasowang@redhat.com
>> <jasowang@redhat.com>; zhenzhong.duan@intel.com <zhenzhong.duan@intel.com>;
>> kevin.tian@intel.com <kevin.tian@intel.com>; yi.l.liu@intel.com
>> <yi.l.liu@intel.com>; joao.m.martins@oracle.com <joao.m.martins@oracle.com>;
>> peterx@redhat.com <peterx@redhat.com>
>> Subject: Re: [PATCH ats_vtd v5 00/22] ATS support for VT-d
>>   
>> 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 Mon, Jun 03, 2024 at 05:59:38AM +0000, CLEMENT MATHIEU--DRIF wrote:
>>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.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://eur06.safelinks.protection.outlook.com/?url=
>> https%3A%2F%2Fgithub.com%2FBullSequana%2FQemu-in-guest-SVM-demo&data=
>> 05%7C02%7Cclement.mathieu--drif%40eviden.com%7Cf5759aefcc5f4e7d4e6c08dc9a08d29a%7C7d1c77852d8a437db8421ed5d8fbe00a%7C0%7C0%7C638554609882544195%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C
>> &sdata=2Gza1VD7hKr1Sx3fOLoRh6tk3taSPKTn5nfimhPLz70%3D&reserved=0
>>
>> I will merge, but could you please resend this using git format-patch
>> for formatting?  The patches have trailing CRs and don't show which sha1
>> they are for, which makes re-applying them after each change painful.
>>
>>
>>
>> Hi Michael,
>> I sent the series again without the trailing new line.
>> Tell me if it's better.
>>
>> Is Zhenzhong's FLTS series merged? If not, it might the cause of the sha1
>> problem you are facing
> 
> I don't think I have FLTS in any queue.
> 
> If your series has a dependency please specify this in
> the cover letter.
> 
> Alternatively just include the dependency in the posting.

seems this is the dependency.

https://lore.kernel.org/qemu-devel/20240522062313.453317-1-zhenzhong.duan@intel.com/#t

> 
> 
> 
> 
>> Thanks
>>> cmd
>>
>>
>>> 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                     | 142 +++++-
>>>   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, 1330 insertions(+), 32 deletions(-)
>>>   create mode 100644 tests/unit/test-atc.c
>>>   create mode 100644 util/atc.c
>>>   create mode 100644 util/atc.h
>>>
>>> --
>>> 2.45.1
>>
>
CLEMENT MATHIEU--DRIF July 2, 2024, 3:27 p.m. UTC | #4 
On 02/07/2024 15:42, 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 20:15, Michael S. Tsirkin wrote:
>> On Tue, Jul 02, 2024 at 05:57:57AM +0000, CLEMENT MATHIEU--DRIF wrote:
>>>
>>>
>>> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
>>>
>>> From: Michael S. Tsirkin <mst@redhat.com>
>>> Sent: 01 July 2024 22:02
>>> To: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>>> Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; jasowang@redhat.com
>>> <jasowang@redhat.com>; zhenzhong.duan@intel.com
>>> <zhenzhong.duan@intel.com>;
>>> kevin.tian@intel.com <kevin.tian@intel.com>; yi.l.liu@intel.com
>>> <yi.l.liu@intel.com>; joao.m.martins@oracle.com
>>> <joao.m.martins@oracle.com>;
>>> peterx@redhat.com <peterx@redhat.com>
>>> Subject: Re: [PATCH ats_vtd v5 00/22] ATS support for VT-d
>>>
>>> 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 Mon, Jun 03, 2024 at 05:59:38AM +0000, CLEMENT MATHIEU--DRIF wrote:
>>>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.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://eur06.safelinks.protection.outlook.com/?url=
>>> https%3A%2F%2Fgithub.com%2FBullSequana%2FQemu-in-guest-SVM-demo&data=
>>> 05%7C02%7Cclement.mathieu--drif%40eviden.com%7Cf5759aefcc5f4e7d4e6c08dc9a08d29a%7C7d1c77852d8a437db8421ed5d8fbe00a%7C0%7C0%7C638554609882544195%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C
>>>
>>> &sdata=2Gza1VD7hKr1Sx3fOLoRh6tk3taSPKTn5nfimhPLz70%3D&reserved=0
>>>
>>> I will merge, but could you please resend this using git format-patch
>>> for formatting?  The patches have trailing CRs and don't show which
>>> sha1
>>> they are for, which makes re-applying them after each change painful.
>>>
>>>
>>>
>>> Hi Michael,
>>> I sent the series again without the trailing new line.
>>> Tell me if it's better.
>>>
>>> Is Zhenzhong's FLTS series merged? If not, it might the cause of the
>>> sha1
>>> problem you are facing
>>
>> I don't think I have FLTS in any queue.
>>
>> If your series has a dependency please specify this in
>> the cover letter.
>>
>> Alternatively just include the dependency in the posting.
>
> seems this is the dependency.
>
> https://lore.kernel.org/qemu-devel/20240522062313.453317-1-zhenzhong.duan@intel.com/#t
>
>
Sorry if I didn't make it clear.

As mentioned in the cover letter, this series is based on Zhenzhong's
and Yi's FLTS implementation which (AFAIK) has only be posted as an RFC
so far (keep me up to date please).

v5 is based on that branch :
https://github.com/yiliu1765/qemu/tree/zhenzhong/iommufd_nesting_rfcv2

>>
>>
>>
>>
>>> Thanks
>>>> cmd
>>>
>>>
>>>> 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                     | 142 +++++-
>>>>   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, 1330 insertions(+), 32 deletions(-)
>>>>   create mode 100644 tests/unit/test-atc.c
>>>>   create mode 100644 util/atc.c
>>>>   create mode 100644 util/atc.h
>>>>
>>>> --
>>>> 2.45.1
>>>
>>
>
> --
> Regards,
> Yi Liu
Michael S. Tsirkin July 2, 2024, 3:28 p.m. UTC | #5 
On Tue, Jul 02, 2024 at 03:27:13PM +0000, CLEMENT MATHIEU--DRIF wrote:
> 
> On 02/07/2024 15:42, 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 20:15, Michael S. Tsirkin wrote:
> >> On Tue, Jul 02, 2024 at 05:57:57AM +0000, CLEMENT MATHIEU--DRIF wrote:
> >>>
> >>>
> >>> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> >>>
> >>> From: Michael S. Tsirkin <mst@redhat.com>
> >>> Sent: 01 July 2024 22:02
> >>> To: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
> >>> Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; jasowang@redhat.com
> >>> <jasowang@redhat.com>; zhenzhong.duan@intel.com
> >>> <zhenzhong.duan@intel.com>;
> >>> kevin.tian@intel.com <kevin.tian@intel.com>; yi.l.liu@intel.com
> >>> <yi.l.liu@intel.com>; joao.m.martins@oracle.com
> >>> <joao.m.martins@oracle.com>;
> >>> peterx@redhat.com <peterx@redhat.com>
> >>> Subject: Re: [PATCH ats_vtd v5 00/22] ATS support for VT-d
> >>>
> >>> 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 Mon, Jun 03, 2024 at 05:59:38AM +0000, CLEMENT MATHIEU--DRIF wrote:
> >>>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.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://eur06.safelinks.protection.outlook.com/?url=
> >>> https%3A%2F%2Fgithub.com%2FBullSequana%2FQemu-in-guest-SVM-demo&data=
> >>> 05%7C02%7Cclement.mathieu--drif%40eviden.com%7Cf5759aefcc5f4e7d4e6c08dc9a08d29a%7C7d1c77852d8a437db8421ed5d8fbe00a%7C0%7C0%7C638554609882544195%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C
> >>>
> >>> &sdata=2Gza1VD7hKr1Sx3fOLoRh6tk3taSPKTn5nfimhPLz70%3D&reserved=0
> >>>
> >>> I will merge, but could you please resend this using git format-patch
> >>> for formatting?  The patches have trailing CRs and don't show which
> >>> sha1
> >>> they are for, which makes re-applying them after each change painful.
> >>>
> >>>
> >>>
> >>> Hi Michael,
> >>> I sent the series again without the trailing new line.
> >>> Tell me if it's better.
> >>>
> >>> Is Zhenzhong's FLTS series merged? If not, it might the cause of the
> >>> sha1
> >>> problem you are facing
> >>
> >> I don't think I have FLTS in any queue.
> >>
> >> If your series has a dependency please specify this in
> >> the cover letter.
> >>
> >> Alternatively just include the dependency in the posting.
> >
> > seems this is the dependency.
> >
> > https://lore.kernel.org/qemu-devel/20240522062313.453317-1-zhenzhong.duan@intel.com/#t
> >
> >
> Sorry if I didn't make it clear.
> 
> As mentioned in the cover letter, this series is based on Zhenzhong's
> and Yi's FLTS implementation which (AFAIK) has only be posted as an RFC
> so far (keep me up to date please).
> 
> v5 is based on that branch :
> https://github.com/yiliu1765/qemu/tree/zhenzhong/iommufd_nesting_rfcv2

Ah, OK so this is not for merge yet until that patchset is ready.


> >>
> >>
> >>
> >>
> >>> Thanks
> >>>> cmd
> >>>
> >>>
> >>>> 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                     | 142 +++++-
> >>>>   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, 1330 insertions(+), 32 deletions(-)
> >>>>   create mode 100644 tests/unit/test-atc.c
> >>>>   create mode 100644 util/atc.c
> >>>>   create mode 100644 util/atc.h
> >>>>
> >>>> --
> >>>> 2.45.1
> >>>
> >>
> >
> > --
> > Regards,
> > Yi Liu