coco/TSM: Implement host-side support for Arm CCA TDISP setup

+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c new file +15 -0

@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 ARM Ltd.
+ */
+
+#include <linux/interrupt.h>
+#include <asm/rmi_smc.h>
+#include <asm/rmi_cmds.h>
+
+#include "arm-smmu-v3.h"
+
+#define	RMI_PSMMU_IRQ_GERROR	BIT(0)
+#define	RMI_PSMMU_IRQ_EVENTQ	BIT(1)
+#define	RMI_PSMMU_IRQ_PRIQ	BIT(2)
+#define	RMI_PSMMU_IRQ_CMDQ	BIT(3)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c new file +2 -0

+		if (rmi_psmmu_irq_notify(smmu->base_phys,
+					 notify_flags, &event)) {

Quoted 17 lines · Aneesh Kumar K.V (Arm)

+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c new file +15 -0

@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 ARM Ltd.
+ */
+
+#include <linux/interrupt.h>
+#include <asm/rmi_smc.h>
+#include <asm/rmi_cmds.h>
+
+#include "arm-smmu-v3.h"
+
+#define	RMI_PSMMU_IRQ_GERROR	BIT(0)
+#define	RMI_PSMMU_IRQ_EVENTQ	BIT(1)
+#define	RMI_PSMMU_IRQ_PRIQ	BIT(2)
+#define	RMI_PSMMU_IRQ_CMDQ	BIT(3)

[...]

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c line 37

Quoted 6 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c new file +6 -0

+	if (irq == smmu->realm_evtq_irq)
+		notify_flags =  RMI_PSMMU_IRQ_EVENTQ;
+	else if (irq == smmu->realm_gerr_irq)
+		notify_flags =  RMI_PSMMU_IRQ_GERROR;
+	else if (irq == smmu->realm_pri_irq)
+		notify_flags =  RMI_PSMMU_IRQ_PRIQ;

These are defined as BIT(x)...

Quoted 2 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c new file +2 -0

+		if (rmi_psmmu_irq_notify(smmu->base_phys,
+					 notify_flags, &event)) {

[...]

Review

Quoted 1 line

(quoted diff) +1 -0

+			rmi_psmmu_event_consume(smmu->base_phys, notify_flags);

.. and passed to RMI_PSMMU_IRQ_NOTIFY and RMI_PSMMU_EVENT_CONSUME.

RMI_PSMMU_IRQ_NOTIFY takes RmiPsmmuIrqSet type, which is a 4-bit field that has a range of [BIT(0), BIT(3)]. So this is correct.

However, RMI_PSMMU_EVENT_CONSUME takes RmiPsmmuIrq type, which is a 2-bit field that has a range of [0x0, 0x3]. So this seems wrong.

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c line 88

Quoted 12 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c new file +11 -0

+void arm_smmu_setup_realm_irqs(struct arm_smmu_device *smmu)
+{
+	int irq, ret;
+
+	irq = smmu->realm_evtq_irq;
+	if (irq) {
+		ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
+						arm_smmu_realm_notify_thread,
+						IRQF_ONESHOT,
+						"arm-smmu-v3-realm-evtq",
+						smmu);

These IRQ numbers are raw numbers forwarded by firmware and then returned by RMI_PSMMU_INFO:

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +5 -0

+		if ((psmmu_info->flags & RMI_PSMMU_IRQCFG_MASK) ==
+		    RMI_PSMMU_IRQCFG_IRQ_WIRED) {
+			smmu->realm_gerr_irq = psmmu_info->gerror_intr_num;
+			smmu->realm_evtq_irq = psmmu_info->eventq_intr_num;
+			smmu->realm_pri_irq = psmmu_info->priq_intr_num;

Should they be converted to Linux IRQ numbers before forwarded to devm_request_threaded_irq?

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h struct arm_smmu_device { · line 792

Quoted 8 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +3 -0

@@ -782,6 +787,9 @@ struct arm_smmu_device {
 
 	int				gerr_irq;
 	int				combined_irq;
+	int				realm_gerr_irq;
+	int				realm_evtq_irq;
+	int				realm_pri_irq;

Nit: this series adds a few realm-specific things in the top SMMU structure. Maybe a "struct arm_realm_psmmu" can make them clearer.

(quoted diff) +17 -0

+static struct irq_domain *arm_smmu_get_wired_irq_domain(struct arm_smmu_device *smmu)
+{
+	int irqs[] = {
+		smmu->combined_irq,
+		smmu->evtq.q.irq,
+		smmu->gerr_irq,
+		smmu->priq.q.irq,
+	};
+	struct irq_domain *domain = NULL;
+	struct irq_data *irq_data;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(irqs); i++) {
+		if (irqs[i] <= 0)
+			continue;
+
+		irq_data = irq_get_irq_data(irqs[i]);

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +7 -0

+static int arm_realm_smmu_v3_vdevice_init(struct iommufd_vdevice *vdev)
+{
+	struct device *dev = iommufd_vdevice_to_device(vdev);
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+	// fixme which stream to pick
+	/* At this moment, iommufd only supports PCI device that has one SID */
+	struct arm_smmu_stream *stream = &master->streams[0];

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +3 -0

+int arm_realm_smmu_v3_init(struct iommufd_viommu *viommu,
+			   const struct iommu_user_data *user_data)
+{

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c void arm_smmu_setup_realm_irqs(struct arm_smmu_device *smmu) · line 196

Quoted 4 lines · Aneesh Kumar K.V (Arm)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +4 -0

+static const struct iommufd_viommu_ops arm_realm_smmu_v3_ops = {
+	.destroy = arm_realm_smmu_v3_destroy,
+	.alloc_domain_nested = arm_vsmmu_alloc_domain_nested,
+	.cache_invalidate = arm_vsmmu_cache_invalidate,

I don't think realm vsmmu should include NS nested domain ops. I wonder if adding here is for some covert reason that prevents us from registering viommu/vdevice objects?

Quoted 7 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +7 -0

+static int arm_realm_smmu_v3_vdevice_init(struct iommufd_vdevice *vdev)
+{
+	struct device *dev = iommufd_vdevice_to_device(vdev);
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+	// fixme which stream to pick
+	/* At this moment, iommufd only supports PCI device that has one SID */
+	struct arm_smmu_stream *stream = &master->streams[0];

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c void arm_smmu_setup_realm_irqs(struct arm_smmu_device *smmu) · line 176

Quoted 10 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +10 -0

+	struct arm_smmu_device *smmu = master->smmu;
+	unsigned long rmi_ret = 0;
+	int ret;
+
+	if (!smmu->realm_initialized)
+		return -EINVAL;
+
+	ret = rmi_psmmu_st_l2_create(smmu->base_phys,
+				     ALIGN_DOWN(stream->id, STRTAB_NUM_L2_STES),
+				     &rmi_ret);

The "vdevice" is for a PSMMU stream table allocation..

Quoted 3 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +3 -0

+int arm_realm_smmu_v3_init(struct iommufd_viommu *viommu,
+			   const struct iommu_user_data *user_data)
+{

[...]

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c void arm_smmu_setup_realm_irqs(struct arm_smmu_device *smmu) · line 257

Quoted 3 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +3 -0

+psmmu_activate:
+	ret = rmi_psmmu_activate(smmu->base_phys, virt_to_phys(params),
+				 &rmi_ret);

.. and the "viommu" is also for PSMMU activation...

include/uapi/linux/iommufd.h enum iommu_viommu_type { · line 1058

Quoted 6 lines

+++ b/include/uapi/linux/iommufd.h

include/uapi/linux/iommufd.h +1 -0

@@ -1055,6 +1055,7 @@ enum iommu_viommu_type {
 	IOMMU_VIOMMU_TYPE_DEFAULT = 0,
 	IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1,
 	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV = 2,
+	IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 = 3,

.. and we demand userspace (VMM) to use IOMMU_VIOMMU_ALLOC ioctl, even if VMM does not actually expose a guest-level SMMU instance. Thus, no user_data.

I can get the reasoning behind the flow using this viommu/vdevice.

But, on the other hand, I can imagine that a Realm VSMMU would add a new flag with a user_data to this VIOMMU. Then, this flow would give some troubles to VMM (QEMU for example):

- For VM with a guest-level SMMU, QEMU creates a realm instance where IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 (with vsmmu) can be allocated.
- For VM w/o a guest-level SMMU, QEMU won't create such a realm instance, while still required to invoke the ioctl (w/o vsmmu).

Taking a step back, I wonder if we really need to use iommufd for PSMMU activation and its stream table allocations?

Here are some facts:
- An iommufd has a ctx, that's one per VM. Similarly, a Realm has an RD.
- For an RMI command that needs an RD, it makes sense to be per iommufd ctx, e.g. RMI_VSMMU_* or RMI_VDEV_* commands.
- PSMMU commands are global; they don't need RD. So they don't seem necessary to tie to an iommufd ctx.

Instead, could the PSMMU activation be done after RMI_PSMMU_INFO check? Is there any reason not to do that? A safer timing might be at the device assignment stage?

Speaking of which, RMI_PSMMU_ST_L2_CREATE doesn't seem necessary to be invoked in a vdevice context either. Maybe it should align with iommufd idev's lifecycle?

Quoted 1 line · Nicolin Chen

On Mon, Apr 27, 2026 at 02:23:31PM +0530, Aneesh Kumar K.V (Arm) wrote:

Quoted 27 lines

.. and we demand userspace (VMM) to use IOMMU_VIOMMU_ALLOC ioctl, even if VMM does not actually expose a guest-level SMMU instance. Thus, no user_data.

I can get the reasoning behind the flow using this viommu/vdevice.

But, on the other hand, I can imagine that a Realm VSMMU would add a new flag with a user_data to this VIOMMU. Then, this flow would give some troubles to VMM (QEMU for example):

- For VM with a guest-level SMMU, QEMU creates a realm instance where IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 (with vsmmu) can be allocated.
- For VM w/o a guest-level SMMU, QEMU won't create such a realm instance, while still required to invoke the ioctl (w/o vsmmu).

Taking a step back, I wonder if we really need to use iommufd for PSMMU activation and its stream table allocations?

Here are some facts:
- An iommufd has a ctx, that's one per VM. Similarly, a Realm has an RD.
- For an RMI command that needs an RD, it makes sense to be per iommufd ctx, e.g. RMI_VSMMU_* or RMI_VDEV_* commands.
- PSMMU commands are global; they don't need RD. So they don't seem necessary to tie to an iommufd ctx.

Quoted 4 lines

Instead, could the PSMMU activation be done after RMI_PSMMU_INFO check? Is there any reason not to do that? A safer timing might be at the device assignment stage?

One of the reasons I added IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 was to avoid creating a psmmu object when we are not using a PCI passthrough VM. That is also the reason for all the refcounting around the psmmu objects.

If we are okay with creating psmmu objects early, then I guess we can go with the above approach.

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c int arm_vsmmu_init(struct iommufd_viommu *viommu, · line 465

Quoted 7 lines · Aneesh Kumar K.V

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

If so it needs its own enum value.

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Quoted 13 lines · Jason Gunthorpe

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c int arm_vsmmu_init(struct iommufd_viommu *viommu, · line 465

Quoted 7 lines · Aneesh Kumar K.V

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

Quoted 6 lines

If so it needs its own enum value.

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Actually, the PSMMU-only viommu object in this series only adds a type with no viommu data structure.

I've drafted the vsmmu case on top of the series, so it would add a structure reusing the same IOMMU_VIOMMU_TYPE_ARM_SMMUV3 with a flag:

Quoted 24 lines · Nicolin Chen

Quoted 13 lines · Jason Gunthorpe

Review line 465

Quoted 7 lines · Aneesh Kumar K.V

(quoted diff) +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

Quoted 6 lines

If so it needs its own enum value.

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Actually, the PSMMU-only viommu object in this series only adds a type with no viommu data structure.

Quoted 3 lines

I've drafted the vsmmu case on top of the series, so it would add a structure reusing the same IOMMU_VIOMMU_TYPE_ARM_SMMUV3 with a flag:

Oh. I replied too fast that I didn't notice this is under the standard IOMMU_VIOMMU_TYPE_ARM_SMMUV3 used by non-secure vSTE.

(quoted diff) +15 -0

+/**
+ * enum iommu_viommu_arm_realm_vsmmuv3_flags - Flags for ARM SMMUv3 Realm
+ * @IOMMU_VIOMMU_ARM_REALM_SMMUV3_FLAGS_VSMMU: Indicates whether the Realm has
+ *                                             a guest-visible VSMMU instance
+ */
+enum iommu_viommu_arm_realm_vsmmuv3_flags {
+	IOMMU_VIOMMU_ARM_REALM_SMMUV3_FLAGS_VSMMU = 1 << 0,
+};
+
+/**
+ * struct iommu_viommu_arm_realm_vsmmuv3 - ARM Realm VSMMUv3 parameters
+ *                                         (IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3)
+ * @flags: Combination of enum iommu_viommu_arm_realm_vsmmuv3_flags
+ * @reg_base: MMIO base address of the VSMMU in the guest VM
+ * @reg_top: MMIO top address of the VSMMU in the guest VM

I think this should work. But I still feel awkward that a non-vsmmu case has to allocate a viommu object for a set of RMI commands that don't need an Realm Descriptor.

Things could be cleaner if we allow RMI_PSMMU_ACTIVATE and RMI_PSMMU_ST_L2_CREATE to be independent on a viommu; then leave IOMMU_VIOMMU_TYPE_ARM_SMMUV3 to vsmmu-visiable case.

Quoted 15 lines · Nicolin Chen

(quoted diff) +15 -0

+/**
+ * enum iommu_viommu_arm_realm_vsmmuv3_flags - Flags for ARM SMMUv3 Realm
+ * @IOMMU_VIOMMU_ARM_REALM_SMMUV3_FLAGS_VSMMU: Indicates whether the Realm has
+ *                                             a guest-visible VSMMU instance
+ */
+enum iommu_viommu_arm_realm_vsmmuv3_flags {
+	IOMMU_VIOMMU_ARM_REALM_SMMUV3_FLAGS_VSMMU = 1 << 0,
+};
+
+/**
+ * struct iommu_viommu_arm_realm_vsmmuv3 - ARM Realm VSMMUv3 parameters
+ *                                         (IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3)
+ * @flags: Combination of enum iommu_viommu_arm_realm_vsmmuv3_flags
+ * @reg_base: MMIO base address of the VSMMU in the guest VM
+ * @reg_top: MMIO top address of the VSMMU in the guest VM

Review

Quoted 10 lines

(quoted diff) +10 -0

+ * @aidr: AIDR register value of the VSMMU in the guest VM
+ * @idr: IDR register values of the VSMMU in the guest VM
+ */
+struct iommu_viommu_arm_realm_vsmmuv3 {
+	__aligned_u64 flags;
+	__aligned_le64 reg_base;
+	__aligned_le64 reg_top;
+	__aligned_le64 aidr;
+	__aligned_le64 idr[7];
+};

Yeah, broadly what I would expect. Pass everything needed to execute RMI_VSMMU_CREATE through this struct.

Is there anything more than RMI_VSMMU_CREATE needed from a RMM perspective? What about that dpt/ats stuff?

Quoted 3 lines

I think this should work. But I still feel awkward that a non-vsmmu case has to allocate a viommu object for a set of RMI commands that don't need an Realm Descriptor.

It is for the RMI_VDEV_CREATE which needs the RD:

	case IOMMU_VDEVICE_TSM_BIND:
		rc = tsm_bind(vdev->idev->dev, kvm, vdev->virt_id);
		break;

It has to be tied to a vdevice on a viommu to pick up the kvm and vSID.

If we don't do that we need a new way to get the virt_id and kvm into the flow, which doesn't really seem worthwhile to me.

Quoted 3 lines

Things could be cleaner if we allow RMI_PSMMU_ACTIVATE and RMI_PSMMU_ST_L2_CREATE to be independent on a viommu; then leave IOMMU_VIOMMU_TYPE_ARM_SMMUV3 to vsmmu-visiable case.

This is why I asked in the other message if RMM spec is clear that PSMMU and STE are not required for anything but VDEV_CREATE. If so, the PDEV create and SPDM stuff is fuly independent.

Which is why I'm saying the split doesn't make sense. PSMMU, interrupts, STE, VDEV are all related objects that should be managed together by the SMMUv3 driver. You need a PDEV to create a STE, and you need a STE to create a VDEV.

PDEV is the SPDM channel and should be managed by the TSM driver.

So, I think the IOMMU_VDEVICE_TSM_BIND is not justified. "BIND" should happen when the SMMUv3 realm viommu ops create the vdevice. The same way the vcmdq sets up the VSID tables when the vdevice is created. Is there a reason to have it in its own command?

Further the implementation of IOMMU_VDEVICE_TSM_BIND in this series *requires* a viommu to work. So OK, let's lean into that. (to be clear I am saying delete tsm_bind)

Quoted 3 lines · Jason Gunthorpe

On Tue, Sep 01, 2026 at 10:13:04AM -0700, Nicolin Chen wrote:
Is there anything more than RMI_VSMMU_CREATE needed from a RMM perspective? What about that dpt/ats stuff?

DPT is a bit hacky currently..

Prior to RMM v2.0 ABI, DPT was allocated statically in RMM; there was no RMI command for DPT allocations.

Now, with RMM v2.0 ABI, DPT can be managed via RMIs. I am making it follow GPT at this point, similar to the static idea. But, in the long run, I will think of decoupling, but I haven't looked at that closely.

The viommu object here is an empty vehicle that almost has no use but to allow another vehicle (vdevice) on top to forward vBDF. It makes VMM code a bit awkward: there is no iommu instance, so VMM doesn't enable iommu driver, then there is no good place to alloc the viomu object for VIOMMU_TYPE_ARM_REALM_SMMUV3.

Quoted 13 lines · Jason Gunthorpe

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c int arm_vsmmu_init(struct iommufd_viommu *viommu, · line 465

Quoted 7 lines · Aneesh Kumar K.V

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

Quoted 6 lines

If so it needs its own enum value.

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Actually, the PSMMU-only viommu object in this series only adds a type with no viommu data structure.

I've drafted the vsmmu case on top of the series, so it would add a structure reusing the same IOMMU_VIOMMU_TYPE_ARM_SMMUV3 with a flag:

(quoted diff) +25 -0

+/**
+ * enum iommu_viommu_arm_realm_vsmmuv3_flags - Flags for ARM SMMUv3 Realm
+ * @IOMMU_VIOMMU_ARM_REALM_SMMUV3_FLAGS_VSMMU: Indicates whether the Realm has
+ *                                             a guest-visible VSMMU instance
+ */
+enum iommu_viommu_arm_realm_vsmmuv3_flags {
+	IOMMU_VIOMMU_ARM_REALM_SMMUV3_FLAGS_VSMMU = 1 << 0,
+};
+
+/**
+ * struct iommu_viommu_arm_realm_vsmmuv3 - ARM Realm VSMMUv3 parameters
+ *                                         (IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3)
+ * @flags: Combination of enum iommu_viommu_arm_realm_vsmmuv3_flags
+ * @reg_base: MMIO base address of the VSMMU in the guest VM
+ * @reg_top: MMIO top address of the VSMMU in the guest VM
+ * @aidr: AIDR register value of the VSMMU in the guest VM
+ * @idr: IDR register values of the VSMMU in the guest VM
+ */
+struct iommu_viommu_arm_realm_vsmmuv3 {
+	__aligned_u64 flags;
+	__aligned_le64 reg_base;
+	__aligned_le64 reg_top;
+	__aligned_le64 aidr;
+	__aligned_le64 idr[7];
+};

I think this should work. But I still feel awkward that a non-vsmmu case has to allocate a viommu object for a set of RMI commands that don't need an Realm Descriptor. Things could be cleaner if we allow RMI_PSMMU_ACTIVATE and RMI_PSMMU_ST_L2_CREATE to be independent on a viommu; then leave IOMMU_VIOMMU_TYPE_ARM_SMMUV3 to vsmmu-visiable case.

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c int arm_vsmmu_init(struct iommufd_viommu *viommu, · line 465

Quoted 7 lines · Aneesh Kumar K.V

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

If so it needs its own enum value.

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Quoted 15 lines · Jason Gunthorpe

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c int arm_vsmmu_init(struct iommufd_viommu *viommu, · line 465

Quoted 7 lines · Aneesh Kumar K.V

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

If so it needs its own enum value.

Quoted 4 lines

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Based on feedback on other emails in this thread, I have now implemented this without using a vdevice or viommu. This should make the CCA and non-CCA cases similar.

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-realm.c +6 -0

+int arm_smmu_realm_tsm_bind(struct device *dev, struct kvm *kvm)
+{
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+	int ret;
+
+	if (!kvm_is_realm(kvm))

iommufd_device_tsm_op_ioctl()

	switch (cmd->type) {
	case IOMMU_DEVICE_TSM_BIND:
		if (!idev->tsm_iommu_bound && ops->tsm_bind) {
			if (WARN_ON_ONCE(!ops->tsm_unbind)) {
				ret = -EOPNOTSUPP;
				break;
			}
			ret = ops->tsm_bind(idev->dev, kvm);
			if (ret)
				break;
			idev->tsm_iommu_bound = true;
			iommu_bound = true;
		}
		ret = tsm_bind(idev->dev, kvm, cmd->tdi_id);

Quoted 22 lines · Aneesh Kumar K.V

Quoted 19 lines · Jason Gunthorpe

Review line 465

Quoted 7 lines · Aneesh Kumar K.V

(quoted diff) +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

If so it needs its own enum value.

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Quoted 3 lines

Based on feedback on other emails in this thread, I have now implemented this without using a vdevice or viommu. This should make the CCA and non-CCA cases similar.

That wasn't the feedback. The feedback was to use the viommu and not make a bunch of new stuff..

Quoted 23 lines · Jason Gunthorpe

Quoted 22 lines · Aneesh Kumar K.V

Quoted 19 lines · Jason Gunthorpe

Review line 465

Quoted 7 lines · Aneesh Kumar K.V

(quoted diff) +3 -0

@@ -463,14 +460,13 @@
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
 	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		if (arm_smmu_is_realm_viommu(viommu))
+			return arm_realm_smmu_v3_init(viommu, user_data);
+

I think the realm vsmmu is going to require a different info struct than the normal psmmu case, isn't it?

If so it needs its own enum value.

It would be nice to see a draft patch showing how the real vsmmu works on top of the RMM spec for it. If we are using a viommu object then non-vsmmu case should be identical just with an option in the info struct to not create the vsmmu object.

Quoted 7 lines

Quoted 3 lines

Based on feedback on other emails in this thread, I have now implemented this without using a vdevice or viommu. This should make the CCA and non-CCA cases similar.

That wasn't the feedback. The feedback was to use the viommu and not make a bunch of new stuff..

That rework was done before I saw your discussion with Nicolin.

To reiterate, for this configuration:

- The viommu will use a stage-1 bypass configuration.
- A new IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 type will create the viommu. The psmmu will be activated at this point to avoid creating psmmu objects early. We will reference-count it to ensure that the same psmmu is shared across realm guests.
- Creating a vdevice will invoke SMC_RMI_PSMMU_ST_L2_CREATE.

I am unclear about the vdev_create suggestion. Creating a vdevice requires an RD, which is created later in the flow above. How do you suggest linking vdevice_alloc to vdev_create?

static int arm_realm_smmu_v3_vdevice_init(struct iommufd_vdevice *vdev)

{
	struct device *dev = iommufd_vdevice_to_device(vdev);
	struct kvm *kvm = vdev->viommu->kvm_file->private_data;
	struct arm_smmu_device *smmu;
	struct arm_smmu_stream *stream;
	struct arm_smmu_master *master;
	unsigned long rmi_ret = 0;
	unsigned long l2_sid;
	int ret;

	if (!tsm_is_configured(dev))
		return 0;

	master = dev_iommu_priv_get(dev);
	/* FIXME which stream to pick */
	/* At this moment, iommufd only supports PCI device that has one SID */
	stream = &master->streams[0];
	smmu = master->smmu;

	l2_sid = ALIGN_DOWN(stream->id, STRTAB_NUM_L2_STES);

	{
		guard(mutex)(&smmu->realm.mutex);

		if (!arm_realm_smmu_active(smmu))
			return -EINVAL;

		ret = rmi_psmmu_st_l2_create(smmu->base_phys, l2_sid,
					     &rmi_ret);
		if (ret || rmi_ret) {
			if (!ret)
				return -EIO;
			if (RMI_RETURN_STATUS(rmi_ret) != RMI_ERROR_PSMMU_ST ||
			    RMI_RETURN_INDEX(rmi_ret) != 2) {
				dev_warn(dev, "failed to create realm stream mapping\n");

				return -EIO;
			}
			/* The L2 stream table already exists. */
		}
	}

	vdev->destroy = arm_realm_smmu_v3_vdevice_destroy;
	return tsm_bind(dev, kvm, vdev->virt_id);

(quoted diff) +3 -1

@@ -513,10 +514,16 @@ static ssize_t cca_tsm_guest_req(struct pci_tdi *tdi,
 		if (copy_from_user((void *)&req_obj, req.user, req_len))
 			return -EFAULT;
 
-		if (req_obj.tdi_state != RHI_DA_TDI_CONFIG_RUN)
+		switch (req_obj.tdi_state) {
+		case RHI_DA_TDI_CONFIG_UNLOCKED:
+			return cca_vdev_device_unlock(pdev);

Quoted 42 lines · Aneesh Kumar K.V

static int arm_realm_smmu_v3_vdevice_init(struct iommufd_vdevice *vdev)

{
	struct device *dev = iommufd_vdevice_to_device(vdev);
	struct kvm *kvm = vdev->viommu->kvm_file->private_data;
	struct arm_smmu_device *smmu;
	struct arm_smmu_stream *stream;
	struct arm_smmu_master *master;
	unsigned long rmi_ret = 0;
	unsigned long l2_sid;
	int ret;

	if (!tsm_is_configured(dev))
		return 0;

	master = dev_iommu_priv_get(dev);
	/* FIXME which stream to pick */
	/* At this moment, iommufd only supports PCI device that has one SID */
	stream = &master->streams[0];
	smmu = master->smmu;

	l2_sid = ALIGN_DOWN(stream->id, STRTAB_NUM_L2_STES);

	{
		guard(mutex)(&smmu->realm.mutex);

		if (!arm_realm_smmu_active(smmu))
			return -EINVAL;

		ret = rmi_psmmu_st_l2_create(smmu->base_phys, l2_sid,
					     &rmi_ret);
		if (ret || rmi_ret) {
			if (!ret)
				return -EIO;
			if (RMI_RETURN_STATUS(rmi_ret) != RMI_ERROR_PSMMU_ST ||
			    RMI_RETURN_INDEX(rmi_ret) != 2) {
				dev_warn(dev, "failed to create realm stream mapping\n");
				return -EIO;
			}
			/* The L2 stream table already exists. */
		}
	}

Quoted 2 lines

	vdev->destroy = arm_realm_smmu_v3_vdevice_destroy;
	return tsm_bind(dev, kvm, vdev->virt_id);

I think we should drop tsm_bind() as an abstraction. It doesn't make sense to take that round about path when we are calling RMIs directly above. It was intended to be an abstraction, but it isn't working out with this viommu based abstraction.

Quoted 17 lines

(quoted diff) +8 -3

@@ -513,10 +514,16 @@ static ssize_t cca_tsm_guest_req(struct pci_tdi *tdi,
 		if (copy_from_user((void *)&req_obj, req.user, req_len))
 			return -EFAULT;
 
-		if (req_obj.tdi_state != RHI_DA_TDI_CONFIG_RUN)
+		switch (req_obj.tdi_state) {
+		case RHI_DA_TDI_CONFIG_UNLOCKED:
+			return cca_vdev_device_unlock(pdev);
+		case RHI_DA_TDI_CONFIG_LOCKED:
+			return cca_vdev_device_lock(pdev);
+		case RHI_DA_TDI_CONFIG_RUN:
+			return cca_vdev_device_start(pdev);
+		default:
 			return -EINVAL;
-
-		return cca_vdev_device_start(pdev);

Review static ssize_t cca_tsm_guest_req(struct pci_tdi *tdi, · line 527

Quoted 2 lines

(quoted diff) +1 -0

+		}
 	}

This stuff cannot flow through sysfs. The VMM must support running in a sandbox so it cannot easially call out to sysfs while the VM is running. That makes the sandboxing more complex and ugly. The flow we have now relies on fd passing from the launcher into the sandbox to get things like vfio and iommufd into the VMM.

So these actions really should work the same way unless there is a strong reason to do otherwise.

Given these are all acting on bound devices, and those can only be created by iommufd, it makes more sense to feed the operations through iommufd into the viommu and vdevice ops. AMD wanted to create such general command ops anyhow for their viommu emulation (non cc).

And.. then you don't need struct pci_tdi. The vdevice is effectively the tdi and the existing locking scheme in iommufd for vdevice takes care of everything the tsm code was trying to do, except in a way that applies to every viommu out there.. This actually makes a lot of sense because the tdi is not really separable from vfio. You cannot have a tdi without a kvm and you cannot link a pci dev to a kvm without vfio.

Finally, there is really nothing about the viommu_ops that has much to do with smmuv3. It would be fairly straightforward for the tsm_ops to be able to create the viommu and provide the viommu_ops. We can get there based on the IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3.

This then would open up a much nicer split where arm-cca-guest.ko can provide a realm smmuv3, and inside those viommu_ops are all the realm & tdi related ops, psmmu, vsmmu, vdevice, "guest req".

The arm-cca-guest can just make a simple function call to SMMUv3 to get the phys and interrupts. Somehow I think Will would like this better than adding to SMMUv3.

Now that the viommu stuff is more developed on the iommufd end, and the RMM spec is more complete with vsmmu, I think this arrangement becomes visible.

What do you think?

Jason

Quoted 8 lines

Quoted 2 lines

	vdev->destroy = arm_realm_smmu_v3_vdevice_destroy;
	return tsm_bind(dev, kvm, vdev->virt_id);

I think we should drop tsm_bind() as an abstraction. It doesn't make sense to take that round about path when we are calling RMIs directly above. It was intended to be an abstraction, but it isn't working out with this viommu based abstraction.

I was considering using the viommu only for explicit pSMMU/vSMMU setup, SID/STE management, and realm stream-table creation. I expected other operations, such as vdevice creation and lock/run state transitions, to be driven by IOMMUFD ioctls and dispatched to the TSM backend through abstractions such as tsm_bind() and tsm_guest_req(). This results in the following split:

- Arm SMMU: pSMMU/vSMMU setup, SID/STE management, and Realm stream-table creation.
- PCI TSM: device association, bind lifetime, DSM lookup, and TDI state.
- arm-cca-host: PDEV/VDEV operations, TDISP transitions, reports, measurements, and IDE interaction.
- IOMMUFD: route userspace requests using the vdevice ID.

Quoted 16 lines

Quoted 16 lines

(quoted diff) +8 -3

@@ -513,10 +514,16 @@ static ssize_t cca_tsm_guest_req(struct pci_tdi *tdi,
 		if (copy_from_user((void *)&req_obj, req.user, req_len))
 			return -EFAULT;
 
-		if (req_obj.tdi_state != RHI_DA_TDI_CONFIG_RUN)
+		switch (req_obj.tdi_state) {
+		case RHI_DA_TDI_CONFIG_UNLOCKED:
+			return cca_vdev_device_unlock(pdev);
+		case RHI_DA_TDI_CONFIG_LOCKED:
+			return cca_vdev_device_lock(pdev);
+		case RHI_DA_TDI_CONFIG_RUN:
+			return cca_vdev_device_start(pdev);
+		default:
 			return -EINVAL;
-
-		return cca_vdev_device_start(pdev);

Quoted 9 lines

Review static ssize_t cca_tsm_guest_req(struct pci_tdi *tdi, · line 527

Quoted 2 lines

(quoted diff) +1 -0

+		}
 	}

This stuff cannot flow through sysfs. The VMM must support running in a sandbox so it cannot easially call out to sysfs while the VM is running. That makes the sandboxing more complex and ugly. The flow we have now relies on fd passing from the launcher into the sandbox to get things like vfio and iommufd into the VMM.

This does not go through sysfs. It uses the following IOMMUFD ioctl:

IOCTL_OP(IOMMU_VDEVICE_TSM_REQ, iommufd_vdevice_tsm_req_ioctl,
struct iommu_vdevice_tsm_req, tsm_code),

I need to spend more time considering your suggestion to handle guest requests through viommu_ops rather than as TSM backend operations. The split described below seemed more natural to me.

- Arm SMMU: pSMMU/vSMMU setup, SID/STE management, and Realm stream-table creation.
- PCI TSM: device association, bind lifetime, DSM lookup, and TDI state.
- arm-cca-host: PDEV/VDEV operations, TDISP transitions, reports, measurements, and IDE interaction.
- IOMMUFD: route userspace requests using the vdevice ID.

It is not yet clear to me whether operations such as MMIO validation (TSM_REQ_VALIDATE_MMIO), setting the TDI lock/unlock/run state (TSM_REQ_SET_TDI_STATE), querying TDISP object details (TSM_REQ_OBJECT_INFO), and reading or regenerating TDISP objects (TSM_REQ_READ_OBJECT and TSM_REQ_REGEN_OBJECT) belong in viommu_ops.

Meanwhile, I will clean up my changes and post them as a patch series so that we can review them more closely?

Quoted 16 lines · Aneesh Kumar K.V

Quoted 8 lines

Quoted 2 lines

	vdev->destroy = arm_realm_smmu_v3_vdevice_destroy;
	return tsm_bind(dev, kvm, vdev->virt_id);

I think we should drop tsm_bind() as an abstraction. It doesn't make sense to take that round about path when we are calling RMIs directly above. It was intended to be an abstraction, but it isn't working out with this viommu based abstraction.

I was considering using the viommu only for explicit pSMMU/vSMMU setup, SID/STE management, and realm stream-table creation. I expected other operations, such as vdevice creation and lock/run state transitions, to be driven by IOMMUFD ioctls and dispatched to the TSM backend through abstractions such as tsm_bind() and tsm_guest_req(). This results in the following split:

Quoted 6 lines

- Arm SMMU: pSMMU/vSMMU setup, SID/STE management, and Realm stream-table creation.
- PCI TSM: device association, bind lifetime, DSM lookup, and TDI state.
- arm-cca-host: PDEV/VDEV operations, TDISP transitions, reports, measurements, and IDE interaction.
- IOMMUFD: route userspace requests using the vdevice ID.

I don't really like it, I think the viommu code should handle the VDEV and VSMU, TSM should handle SPDM.

"TDI state" is the existance of a iommufd vdevice, it doesn't make sense to have a seperate "TDI state" concept and a parallel set of APIs out side the iommufd object model that concretely defines the lifecylce of the VDEV.

Is there a reason to have this split aside from it matches the tsm prototypes that were sketched?

Quoted 26 lines

Quoted 16 lines

Quoted 16 lines

(quoted diff) +8 -3

@@ -513,10 +514,16 @@ static ssize_t cca_tsm_guest_req(struct pci_tdi *tdi,
 		if (copy_from_user((void *)&req_obj, req.user, req_len))
 			return -EFAULT;
 
-		if (req_obj.tdi_state != RHI_DA_TDI_CONFIG_RUN)
+		switch (req_obj.tdi_state) {
+		case RHI_DA_TDI_CONFIG_UNLOCKED:
+			return cca_vdev_device_unlock(pdev);
+		case RHI_DA_TDI_CONFIG_LOCKED:
+			return cca_vdev_device_lock(pdev);
+		case RHI_DA_TDI_CONFIG_RUN:
+			return cca_vdev_device_start(pdev);
+		default:
 			return -EINVAL;
-
-		return cca_vdev_device_start(pdev);

Quoted 9 lines

Review static ssize_t cca_tsm_guest_req(struct pci_tdi *tdi, · line 527

Quoted 2 lines

(quoted diff) +1 -0

+		}
 	}

This stuff cannot flow through sysfs. The VMM must support running in a sandbox so it cannot easially call out to sysfs while the VM is running. That makes the sandboxing more complex and ugly. The flow we have now relies on fd passing from the launcher into the sandbox to get things like vfio and iommufd into the VMM.

Quoted 4 lines

This does not go through sysfs. It uses the following IOMMUFD ioctl:

IOCTL_OP(IOMMU_VDEVICE_TSM_REQ, iommufd_vdevice_tsm_req_ioctl,
struct iommu_vdevice_tsm_req, tsm_code),

I see, I saw this when I was grepping:

static ssize_t tsm_request_store(struct device *dev,
struct device_attribute *attr, const char *__buf, size_t count)

Which the name and sysfs parts confused me, it looked like core code. Turns out it is the sample driver.

Quoted 11 lines

I need to spend more time considering your suggestion to handle guest requests through viommu_ops rather than as TSM backend operations. The split described below seemed more natural to me.

- Arm SMMU: pSMMU/vSMMU setup, SID/STE management, and Realm stream-table creation.
- PCI TSM: device association, bind lifetime, DSM lookup, and TDI state.
- arm-cca-host: PDEV/VDEV operations, TDISP transitions, reports, measurements, and IDE interaction.
- IOMMUFD: route userspace requests using the vdevice ID.

Quoted 5 lines

It is not yet clear to me whether operations such as MMIO validation (TSM_REQ_VALIDATE_MMIO), setting the TDI lock/unlock/run state (TSM_REQ_SET_TDI_STATE), querying TDISP object details (TSM_REQ_OBJECT_INFO), and reading or regenerating TDISP objects (TSM_REQ_READ_OBJECT and TSM_REQ_REGEN_OBJECT) belong in viommu_ops.

They way I'm looking at it is the "TDI" is the RMM's VDEV.

The RMM VDEV has to be created from an iommufd vdevice and be 1:1 with that. Thus the iommufd vdevice is the TDI.

Therefore, the struct cca_host_tdi should be the driver specific struct of a struct iommufd_vdevice.

So if you want to get the information in the cca_host_tdi you have to come in through the viommu ops with a vdevice object in hand.

This seems like a much cleaner logical seperation than trying to maintain a cca_host_tdi external to the iommufd vdevice object that actually directly controls its lifecycle.

So if you did this then the cca_tsm_guest_req would flow through some generic viommu op similar to what AMD proposed:

struct iommu_viommu_op {
	__u32 size;
	__u32 vdevice_id;
	__u32 viommu_type;  // enum iommu_viommu_type
	__u32 operation; // unique enum per type
	__u32 req_len;
	__u32 resp_len;
	__aligned_u64 req_uptr;
	__aligned_u64 resp_uptr;
};

enum {
IOMMUFD_VIOMMU_OP_CCA_OBJECT_SIZE
IOMMUFD_VIOMMU_OP_CCA_OBJECT_READ
IOMMUFD_VIOMMU_OP_CCA_UPDATE_INTERFACE_REPORT
IOMMUFD_VIOMMU_OP_CCA_UPDATE_MEASUREMENTS
IOMMUFD_VIOMMU_OP_CCA_VDEV_MAP
IOMMUFD_VIOMMU_OP_CCA_SET_TDI_STATE

And none of the TDI information leaks out side the iommufd world.

The iommufd side change to introduce CCA support would then only be adding the IOCTL for iommu_viommu_op and some fiddling with how the viommu is created.

Then everyone can use the same infrastructure for their related problems.

Quoted 2 lines

Meanwhile, I will clean up my changes and post them as a patch series so that we can review them more closely?

Well, OK, but I'm am still very interested in focusing on the viommu. I cc'd you on another thread so you can see the other topics I'm looking at here that all come down to very similar patterns.

I think the TDI related tsm ops were developed well before iommufd was completed and you have raised a good point now to re-evaluate if the we even need them since we now understand that the iommufd vdevice is in fact the concrete TDI object in the uAPI.

I looked into this, and it becomes fairly complicated. We can move all vdev/TDI-related code to arm-smmu-realm-v3.c, but that would result in:

1. Adding more CCA-specific code to the SMMU driver.
2. arm-cca-host continuing to own the TSM link setup (IDE).
3. Adding callbacks from the device communication helpers back into arm-cca-host, since device communication still goes through DOE.
4. Moving the device communication helpers to firmware/arm-rmm and adding something like:

The locking also becomes more complex. Unlocking a vdev can trigger a stream key refresh, which is owned by arm-cca-host. Currently, the locking is simpler, using pci_tsm_rwsem and pci_tsm_pf0::lock. With the SMMU driver owning the vdev/TDI, we would have:

- pci_tsm_rwsem protecting the lifetime and registration state of pdev->tsm.
- pci_tsm::tdi_lock protecting the function's pci_tdi pointer.

Quoted 2 lines · Aneesh Kumar K.V

I looked into this, and it becomes fairly complicated. We can move all vdev/TDI-related code to arm-smmu-realm-v3.c, but that would result in:

I was going for the opposite, you'd move everything out of arm-smmu-v3 and into the arm-cca-host and obtain the viommu through tsm_ops not through iommu_ops.

I guess I pointed to that in another email.

The only thing arm-smmu-v3 should provide is a simple function to give the pdev phys and irq parameters. arm-cca-host calls that when it creates an viommu object.

Quoted 9 lines · Jason Gunthorpe

Quoted 2 lines · Aneesh Kumar K.V

I looked into this, and it becomes fairly complicated. We can move all vdev/TDI-related code to arm-smmu-realm-v3.c, but that would result in:

I was going for the opposite, you'd move everything out of arm-smmu-v3 and into the arm-cca-host and obtain the viommu through tsm_ops not through iommu_ops.

Quoted 6 lines

I guess I pointed to that in another email.

The only thing arm-smmu-v3 should provide is a simple function to give the pdev phys and irq parameters. arm-cca-host calls that when it creates an viommu object.

So ended up with

static const struct tsm_viommu_ops cca_tsm_viommu_ops = {
.owner = THIS_MODULE,
.type = IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3, .get_size = cca_viommu_get_size,
.init = cca_viommu_init,
};

struct cca_viommu {
	struct cca_psmmu *psmmu;
	struct iommu_viommu_provider *iommu_provider;
	const struct iommufd_viommu_ops *iommu_ops; // backing SMMU ops

static struct iommu_domain * cca_viommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
const struct iommu_user_data *user_data)

{
	struct cca_viommu *cca = viommu->provider_data;

	return cca->iommu_ops->alloc_domain_nested(viommu, flags, user_data);
}

static int cca_viommu_cache_invalidate(struct iommufd_viommu *viommu,
struct iommu_user_data_array *array)

{
	struct cca_viommu *cca = viommu->provider_data;

	return cca->iommu_ops->cache_invalidate(viommu, array);
}

struct cca_vdevice {
	struct iommufd_vdevice core;
	struct pci_tsm_context *tsm_context;
	struct cca_host_tdi host_tdi;

	u32 l2_sid;
};

static const struct iommufd_viommu_ops cca_viommu_ops = {
.destroy = cca_viommu_destroy,
.alloc_domain_nested = cca_viommu_alloc_domain_nested, .cache_invalidate = cca_viommu_cache_invalidate, .vdevice_size = VDEVICE_STRUCT_SIZE(struct cca_vdevice, core), .vdevice_init = cca_vdevice_init,
.vdevice_tsm_req = cca_vdevice_tsm_req,
};

and on iommu side

struct iommu_viommu_provider {
	size_t size;
	int (*init)(struct iommufd_viommu *viommu, struct device *dev,
		    enum iommu_viommu_type type,
		    struct iommu_domain *parent_domain,
		    const struct iommu_user_data *user_data);
	int (*get_params)(struct iommu_viommu_provider *provider,
			  struct device *dev, enum iommu_viommu_type type,
			  void *params, size_t params_size);
	void (*release)(struct iommu_viommu_provider *provider);
	void *data;
};

The TSM disconnect path will now fail while any vdevice is alive or active.

Destroying a vdevice will unlock and destroy the VDEV.

I think we can also unmap its MMIO mappings at that point, provided we track the mapping requests in a list alongside the vdevice details.

All CCA operations will use pci_tsm_pf0::lock, though I think the locking can be made more fine-grained.

I will send a cleaned-up series so that we can review the code changes.

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, September 9, 2026 8:46 PM

On Wed, Sep 09, 2026 at 03:39:31PM +0530, Aneesh Kumar K.V wrote:

Quoted 2 lines

The TSM disconnect path will now fail while any vdevice is alive or active.

Yes, that's makes sense.

Quoted 1 line

Destroying a vdevice will unlock and destroy the VDEV.

Yes

Quoted 3 lines

I think we can also unmap its MMIO mappings at that point, provided we track the mapping requests in a list alongside the vdevice details.

The mmio is owned by vfio, there should be a handshake in VFIO to remove mmio when it becomes private, otherwise I don't think we need to do anything more?

Quoted 3 lines

All CCA operations will use pci_tsm_pf0::lock, though I think the locking can be made more fine-grained.

Sure

Quoted 1 line

I will send a cleaned-up series so that we can review the code changes.

Does it seems reasonable to you? Was there any oddness with modeling the vdev/bind through the viommu?

Quoted 3 lines

The mmio is owned by vfio, there should be a handshake in VFIO to remove mmio when it becomes private, otherwise I don't think we need to do anything more?

for TDX a callback in VFIO is invoked to revoke the dmabuf.

btw I didn't see any dmabuf related in this series (even in a hackish way as Yilun sent out last year). Is it not required by CCA or just skipped now waiting for the framework to be settled?

So viommu will become the abstraction point for tsm operations and directly talks to the tsm driver instead of going through the merged tsm_ops (bind/unbind/guest_req).

currently what tsm_ops provides additionally is more about synchronization with connect/disconnect, otherwise just calls into underlying tsm driver. As long as viommu creation holds a reference to tsm then disconnection is blocked then it's safe.

and IOMMU_VDEVICE_TSM_BIND will be removed.

Seems originally this series
does SMC_RMI_VDEV_CREATE and SMC_RMI_VDEV_LOCK both in tsm bind op. Then the proposal is moving them all to vdevice creation time so no separate bind step is required.

Does it mean that ARM CCA is essentially an early-bind model i.e. the TDI starts in locked state from guest p.o.v.?

but https://lore.kernel.org/all/[email protected]/ seems to indicate that guest_req() still handles the bind request from the guest:

"
- cca_tsm_guest_req() now handles TSM_REQ_SET_TDI_STATE requests for the unlocked, locked, and running states.
"

a bit confusing, but maybe due to stale info cited from different timing...

for TDX we're currently pursuing a late-bind model (i.e. TDI starts in shared state in guest), so a separate bind interface is still necessary. We could look at whether it should be an explicit @bind viommu op or carried by @guest_req (say, if most preparation works can be moved to vdevice creation).

btw per past discussions VFIO should block some operations (reset, etc.) while a TDI is locked. Do we expect a callback into VFIO to notify the locked state, or require VFIO's own interface to place a cdev into a special state which prevents sensitive operations even before binding it to iommufd (and allow creating TDI vdevice only on such idevice)?

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, September 10, 2026 8:47 PM

Quoted 6 lines · Kevin

Quoted 3 lines

The mmio is owned by vfio, there should be a handshake in VFIO to remove mmio when it becomes private, otherwise I don't think we need to do anything more?

for TDX a callback in VFIO is invoked to revoke the dmabuf.

Quoted 3 lines

btw I didn't see any dmabuf related in this series (even in a hackish way as Yilun sent out last year). Is it not required by CCA or just skipped now waiting for the framework to be settled?

I think just one step at a time :\ I think CCA needs it as well, you also cannot safely leave private MMIO floating around the host userspace on CCA.

Quoted 4 lines

So viommu will become the abstraction point for tsm operations and directly talks to the tsm driver instead of going through the merged tsm_ops (bind/unbind/guest_req).

More precisely for what was being called the TDI - ie it is the abstraction point for the host side of a guest virtual PCI device.

iommufd vdevice already represents this, so we use it again.

Quoted 4 lines

currently what tsm_ops provides additionally is more about synchronization with connect/disconnect, otherwise just calls into underlying tsm driver. As long as viommu creation holds a reference to tsm then disconnection is blocked then it's safe.

Yes, this already has to be true or our lifecylce model is nonsense. Once the iommufd creates the vdevice the guest is running and we cannot disconnect it without unplugging it from the guest. Thus the locking can rely on that. Within an iommufd ioctl context the vdev and underlying connection must be stable.

Quoted 2 lines

and IOMMU_VDEVICE_TSM_BIND will be removed.

Replaced, the same logical operations should flow through the viommu command ioctl

Quoted 4 lines

Seems originally this series
does SMC_RMI_VDEV_CREATE and SMC_RMI_VDEV_LOCK both in tsm bind op. Then the proposal is moving them all to vdevice creation time so no separate bind step is required.

Hmm, lock still should be guest visible on CCA, and the device has to start up in the guest as TDISP unlocked. I'm not 100% on the RMI side of the flow, but this does not look right.

VDEV create should be done before the realm is started. We need this setup right because Linux VM is going to validate the vdev during boot if it is affiliated to a vSMMU. lock should be done only when requested by the guest. The guest must start with a normal unlocked T=0 PCI device.

Quoted 3 lines

Does it mean that ARM CCA is essentially an early-bind model i.e. the TDI starts in locked state from guest p.o.v.?

No

Quoted 1 line

but https://lore.kernel.org/all/[email protected]/ seems to

indicate

Quoted 6 lines

that guest_req() still handles the bind request from the guest:

"
- cca_tsm_guest_req() now handles TSM_REQ_SET_TDI_STATE requests for the unlocked, locked, and running states.
"

Right the guest side request to lock should come that way.

Quoted 7 lines

a bit confusing, but maybe due to stale info cited from different timing...

for TDX we're currently pursuing a late-bind model (i.e. TDI starts in shared state in guest), so a separate bind interface is still necessary. We could look at whether it should be an explicit @bind viommu op or carried by @guest_req (say, if most preparation works can be moved to vdevice

creation).

It should go over the viommu command ioctl. It looked like there was alway some iommu specific format to the command in the tsm version of this, so that is the appropriate way.

Quoted 5 lines

btw per past discussions VFIO should block some operations (reset, etc.) while a TDI is locked. Do we expect a callback into VFIO to notify the locked state, or require VFIO's own interface to place a cdev into a special state which prevents sensitive operations even before binding it to iommufd (and allow creating TDI vdevice only on such idevice)?

At a minimum I think we have to synchronize with vfio, somehow, that when the device enforces its private MMIO so it can unmap it. Thay may be an argument we need a few standard viommu ops so the core code can capture lock/unlocks and do this prep work. Or maybe a special cdev state is simpler.

This should be figured out before any uapi is settled.

I don't know about reset, that sounds like something the pci core should deal with?

Quoted 5 lines

Yes, this already has to be true or our lifecylce model is nonsense. Once the iommufd creates the vdevice the guest is running and we cannot disconnect it without unplugging it from the guest. Thus the locking can rely on that. Within an iommufd ioctl context the vdev and underlying connection must be stable.

btw looks a related suggestion is having the tsm driver (arm-cca-host here) implement the viommu ops while leaving the original smmu driver largely intact, and "obtain the viommu through tsm_ops not through iommu_ops".

It's not super clear to me about the implication behind after reading related discussions and what the "obtain" part means.

Could you help elaborate this part?

The old way is kind of:

vdevice_tsm_ops -> tsm_ops -> tsm driver -> iommu driver

then it will become:

viommu_ops (tsm related) -> tsm_driver

where tsm driver will fully handle tsm related viommu_ops?

btw I held the impression of blocked operations from past discussion [1]. Initial attempt tried to proactively unbind the TDI upon any operations which may transit the TDI to the ERROR state. Then the suggestion at the moment was:

" But now the suggestion is never let VFIO do unbind, instead VFIO should block these operations when device is bound. "

[1] https://lore.kernel.org/all/aEFmPaYorqaYCKBY@yilunxu-OptiPlex-7050/

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, September 15, 2026 9:43 PM

Quoted 18 lines · Kevin

Quoted 5 lines

Yes, this already has to be true or our lifecylce model is nonsense. Once the iommufd creates the vdevice the guest is running and we cannot disconnect it without unplugging it from the guest. Thus the locking can rely on that. Within an iommufd ioctl context the vdev and underlying connection must be stable.

btw looks a related suggestion is having the tsm driver (arm-cca-host here) implement the viommu ops while leaving the original smmu driver largely intact, and "obtain the viommu through tsm_ops not through iommu_ops".

It's not super clear to me about the implication behind after reading related discussions and what the "obtain" part means.

Could you help elaborate this part?

The old way is kind of:

Quoted 7 lines

vdevice_tsm_ops -> tsm_ops -> tsm driver -> iommu driver

then it will become:

viommu_ops (tsm related) -> tsm_driver

where tsm driver will fully handle tsm related viommu_ops?

At least for ARM there is effectively no entanglement with the actual host iommu driver. The viommu is entirely provided by software in the RMM world, so it can have its own dedicated driver. In ARM T=1 transactions are alwayus routed to the RMM's iommu and there is no relation to the host.

I am interested how Intel works here, but I thought it was similar.

AMD is different and I suspect AMD will have to continue to use the viommu from the AMD iommu driver, but I am not sure.

Quoted 5 lines

btw I held the impression of blocked operations from past discussion [1]. Initial attempt tried to proactively unbind the TDI upon any operations which may transit the TDI to the ERROR state. Then the suggestion at the moment was:

With this construction the iommufd vdev is created and permanently exist as long as the VM and viommu exist.

How/when the tsm driver links this to a arch specific "bind/unbind" operation is more up to that driver, but I would expect what is thought of as "bind" should be the affiliation of the device's T=1 stream with the viommu and the target VM. It should not be sensitive to the TDISP state.

That is not prohibited, the TSM driver could do some auto "bind/unbind" whatever that means triggered by ops or tdisp state changing under the covers. But this cannot leak out as some kind of asynchronous vdev destruction.

Quoted 5 lines

" But now the suggestion is never let VFIO do unbind, instead VFIO should block these operations when device is bound. "

[1] https://lore.kernel.org/all/aEFmPaYorqaYCKBY@yilunxu-OptiPlex-7050/

IDK, if the host wants to put a device into error I don't see why the kernel should block it? There are many reasons a device can reach error, that needs to be handled.

Quoted 7 lines

At least for ARM there is effectively no entanglement with the actual host iommu driver. The viommu is entirely provided by software in the RMM world, so it can have its own dedicated driver. In ARM T=1 transactions are alwayus routed to the RMM's iommu and there is no relation to the host.

I am interested how Intel works here, but I thought it was similar.

Largely yes. Main difference at Intel side is that TDX still relies on the host to initiate iotlb invalidation (upon notification from KVM on S-EPT change). Currently we put this logic in intel-iommu driver but it's more about wrapping invalidation info and passing it to the firmware. Moving it into the tsm driver should be straightforward.

Maybe there'll be other subtle connections to host iommu driver but it doesn't sound a hard problem to solve.

Quoted 2 lines

AMD is different and I suspect AMD will have to continue to use the viommu from the AMD iommu driver, but I am not sure.

ARM/Intel may support guest viommu in the future.

So AMD's case is a good reference.

Quoted 5 lines

How/when the tsm driver links this to a arch specific "bind/unbind" operation is more up to that driver, but I would expect what is thought of as "bind" should be the affiliation of the device's T=1 stream with the viommu and the target VM. It should not be sensitive to the TDISP state.

Not sure about this part.

Each arch has its own definition about the binding flow (about 'how'), but sharing a common step by sending TDISP message to transit the TDI into the CONFIG_LOCKED state upon guest request (i.e. 'when').

According to the TDISP spec, memory reads/writes with T bit set is accepted only when the TDI is in RUN state (except MSI/MSI-X writes are allowed with T bit set in LOCKED but I don't think any arch supports it yet).

So your definition of 'bind' essentially affiliate it to the RUN state?

Quoted 4 lines

That is not prohibited, the TSM driver could do some auto "bind/unbind" whatever that means triggered by ops or tdisp state changing under the covers. But this cannot leak out as some kind of asynchronous vdev destruction.

Maybe it'd be clearer using an example e.g. ARM to clarify the suggested split. Or wait for Aneesh's next version...

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, September 16, 2026 8:40 PM

Quoted 14 lines · Kevin

Quoted 7 lines

At least for ARM there is effectively no entanglement with the actual host iommu driver. The viommu is entirely provided by software in the RMM world, so it can have its own dedicated driver. In ARM T=1 transactions are alwayus routed to the RMM's iommu and there is no relation to the host.

I am interested how Intel works here, but I thought it was similar.

Largely yes. Main difference at Intel side is that TDX still relies on the host to initiate iotlb invalidation (upon notification from KVM on S-EPT change). Currently we put this logic in intel-iommu driver but it's more about wrapping invalidation info and passing it to the firmware. Moving it into the tsm driver should be straightforward.

Quoted 2 lines

Maybe there'll be other subtle connections to host iommu driver but it doesn't sound a hard problem to solve.

Okay, so I saw the driver posting for basic iommu support, can we try to rework that to be split out like Aneesh is doing so everything about TDX calls lives in tsm and intel iommu only provides a small API surface to exchange whatever details are needed to bootstrap TDX module?

Quoted 5 lines

Quoted 3 lines

AMD is different and I suspect AMD will have to continue to use the viommu from the AMD iommu driver, but I am not sure.

ARM/Intel may support guest viommu in the future.

ARM supports guest viommu today, it is in the public spec. Secure

guest vSMMU is entirely handled inside the RMM and has no connection to the host iommu driver. It is a <100 line ++ on top of Aneesh's work, Nicolin posted a draft at one point in those threads.

I anticipate a future intel guest T=1 viommu should be the same.

Thus I expect Intel/ARM to have two viommus, one that handles the T=1 stream owned by the TSM driver and implemented entirely by calling TDX/RMM.

One that handles the T=0 stream owned by the iommu driver - and it already exists.

Quoted 2 lines

So AMD's case is a good reference.

I think, AMD is completely different. I keep forgetting thier thing, but IIRC they have a secure DTE but instead of having the secure word control the translation it controls the RMP and you end up using the host's translation for T=1 traffic. This is fundamentally different from how Intel and ARM are doing it where the actually IOVA translate is under the control of the secure world.

Both Intel and ARM put the S-EPT into the iommu HW directly.

So, I expect Intel to have an API similar to ARM. When you create the TSM viommu you tell it if the TDX module should create a secure guest visible VT-d emulation. TDX module has to perform the entire emulation because it must be trusted. Existing viommu ops should cover the remaining to register pdevices as vdevices, provide the vBDF and so on.

Quoted 7 lines

Quoted 6 lines

How/when the tsm driver links this to a arch specific "bind/unbind" operation is more up to that driver, but I would expect what is thought of as "bind" should be the affiliation of the device's T=1 stream with the viommu and the target VM. It should not be sensitive to the TDISP state.

Quoted 5 lines

Not sure about this part.

Each arch has its own definition about the binding flow (about 'how'), but sharing a common step by sending TDISP message to transit the TDI into the CONFIG_LOCKED state upon guest request (i.e. 'when').

Sure, the LOCKED command can be relayed from the guest, but that shouldn't be called BIND. locked/unlock/run/err is taking a iommufd vdev that is already affiliated with the VM to a specific TDISP state

Quoted 4 lines

According to the TDISP spec, memory reads/writes with T bit set is accepted only when the TDI is in RUN state (except MSI/MSI-X writes are allowed with T bit set in LOCKED but I don't think any arch supports it yet).

Sure

Quoted 1 line

So your definition of 'bind' essentially affiliate it to the RUN state?

No, it is informing the secure world that a physical PCI function is now a virtual PCI function, is a TDI, and is in a certain VM.

Outside virtual hotplug this is a permanent action when the VM is created.

Quoted 5 lines

Quoted 4 lines

That is not prohibited, the TSM driver could do some auto "bind/unbind" whatever that means triggered by ops or tdisp state changing under the covers. But this cannot leak out as some kind of asynchronous vdev destruction.

Quoted 2 lines

Maybe it'd be clearer using an example e.g. ARM to clarify the suggested split. Or wait for Aneesh's next version...

In ARM:

BIND is RMI_VDEV_CREATE it links a physical device to a virtual device in a realm.

RMI_VSMMU_CREATE can attach a vSMMU to the realm and there is some way to link the VDEV And the VSMMU together

Some sequence of RMI_VDEV_COMMUNICATE, RMI_VDEV_LOCK, RMI_VDEV_UNLOCK
and a few others manipulate the UNLOCKED/LOCKED/RUN/ERR TDISP state of the VDEV.

I assume TDX has the same general shape, I don't know how you could implement this in a radically different way?

So iommufd viommu create calls RMI_VSMMU_CREATE
iommufd vdev create calls RMI_VDEV_CREATE iommufd viommu op ioctl calls the COMMUNICATE/LOCK/UNLOCK

Quoted 13 lines · Kevin

Quoted 7 lines

At least for ARM there is effectively no entanglement with the actual host iommu driver. The viommu is entirely provided by software in the RMM world, so it can have its own dedicated driver. In ARM T=1 transactions are alwayus routed to the RMM's iommu and there is no relation to the host.

I am interested how Intel works here, but I thought it was similar.

Largely yes. Main difference at Intel side is that TDX still relies on the host to initiate iotlb invalidation (upon notification from KVM on S-EPT change). Currently we put this logic in intel-iommu driver but it's more about wrapping invalidation info and passing it to the firmware. Moving it into the tsm driver should be straightforward.

Quoted 2 lines

Maybe there'll be other subtle connections to host iommu driver but it doesn't sound a hard problem to solve.

Okay, so I saw the driver posting for basic iommu support, can we try to rework that to be split out like Aneesh is doing so everything about TDX calls lives in tsm and intel iommu only provides a small API surface to exchange whatever details are needed to bootstrap TDX module?

To reiterate, for this configuration:

- The viommu will use a stage-1 bypass configuration.
- A new IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 type will create the viommu. The psmmu will be activated at this point to avoid creating psmmu objects early. We will reference-count it to ensure that the same psmmu is shared across realm guests.
- Creating a vdevice will invoke SMC_RMI_PSMMU_ST_L2_CREATE.

I am unclear about the vdev_create suggestion. Creating a vdevice requires an RD, which is created later in the flow above. How do you suggest linking vdevice_alloc to vdev_create?

From the RMM's perspective, the sequence is as follows:

viommu alloc

[   rmm ] SMC_RMI_PSMMU_ACTIVATE            2b400000 8819bb000 > RMI_INCOMPLETE 0 10008
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 2 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 1 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 1 0
[   rmm ] L1 StrTab: PA 0x881baa000 VA 0x80003c0000 size 0x2000
[   rmm ] CMDQ: PA 0x88066a000 VA 0x80003c2000
[   rmm ] EVTQ: PA 0x8815c5000 VA 0x80003c3000
[   rmm ] PSMMU 0x2b400000 activated
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

[   rmm ] SMC_RMI_PSMMU_ST_L2_CREATE        2b400000 300 > RMI_INCOMPLETE 0 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 1 0
[   rmm ] smmu->strtab_base[12] 0x0 @0x80003c0060
[   rmm ] L1STD[12] 0x8819bb007 for SID 0x300: L2 table VA 0x80003d0000 PA 0x8819bb000
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

vdevice alloc

[   rmm ] SMC_RMI_PSMMU_ST_L2_CREATE        2b400000 200 > RMI_INCOMPLETE 0 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 882648098 1 > RMI_INCOMPLETE 1 0
[   rmm ] smmu->strtab_base[8] 0x0 @0x80003c0040
[   rmm ] L1STD[8] 0x882506007 for SID 0x200: L2 table VA 0x80003cc000 PA 0x882506000
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

RD gets allocated here

[   rmm ] SMC_RMI_REALM_CREATE              881b03000 8819a1000 > RMI_INCOMPLETE 0 24
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881605098 9 > RMI_INCOMPLETE 9 0
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

Quoted 9 lines · Aneesh Kumar K.V

To reiterate, for this configuration:

- The viommu will use a stage-1 bypass configuration.
- A new IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 type will create the viommu. The psmmu will be activated at this point to avoid creating psmmu objects early. We will reference-count it to ensure that the same psmmu is shared across realm guests.
- Creating a vdevice will invoke SMC_RMI_PSMMU_ST_L2_CREATE.

Quoted 3 lines

I am unclear about the vdev_create suggestion. Creating a vdevice requires an RD, which is created later in the flow above. How do you suggest linking vdevice_alloc to vdev_create?

I was thinking we'd make sure the KVM is associated with the viommu and/or possibly the S2 domain. That was always sort of broadly the idea in this space. There are several topics unrelated to CC that needed this.
In any case, when you create the iommufd viommu you should also do VSMMU_CREATE which needs the RD. It seems reasonable to assume the RD is available during vdevice create.

[There is an aside here I will mention: several other use cases need this idea of an "external" domain where the HWPT would be created but not controlled by iommufd or the iommu subsystem. Xen and Hyperv for example. There is probably some merit in thinking more about exactly what the nested parent domain should be for this viommu, but it isn't critical.]

Quoted 14 lines

From the RMM's perspective, the sequence is as follows:

viommu alloc

[   rmm ] SMC_RMI_PSMMU_ACTIVATE            2b400000 8819bb000 > RMI_INCOMPLETE 0 10008
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 2 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 1 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 1 0
[   rmm ] L1 StrTab: PA 0x881baa000 VA 0x80003c0000 size 0x2000
[   rmm ] CMDQ: PA 0x88066a000 VA 0x80003c2000
[   rmm ] EVTQ: PA 0x8815c5000 VA 0x80003c3000
[   rmm ] PSMMU 0x2b400000 activated
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

Quoted 5 lines

[   rmm ] SMC_RMI_PSMMU_ST_L2_CREATE        2b400000 300 > RMI_INCOMPLETE 0 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881bac098 1 > RMI_INCOMPLETE 1 0
[   rmm ] smmu->strtab_base[12] 0x0 @0x80003c0060
[   rmm ] L1STD[12] 0x8819bb007 for SID 0x300: L2 table VA 0x80003d0000 PA 0x8819bb000
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

What was this one for during viommu alloc?

Quoted 8 lines

vdevice alloc

[   rmm ] SMC_RMI_PSMMU_ST_L2_CREATE        2b400000 200 > RMI_INCOMPLETE 0 10004
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 882648098 1 > RMI_INCOMPLETE 1 0
[   rmm ] smmu->strtab_base[8] 0x0 @0x80003c0040
[   rmm ] L1STD[8] 0x882506007 for SID 0x200: L2 table VA 0x80003cc000 PA 0x882506000
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

Quoted 5 lines

RD gets allocated here

[   rmm ] SMC_RMI_REALM_CREATE              881b03000 8819a1000 > RMI_INCOMPLETE 0 24
[   rmm ]       SMC_RMI_OP_MEM_DONATE       0 881605098 9 > RMI_INCOMPLETE 9 0
[   rmm ]       SMC_RMI_OP_CONTINUE         0 0 > RMI_SUCCESS 0 0

Why? The VMM needs to create the kvm before starting the iommu stuff.

I would expect that KVM knows it is going to a realm very early on? I had assumed the RD would also be created early by KVM?

What triggers RD creation? Why can't the VMM do it earlier?

Your followup said:
- The realm is created during viommu allocation, ensuring that the vSMMU can be created here.

Do you mean the SMMUv3 driver triggers RD creation? That feels wrong. Did you mean the VMM just does it earlier?

The draft in the next message looks promising, did you discover any other gotchas when exploring it?

When you repost this can you take some care to explain the general idea of this modelling in the commit messages so AMD and Intel can confirm they can use it for both their non-viommu and viommu cases?

Thanks,
Jason

Quoted 2 lines

Do you mean the SMMUv3 driver triggers RD creation? That feels wrong. Did you mean the VMM just does it earlier?

Currently, CCA creates the realm lazily as part of another operation. Realm creation is triggered by kvm_arm_rmi_populate() or kvm_arch_vcpu_run_pid_change()

Quoted 6 lines · Aneesh Kumar K.V

Quoted 2 lines

Do you mean the SMMUv3 driver triggers RD creation? That feels wrong. Did you mean the VMM just does it earlier?

Currently, CCA creates the realm lazily as part of another operation. Realm creation is triggered by kvm_arm_rmi_populate() or kvm_arch_vcpu_run_pid_change()

Okay, that's not what I would have guessed. But critically is there some way for userspace to specify any realm creation parameters (like rpv, ats_plane and so one) prior to the lazy creation?

Why lazy? Why not just have userspace provide the creation parameters and create the RD right then?

Jason

Quoted 11 lines · Jason Gunthorpe

Quoted 6 lines · Aneesh Kumar K.V

Quoted 2 lines

Do you mean the SMMUv3 driver triggers RD creation? That feels wrong. Did you mean the VMM just does it earlier?

Currently, CCA creates the realm lazily as part of another operation. Realm creation is triggered by kvm_arm_rmi_populate() or kvm_arch_vcpu_run_pid_change()

Okay, that's not what I would have guessed. But critically is there some way for userspace to specify any realm creation parameters (like rpv, ats_plane and so one) prior to the lazy creation?

There would be, eventually. Based on the feedback from KVM arm64 maintainers, we are trying to keep the ABI as generic as possible as the normal VMs. Thus, for now we have not included the Realm specific parameter configuration (which was present in the earlier versions) in the latest version of the KVM support.

We have some ideas on how to do the configuration step. e.g. for MEC, see the following thread :

https://lore.kernel.org/all/[email protected]/

Quoted 21 lines · Nicolin Chen

But, on the other hand, I can imagine that a Realm VSMMU would add a new flag with a user_data to this VIOMMU. Then, this flow would give some troubles to VMM (QEMU for example):

- For VM with a guest-level SMMU, QEMU creates a realm instance where IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 (with vsmmu) can be allocated.
- For VM w/o a guest-level SMMU, QEMU won't create such a realm instance, while still required to invoke the ioctl (w/o vsmmu).

Taking a step back, I wonder if we really need to use iommufd for PSMMU activation and its stream table allocations?

Here are some facts:
- An iommufd has a ctx, that's one per VM. Similarly, a Realm has an RD.
- For an RMI command that needs an RD, it makes sense to be per iommufd ctx, e.g. RMI_VSMMU_* or RMI_VDEV_* commands.
- PSMMU commands are global; they don't need RD. So they don't seem necessary to tie to an iommufd ctx.

Quoted 4 lines

Instead, could the PSMMU activation be done after RMI_PSMMU_INFO check? Is there any reason not to do that? A safer timing might be at the device assignment stage?

One of the reasons I added IOMMU_VIOMMU_TYPE_ARM_REALM_SMMUV3 was to avoid creating a psmmu object when we are not using a PCI passthrough VM. That is also the reason for all the refcounting around the psmmu objects.

If we are okay with creating psmmu objects early, then I guess we can go with the above approach.