|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 20/22] xen/arm: vIOMMU vSID->pSID mapping layer
On Tue, Nov 03, 2026 at 17:30:54 +0000, Milan Djokic wrote:
> Introduce a vIOMMU mapping layer to support passthrough of IOMMU devices
> attached to different physical IOMMUs, including devices with the same
> Stream ID.
>
> Add a new generic vIOMMU API, viommu_allocate_vsid(), which allocates a
> guest vSID and maps it to the input pSID and the backing physical SMMU.
>
> Once the mapping is established, the guest uses the vSID for stage-1
> commands, and Xen translates the vSID to the corresponding pSID before
> processing the command on the physical SMMU.
>
> Implement usage of the new API for dom0less guests. Allocate vSIDs when
> creating the guest device tree and replace the original pSIDs with vSIDs
> that are subsequently used by the guest driver.
>
> Signed-off-by: Milan Djokic <milan_djokic@xxxxxxxx>
> ---
> xen/arch/arm/include/asm/viommu.h | 12 ++
> xen/common/device-tree/dom0less-build.c | 35 +++--
> xen/drivers/passthrough/arm/smmu-v3.c | 10 +-
> xen/drivers/passthrough/arm/viommu.c | 9 ++
> xen/drivers/passthrough/arm/vsmmu-v3.c | 184 +++++++++++++++++++++++-
> xen/include/xen/iommu.h | 6 +-
> 6 files changed, 242 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/viommu.h
> b/xen/arch/arm/include/asm/viommu.h
> index 5ae21042f0..3d0eb92655 100644
> --- a/xen/arch/arm/include/asm/viommu.h
> +++ b/xen/arch/arm/include/asm/viommu.h
> @@ -34,6 +34,16 @@ struct viommu_ops {
> * Called during domain destruction to free resources used by vIOMMU.
> */
> int (*relinquish_resources)(struct domain *d);
> +
> + /*
> + * Allocate free vSID/vRID for the guest device and establish vID->pID
> mapping
> + * Called during domain device assignment.
> + * Returns 0 on success and sets vsid argument to newly allocated
> vSID/vRID
> + * mapped to physical ID (id argument).
> + * Negative error code returned if allocation fails.
> + */
> + int (*allocate_vsid)(struct domain *d, uint32_t id, uint32_t phandle,
> + uint32_t *vsid);
> };
>
> struct viommu_desc {
> @@ -54,6 +64,8 @@ extern struct list_head host_iommu_list;
> int domain_viommu_init(struct domain *d, uint8_t viommu_type);
> int viommu_relinquish_resources(struct domain *d);
> uint8_t viommu_get_type(void);
> +int viommu_allocate_vsid(struct domain *d, uint32_t id, uint32_t phandle,
> + uint32_t *vsid);
> void add_to_host_iommu_list(paddr_t addr, paddr_t size,
> const struct dt_device_node *node,
> const void *iommu);
> diff --git a/xen/common/device-tree/dom0less-build.c
> b/xen/common/device-tree/dom0less-build.c
> index 970f89ef68..3b2a888698 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -32,6 +32,10 @@
> #include <xen/static-memory.h>
> #include <xen/static-shmem.h>
>
> +#ifdef CONFIG_ARM_VIRTUAL_IOMMU
> +#include <asm/viommu.h>
> +#endif
> +
> #define XENSTORE_PFN_LATE_ALLOC UINT64_MAX
>
> static domid_t __initdata xs_domid = DOMID_INVALID;
> @@ -333,11 +337,12 @@ static int __init handle_prop_pfdt(struct kernel_info
> *kinfo,
> }
>
> #ifdef CONFIG_ARM_VIRTUAL_IOMMU
> -static int __init modify_pfdt_node(void *pfdt, int nodeoff)
> +static int __init modify_pfdt_node(void *pfdt, int nodeoff, struct domain *d)
> {
> int proplen, i, rc;
> const fdt32_t *prop;
> fdt32_t *prop_c;
> + uint32_t psid, phandle, vsid;
>
> prop = fdt_getprop(pfdt, nodeoff, "iommus", &proplen);
> if ( !prop )
> @@ -347,10 +352,23 @@ static int __init modify_pfdt_node(void *pfdt, int
> nodeoff)
> if ( !prop_c )
> return -ENOMEM;
>
> + /*
> + * Assign <vIOMMU vSID> pairs to iommus property and establish
> + * vSID->pSID mappings
> + */
> for ( i = 0; i < proplen / 8; ++i )
> {
> + phandle = fdt32_to_cpu(prop[i * 2]);
> + psid = fdt32_to_cpu(prop[i * 2 + 1]);
> + rc = viommu_allocate_vsid(d, psid, phandle, &vsid);
> + if ( rc )
> + {
> + dprintk(XENLOG_ERR, "(DOMID=%u) Failed to allocate vSID for "
> + "pSID: %u, pIOMMU: %u", d->domain_id, psid, phandle);
Missing \n?
> + goto out;
> + }
> prop_c[i * 2] = cpu_to_fdt32(GUEST_PHANDLE_VSMMUV3);
> - prop_c[i * 2 + 1] = prop[i * 2 + 1];
> + prop_c[i * 2 + 1] = cpu_to_fdt32(vsid);
> }
>
> rc = fdt_setprop(pfdt, nodeoff, "iommus", prop_c, proplen);
> @@ -365,7 +383,8 @@ static int __init modify_pfdt_node(void *pfdt, int
> nodeoff)
> return rc;
> }
> #else
> -static inline int __init modify_pfdt_node(void *pfdt, int nodeoff)
> +static inline int __init modify_pfdt_node(void *pfdt, int nodeoff,
> + struct domain *d)
> {
> return -EOPNOTSUPP;
> }
> @@ -374,7 +393,7 @@ static inline int __init modify_pfdt_node(void *pfdt, int
> nodeoff)
> static int __init scan_pfdt_node(struct kernel_info *kinfo, void *pfdt,
> int nodeoff,
> uint32_t address_cells, uint32_t size_cells,
> - bool scan_passthrough_prop)
> + bool scan_passthrough_prop, struct domain
> *d)
> {
> int rc = 0;
> void *fdt = kinfo->fdt;
> @@ -397,12 +416,12 @@ static int __init scan_pfdt_node(struct kernel_info
> *kinfo, void *pfdt,
> node_next = fdt_first_subnode(pfdt, nodeoff);
> while ( node_next > 0 )
> {
> - rc = modify_pfdt_node(pfdt, node_next);
> + rc = modify_pfdt_node(pfdt, node_next, d);
> if ( rc )
> return rc;
>
> rc = scan_pfdt_node(kinfo, pfdt, node_next, address_cells,
> size_cells,
> - scan_passthrough_prop);
> + scan_passthrough_prop, d);
> if ( rc )
> return rc;
>
> @@ -489,7 +508,7 @@ static int __init domain_handle_dtb_boot_module(struct
> domain *d,
> res = scan_pfdt_node(kinfo, pfdt, node_next,
> DT_ROOT_NODE_ADDR_CELLS_DEFAULT,
> DT_ROOT_NODE_SIZE_CELLS_DEFAULT,
> - false);
> + false, d);
> if ( res )
> goto out;
> continue;
> @@ -499,7 +518,7 @@ static int __init domain_handle_dtb_boot_module(struct
> domain *d,
> res = scan_pfdt_node(kinfo, pfdt, node_next,
> DT_ROOT_NODE_ADDR_CELLS_DEFAULT,
> DT_ROOT_NODE_SIZE_CELLS_DEFAULT,
> - true);
> + true, d);
> if ( res )
> goto out;
> continue;
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index f213b3de12..b0ec004960 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -3103,7 +3103,7 @@ static struct arm_smmu_device
> *arm_smmu_get_by_dev(const struct device *dev)
> }
>
> static struct iommu_domain *arm_smmu_get_domain_by_sid(struct domain *d,
> - u32 sid)
> + u32 sid, const struct arm_smmu_device *smmu)
> {
> int i;
> unsigned long flags;
> @@ -3119,6 +3119,9 @@ static struct iommu_domain
> *arm_smmu_get_domain_by_sid(struct domain *d,
> list_for_each_entry(io_domain, &xen_domain->contexts, list) {
> smmu_domain = to_smmu_domain(io_domain);
>
> + if (smmu_domain->smmu != smmu)
> + continue;
> +
> spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> list_for_each_entry(master, &smmu_domain->devices, domain_head)
> {
> for (i = 0; i < master->num_streams; i++) {
> @@ -3347,7 +3350,8 @@ static void arm_smmu_iommu_xen_domain_teardown(struct
> domain *d)
> }
>
> static int arm_smmu_attach_guest_config(struct domain *d, uint32_t sid,
> - const struct iommu_guest_config *cfg)
> + const struct iommu_guest_config *cfg,
> + const struct arm_smmu_device *smmu)
> {
> int ret = -EINVAL;
> unsigned long flags;
> @@ -3357,7 +3361,7 @@ static int arm_smmu_attach_guest_config(struct domain
> *d, uint32_t sid,
> struct iommu_domain *io_domain;
>
> spin_lock(&xen_domain->lock);
> - io_domain = arm_smmu_get_domain_by_sid(d, sid);
> + io_domain = arm_smmu_get_domain_by_sid(d, sid, smmu);
> if (!io_domain) {
> ret = -ENODEV;
> goto out;
> diff --git a/xen/drivers/passthrough/arm/viommu.c
> b/xen/drivers/passthrough/arm/viommu.c
> index b4b91ab1cb..e476567b49 100644
> --- a/xen/drivers/passthrough/arm/viommu.c
> +++ b/xen/drivers/passthrough/arm/viommu.c
> @@ -76,6 +76,15 @@ int viommu_relinquish_resources(struct domain *d)
> return cur_viommu->ops->relinquish_resources(d);
> }
>
> +int viommu_allocate_vsid(struct domain *d, uint32_t id, uint32_t phandle,
> + uint32_t *vsid)
> +{
> + if ( !cur_viommu )
> + return -ENODEV;
> +
> + return cur_viommu->ops->allocate_vsid(d, id, phandle, vsid);
> +}
> +
> uint8_t viommu_get_type(void)
> {
> if ( !cur_viommu || host_iommu_list_incomplete ||
> diff --git a/xen/drivers/passthrough/arm/vsmmu-v3.c
> b/xen/drivers/passthrough/arm/vsmmu-v3.c
> index 24588affe9..5d225ccc08 100644
> --- a/xen/drivers/passthrough/arm/vsmmu-v3.c
> +++ b/xen/drivers/passthrough/arm/vsmmu-v3.c
> @@ -86,6 +86,8 @@
> #define smmu_get_ste_s1ctxptr(x) FIELD_PREP(STRTAB_STE_0_S1CTXPTR_MASK, \
> FIELD_GET(STRTAB_STE_0_S1CTXPTR_MASK, x))
>
> +#define MAX_VSID (1 << SMMU_IDR1_SIDSIZE)
> +
> /* event queue entry */
> struct arm_smmu_evtq_ent {
> /* Common fields */
> @@ -133,6 +135,14 @@ struct arm_vsmmu_queue {
> uint8_t max_n_shift;
> };
>
> +/* vSID->pSID mapping entry */
> +struct vsid_entry {
> + struct list_head entry;
> + uint32_t vsid;
> + uint32_t psid;
> + struct arm_smmu_device *smmu;
> +};
> +
> struct virt_smmu {
> struct domain *d;
> struct list_head viommu_list;
> @@ -157,6 +167,8 @@ struct virt_smmu {
> spinlock_t cr2_lock;
> spinlock_t strtab_cfg_lock;
> spinlock_t irq_cfg_lock;
> + spinlock_t vsids_lock;
> + struct list_head vsids;
> };
>
> /* Helper functions */
> @@ -379,8 +391,31 @@ int arm_vsmmu_handle_evt(struct domain *d, struct device
> *dev, uint64_t *evt)
> }
> else
> {
> + uint32_t sid = FIELD_GET(EVTQ_0_SID, evt[0]);
> + struct vsid_entry *vsid;
> + bool found = false;
> +
> smmu = list_entry(d->arch.viommu_list.next,
> struct virt_smmu, viommu_list);
> +
> + spin_lock(&smmu->vsids_lock);
> +
> + /* Translate pSID->vSID */
> + list_for_each_entry(vsid, &smmu->vsids, entry)
> + {
> + if ( vsid->psid == sid && vsid->smmu->dev == dev )
> + {
> + evt[0] &= ~EVTQ_0_SID;
> + evt[0] |= FIELD_PREP(EVTQ_0_SID, vsid->vsid);
> + found = true;
> + break;
> + }
> + }
> +
> + spin_unlock(&smmu->vsids_lock);
> +
> + if ( !found )
> + return -ENOENT;
> }
>
> ret = arm_vsmmu_write_evtq(smmu, evt);
> @@ -558,6 +593,71 @@ static int arm_vsmmu_handle_cfgi_ste(struct virt_smmu
> *smmu, uint64_t *cmdptr)
> struct arm_vsmmu_s1_trans_cfg s1_cfg = {0};
> uint32_t sid = smmu_cmd_get_sid(cmdptr[0]);
> struct iommu_guest_config guest_cfg = {0};
> + struct arm_smmu_device *phys_smmu = NULL;
> + struct host_iommu *host_iommu;
> + uint32_t psid;
> + struct arm_smmu_evtq_ent ent = {
> + .opcode = EVT_ID_BAD_STREAMID,
> + .sid = sid,
> + .c_bad_ste_streamid = {
> + .ssid = 0,
> + .ssv = false,
> + },
> + };
> +
> + /* vIOMMU->pIOMMU identity mapped for HW domain */
> + if ( domain_use_host_layout(d) )
> + {
> + psid = sid;
> + list_for_each_entry(host_iommu, &host_iommu_list, entry)
> + {
> + if ( host_iommu->addr == smmu->addr )
> + {
> + phys_smmu = (struct arm_smmu_device *) host_iommu->iommu;
> + break;
> + }
> + }
> + }
> + else {
> + struct vsid_entry *vsid_entry;
> + bool entry_found = false;
> +
> + spin_lock(&smmu->vsids_lock);
> + /* vSID out of range or no vSIDs allocated */
> + if ( sid >= MAX_VSID || list_empty(&smmu->vsids) )
> + {
> + spin_unlock(&smmu->vsids_lock);
> + arm_vsmmu_send_event(smmu, &ent);
> + return -EINVAL;
> + }
> +
> + /* get vSID entry corresponding to passed vSID */
> + list_for_each_entry(vsid_entry, &smmu->vsids, entry)
> + if ( vsid_entry->vsid == sid )
> + {
> + entry_found = true;
> + break;
> + }
> +
> + /* vSID not mapped */
> + if ( !entry_found )
> + {
> + spin_unlock(&smmu->vsids_lock);
> + arm_vsmmu_send_event(smmu, &ent);
> + return -EINVAL;
> + }
> +
> + psid = vsid_entry->psid;
> + phys_smmu = vsid_entry->smmu;
> + spin_unlock(&smmu->vsids_lock);
> + }
> +
> + /* vSID mapped to invalid physical SMMU */
> + if (!phys_smmu)
> + {
> + arm_vsmmu_send_event(smmu, &ent);
> + return -EINVAL;
> + }
>
> ret = arm_vsmmu_find_ste(smmu, sid, ste);
> if ( ret )
> @@ -578,7 +678,7 @@ static int arm_vsmmu_handle_cfgi_ste(struct virt_smmu
> *smmu, uint64_t *cmdptr)
> else
> guest_cfg.config = ARM_SMMU_DOMAIN_S1;
>
> - ret = hd->platform_ops->attach_guest_config(d, sid, &guest_cfg);
> + ret = hd->platform_ops->attach_guest_config(d, psid, &guest_cfg,
> phys_smmu);
> if ( ret )
> return ret;
>
> @@ -1189,6 +1289,13 @@ static int vsmmuv3_init_single(struct domain *d,
> paddr_t addr,
> smmu->cmdq.ent_size = CMDQ_ENT_DWORDS * DWORDS_BYTES;
> smmu->evtq.ent_size = EVTQ_ENT_DWORDS * DWORDS_BYTES;
>
> + /*
> + * vSID->pSID mapping layer is only used in the context of domains that
> + * are not direct-mapped
> + */
> + if ( !domain_use_host_layout(d) )
> + INIT_LIST_HEAD(&smmu->vsids);
> +
> spin_lock_init(&smmu->cmd_queue_lock);
> spin_lock_init(&smmu->evt_queue_lock);
> spin_lock_init(&smmu->gerror_lock);
> @@ -1197,6 +1304,7 @@ static int vsmmuv3_init_single(struct domain *d,
> paddr_t addr,
> spin_lock_init(&smmu->cr2_lock);
> spin_lock_init(&smmu->strtab_cfg_lock);
> spin_lock_init(&smmu->irq_cfg_lock);
> + spin_lock_init(&smmu->vsids_lock);
>
> ret = vgic_reserve_virq(d, virq);
> if ( !ret )
> @@ -1317,8 +1425,16 @@ static int vsmmuv3_relinquish_resources(struct domain
> *d)
> if ( list_head_is_null(&d->arch.viommu_list) )
> return 0;
>
> - list_for_each_entry_safe(pos, temp, &d->arch.viommu_list, viommu_list )
> + list_for_each_entry_safe(pos, temp, &d->arch.viommu_list, viommu_list)
> {
> + struct vsid_entry *vsid, *vsid_temp;
> +
> + list_for_each_entry_safe(vsid, vsid_temp, &pos->vsids, entry)
> + {
> + list_del(&vsid->entry);
> + xfree(vsid);
> + }
Can this break for host_layout domains? The list is only initialized for non
host_layout ones.
> +
> list_del(&pos->viommu_list);
> xfree(pos);
> }
> @@ -1326,8 +1442,72 @@ static int vsmmuv3_relinquish_resources(struct domain
> *d)
> return 0;
> }
>
> +static int vsmmuv3_allocate_vsid(struct domain *d, uint32_t id,
> + uint32_t phandle, uint32_t *vsid)
> +{
> + uint32_t i = 0;
> + struct virt_smmu *smmu;
> + struct host_iommu *host_iommu;
> + struct arm_smmu_device *phys_smmu = NULL;
> + struct vsid_entry *entry;
> + struct list_head *vsid_head;
> +
> + /* vSID->pSID mapping layer not used for direct-mapped domains */
> + if ( domain_use_host_layout(d) )
> + return -EOPNOTSUPP;
> +
> + if ( list_head_is_null(&d->arch.viommu_list) )
> + return -ENODEV;
> +
> + smmu = list_first_entry(&d->arch.viommu_list, struct virt_smmu,
> viommu_list);
> +
> + /*
> + * Find the physical SMMU corresponding to the device-tree phandle.
> + */
> + list_for_each_entry(host_iommu, &host_iommu_list, entry)
> + {
> + if ( host_iommu->dt_node->phandle == phandle )
> + {
> + phys_smmu = (struct arm_smmu_device *) host_iommu->iommu;
> + break;
> + }
> + }
> +
> + if ( !phys_smmu )
> + return -ENODEV;
> +
> + spin_lock(&smmu->vsids_lock);
> + /* Get first free vSID index */
> + list_for_each(vsid_head, &smmu->vsids)
> + i++;
> +
> + /* Max number of vSIDs already allocated? */
> + if ( i == MAX_VSID )
> + {
> + spin_unlock(&smmu->vsids_lock);
> + return -ENOMEM;
> + }
> +
> + entry = xzalloc(struct vsid_entry);
> + if ( !entry )
> + {
> + spin_unlock(&smmu->vsids_lock);
> + return -ENOMEM;
> + }
> +
> + /* Establish vSID->pSID mapping */
> + entry->vsid = i;
> + entry->psid = id;
> + entry->smmu = phys_smmu;
> + *vsid = entry->vsid;
> + list_add_tail(&entry->entry, &smmu->vsids);
> + spin_unlock(&smmu->vsids_lock);
> + return 0;
> +}
> +
Is it okay for multiple vsids to be mapped to the same psid?
> static const struct viommu_ops vsmmuv3_ops = {
> .domain_init = domain_vsmmuv3_init,
> + .allocate_vsid = vsmmuv3_allocate_vsid,
> .relinquish_resources = vsmmuv3_relinquish_resources,
> };
>
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 253f8bb6cb..c94ee4f08e 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -312,7 +312,10 @@ static inline int iommu_add_dt_pci_sideband_ids(struct
> pci_dev *pdev)
> #endif /* HAS_DEVICE_TREE_DISCOVERY */
>
> struct page_info;
> +#ifdef CONFIG_ARM
> struct iommu_guest_config;
> +struct arm_smmu_device;
> +#endif
>
> /*
> * Any non-zero value returned from callbacks of this type will cause the
> @@ -391,7 +394,8 @@ struct iommu_ops {
>
> #ifdef CONFIG_ARM
> int (*attach_guest_config)(struct domain *d, uint32_t sid,
> - const struct iommu_guest_config *cfg);
> + const struct iommu_guest_config *cfg,
> + const struct arm_smmu_device *smmu);
> #endif
> };
Isn't arm_smmu_device a little too specific? This will be a problem if other
IOMMU implementation tries to add support for this. Maybe replace with void* and
let specific driver cast it to it's own type?
Thanks,
Mykyta
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |