[v9] SMMU handling for PCIe Passthrough on ARM

[PATCH v9 8/8] xen/arm: Map ITS doorbell register to IOMMU page tables

Posted by Mykyta Poturai 11 months ago

From: Rahul Singh <rahul.singh@arm.com>

When ITS is enabled and PCI devices that are behind an SMMU generate an
MSI interrupt, SMMU fault will be observed as there is currently no
mapping in p2m table for the ITS translation register (GITS_TRANSLATER).

A mapping is required in the iommu page tables so that the device can
generate the MSI interrupt writing to the GITS_TRANSLATER register.

The GITS_TRANSLATER register is a 32-bit register, and there is nothing
else in a page containing it, so map that page.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
This patch was originally picked up from [1], and commit description
loosely borrowed from [2].

Example SMMUv3 fault (qemu-system-aarch64 virt model), ITS base 0x8080000:

(XEN) SMMUv3: /smmuv3@9050000: event 0x10 received:
(XEN) SMMUv3: /smmuv3@9050000:  0x0000000800000010
(XEN) SMMUv3: /smmuv3@9050000:  0x0000008000000000
(XEN) SMMUv3: /smmuv3@9050000:  0x0000000008090040
(XEN) SMMUv3: /smmuv3@9050000:  0x0000000000000000

Example SMMUv2 fault (AMD/Xilinx Versal), ITS base 0xf9020000:

(XEN) smmu: /axi/smmu@fd800000: Unhandled context fault: fsr=0x402, iova=0xf9030040, fsynr=0x12, cb=0

v8->v9:
* no changes

v7->v8:
* no changes

v6->v7:
* add tlb flush after mapping
* style: update formatting
* revert back to printk with XENLOG_G_ERR

v5->v6:
* switch to iommu_map() interface
* fix page_count argument
* style fixup
* use gprintk instead of printk
* add my Signed-off-by
* move to vgic_v3_its_init_virtual()

v4->v5:
* new patch

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00483.html
[2] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/6232a0d53377009bb7fbc3c3ab81d0153734be6b
---
 xen/arch/arm/vgic-v3-its.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index c65c1dbf52..376254f206 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -1478,6 +1478,26 @@ static int vgic_v3_its_init_virtual(struct domain *d, paddr_t guest_addr,
 
     register_mmio_handler(d, &vgic_its_mmio_handler, guest_addr, SZ_64K, its);
 
+    if ( is_iommu_enabled(its->d) )
+    {
+        mfn_t mfn = maddr_to_mfn(its->doorbell_address);
+        unsigned int flush_flags = 0;
+        int ret = iommu_map(its->d, _dfn(mfn_x(mfn)), mfn, 1, IOMMUF_writable,
+                            &flush_flags);
+
+        if ( ret < 0 )
+        {
+            printk(XENLOG_G_ERR
+                    "GICv3: Map ITS translation register for %pd failed.\n",
+                    its->d);
+            return ret;
+        }
+
+        ret = iommu_iotlb_flush(its->d, _dfn(mfn_x(mfn)), 1, flush_flags);
+        if ( ret < 0 )
+            return ret;
+    }
+
     /* Register the virtual ITS to be able to clean it up later. */
     list_add_tail(&its->vits_list, &d->arch.vgic.vits_list);
 
-- 
2.34.1

Re: [PATCH v9 8/8] xen/arm: Map ITS doorbell register to IOMMU page tables

Posted by Stefano Stabellini 9 months, 3 weeks ago

On Fri, 14 Mar 2025, Mykyta Poturai wrote:
> From: Rahul Singh <rahul.singh@arm.com>
> 
> When ITS is enabled and PCI devices that are behind an SMMU generate an
> MSI interrupt, SMMU fault will be observed as there is currently no
> mapping in p2m table for the ITS translation register (GITS_TRANSLATER).
> 
> A mapping is required in the iommu page tables so that the device can
> generate the MSI interrupt writing to the GITS_TRANSLATER register.
> 
> The GITS_TRANSLATER register is a 32-bit register, and there is nothing
> else in a page containing it, so map that page.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
> ---
> This patch was originally picked up from [1], and commit description
> loosely borrowed from [2].
> 
> Example SMMUv3 fault (qemu-system-aarch64 virt model), ITS base 0x8080000:
> 
> (XEN) SMMUv3: /smmuv3@9050000: event 0x10 received:
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000000800000010
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000008000000000
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000000008090040
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000000000000000
> 
> Example SMMUv2 fault (AMD/Xilinx Versal), ITS base 0xf9020000:
> 
> (XEN) smmu: /axi/smmu@fd800000: Unhandled context fault: fsr=0x402, iova=0xf9030040, fsynr=0x12, cb=0
> 
> v8->v9:
> * no changes
> 
> v7->v8:
> * no changes
> 
> v6->v7:
> * add tlb flush after mapping
> * style: update formatting
> * revert back to printk with XENLOG_G_ERR
> 
> v5->v6:
> * switch to iommu_map() interface
> * fix page_count argument
> * style fixup
> * use gprintk instead of printk
> * add my Signed-off-by
> * move to vgic_v3_its_init_virtual()
> 
> v4->v5:
> * new patch
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00483.html
> [2] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/6232a0d53377009bb7fbc3c3ab81d0153734be6b
> ---
>  xen/arch/arm/vgic-v3-its.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index c65c1dbf52..376254f206 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -1478,6 +1478,26 @@ static int vgic_v3_its_init_virtual(struct domain *d, paddr_t guest_addr,
>  
>      register_mmio_handler(d, &vgic_its_mmio_handler, guest_addr, SZ_64K, its);
>  
> +    if ( is_iommu_enabled(its->d) )
> +    {
> +        mfn_t mfn = maddr_to_mfn(its->doorbell_address);
> +        unsigned int flush_flags = 0;
> +        int ret = iommu_map(its->d, _dfn(mfn_x(mfn)), mfn, 1, IOMMUF_writable,
> +                            &flush_flags);

Should this be:

        int ret = iommu_map(its->d, _dfn(PFN_DOWN(its->doorbell_address)), mfn, 1, IOMMUF_writable,
                            &flush_flags);

?


> +        if ( ret < 0 )
> +        {
> +            printk(XENLOG_G_ERR
> +                    "GICv3: Map ITS translation register for %pd failed.\n",
> +                    its->d);
> +            return ret;
> +        }
> +
> +        ret = iommu_iotlb_flush(its->d, _dfn(mfn_x(mfn)), 1, flush_flags);

And this:

       ret = iommu_iotlb_flush(its->d, _dfn(PFN_DOWN(its->doorbell_address)), 1, flush_flags);

?

The original code in this patch assumes that the mapping is 1:1 but
actually its->doorbell_address is set to guest_addr +
ITS_DOORBELL_OFFSET and could potentially be not 1:1 ?

> +        if ( ret < 0 )
> +            return ret;
> +    }
> +
>      /* Register the virtual ITS to be able to clean it up later. */
>      list_add_tail(&its->vits_list, &d->arch.vgic.vits_list);
>  
> -- 
> 2.34.1
>

Re: [PATCH v9 8/8] xen/arm: Map ITS doorbell register to IOMMU page tables

Posted by Julien Grall 9 months, 4 weeks ago

Hi Mykyta,

On 14/03/2025 22:34, Mykyta Poturai wrote:
> From: Rahul Singh <rahul.singh@arm.com>
> 
> When ITS is enabled and PCI devices that are behind an SMMU generate an
> MSI interrupt, SMMU fault will be observed as there is currently no
> mapping in p2m table for the ITS translation register (GITS_TRANSLATER).
> 
> A mapping is required in the iommu page tables so that the device can
> generate the MSI interrupt writing to the GITS_TRANSLATER register.
> 
> The GITS_TRANSLATER register is a 32-bit register, and there is nothing
> else in a page containing it, so map that page.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
> ---
> This patch was originally picked up from [1], and commit description
> loosely borrowed from [2].
> 
> Example SMMUv3 fault (qemu-system-aarch64 virt model), ITS base 0x8080000:
> 
> (XEN) SMMUv3: /smmuv3@9050000: event 0x10 received:
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000000800000010
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000008000000000
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000000008090040
> (XEN) SMMUv3: /smmuv3@9050000:  0x0000000000000000
> 
> Example SMMUv2 fault (AMD/Xilinx Versal), ITS base 0xf9020000:
> 
> (XEN) smmu: /axi/smmu@fd800000: Unhandled context fault: fsr=0x402, iova=0xf9030040, fsynr=0x12, cb=0
> 
> v8->v9:
> * no changes
> 
> v7->v8:
> * no changes
> 
> v6->v7:
> * add tlb flush after mapping
> * style: update formatting
> * revert back to printk with XENLOG_G_ERR
> 
> v5->v6:
> * switch to iommu_map() interface
> * fix page_count argument
> * style fixup
> * use gprintk instead of printk
> * add my Signed-off-by
> * move to vgic_v3_its_init_virtual()
> 
> v4->v5:
> * new patch
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00483.html
> [2] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/6232a0d53377009bb7fbc3c3ab81d0153734be6b
> ---
>   xen/arch/arm/vgic-v3-its.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index c65c1dbf52..376254f206 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -1478,6 +1478,26 @@ static int vgic_v3_its_init_virtual(struct domain *d, paddr_t guest_addr,
>   
>       register_mmio_handler(d, &vgic_its_mmio_handler, guest_addr, SZ_64K, its);
>   
> +    if ( is_iommu_enabled(its->d) )
> +    {
> +        mfn_t mfn = maddr_to_mfn(its->doorbell_address);
> +        unsigned int flush_flags = 0;
> +        int ret = iommu_map(its->d, _dfn(mfn_x(mfn)), mfn, 1, IOMMUF_writable,
> +                            &flush_flags);

While looking at your patches for the guest vITS support. I noticed this 
line is not updated for domU. Regardless of the one vs many vITS 
discussion, I don't think this wants to be mapped 1:1. Instead, you want 
to map the guest doorbell page to the host doorbell page.

The flush would need to be updated accordingly.

Cheers,

> +
> +        if ( ret < 0 )
> +        {
> +            printk(XENLOG_G_ERR
> +                    "GICv3: Map ITS translation register for %pd failed.\n",
> +                    its->d);
> +            return ret;
> +        }
> +
> +        ret = iommu_iotlb_flush(its->d, _dfn(mfn_x(mfn)), 1, flush_flags);
> +        if ( ret < 0 )
> +            return ret;
> +    }
> +
>       /* Register the virtual ITS to be able to clean it up later. */
>       list_add_tail(&its->vits_list, &d->arch.vgic.vits_list);
>   

-- 
Julien Grall

[PATCH v9 1/8] iommu/arm: Add iommu_dt_xlate()
[PATCH v9 2/8] iommu/arm: Introduce iommu_add_dt_pci_sideband_ids API
[PATCH v9 3/8] iommu/arm: iommu_add_dt_pci_sideband_ids phantom handling
[PATCH v9 4/8] xen/arm: smmuv2: Add PCI devices support for SMMUv2
[PATCH v9 5/8] xen/arm: smmuv3: Add PCI devices support for SMMUv3
[PATCH v9 6/8] xen/arm: Fix mapping for PCI bridge mmio region
[PATCH v9 7/8] xen/arm: enable dom0 to use PCI devices with pci-passthrough=no
[PATCH v9 8/8] xen/arm: Map ITS doorbell register to IOMMU page tables