On Fri, 14 Mar 2025, Mykyta Poturai wrote:
> From: Rahul Singh <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Stewart Hildebrand <[email protected]>
> Signed-off-by: Mykyta Poturai <[email protected]>
> ---
> 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
>
