Add Tegra241 (Grace) CMDQV Support (part 2/2)

include/linux/iommufd.h +4 -0

index ffc3a949f837..a0cb08a4b653 100644
@@ -9,6 +9,7 @@
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/err.h>
+#include <linux/refcount.h>
 
 struct device;
 struct iommufd_device;

@@ -18,6 +19,28 @@ struct iommufd_access;
 struct file;
 struct iommu_group;
 
+enum iommufd_object_type {
+	IOMMUFD_OBJ_NONE,
+	IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE,

Quoted 9 lines · Nicolin Chen

include/linux/iommufd.h +8 -0

+enum iommufd_object_type {
+	IOMMUFD_OBJ_NONE,
+	IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE,
+	IOMMUFD_OBJ_DEVICE,
+	IOMMUFD_OBJ_HWPT_PAGING,
+	IOMMUFD_OBJ_HWPT_NESTED,
+	IOMMUFD_OBJ_IOAS,
+	IOMMUFD_OBJ_ACCESS,

include/linux/iommufd.h struct iommufd_access; · line 34

Quoted 5 lines

include/linux/iommufd.h +5 -0

+#ifdef CONFIG_IOMMUFD_TEST
+	IOMMUFD_OBJ_SELFTEST,
+#endif
+	IOMMUFD_OBJ_MAX,
+};

Can we just forward declare the enum? It would be nice to keep it in the private header

We could change the "enum iommufd_object_type type" in struct iommufd_object to "unsigned int type" though...

Currently, the object allocation function calls:

level-0: iommufd_object_alloc()
level-1:     ___iommufd_object_alloc()
level-2:         _iommufd_object_alloc()

Quoted 4 lines · Nicolin Chen

Currently, the object allocation function calls:

level-0: iommufd_object_alloc()
level-1:     ___iommufd_object_alloc()
level-2:         _iommufd_object_alloc()

Let's give __iommufd_object_alloc() a better name then

It is a less general version of iommufd_object_alloc(), maybe iommufd_object_alloc_elm() ?

Quoted 10 lines · Jason Gunthorpe

Quoted 4 lines · Nicolin Chen

Currently, the object allocation function calls:

level-0: iommufd_object_alloc()
level-1:     ___iommufd_object_alloc()
level-2:         _iommufd_object_alloc()

Let's give __iommufd_object_alloc() a better name then

It is a less general version of iommufd_object_alloc(), maybe iommufd_object_alloc_elm() ?

With the level-3 allocator, something like the followings?

level-0: iommufd_object_alloc()
level-1:     __iommufd_object_alloc()
level-2:         iommufd_object_alloc_elm()
level-3:             __iommufd_object_alloc_elm()

In this case, this patch will be:
"iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm"

Quoted 10 lines · Jason Gunthorpe

Quoted 4 lines · Nicolin Chen

Currently, the object allocation function calls:

level-0: iommufd_object_alloc()
level-1:     ___iommufd_object_alloc()
level-2:         _iommufd_object_alloc()

Let's give __iommufd_object_alloc() a better name then

It is a less general version of iommufd_object_alloc(), maybe iommufd_object_alloc_elm() ?

With the level-3 allocator, something like the followings?

level-0: iommufd_object_alloc()
level-1:     __iommufd_object_alloc()
level-2:         iommufd_object_alloc_elm()
level-3:             __iommufd_object_alloc_elm()

In this case, this patch will be:
"iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm"

drivers/iommu/iommufd/iommufd_private.h +9 -0

+static inline struct iommufd_object *___iommufd_object_alloc(size_t size)
+{
+	struct iommufd_object *obj;
+
+	obj = kzalloc(size, GFP_KERNEL_ACCOUNT);
+	if (!obj)
+		return ERR_PTR(-ENOMEM);
+
+	/* Starts out bias'd by 1 until it is removed from the xarray */

Quoted 9 lines · Nicolin Chen

drivers/iommu/iommufd/iommufd_private.h +9 -0

+static inline struct iommufd_object *___iommufd_object_alloc(size_t size)
+{
+	struct iommufd_object *obj;
+
+	obj = kzalloc(size, GFP_KERNEL_ACCOUNT);
+	if (!obj)
+		return ERR_PTR(-ENOMEM);
+
+	/* Starts out bias'd by 1 until it is removed from the xarray */

drivers/iommu/iommufd/iommufd_private.h void iommufd_object_abort_and_destroy(struct iommufd_ctx *ictx, · line 177

Quoted 10 lines

drivers/iommu/iommufd/iommufd_private.h +10 -0

+	refcount_set(&obj->shortterm_users, 1);
+	refcount_set(&obj->users, 1);
+
+	/*
+	 * The allocation of an obj->id needs an ictx, so it has to be done
+	 * after this ___iommufd_object_alloc() callback.
+	 */
+
+	return obj;
+}

It is probably cleaner to just make the existing allocation work with a NULL ictx for this case? Then we can use the existing alloc functions.

drivers/iommu/iommufd/viommu.c line 14

Quoted 6 lines

drivers/iommu/iommufd/viommu.c new file +6 -0

+#define viommu_struct_alloc(name)                                              \
+	struct iommufd_##name *_iommufd_##name##_alloc(size_t size)            \
+	{                                                                      \
+		struct iommufd_object *obj;                                    \
+		if (WARN_ON(size < sizeof(struct iommufd_##name)))             \
+			return NULL;                                           \

Then here you'd just use the exisint container_of based flow with the driver sub struct name passed as the 'obj'

Add a new iommufd_viommu core structure to represent a vIOMMU instance in the user space, typically backed by a HW-accelerated feature of an IOMMU, e.g. NVIDIA CMDQ-Virtualization (an ARM SMMUv3 extension) and AMD Hardware Accelerated Virtualized IOMMU (vIOMMU).

A driver should embed this core structure in its driver viommu structure and call the new iommufd_viommu_alloc() helper to allocate a core/driver structure bundle and fill its core viommu->ops:

    struct my_driver_viommu {
        struct iommufd_viommu core;
	....
    };

    static const struct iommufd_viommu_ops my_driver_viommu_ops = {
        .free = my_driver_viommu_free,
    };

    struct my_driver_viommu *my_viommu =
            iommufd_viommu_alloc(my_driver_viommu, core);

Quoted 4 lines · Nicolin Chen

Add a new iommufd_viommu core structure to represent a vIOMMU instance in the user space, typically backed by a HW-accelerated feature of an IOMMU, e.g. NVIDIA CMDQ-Virtualization (an ARM SMMUv3 extension) and AMD Hardware Accelerated Virtualized IOMMU (vIOMMU).

I expect this will also be the only way to pass in an associated KVM, userspace would supply the kvm when creating the viommu.

The tricky bit of this flow is how to manage the S2. It is necessary that the S2 be linked to the viommu:

1) ARM BTM requires the VMID to be shared with KVM
2) AMD and others need the S2 translation because some of the HW acceleration is done inside the guest address space

I haven't looked closely at AMD but presumably the VIOMMU create will have to install the S2 into a DID or something?

So we need the S2 to exist before the VIOMMU is created, but the drivers are going to need some more fixing before that will fully work.

Does the nesting domain create need the viommu as well (in place of the S2 hwpt)? That feels sort of natural.

There is still a lot of fixing before everything can work fully, but do we need to make some preperations here in the uapi? Like starting to thread the S2 through it as I described?

Kevin, does Intel forsee any viommu needs on current/future Intel HW? I assume you are thinking about invalidation queue bypass like everyone else. I think it is an essential feature for vSVA.

Quoted 12 lines

A driver should embed this core structure in its driver viommu structure and call the new iommufd_viommu_alloc() helper to allocate a core/driver structure bundle and fill its core viommu->ops:

    struct my_driver_viommu {
        struct iommufd_viommu core;
	....
    };

    static const struct iommufd_viommu_ops my_driver_viommu_ops = {
        .free = my_driver_viommu_free,
    };

Quoted 2 lines

struct my_driver_viommu *my_viommu =
iommufd_viommu_alloc(my_driver_viommu, core);

Why don't we have an ictx here anyhow? The caller has it? Just pass it down and then it is normal:

my_viommu = iommufd_object_alloc_elm(ictx, my_viommu, IOMMUFD_OBJ_HWPT_VIOMMU, core.obj);

Quoted 23 lines · Jason Gunthorpe

Quoted 4 lines · Nicolin Chen

Add a new iommufd_viommu core structure to represent a vIOMMU instance in the user space, typically backed by a HW-accelerated feature of an IOMMU, e.g. NVIDIA CMDQ-Virtualization (an ARM SMMUv3 extension) and AMD Hardware Accelerated Virtualized IOMMU (vIOMMU).

I expect this will also be the only way to pass in an associated KVM, userspace would supply the kvm when creating the viommu.

The tricky bit of this flow is how to manage the S2. It is necessary that the S2 be linked to the viommu:

1) ARM BTM requires the VMID to be shared with KVM
2) AMD and others need the S2 translation because some of the HW acceleration is done inside the guest address space

I haven't looked closely at AMD but presumably the VIOMMU create will have to install the S2 into a DID or something?

So we need the S2 to exist before the VIOMMU is created, but the drivers are going to need some more fixing before that will fully work.

Quoted 2 lines

Does the nesting domain create need the viommu as well (in place of the S2 hwpt)? That feels sort of natural.

Yes, I had a similar thought initially: each viommu is backed by a nested IOMMU HW, and a special HW accelerator like VCMDQ could be treated as an extension on top of that. It might not be very straightforward like the current design having vintf<->viommu and vcmdq <-> vqueue though...

In that case, we can then support viommu_cache_invalidate, which is quite natural for SMMUv3. Yet, I recall Kevin said that VT-d doesn't want or need that.

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, May 14, 2024 11:56 PM

On Sun, May 12, 2024 at 08:34:02PM -0700, Nicolin Chen wrote:

Quoted 3 lines · Jason Gunthorpe

Quoted 2 lines · Nicolin Chen

Quoted 1 line

Add a new iommufd_viommu core structure to represent a vIOMMU

instance in

Quoted 1 line

Quoted 1 line

Quoted 1 line

the user space, typically backed by a HW-accelerated feature of an

IOMMU,

Quoted 1 line

Quoted 1 line

Quoted 1 line

e.g. NVIDIA CMDQ-Virtualization (an ARM SMMUv3 extension) and

AMD Hardware

Quoted 18 lines

Quoted 18 lines

Quoted 1 line

Accelerated Virtualized IOMMU (vIOMMU).

I expect this will also be the only way to pass in an associated KVM, userspace would supply the kvm when creating the viommu.

The tricky bit of this flow is how to manage the S2. It is necessary that the S2 be linked to the viommu:

1) ARM BTM requires the VMID to be shared with KVM
2) AMD and others need the S2 translation because some of the HW acceleration is done inside the guest address space

I haven't looked closely at AMD but presumably the VIOMMU create will have to install the S2 into a DID or something?

So we need the S2 to exist before the VIOMMU is created, but the drivers are going to need some more fixing before that will fully work.

Quoted 9 lines

Quoted 3 lines

Does the nesting domain create need the viommu as well (in place of the S2 hwpt)? That feels sort of natural.

Yes, I had a similar thought initially: each viommu is backed by a nested IOMMU HW, and a special HW accelerator like VCMDQ could be treated as an extension on top of that. It might not be very straightforward like the current design having vintf<->viommu and vcmdq <-> vqueue though...

vqueue should be considered a sub object of the viommu and hold a refcount on the viommu object for its lifetime.

Quoted 3 lines

In that case, we can then support viommu_cache_invalidate, which is quite natural for SMMUv3. Yet, I recall Kevin said that VT-d doesn't want or need that.

Right, Intel currently doesn't need it, but I feel like everyone will need this eventually as the fast invalidation path is quite important.

Quoted 4 lines

From: Jason Gunthorpe<[email protected]>
Sent: Tuesday, May 14, 2024 11:56 PM

On Sun, May 12, 2024 at 08:34:02PM -0700, Nicolin Chen wrote:

Quoted 25 lines

Quoted 3 lines · Jason Gunthorpe

Quoted 2 lines · Nicolin Chen

Quoted 1 line

Add a new iommufd_viommu core structure to represent a vIOMMU

instance in

Quoted 1 line

Quoted 1 line

Quoted 1 line

the user space, typically backed by a HW-accelerated feature of an

IOMMU,

Quoted 1 line

Quoted 1 line

Quoted 1 line

e.g. NVIDIA CMDQ-Virtualization (an ARM SMMUv3 extension) and

AMD Hardware

Quoted 17 lines

Quoted 17 lines

Quoted 1 line

Accelerated Virtualized IOMMU (vIOMMU).

I expect this will also be the only way to pass in an associated KVM, userspace would supply the kvm when creating the viommu.

The tricky bit of this flow is how to manage the S2. It is necessary that the S2 be linked to the viommu:

1) ARM BTM requires the VMID to be shared with KVM
2) AMD and others need the S2 translation because some of the HW acceleration is done inside the guest address space

I haven't looked closely at AMD but presumably the VIOMMU create will have to install the S2 into a DID or something?

So we need the S2 to exist before the VIOMMU is created, but the drivers are going to need some more fixing before that will fully work.

Can you elaborate on this point? VIOMMU is a dummy container when it's created and the association to S2 comes relevant only until when VQUEUE is created inside and linked to a device? then there should be a window in between allowing the userspace to configure S2.

Not saying against setting S2 up before vIOMMU creation. Just want to better understand the rationale here.

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

Does the nesting domain create need the viommu as well (in place of the S2 hwpt)? That feels sort of natural.

Yes, I had a similar thought initially: each viommu is backed by a nested IOMMU HW, and a special HW accelerator like VCMDQ could be treated as an extension on top of that. It might not be very straightforward like the current design having vintf<->viommu and vcmdq <-> vqueue though...

Quoted 9 lines

vqueue should be considered a sub object of the viommu and hold a refcount on the viommu object for its lifetime.

Quoted 3 lines

In that case, we can then support viommu_cache_invalidate, which is quite natural for SMMUv3. Yet, I recall Kevin said that VT-d doesn't want or need that.

Right, Intel currently doesn't need it, but I feel like everyone will need this eventually as the fast invalidation path is quite important.

yes, there is no need but I don't see any harm of preparing for such extension on VT-d. Logically it's clearer, e.g. if we decide to move device TLB invalidation to a separate uAPI then vIOMMU is certainly a clearer object to carry it. and hardware extensions really looks like optimization on software implementations.

and we do need make a decision now, given if we make vIOMMU as a generic object for all vendors it may have potential impact on the user page fault support which Baolu is working on. the so-called fault object will be contained in vIOMMU, which is software managed on VT-d/SMMU but passed through on AMD. And probably we don't need another handle mechanism in the attach path, suppose the vIOMMU object already contains necessary information to find out iommufd_object for a reported fault.

Quoted 27 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, May 14, 2024 11:56 PM

Quoted 3 lines · Nicolin Chen

Quoted 2 lines · Jason Gunthorpe

Quoted 1 line · Nicolin Chen

Add a new iommufd_viommu core structure to represent a vIOMMU

instance in

Quoted 1 line

Quoted 1 line

Quoted 1 line

the user space, typically backed by a HW-accelerated feature of an

IOMMU,

Quoted 1 line

Quoted 1 line

Quoted 1 line

e.g. NVIDIA CMDQ-Virtualization (an ARM SMMUv3 extension) and

AMD Hardware

Quoted 15 lines

Quoted 15 lines

Quoted 1 line

Accelerated Virtualized IOMMU (vIOMMU).

I expect this will also be the only way to pass in an associated KVM, userspace would supply the kvm when creating the viommu.

The tricky bit of this flow is how to manage the S2. It is necessary that the S2 be linked to the viommu:

1) ARM BTM requires the VMID to be shared with KVM
2) AMD and others need the S2 translation because some of the HW acceleration is done inside the guest address space

I haven't looked closely at AMD but presumably the VIOMMU create will have to install the S2 into a DID or something?

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

So we need the S2 to exist before the VIOMMU is created, but the drivers are going to need some more fixing before that will fully work.

Can you elaborate on this point? VIOMMU is a dummy container when it's created and the association to S2 comes relevant only until when VQUEUE is created inside and linked to a device?

Quoted 2 lines

Right, Intel currently doesn't need it, but I feel like everyone will need this eventually as the fast invalidation path is quite important.

yes, there is no need but I don't see any harm of preparing for such extension on VT-d. Logically it's clearer, e.g. if we decide to move device TLB invalidation to a separate uAPI then vIOMMU is certainly a clearer object to carry it. and hardware extensions really looks like optimization on software implementations.

and we do need make a decision now, given if we make vIOMMU as a generic object for all vendors it may have potential impact on the user page fault support which Baolu is working on.

the so-called
fault object will be contained in vIOMMU, which is software managed on VT-d/SMMU but passed through on AMD.

And probably we don't need another handle mechanism in the attach path, suppose the vIOMMU object already contains necessary information to find out iommufd_object for a reported fault.

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 9:39 PM

Quoted 10 lines · Kevin

Quoted 6 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, May 14, 2024 11:56 PM

Quoted 3 lines

Quoted 3 lines

So we need the S2 to exist before the VIOMMU is created, but the drivers are going to need some more fixing before that will fully work.

Can you elaborate on this point? VIOMMU is a dummy container when it's created and the association to S2 comes relevant only until when VQUEUE is created inside and linked to a device?

VIOMMU contains:
- A nesting parent
- A KVM
- Any global per-VM data the driver needs
* In ARM case this is VMID, sometimes shared with KVM

* In AMD case this is will allocate memory in the "viommu backing storage memory"

Objects can be created on top of a VIOMMU:
- A nested HWPT (iommu_hwpt_alloc::pt_id can be a viommu)
- A vqueue (ARM/AMD)
- Other AMD virtualized objects (EventLog, PPRLog)

It is desirable to keep the VIOMMU linked to only a single nesting parent that never changes. Given it seems to be a small ask to allocate the nesting parent before the VIOMMU providing it at VIOMMU creation time looks like it will simplify the drivers because they can rely on it always existing and never changing.

I think this lends itself to a logical layered VMM design..

- If VFIO is being used get an iommufd
- Allocate an IOAS for the entire guest physical
- Determine the vIOMMU driver to use
- Allocate a HWPT for use by all the vIOMMU instances
- Allocate a VIOMMU per vIOMMU instance

On ARM the S2 is not divorced from the VIOMMU, ARM requires a single VMID, shared with KVM, and localized to a single VM for some of the bypass features (vBTM, vCMDQ). So to attach a S2 you actually have to attach the VIOMMU to pick up the correct VMID.

I imagine something like this:
hwpt_alloc(deva, nesting_parent=true) = shared_s2 viommu_alloc(deva, shared_s2) = viommu1 viommu_alloc(devb, shared_s2) = viommu2 hwpt_alloc(deva, viommu1, vste) = deva_vste hwpt_alloc(devb, viommu2, vste) = devb_vste attach(deva, deva_vste)
attach(devb, devb_vste)
attach(devc, shared_s2)

The driver will then know it should program three different VMIDs for the same S2 page table, which matches the ARM expectation for VMID. That is to say we'd pass in the viommu as the pt_id for the iommu_hwpt_alloc. The viommu would imply both the S2 page table and any meta information like VMID the driver needs.

Both AMD and the vCMDQ thing need to translate some PFNs through the S2 and program them elsewhere, this is manually done by SW, and there are three choices I guess:
- Have the VMM do it and provide void __user * to the driver

- Have the driver do it through the S2 directly and track S2 invalidations

- Have the driver open an access on the IOAS and use the access unmap

Not sure which is the best..

Quoted 12 lines

Quoted 2 lines

Right, Intel currently doesn't need it, but I feel like everyone will need this eventually as the fast invalidation path is quite important.

yes, there is no need but I don't see any harm of preparing for such extension on VT-d. Logically it's clearer, e.g. if we decide to move device TLB invalidation to a separate uAPI then vIOMMU is certainly a clearer object to carry it. and hardware extensions really looks like optimization on software implementations.

and we do need make a decision now, given if we make vIOMMU as a generic object for all vendors it may have potential impact on the user page fault support which Baolu is working on.

Quoted 3 lines

the so-called
fault object will be contained in vIOMMU, which is software managed on VT-d/SMMU but passed through on AMD.

Hmm, given we currently have no known hardware entanglement between PRI and VIOMMU it does seem OK for PRI to just exist seperate for

now. If someone needs them linked someday we can add a viommu_id to the create pri queue command.

Quoted 7 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 9:39 PM
VIOMMU contains:
- A nesting parent
- A KVM
- Any global per-VM data the driver needs
* In ARM case this is VMID, sometimes shared with KVM

In which case is it not shared with KVM? I had the impression that VMID always comes from KVM in this VCMDQ usage. 😊

Quoted 5 lines

On ARM the S2 is not divorced from the VIOMMU, ARM requires a single VMID, shared with KVM, and localized to a single VM for some of the bypass features (vBTM, vCMDQ). So to attach a S2 you actually have to attach the VIOMMU to pick up the correct VMID.

Quoted 9 lines

I imagine something like this:
hwpt_alloc(deva, nesting_parent=true) = shared_s2 viommu_alloc(deva, shared_s2) = viommu1 viommu_alloc(devb, shared_s2) = viommu2 hwpt_alloc(deva, viommu1, vste) = deva_vste hwpt_alloc(devb, viommu2, vste) = devb_vste attach(deva, deva_vste)
attach(devb, devb_vste)
attach(devc, shared_s2)

I wonder whether we want to make viommu as the 1st-class citizen for any nested hwpt if it is desirable to enable it even for VT-d which lacks of a hw viommu concept at the moment.

Quoted 5 lines

The driver will then know it should program three different VMIDs for the same S2 page table, which matches the ARM expectation for VMID. That is to say we'd pass in the viommu as the pt_id for the iommu_hwpt_alloc. The viommu would imply both the S2 page table and any meta information like VMID the driver needs.

Can you elaborate the aspect about "three different VMIDs"? They are all for the same VM hence sharing the same VMID per the earlier description. This is also echo-ed in patch14:

tegra241_cmdqv_viommu_alloc()

        vintf->vmid = smmu_domain->vmid;

Quoted 2 lines

now. If someone needs them linked someday we can add a viommu_id to the create pri queue command.

I'm more worried about the potential conflict between the vqueue object here and the fault queue object in Baolu's series, if we want to introduce vIOMMU concept to platforms which lack of the hw support.

From: Nicolin Chen <[email protected]>
Sent: Thursday, May 23, 2024 12:02 PM

Quoted 8 lines · Kevin

Quoted 7 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 9:39 PM
VIOMMU contains:
- A nesting parent
- A KVM
- Any global per-VM data the driver needs
* In ARM case this is VMID, sometimes shared with KVM

Quoted 2 lines

In which case is it not shared with KVM? I had the impression that VMID always comes from KVM in this VCMDQ usage. 😊

Not actually. I guess that shared VMID is for BTM.

Quoted 4 lines

Quoted 4 lines

On ARM the S2 is not divorced from the VIOMMU, ARM requires a single VMID, shared with KVM, and localized to a single VM for some of the bypass features (vBTM, vCMDQ). So to attach a S2 you actually have

Quoted 12 lines

Quoted 11 lines

attach the VIOMMU to pick up the correct VMID.

I imagine something like this:
hwpt_alloc(deva, nesting_parent=true) = shared_s2 viommu_alloc(deva, shared_s2) = viommu1 viommu_alloc(devb, shared_s2) = viommu2 hwpt_alloc(deva, viommu1, vste) = deva_vste hwpt_alloc(devb, viommu2, vste) = devb_vste attach(deva, deva_vste)
attach(devb, devb_vste)
attach(devc, shared_s2)

Quoted 3 lines

I wonder whether we want to make viommu as the 1st-class citizen for any nested hwpt if it is desirable to enable it even for VT-d which lacks of a hw viommu concept at the moment.

I think Jason is completely using SMMU as an example here.

Also FWIW, I am trying a core-allocated core-managed viommu for IOMMU_VIOMMU_TYPE_DEFAULT. So VT-d driver doesn't need to hold a viommu while VMM could still allocate one if it wants. And the VIOMMU interface can provide some helpers if driver wants some info from the core-managed viommu: a virtual dev ID to physical dev ID (returning device pointer) translation for example. And we can add more after we brain storm.

Quoted 10 lines

Quoted 5 lines

The driver will then know it should program three different VMIDs for the same S2 page table, which matches the ARM expectation for VMID. That is to say we'd pass in the viommu as the pt_id for the iommu_hwpt_alloc. The viommu would imply both the S2 page table and any meta information like VMID the driver needs.

Can you elaborate the aspect about "three different VMIDs"? They are all for the same VM hence sharing the same VMID per the earlier description. This is also echo-ed in patch14:

Quoted 2 lines

tegra241_cmdqv_viommu_alloc()

        vintf->vmid = smmu_domain->vmid;

The design in the series is still old using a 1:1 relationship between a viommu and an S2 domain. I think the "three" is from his SMMU example above? Leaving it to Jason to reply though.

Quoted 15 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 9:39 PM

Quoted 10 lines · Kevin

Quoted 6 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, May 14, 2024 11:56 PM

Quoted 3 lines

Quoted 3 lines

So we need the S2 to exist before the VIOMMU is created, but the drivers are going to need some more fixing before that will fully work.

Can you elaborate on this point? VIOMMU is a dummy container when it's created and the association to S2 comes relevant only until when VQUEUE is created inside and linked to a device?

Quoted 5 lines

VIOMMU contains:
- A nesting parent
- A KVM
- Any global per-VM data the driver needs
* In ARM case this is VMID, sometimes shared with KVM

In which case is it not shared with KVM? I had the impression that VMID always comes from KVM in this VCMDQ usage. 😊

Quoted 5 lines

On ARM the S2 is not divorced from the VIOMMU, ARM requires a single VMID, shared with KVM, and localized to a single VM for some of the bypass features (vBTM, vCMDQ). So to attach a S2 you actually have to attach the VIOMMU to pick up the correct VMID.

Quoted 9 lines

I imagine something like this:
hwpt_alloc(deva, nesting_parent=true) = shared_s2 viommu_alloc(deva, shared_s2) = viommu1 viommu_alloc(devb, shared_s2) = viommu2 hwpt_alloc(deva, viommu1, vste) = deva_vste hwpt_alloc(devb, viommu2, vste) = devb_vste attach(deva, deva_vste)
attach(devb, devb_vste)
attach(devc, shared_s2)

I wonder whether we want to make viommu as the 1st-class citizen for any nested hwpt if it is desirable to enable it even for VT-d which lacks of a hw viommu concept at the moment.

Quoted 5 lines

The driver will then know it should program three different VMIDs for the same S2 page table, which matches the ARM expectation for VMID. That is to say we'd pass in the viommu as the pt_id for the iommu_hwpt_alloc. The viommu would imply both the S2 page table and any meta information like VMID the driver needs.

Can you elaborate the aspect about "three different VMIDs"?

tegra241_cmdqv_viommu_alloc()

	vintf->vmid = smmu_domain->vmid;

Quoted 4 lines

Both AMD and the vCMDQ thing need to translate some PFNs through the S2 and program them elsewhere, this is manually done by SW, and there are three choices I guess:
- Have the VMM do it and provide void __user * to the driver

this sounds redundant to what S2 already provides

Quoted 2 lines

- Have the driver do it through the S2 directly and track S2 invalidations

this makes more sense to me. Just like the driver already needs to track S2 invalidations to flush any nested cache related to the affected S2 range.

Quoted 1 line

- Have the driver open an access on the IOAS and use the access unmap

it requires adding more iommufd awareness into the iommu driver. I'm inclined to do it only at minimal necessity.

Quoted 2 lines

Hmm, given we currently have no known hardware entanglement between PRI and VIOMMU it does seem OK for PRI to just exist seperate for

Isn't AMD vPPRLog for directly sending PRI request into the guest?

Quoted 2 lines

now. If someone needs them linked someday we can add a viommu_id to the create pri queue command.

I'm more worried about the potential conflict between the vqueue object here and the fault queue object in Baolu's series, if we want to introduce vIOMMU concept to platforms which lack of the hw support.

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, May 23, 2024 8:59 PM

Quoted 10 lines · Kevin

Quoted 8 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 9:39 PM

The driver will then know it should program three different VMIDs for the same S2 page table, which matches the ARM expectation for VMID. That is to say we'd pass in the viommu as the pt_id for the iommu_hwpt_alloc. The viommu would imply both the S2 page table and any meta information like VMID the driver needs.

Can you elaborate the aspect about "three different VMIDs"?

In SMMUv3 the cache is tagged by (VMID,ASID) where ASID is completely controlled by the guest.

Every time the guest observes a SMMUv3 instance it is allowed to creates its own private ASID number space for that instance. The guest could re-use ASID #1 on two instances.

So every SMMUv3 instance plugged into the guest needs to have its own unique VMID so that the overlapping ASID's are disambiguate. The above would create a VM where:

deva -> vSMMUv3 #1
devb -> vSMMUv3 #2
devc -> No IOMMU

Quoted 1 line

Quoted 1 line

Hmm, given we currently have no known hardware entanglement

between

Quoted 3 lines

Quoted 1 line

PRI and VIOMMU it does seem OK for PRI to just exist seperate for

Isn't AMD vPPRLog for directly sending PRI request into the guest?

I think it is, but that would be a vQUEUE on the VIOMMU not adding a VIOMMU to Lu's patches, which is what I ment.

Quoted 3 lines

Quoted 2 lines

now. If someone needs them linked someday we can add a viommu_id to the create pri queue command.

Quoted 4 lines

I'm more worried about the potential conflict between the vqueue object here and the fault queue object in Baolu's series, if we want to introduce vIOMMU concept to platforms which lack of the hw support.

I assume the vPPRLog will steal all the PRI before it reaches the kernel, so once this is turned on Lu's path won't see anything.

Quoted 25 lines

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, May 23, 2024 8:59 PM

Quoted 10 lines · Kevin

Quoted 8 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 9:39 PM

The driver will then know it should program three different VMIDs for the same S2 page table, which matches the ARM expectation for VMID. That is to say we'd pass in the viommu as the pt_id for the iommu_hwpt_alloc. The viommu would imply both the S2 page table and any meta information like VMID the driver needs.

Can you elaborate the aspect about "three different VMIDs"?

In SMMUv3 the cache is tagged by (VMID,ASID) where ASID is completely controlled by the guest.

Every time the guest observes a SMMUv3 instance it is allowed to creates its own private ASID number space for that instance. The guest could re-use ASID #1 on two instances.

So every SMMUv3 instance plugged into the guest needs to have its own unique VMID so that the overlapping ASID's are disambiguate. The above would create a VM where:

Quoted 3 lines

deva -> vSMMUv3 #1
devb -> vSMMUv3 #2
devc -> No IOMMU

This assumes that each vSMMUv3 instance has only one ASID space i.e. the guest cannot create multiple VMID's itself?

Quoted 2 lines

I assume the vPPRLog will steal all the PRI before it reaches the kernel, so once this is turned on Lu's path won't see anything.

Okay, then we expect this vqueue object only for HW acceleration while software-based fault logging is still routed via Baolu's work.

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, May 23, 2024 8:59 PM

Quoted 7 lines

Quoted 2 lines

now. If someone needs them linked someday we can add a viommu_id to the create pri queue command.

I'm more worried about the potential conflict between the vqueue object here and the fault queue object in Baolu's series, if we want to introduce vIOMMU concept to platforms which lack of the hw support.

I assume the vPPRLog will steal all the PRI before it reaches the kernel, so once this is turned on Lu's path won't see anything.

I don't know if AMD can turn vPPRLog on individually or if it is a whole package once they switch on VIOMMU..

drivers/iommu/iommufd/viommu.c +29 -0

+int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)
+{
+	struct iommu_viommu_alloc *cmd = ucmd->cmd;
+	struct iommufd_hwpt_paging *hwpt_paging;
+	struct iommu_device *iommu_dev;
+	struct iommufd_viommu *viommu;
+	struct iommufd_device *idev;
+	int rc;
+
+	if (cmd->flags)
+		return -EOPNOTSUPP;
+
+	idev = iommufd_get_device(ucmd, cmd->dev_id);
+	if (IS_ERR(idev))
+		return PTR_ERR(idev);
+	iommu_dev = idev->dev->iommu->iommu_dev;
+
+	if (!iommu_dev->ops->viommu_alloc) {
+		rc = -EOPNOTSUPP;
+		goto out_put_idev;
+	}
+
+	hwpt_paging = iommufd_get_hwpt_paging(ucmd, cmd->hwpt_id);
+	if (IS_ERR(hwpt_paging)) {
+		rc = PTR_ERR(hwpt_paging);
+		goto out_put_idev;
+	}
+
+	if (!hwpt_paging->nest_parent) {

drivers/iommu/iommufd/viommu.c +9 -0

+	viommu->type = cmd->type;
+
+	viommu->ictx = ucmd->ictx;
+	viommu->hwpt = hwpt_paging;
+	viommu->iommu_dev = idev->dev->iommu->iommu_dev;
+	cmd->out_viommu_id = viommu->obj.id;
+	rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
+	if (rc)
+		goto out_erase_xa;

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

+/**
+ * struct iommu_viommu_alloc - ioctl(IOMMU_VIOMMU_ALLOC)
+ * @size: sizeof(struct iommu_viommu_alloc)
+ * @flags: Must be 0
+ * @type: Type of the VIOMMU object. Must be defined in enum iommu_viommu_type
+ * @dev_id: The device to allocate this virtual IOMMU for
+ * @hwpt_id: ID of a nested parent HWPT
+ * @out_viommu_id: Output virtual IOMMU ID for the allocated object
+ *

drivers/iommu/iommufd/viommu.c line 71

Quoted 6 lines · Nicolin Chen

drivers/iommu/iommufd/viommu.c +6 -0

+	viommu = iommu_dev->ops->viommu_alloc(idev->dev, cmd->type,
+					      hwpt_paging->common.domain);
+	if (IS_ERR(viommu)) {
+		rc = PTR_ERR(viommu);
+		goto out_put_hwpt;
+	}

Ah you did already include the S2, So should it be domain->viommu_alloc() then?

drivers/iommu/iommufd/viommu.c line 79

Quoted 8 lines

drivers/iommu/iommufd/viommu.c +8 -0

+
+	/* iommufd_object_finalize will store the viommu->obj.id */
+	rc = xa_alloc(&ucmd->ictx->objects, &viommu->obj.id, XA_ZERO_ENTRY,
+		      xa_limit_31b, GFP_KERNEL_ACCOUNT);
+	if (rc)
+		goto out_free;
+
+	viommu->obj.type = IOMMUFD_OBJ_VIOMMU;

See my other notes, lets try not to open code this.

Quoted 14 lines

drivers/iommu/iommufd/viommu.c +14 -0

+	viommu->type = cmd->type;
+
+	viommu->ictx = ucmd->ictx;
+	viommu->hwpt = hwpt_paging;
+	viommu->iommu_dev = idev->dev->iommu->iommu_dev;
+	cmd->out_viommu_id = viommu->obj.id;
+	rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
+	if (rc)
+		goto out_erase_xa;
+	iommufd_object_finalize(ucmd->ictx, &viommu->obj);
+	refcount_inc(&viommu->hwpt->common.obj.users);
+	goto out_put_hwpt;
+
+out_erase_xa:

drivers/iommu/iommufd/viommu.c line 98

Quoted 5 lines

drivers/iommu/iommufd/viommu.c +5 -0

+	xa_erase(&ucmd->ictx->objects, viommu->obj.id);
+out_free:
+	if (viommu->ops && viommu->ops->free)
+		viommu->ops->free(viommu);
+	kfree(viommu);

This really should use the abort flow. The driver free callback has to be in the object release..

include/uapi/linux/iommufd.h struct iommu_hwpt_invalidate { · line 703

Quoted 8 lines

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

+
+/**
+ * enum iommu_viommu_type - VIOMMU Type
+ * @IOMMU_VIOMMU_TEGRA241_CMDQV: NVIDIA Tegra241 CMDQV Extension for SMMUv3
+ */
+enum iommu_viommu_type {
+	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
+};

At least the 241 line should be in a following patch

Quoted 9 lines

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

+/**
+ * struct iommu_viommu_alloc - ioctl(IOMMU_VIOMMU_ALLOC)
+ * @size: sizeof(struct iommu_viommu_alloc)
+ * @flags: Must be 0
+ * @type: Type of the VIOMMU object. Must be defined in enum iommu_viommu_type
+ * @dev_id: The device to allocate this virtual IOMMU for
+ * @hwpt_id: ID of a nested parent HWPT
+ * @out_viommu_id: Output virtual IOMMU ID for the allocated object
+ *

include/uapi/linux/iommufd.h struct iommu_hwpt_invalidate { · line 723

Quoted 10 lines

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

+ * Allocate an virtual IOMMU object that holds a (shared) nested parent HWPT
+ */
+struct iommu_viommu_alloc {
+	__u32 size;
+	__u32 flags;
+	__u32 type;
+	__u32 dev_id;
+	__u32 hwpt_id;
+	__u32 out_viommu_id;
+};

This seems fine.

Let's have a following patch to change the hwpt_alloc to accept the viommu as a hwpt as a uAPI change as well.

The more I think about how this needs to work the more sure I am that we need to do that.

ARM will need a fairly tricky set of things to manage the VMID lifecycle. In BTM mode the VMID must come from the KVM. For vcmdq the VMID is needed to create the queue/viommu. For AMD the S2 is needed to create the VIOMMU in the first place.

So, to make this all work perfectly we need approx the following
- S2 sharing across instances in ARM - meaning the VMID is allocated at attach not domain alloc
- S2 hwpt is refcounted by the VIOMMU in the iommufd layer
- VIOMMU is refcounted by every nesting child in the iommufd layer
- The nesting child holds a pointer to both the S2 and the VIOMMU (viommu optional)
- When the nesting child attaches to a device the STE will source the VMID from the VIOMMU if present otherwise from the S2
- "RID" attach (ie naked S2) will have to be done with a Nesting Child using a vSTE that indicates Identity. Then the attach logic will have enough information to get the VMID from the VIOMMU

- In full VIOMMU mode the S2 will never get a VMID of its own, it will always use the VIOMMU. Life cycle is simple, the VMID is freed when the VIOMMU is freed. That can't happen until all Nesting Children are freed. That can't happen until all Nesting Children are detached from devices. Detatching removes the HW touch of the VMID.

At this point you don't need the full generality, but let's please get ready and get the viommu pointer available in all the right spots and we can keep the current logic to borrow the VMID from the S2 for the VIOMMU.

Quoted 10 lines

include/uapi/linux/iommufd.h struct iommu_hwpt_invalidate { · line 703

Quoted 8 lines

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

+
+/**
+ * enum iommu_viommu_type - VIOMMU Type
+ * @IOMMU_VIOMMU_TEGRA241_CMDQV: NVIDIA Tegra241 CMDQV Extension for SMMUv3
+ */
+enum iommu_viommu_type {
+	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
+};

At least the 241 line should be in a following patch

It's for the "enum iommu_viommu_type" mentioned in the following structure. Yi told me that you don't like an empty enum, and he did something like this in HWPT_INVALIDATE series:
https://lore.kernel.org/linux-iommu/[email protected]/

Quoted 4 lines

So, to make this all work perfectly we need approx the following
- S2 sharing across instances in ARM - meaning the VMID is allocated at attach not domain alloc
- S2 hwpt is refcounted by the VIOMMU in the iommufd layer

Quoted 8 lines

- VIOMMU is refcounted by every nesting child in the iommufd layer
- The nesting child holds a pointer to both the S2 and the VIOMMU (viommu optional)
- When the nesting child attaches to a device the STE will source the VMID from the VIOMMU if present otherwise from the S2
- "RID" attach (ie naked S2) will have to be done with a Nesting Child using a vSTE that indicates Identity. Then the attach logic will have enough information to get the VMID from the VIOMMU

What is this RID attach (naked S2) case? S1DSS_BYPASS + SVA?

Quoted 5 lines

- In full VIOMMU mode the S2 will never get a VMID of its own, it will always use the VIOMMU. Life cycle is simple, the VMID is freed when the VIOMMU is freed. That can't happen until all Nesting Children are freed. That can't happen until all Nesting Children are detached from devices. Detatching removes the HW touch of the VMID.

So, each VM will have one S2 HWPT/domain/iopt, but each VM can have multiple VIOMMU instances sharing that single S2 HWPT, and each VIOMMU instance (in the SMMU driver at least) holds a vmid.

Quoted 11 lines

Quoted 10 lines

include/uapi/linux/iommufd.h struct iommu_hwpt_invalidate { · line 703

Quoted 8 lines

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

+
+/**
+ * enum iommu_viommu_type - VIOMMU Type
+ * @IOMMU_VIOMMU_TEGRA241_CMDQV: NVIDIA Tegra241 CMDQV Extension for SMMUv3
+ */
+enum iommu_viommu_type {
+	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
+};

At least the 241 line should be in a following patch

Quoted 4 lines

It's for the "enum iommu_viommu_type" mentioned in the following structure. Yi told me that you don't like an empty enum, and he did something like this in HWPT_INVALIDATE series:
https://lore.kernel.org/linux-iommu/[email protected]/

I suspect 0 should be reserved as a non-set value for some basic sanity in all these driver type enums.

Quoted 13 lines

Quoted 4 lines

So, to make this all work perfectly we need approx the following
- S2 sharing across instances in ARM - meaning the VMID is allocated at attach not domain alloc
- S2 hwpt is refcounted by the VIOMMU in the iommufd layer

Quoted 8 lines

- VIOMMU is refcounted by every nesting child in the iommufd layer
- The nesting child holds a pointer to both the S2 and the VIOMMU (viommu optional)
- When the nesting child attaches to a device the STE will source the VMID from the VIOMMU if present otherwise from the S2
- "RID" attach (ie naked S2) will have to be done with a Nesting Child using a vSTE that indicates Identity. Then the attach logic will have enough information to get the VMID from the VIOMMU

Quoted 1 line

What is this RID attach (naked S2) case? S1DSS_BYPASS + SVA?

No, when the guest installs a vSTE that simply says bypass with no CD table pointer. That should result in a pSTE that is the S2 with on CD pointer.

I was originally thinking that the VMM would simply directly attach the S2 HWPT in this caes, but given the above issue with the VMID lifetime it makes more sense to 'attach' the viommu which holds the correct VMID.

The issue with direct attach the S2 HWPT is the VMID lifetime, as it would have to borrow the VMID from the viommu but then the lifetime becomes more complex as it has to live beyond VIOMMU destruction. Not unsolvable but it seems easier to just avoid it entirely.

Quoted 2 lines

I suspect 0 should be reserved as a non-set value for some basic sanity in all these driver type enums.

We have an IOMMU_HWPT_DATA_NONE for HWPT_ALLOC to compatible with an S2 hwpt, since it doesn't need a data.

Maybe we can have an IOMMU_VIOMMU_TYPE_DEFAULT to be 0, for an IOMMU driver (e.g. VT-d) that doesn't need to handle nor be aware of any viommu object?

That makes a lot sense. I'd need to go through QEMU code and see how we will accommodate these two more naturally: likely the QEMU core should allocate an S2 HWPT for a VM, while the viommu code should allocate a VIOMMU for each instance.

Quoted 6 lines · Nicolin Chen

Quoted 2 lines

I suspect 0 should be reserved as a non-set value for some basic sanity in all these driver type enums.

We have an IOMMU_HWPT_DATA_NONE for HWPT_ALLOC to compatible with an S2 hwpt, since it doesn't need a data.

Quoted 3 lines

Maybe we can have an IOMMU_VIOMMU_TYPE_DEFAULT to be 0, for an IOMMU driver (e.g. VT-d) that doesn't need to handle nor be aware of any viommu object?

Seems like a good practice, and perhaps userspace will find value in a generic viommu object that is always present.

Quoted 4 lines

That makes a lot sense. I'd need to go through QEMU code and see how we will accommodate these two more naturally: likely the QEMU core should allocate an S2 HWPT for a VM, while the viommu code should allocate a VIOMMU for each instance.

I'd suggest that core qemu should allocate the S2 IOAS and pass that to the qemu viommu driver

The qemu viommu driver should create the hwpt and then the viommu and perhaps return the viommu or hwpt back to the core code.

If the vSTE flow above is used for identity then the qemu viommu driver would also have to go an create vSTEs for identity and attach them to all devices before the VM starts up. Then when the OS activates the SMMU it would have to mirror the real vSTE from guest memory to the kernel.

Not sure there is value in having the core qemu code directly access the hwpt/viommu?

Yea. VMM is always allowed to create a viommu to wrap an S2 HWPT. Then, I assume iommufd in this case should allocate a viommu object if !domain_ops->viommu_alloc.

On one side, it may not be straightforward for a qemu viommu driver to hold a shared S2 hwpt, as the driver is typically per instance, though I think it can keep viommu to its own. So passing the S2 hwpt back to qemu core and tie to iommufd handler (ictx) makes sense.

On the other side, there can be some future HW potentially supporting two+ kinds of IO page tables so a VM may have two+ S2 hwpts? Then the core would hold a list of S2 hwpts and the viommu driver would need to try-n-allocate viommu against the list..

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by iommufd to support some HW-acceleration feature from the host level.

For instance, every device behind an ARM SMMU has a Stream ID. The ID is used by ATC invalidation commands so SMMU HW can direct invalidation requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the guest system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface to execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

Quoted 4 lines · Nicolin Chen

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by iommufd to support some HW-acceleration feature from the host level.

Quoted 9 lines

For instance, every device behind an ARM SMMU has a Stream ID. The ID is used by ATC invalidation commands so SMMU HW can direct invalidation requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the guest system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface to execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

I imagine using this as well for the ATC invalidation commands. It would be very easy and simplifying if the command fixup just extracted the vSID from the ATC invalidation and used an xarray to turn it into a pSID and then pushed the resulting command.

Quoted 15 lines · Jason Gunthorpe

Quoted 13 lines · Nicolin Chen

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by iommufd to support some HW-acceleration feature from the host level.

For instance, every device behind an ARM SMMU has a Stream ID. The ID is used by ATC invalidation commands so SMMU HW can direct invalidation requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the guest system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface to execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

Quoted 4 lines

I imagine using this as well for the ATC invalidation commands. It would be very easy and simplifying if the command fixup just extracted the vSID from the ATC invalidation and used an xarray to turn it into a pSID and then pushed the resulting command.

You mean the nested SMMU series right? Actually the set_dev_id ioctl was a part of that until we wanted to try DEV_INVALIDATE.

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

Quoted 16 lines · Nicolin Chen

Quoted 15 lines · Jason Gunthorpe

Quoted 13 lines · Nicolin Chen

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by iommufd to support some HW-acceleration feature from the host level.

For instance, every device behind an ARM SMMU has a Stream ID. The ID is used by ATC invalidation commands so SMMU HW can direct invalidation requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the guest system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface to execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

Quoted 7 lines

Quoted 4 lines

I imagine using this as well for the ATC invalidation commands. It would be very easy and simplifying if the command fixup just extracted the vSID from the ATC invalidation and used an xarray to turn it into a pSID and then pushed the resulting command.

You mean the nested SMMU series right? Actually the set_dev_id ioctl was a part of that until we wanted to try DEV_INVALIDATE.

Yes, there is nothing inherently wrong with DEV_INVALIDATE, we could continue to use that as the API and automatically pick up the VIOMMU instance from the nesting domain to process the ATS.

The VMM needs a reliable place to send the CMDQ data, on ARM/AMD this needs to be an always available global-to-the-viommu thing. Intel needs to associate the virtual invalidation with the correct nesting domain as well.

So I original thought it would nice and simple to have a VIOMMU_INVALIDATE as well.

But.. If we need a nesting domain that is indentity (ie the S2) then when the VIOMMU is created then we'd also create an identity nesting domain as well.

Functionally we could use that global nesting domain to deliver the DEV_INVALIDATE too.

Quoted 2 lines

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

I would like to do this bit by bit. viommu is a big series on its own.

DEV_INVALIDATE is fine, it just can't do ATS invalidation.

We can add ATS invalidation after either as an enhancement as part of adding the VIOMMU either as DEV_INVALIDATE or VIOMMU_INVALIDATE (or both)

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

Quoted 2 lines

Functionally we could use that global nesting domain to deliver the DEV_INVALIDATE too.

If my narrative above is correct, the device is actually still attached to S2 domain via a proxy nested S1 domain. What cache do we need to invalidate except S2 mappings in this case?

Quoted 6 lines

Quoted 2 lines

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

I would like to do this bit by bit. viommu is a big series on its own.

DEV_INVALIDATE is fine, it just can't do ATS invalidation.

I am not very sure about AMD.

Same question: any other case can we use the DEV_INVALIDATE for?

Quoted 3 lines

We can add ATS invalidation after either as an enhancement as part of adding the VIOMMU either as DEV_INVALIDATE or VIOMMU_INVALIDATE (or both)

Yea, maybe step by step like this:

Part-1 VIOMMU_ALLOC and VIOMMU_ATTACH
Part-2 VIOMMU_SET/UNSET_VDEV_ID
Part-3 VIOMMU_INVALIDATE
Part-4 VQUEUE_ALLOC
...

Quoted 5 lines · Nicolin Chen

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

Quoted 6 lines

Quoted 2 lines

Functionally we could use that global nesting domain to deliver the DEV_INVALIDATE too.

If my narrative above is correct, the device is actually still attached to S2 domain via a proxy nested S1 domain. What cache do we need to invalidate except S2 mappings in this case?

qemu needs a reliable place to send the invalidation commands to (ie what ID to provide).

If we add IOMMU_VIOMMU_INVALIDATE then the ID is the viommu id.

If we enhance IOMMU_HWPT_INVALIDATE then the ID is the identity nesting domain.

Either case leads to the viommu object in the kernel.

I don't know if there is merit one way or the other. A more specific API surface is nice, but the two APIs are completely duplicating.

So maybe:

#define IOMMU_VIOMMU_INVALIDATE IOMMU_HWPT_INVALIDATE

As documentation and have the kernel just detect based on the type of the passed ID?

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

I would like to do this bit by bit. viommu is a big series on its own.

DEV_INVALIDATE is fine, it just can't do ATS invalidation.

I am not very sure about AMD.

AMD will need the same vRID -> pRID stuff and we want that to run on the VIOMMU

Quoted 1 line

Same question: any other case can we use the DEV_INVALIDATE for?

DEV_INVALIDATE was interesting before the viommu idea because userspace could process each invalidation command and when it reaches ATS it would invoke the correct DEV_INVALIDATE.

With viommu we expect ATS supporting drivers to support viommu and then to do vRID -> pRID in the other invalidation paths. In this case I don't see a reason to do DEV_INVALIDATE right now.

Quoted 11 lines

Quoted 3 lines

We can add ATS invalidation after either as an enhancement as part of adding the VIOMMU either as DEV_INVALIDATE or VIOMMU_INVALIDATE (or both)

Yea, maybe step by step like this:

Part-1 VIOMMU_ALLOC and VIOMMU_ATTACH
Part-2 VIOMMU_SET/UNSET_VDEV_ID
Part-3 VIOMMU_INVALIDATE
Part-4 VQUEUE_ALLOC
...

So we have this stuff still open:
- Identity STE with PASID (part 2b)
- IOMMU_GET_HW_INFO (part 3)
- IOMMU_HWPT_ALLOC_NEST_PARENT (part 3)
- IOMMU_HWPT_DATA_ARM_SMMUV3 (part 3)
- IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3
- VIOMMU_ALLOC, VIOMMU_ATTACH
- VIOMMU_INVALIDATE
- VIOMMU_SET/UNSET_VDEV_ID
- VQUEUE_ALLOC / vCMDQ

I feel like IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3 is a reasonable fit to part 3. Then part 4 would be VIOMMU_ALLOC -> VIOMMU_SET/UNSET_VDEV_ID which brings ATS support the API.

vCMDQ hypervisor support would sit on top of that with just VQUEUE?

Quoted 7 lines · Jason Gunthorpe

Quoted 5 lines · Nicolin Chen

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

Quoted 2 lines

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

Yea. For vSTE=abort, likely we need a nested block domain too?

Quoted 7 lines

I don't know if there is merit one way or the other. A more specific API surface is nice, but the two APIs are completely duplicating.

So maybe:

#define IOMMU_VIOMMU_INVALIDATE IOMMU_HWPT_INVALIDATE

Quoted 2 lines

As documentation and have the kernel just detect based on the type of the passed ID?

Yea, the only difference is viommu_id v.s. hwpt_id that we can document.

Then in this case, we have two mostly identical uAPIs for the SMMU driver to use. Should we implement both?

Quoted 23 lines

Quoted 11 lines

Quoted 3 lines

We can add ATS invalidation after either as an enhancement as part of adding the VIOMMU either as DEV_INVALIDATE or VIOMMU_INVALIDATE (or both)

Yea, maybe step by step like this:

Part-1 VIOMMU_ALLOC and VIOMMU_ATTACH
Part-2 VIOMMU_SET/UNSET_VDEV_ID
Part-3 VIOMMU_INVALIDATE
Part-4 VQUEUE_ALLOC
...

So we have this stuff still open:
- Identity STE with PASID (part 2b)
- IOMMU_GET_HW_INFO (part 3)
- IOMMU_HWPT_ALLOC_NEST_PARENT (part 3)
- IOMMU_HWPT_DATA_ARM_SMMUV3 (part 3)
- IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3
- VIOMMU_ALLOC, VIOMMU_ATTACH
- VIOMMU_INVALIDATE
- VIOMMU_SET/UNSET_VDEV_ID
- VQUEUE_ALLOC / vCMDQ

Quoted 3 lines

I feel like IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3 is a reasonable fit to part 3. Then part 4 would be VIOMMU_ALLOC -> VIOMMU_SET/UNSET_VDEV_ID which brings ATS support the API.

There is some conflict at passing in viommu_id/viommu v.s. parent hwpt_id/domain for a nested domain allocation. Do you think that should be addressed later in VIOMMU series v.s. part3?

More specifically, I have two drafts in my viommu series:
87a659e65229 WAR: iommufd: Allow pt_it to carry viommu_id 7c5fd8f50bc9 WAR pass in viommu pointer to domain_alloc_user op

I know that these two only make sense with VIOMMU_ALOC. Yet, will there be a problem, if we establish nested domain allocation with parent domain/hwpt by part3, in the uAPI, and then change later? Will we end up with supporting two for backward compatibility?

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 2:25 AM

On Tue, May 14, 2024 at 06:59:07PM -0700, Nicolin Chen wrote:

Quoted 5 lines

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

Quoted 10 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 2:25 AM

Quoted 5 lines · Nicolin Chen

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

Quoted 3 lines

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

why does ATC invalidation need to know about VMID?

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, May 23, 2024 11:01 PM

Quoted 14 lines · Kevin

Quoted 13 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 2:25 AM

Quoted 5 lines · Nicolin Chen

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

Quoted 1 line

why does ATC invalidation need to know about VMID?

ATC invalidation always requires a vRID to pRID translation and the VIOMMU will hold that translation.

On vCMDQ HW and on AMD HW the vRID to pRID translation is pushed into HW, and vCMDQ requires the VMID to do that.

Quoted 18 lines

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, May 23, 2024 11:01 PM

Quoted 14 lines · Kevin

Quoted 13 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 2:25 AM

Quoted 5 lines · Nicolin Chen

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

Quoted 8 lines

Quoted 1 line

why does ATC invalidation need to know about VMID?

ATC invalidation always requires a vRID to pRID translation and the VIOMMU will hold that translation.

On vCMDQ HW and on AMD HW the vRID to pRID translation is pushed into HW, and vCMDQ requires the VMID to do that.

At a quick glance VMID and vRID->pRID translation are both configurations of a vintf.

My impression was that vintf->vmid is added to guest cmd when it's about iotlb invalidation.

then vintf->sid_slots is walked when handling a guest cmd for ATC invalidation.

I'm not sure why the latter one requires a valid VMID to do the walking except it's a implementation choice made by vCMDQ?

From: Nicolin Chen <[email protected]>
Sent: Friday, May 24, 2024 11:26 AM

Quoted 4 lines · Kevin

Quoted 4 lines

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, May 23, 2024 11:01 PM

On Thu, May 23, 2024 at 06:19:59AM +0000, Tian, Kevin wrote:

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 2:25 AM

Quoted 4 lines · Nicolin Chen

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device

can't

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Quoted 1 line

attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

Quoted 6 lines

Quoted 6 lines

Quoted 1 line

why does ATC invalidation need to know about VMID?

ATC invalidation always requires a vRID to pRID translation and the VIOMMU will hold that translation.

On vCMDQ HW and on AMD HW the vRID to pRID translation is pushed

into

Quoted 12 lines

Quoted 2 lines

HW, and vCMDQ requires the VMID to do that.

At a quick glance VMID and vRID->pRID translation are both configurations of a vintf.

My impression was that vintf->vmid is added to guest cmd when it's about iotlb invalidation.

then vintf->sid_slots is walked when handling a guest cmd for ATC invalidation.

Quoted 2 lines

I'm not sure why the latter one requires a valid VMID to do the walking except it's a implementation choice made by vCMDQ?

Well, we haven't thought about a case of doing ATC invalidation via VINTF/VCMDQ without setting up a VMID, as "VMID" is a field in the VINTF_CONFIG register next to the Enable bit and must be set prior to enabling a VINTF, though your understanding of the HW work flow is probably accurate :)

And the narrative at the top was trying to describe the links:
[ device ] => [ proxy identity S1 ] => [ viommu [ shared S2 ] ]
v.s.
[ device ] => [ non-shareable S2 ]

So the first case can take advantage of VIOMMU_INVALIDATE v.s. the second case requires a DEV_INVALIDATE.

Another conclusion between the lines: since ARM SMMU will have the first case (with viommu), there is no longer any use of a DEV_INVALIDATE ioctl. So, we would likely drop it in the coming version.

Thanks
Nicolin

Quoted 16 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines · Nicolin Chen

Quoted 4 lines

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device

can't

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

And the narrative at the top was trying to describe the links:
[ device ] => [ proxy identity S1 ] => [ viommu [ shared S2 ] ]
v.s.
[ device ] => [ non-shareable S2 ]

Quoted 2 lines

So the first case can take advantage of VIOMMU_INVALIDATE v.s. the second case requires a DEV_INVALIDATE.

and one side-effect in the first case is to save one VMID for non-shareable S2 hence improves iotlb efficiency.

From: Nicolin Chen <[email protected]>
Sent: Friday, May 24, 2024 1:58 PM

Quoted 4 lines · Kevin

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines · Nicolin Chen

Quoted 3 lines

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver

case,

Quoted 5 lines

Quoted 5 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

an arm_smmu_domain won't have an smmu pointer, so a device

can't

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it

means

Quoted 7 lines

Quoted 7 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

the kernel has to decode more types of vSTE.

And the narrative at the top was trying to describe the links:
[ device ] => [ proxy identity S1 ] => [ viommu [ shared S2 ] ]
v.s.
[ device ] => [ non-shareable S2 ]

Quoted 5 lines

Quoted 2 lines

So the first case can take advantage of VIOMMU_INVALIDATE v.s. the second case requires a DEV_INVALIDATE.

and one side-effect in the first case is to save one VMID for non-shareable S2 hence improves iotlb efficiency.

Hmm, how is that?

VMID is currently stored in an S2 domain, actually. The viommu is a VMID holder to potentially decouple VMID from S2 domain, because VMID is per SMMU instance while S2 domain is shareable:

   [ dev0 ] => [ S1 dom0 ] => [ viommu0 (VMID0) [ shared S2 ] ]
   [ dev1 ] => [ S1 dom1 ] => [ viommu1 (VMID1) [ shared S2 ] ]

My point was based on Jason's example about 3 VMIDs:

hwpt_alloc(deva, nesting_parent=true) = shared_s2 viommu_alloc(deva, shared_s2) = viommu1 viommu_alloc(devb, shared_s2) = viommu2 hwpt_alloc(deva, viommu1, vste) = deva_vste hwpt_alloc(devb, viommu2, vste) = devb_vste attach(deva, deva_vste)
attach(devb, devb_vste)
attach(devc, shared_s2)

for devc it could be:
hwpt_alloc(deva, viommu1, vproxy_s1) = devc_proxys1 attach(devc, devc_proxys1)

then devc will reuse VMID of viommu1 and we save one VMID.

Does that not work so we need create another viommu to hold the proxy identity s1 then still need a 3rd VMID?

Quoted 18 lines

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, May 23, 2024 11:01 PM

Quoted 14 lines · Kevin

Quoted 13 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 22, 2024 2:25 AM

Quoted 5 lines · Nicolin Chen

So, you want a proxy S1 domain for a device to attach, in case of a stage-2 only setup, because an S2 domain will no longer has a VMID, since it's shared among viommus. In the SMMU driver case, an arm_smmu_domain won't have an smmu pointer, so a device can't attach to an S2 domain but always an nested S1 domain, right?

That seems like a simple solution to the VMID lifetime, but it means the kernel has to decode more types of vSTE.

Quoted 8 lines

Quoted 1 line

why does ATC invalidation need to know about VMID?

ATC invalidation always requires a vRID to pRID translation and the VIOMMU will hold that translation.

On vCMDQ HW and on AMD HW the vRID to pRID translation is pushed into HW, and vCMDQ requires the VMID to do that.

At a quick glance VMID and vRID->pRID translation are both configurations of a vintf.

My impression was that vintf->vmid is added to guest cmd when it's about iotlb invalidation.

then vintf->sid_slots is walked when handling a guest cmd for ATC invalidation.

I'm not sure why the latter one requires a valid VMID to do the walking except it's a implementation choice made by vCMDQ?

From: Nicolin Chen <[email protected]>
Sent: Monday, May 13, 2024 12:39 PM

Quoted 7 lines · Jason Gunthorpe

Quoted 6 lines · Nicolin Chen

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by iommufd to support some HW-acceleration feature from the host level.

For instance, every device behind an ARM SMMU has a Stream ID. The ID is used by ATC invalidation commands so SMMU HW can direct

invalidation

Quoted 4 lines

Quoted 4 lines

requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the guest system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface

to

Quoted 8 lines

Quoted 3 lines

execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

I imagine using this as well for the ATC invalidation commands. It would be very easy and simplifying if the command fixup just extracted the vSID from the ATC invalidation and used an xarray to turn it into a pSID and then pushed the resulting command.

You mean the nested SMMU series right? Actually the set_dev_id ioctl was a part of that until we wanted to try DEV_INVALIDATE.

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

Quoted 26 lines

From: Nicolin Chen <[email protected]>
Sent: Monday, May 13, 2024 12:39 PM

Quoted 7 lines · Jason Gunthorpe

Quoted 6 lines · Nicolin Chen

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by iommufd to support some HW-acceleration feature from the host level.

For instance, every device behind an ARM SMMU has a Stream ID. The ID is used by ATC invalidation commands so SMMU HW can direct

invalidation

Quoted 4 lines

Quoted 4 lines

requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the guest system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface

to

Quoted 8 lines

Quoted 3 lines

execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

I imagine using this as well for the ATC invalidation commands. It would be very easy and simplifying if the command fixup just extracted the vSID from the ATC invalidation and used an xarray to turn it into a pSID and then pushed the resulting command.

Quoted 5 lines

You mean the nested SMMU series right? Actually the set_dev_id ioctl was a part of that until we wanted to try DEV_INVALIDATE.

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

I'm right about to ask how the nesting series is going. Per earlier discussion iirc the nesting series will go in before VCMDQ?

So we have this stuff still open:
- Identity STE with PASID (part 2b)
- IOMMU_GET_HW_INFO (part 3)
- IOMMU_HWPT_ALLOC_NEST_PARENT (part 3)
- IOMMU_HWPT_DATA_ARM_SMMUV3 (part 3)
- IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3
- VIOMMU_ALLOC, VIOMMU_ATTACH
- VIOMMU_INVALIDATE

From: Nicolin Chen <[email protected]>
Sent: Thursday, May 23, 2024 2:10 PM

Quoted 7 lines · Kevin

Quoted 7 lines

From: Nicolin Chen <[email protected]>
Sent: Monday, May 13, 2024 12:39 PM

Quoted 3 lines · Jason Gunthorpe

Quoted 2 lines · Nicolin Chen

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by

iommufd

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

to support some HW-acceleration feature from the host level.

For instance, every device behind an ARM SMMU has a Stream ID. The

ID

Quoted 5 lines

Quoted 5 lines

Quoted 1 line

Quoted 1 line

is used by ATC invalidation commands so SMMU HW can direct

invalidation

Quoted 3 lines

Quoted 3 lines

requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the

guest

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

system. NVIDIA's CMDQV extension for SMMUv3 provides a v-

interface

Quoted 10 lines

Quoted 10 lines

to

Quoted 8 lines

Quoted 3 lines

execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

I imagine using this as well for the ATC invalidation commands. It would be very easy and simplifying if the command fixup just extracted the vSID from the ATC invalidation and used an xarray to turn it into a pSID and then pushed the resulting command.

Quoted 8 lines

Quoted 5 lines

You mean the nested SMMU series right? Actually the set_dev_id ioctl was a part of that until we wanted to try DEV_INVALIDATE.

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

I'm right about to ask how the nesting series is going. Per earlier discussion iirc the nesting series will go in before VCMDQ?

Yes. It still should. Yet we ended up with adding VIOMMU to the nested SMMU series too. A shared S2 domain/hwpt isn't exclusive for VCMDQ use case but also for regular nesting on a multi-SMMU setup. So, VIOMMU turns out to be the best object that we have at this moment to hold individual VMIDs for different physical SMMUs sharing a single S2 domain. Its virtual device ID lookup feature can also allow us to forget about DEV_INVALIDATE ioctl for now.

Jason listed all the tasks ahead in this thread too, using SMMU as an example:

Quoted 8 lines

So we have this stuff still open:
- Identity STE with PASID (part 2b)
- IOMMU_GET_HW_INFO (part 3)
- IOMMU_HWPT_ALLOC_NEST_PARENT (part 3)
- IOMMU_HWPT_DATA_ARM_SMMUV3 (part 3)
- IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3
- VIOMMU_ALLOC, VIOMMU_ATTACH
- VIOMMU_INVALIDATE

By this series nesting setup is done. We need Baolu's solution or VQUEUE for fault reporting after that.

Quoted 26 lines · Jason Gunthorpe

Quoted 7 lines · Nicolin Chen

Quoted 6 lines

Add a pair of ops to set and unet device's virtual ID that belongs to a viommu object. They will be used, in the following patch, by iommufd to support some HW-acceleration feature from the host level.

For instance, every device behind an ARM SMMU has a Stream ID. The ID is used by ATC invalidation commands so SMMU HW can direct

invalidation

Quoted 4 lines

Quoted 4 lines

requests to the corresponding PCI device where the ID belongs to. In a virtualization use case, a passthroughed device in the VM will have a virtuail Stream ID, used by the ATC invalidation commands in the guest system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface

to

Quoted 8 lines

Quoted 3 lines

execute the guest-level ATC invalidation commands directly, yet needs the HW to be aware of its virtual Stream ID so it can replace with its physical Stream ID.

I imagine using this as well for the ATC invalidation commands. It would be very easy and simplifying if the command fixup just extracted the vSID from the ATC invalidation and used an xarray to turn it into a pSID and then pushed the resulting command.

You mean the nested SMMU series right? Actually the set_dev_id ioctl was a part of that until we wanted to try DEV_INVALIDATE.

Quoted 3 lines

So again, yes, it makes sense to me that we move viommu and the set_dev_id to the nested series, and then drop DEV_INVALIDATE.

I'm right about to ask how the nesting series is going. Per earlier discussion iirc the nesting series will go in before VCMDQ?

Introduce a new ioctl to set a per-viommu device virtual id that should be linked to the physical device id (or just a struct device pointer).

Since a viommu (user space IOMMU instance) can have multiple devices while it's not ideal to confine a device to one single user space IOMMU instance either, these two shouldn't just do a 1:1 mapping. Add two xarrays in their structures to bind them bidirectionally.

drivers/iommu/iommufd/viommu.c +29 -0

+int iommufd_viommu_set_device_id(struct iommufd_ucmd *ucmd)
+{
+	struct iommu_viommu_set_dev_id *cmd = ucmd->cmd;
+	unsigned int dev_id, viommu_id;
+	struct iommufd_viommu *viommu;
+	struct iommufd_device *idev;
+	int rc;
+
+	idev = iommufd_get_device(ucmd, cmd->dev_id);
+	if (IS_ERR(idev))
+		return PTR_ERR(idev);
+	dev_id = idev->obj.id;
+
+	viommu = iommufd_get_viommu(ucmd, cmd->viommu_id);
+	if (IS_ERR(viommu)) {
+		rc = PTR_ERR(viommu);
+		goto out_put_idev;
+	}
+
+	if (viommu->iommu_dev != idev->dev->iommu->iommu_dev) {
+		rc = -EINVAL;
+		goto out_put_viommu;
+	}
+
+	if (!viommu->ops->set_dev_id || !viommu->ops->unset_dev_id) {
+		rc = -EOPNOTSUPP;
+		goto out_put_viommu;
+	}
+

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

+
+/**
+ * struct iommu_viommu_set_dev_id - ioctl(IOMMU_VIOMMU_SET_DEV_ID)
+ * @size: sizeof(struct iommu_viommu_set_dev_id)
+ * @viommu_id: viommu ID to associate with the device to store its virtual ID
+ * @dev_id: device ID to set a device virtual ID
+ * @__reserved: Must be 0

Quoted 7 lines · Nicolin Chen

Introduce a new ioctl to set a per-viommu device virtual id that should be linked to the physical device id (or just a struct device pointer).

Since a viommu (user space IOMMU instance) can have multiple devices while it's not ideal to confine a device to one single user space IOMMU instance either, these two shouldn't just do a 1:1 mapping. Add two xarrays in their structures to bind them bidirectionally.

Since I would like to retain the dev_id, I think this is probably better done with an allocated struct per dev-id:

struct dev_id {
    struct iommufd_device *idev;
    struct iommufd_viommu *viommu;
    u64 vdev_id;
    u64 driver_private; // Ie the driver can store the pSID here
    struct list_head idev_entry;
};

Quoted 7 lines · Nicolin Chen

Introduce a new ioctl to set a per-viommu device virtual id that should be linked to the physical device id (or just a struct device pointer).

Since a viommu (user space IOMMU instance) can have multiple devices while it's not ideal to confine a device to one single user space IOMMU instance either, these two shouldn't just do a 1:1 mapping. Add two xarrays in their structures to bind them bidirectionally.

Since I would like to retain the dev_id, I think this is probably better done with an allocated struct per dev-id:

struct dev_id {
    struct iommufd_device *idev;
    struct iommufd_viommu *viommu;
    u64 vdev_id;
    u64 driver_private; // Ie the driver can store the pSID here
    struct list_head idev_entry;
};

Quoted 7 lines

drivers/iommu/iommufd/device.c +2 -0

@@ -135,7 +135,16 @@ void iommufd_device_destroy(struct iommufd_object *obj)
 {
 	struct iommufd_device *idev =
 		container_of(obj, struct iommufd_device, obj);
+	struct iommufd_viommu *viommu;
+	unsigned long index;
 

drivers/iommu/iommufd/device.c void iommufd_device_destroy(struct iommufd_object *obj) · line 146

Quoted 6 lines

drivers/iommu/iommufd/device.c +6 -0

+	xa_for_each(&idev->viommus, index, viommu) {
+		if (viommu->ops->unset_dev_id)
+			viommu->ops->unset_dev_id(viommu, idev->dev);
+		xa_erase(&viommu->idevs, idev->obj.id);
+		xa_erase(&idev->viommus, index);
+	}

Then this turns into list_for_each(idev->viommu_vdevid_list)

Quoted 2 lines

drivers/iommu/iommufd/viommu.c +2 -0

+int iommufd_viommu_set_device_id(struct iommufd_ucmd *ucmd)
+{

drivers/iommu/iommufd/viommu.c int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) · line 145

Quoted 10 lines

drivers/iommu/iommufd/viommu.c +10 -0

+	rc = xa_alloc(&idev->viommus, &viommu_id, viommu,
+		      XA_LIMIT(viommu->obj.id, viommu->obj.id),
+		      GFP_KERNEL_ACCOUNT);
+	if (rc)
+		goto out_put_viommu;
+
+	rc = xa_alloc(&viommu->idevs, &dev_id, idev,
+		      XA_LIMIT(dev_id, dev_id), GFP_KERNEL_ACCOUNT);
+	if (rc)
+		goto out_xa_erase_viommu;

Both of these are API mis-uses, you don't want an allocating xarray you just want to use xa_cmpxchg

Put an xarray in the viommu object and fill it with pointers to the struct dev_id thing above

The driver can piggyback on this xarray too if it wants, so we only need one.

I implemented it with a small tweak, to align with viommu_alloc and vqueue_alloc:

	// core
	struct iommufd_vdev_id {
		struct iommufd_viommu *viommu;
		struct device *dev;
		u64 vdev_id;
		struct list_head idev_item;
	};

	// driver
	struct my_driver_vdev_id {
	    struct iommufd_vdev_id core;
	    unsigned int private_attrs;
	};

	static struct iommufd_vdev_id *
	my_driver_set_vdev_id(struct iommufd_viommu *viommu,
			      struct device *dev, u64 id)
	{
	    struct my_driver_vdev_id *my_vdev_id;

	    my_vdev_id = kzalloc(sizeof(*my_vdev_id), GFP_KERNEL);
	    .... /* set private_attrs */
	    return &my_driver_vdev_id->core;
	}

	static void my_driver_unset_vdev_id(struct iommufd_vdev_id *vdev_id)
	{
	    struct my_driver_vdev_id *my_vdev_id =
		    container_of(vdev_id, struct my_driver_vdev_id, core);
	    .... /* unset private_attrs */
	}

Please let me know if you like it inverted as you wrote above.

I also moved xa_cmpxchg to the driver, as I feel that the vdev_id here is very driver/iommu specific. There can be some complication if iommufd core handles this u64 vdev_id: most likely we will use this u64 vdev_id to index the xarray that takes an unsigned-long xa_index for a fast vdev_id-to-pdev_id lookup, while only a driver knows whether u64 vdev_id is compatible with unsigned long or not.

And, we have a list_head in the structure idev, so a device unbind will for-each the list and unset all the vdev_ids in it, meanwhile the viommu stays. I wonder if we need to add another list_head in the structure viommu, so a viommu tear down will for-each its list and unset all the vdev_ids on its side while a device (idev) stays. I don't see a use case of that though..any thought?

Quoted 6 lines · Nicolin Chen

I also moved xa_cmpxchg to the driver, as I feel that the vdev_id here is very driver/iommu specific. There can be some complication if iommufd core handles this u64 vdev_id: most likely we will use this u64 vdev_id to index the xarray that takes an unsigned-long xa_index for a fast vdev_id-to-pdev_id lookup, while only a driver knows whether u64 vdev_id is compatible with unsigned long or not.

This seems like the most general part.. The core code should just have a check like:

  if (vdevid >= ULONG_MAX) return -EINVAL;

And if someone wants to make 32 bit kernels support a 64bit vdevid then they can sort it out someday :) I think this is not a big issue as all iommus seem to have some kind of radix lookup for vdevid and want it to be small.

Matthew has been talking about support for 64bit indexes in maple/xarray or something for a bit so it might sort itself out.

Quoted 6 lines

And, we have a list_head in the structure idev, so a device unbind will for-each the list and unset all the vdev_ids in it, meanwhile the viommu stays. I wonder if we need to add another list_head in the structure viommu, so a viommu tear down will for-each its list and unset all the vdev_ids on its side while a device (idev) stays. I don't see a use case of that though..any thought?

I think you need to support viommu teardown, at least from a correctness perspective. The API permits it. But isn't it just list_del everything in the xarray and that will clean it up enough?

From: Nicolin Chen <[email protected]>
Sent: Saturday, April 13, 2024 11:47 AM

Introduce a new ioctl to set a per-viommu device virtual id that should be linked to the physical device id (or just a struct device pointer).

Since a viommu (user space IOMMU instance) can have multiple devices

while
it's not ideal to confine a device to one single user space IOMMU instance either, these two shouldn't just do a 1:1 mapping. Add two xarrays in

Quoted 7 lines

From: Nicolin Chen <[email protected]>
Sent: Saturday, April 13, 2024 11:47 AM

Introduce a new ioctl to set a per-viommu device virtual id that should be linked to the physical device id (or just a struct device pointer).

Since a viommu (user space IOMMU instance) can have multiple devices

this is true...

Quoted 3 lines

while
it's not ideal to confine a device to one single user space IOMMU instance either, these two shouldn't just do a 1:1 mapping. Add two xarrays in

...but why would one device link to multiple viommu instances?

Or is it referring to Tegra194 as arm-smmu-nvidia.c tries to support?

btw there is a check in the following code:

drivers/iommu/iommufd/viommu.c +4 -0

+       if (viommu->iommu_dev != idev->dev->iommu->iommu_dev) {
+               rc = -EINVAL;
+               goto out_put_viommu;
+       }

I vaguely remember an earlier discussion about multiple vSMMU instances following the physical SMMU topology, but don't quite recall the exact reason.

What is the actual technical obstacle prohibiting one to put multiple VCMDQ's from different SMMU's into one vIOMMU instance?

From: Nicolin Chen <[email protected]>
Sent: Friday, May 24, 2024 1:40 PM

Quoted 7 lines · Kevin

btw there is a check in the following code:

drivers/iommu/iommufd/viommu.c +4 -0

+       if (viommu->iommu_dev != idev->dev->iommu->iommu_dev) {
+               rc = -EINVAL;
+               goto out_put_viommu;
+       }

Quoted 6 lines

I vaguely remember an earlier discussion about multiple vSMMU instances following the physical SMMU topology, but don't quite recall the exact reason.

What is the actual technical obstacle prohibiting one to put multiple VCMDQ's from different SMMU's into one vIOMMU instance?

Because a VCMDQ passthrough involves a direct mmap of a HW MMIO page to the guest-level MMIO region. The MMIO page provides the read/write of queue's head and tail indexes.

With a single pSMMU and a single vSMMU, it's simply 1:1 mapping.

With a multi-pSMMU and a single vSMMU, the single vSMMU will see one guest-level MMIO region backed by multiple physical pages. Since we are talking about MMIO, technically it cannot select the corresponding MMIO page to the device, not to mention we don't really want VMM to involve, i.e. no VM exist, when using VCMDQ.

So, there must be some kind of multi-instanced carriers to hold those MMIO pages, by attaching devices behind different pSMMUs to corresponding carriers. And today we have VIOMMU as the carrier.

One step back, even without VCMDQ feature, a multi-pSMMU setup will have multiple viommus (with our latest design) being added to a viommu list of a single vSMMU's. Yet, vSMMU in this case always traps regular SMMU CMDQ, so it can do viommu selection or even broadcast (if it has to).

Quoted 24 lines

From: Nicolin Chen <[email protected]>
Sent: Friday, May 24, 2024 1:40 PM

Quoted 7 lines · Kevin

btw there is a check in the following code:

drivers/iommu/iommufd/viommu.c +4 -0

+       if (viommu->iommu_dev != idev->dev->iommu->iommu_dev) {
+               rc = -EINVAL;
+               goto out_put_viommu;
+       }

Quoted 6 lines

I vaguely remember an earlier discussion about multiple vSMMU instances following the physical SMMU topology, but don't quite recall the exact reason.

What is the actual technical obstacle prohibiting one to put multiple VCMDQ's from different SMMU's into one vIOMMU instance?

Because a VCMDQ passthrough involves a direct mmap of a HW MMIO page to the guest-level MMIO region. The MMIO page provides the read/write of queue's head and tail indexes.

With a single pSMMU and a single vSMMU, it's simply 1:1 mapping.

Quoted 5 lines

With a multi-pSMMU and a single vSMMU, the single vSMMU will see one guest-level MMIO region backed by multiple physical pages. Since we are talking about MMIO, technically it cannot select the corresponding MMIO page to the device, not to mention we don't really want VMM to involve, i.e. no VM exist, when using VCMDQ.

can a vSMMU report to support multiple CMDQ's then there are several MMIO regions each mapped to a different backend VCMDQ?

but I guess even if it's supported there is still a problem describing the association between assigned devices and the CMDQ's of the single vIOMMU instance.

I'm curious to learn the real reason of that design. Is it because you want to do certain load-balance between viommu's or due to other reasons in the kernel smmuv3 driver which e.g. cannot support a viommu spanning multiple pSMMU?

From: Jason Gunthorpe <[email protected]>
Sent: Friday, May 24, 2024 9:19 PM

Quoted 4 lines · Kevin

I'm curious to learn the real reason of that design. Is it because you want to do certain load-balance between viommu's or due to other reasons in the kernel smmuv3 driver which e.g. cannot support a viommu spanning multiple pSMMU?

Yeah, there is no concept of support for a SMMUv3 instance where it's command Q's can only work on a subset of devices.

My expectation was that VIOMMU would be 1:1 with physical iommu instances, I think AMD needs this too??

Quoted 9 lines

From: Jason Gunthorpe <[email protected]>
Sent: Friday, May 24, 2024 9:19 PM

Quoted 4 lines · Kevin

I'm curious to learn the real reason of that design. Is it because you want to do certain load-balance between viommu's or due to other reasons in the kernel smmuv3 driver which e.g. cannot support a viommu spanning multiple pSMMU?

Quoted 6 lines

Yeah, there is no concept of support for a SMMUv3 instance where it's command Q's can only work on a subset of devices.

My expectation was that VIOMMU would be 1:1 with physical iommu instances, I think AMD needs this too??

Yes this part is clear now regarding to VCMDQ.

But Nicoline said:

"
One step back, even without VCMDQ feature, a multi-pSMMU setup will have multiple viommus (with our latest design) being added to a viommu list of a single vSMMU's. Yet, vSMMU in this case always traps regular SMMU CMDQ, so it can do viommu selection or even broadcast (if it has to).
"

I don't think there is an arch limitation mandating that?

Quoted 28 lines · Kevin

Quoted 9 lines

From: Jason Gunthorpe <[email protected]>
Sent: Friday, May 24, 2024 9:19 PM

Quoted 4 lines · Kevin

I'm curious to learn the real reason of that design. Is it because you want to do certain load-balance between viommu's or due to other reasons in the kernel smmuv3 driver which e.g. cannot support a viommu spanning multiple pSMMU?

Quoted 6 lines

Yeah, there is no concept of support for a SMMUv3 instance where it's command Q's can only work on a subset of devices.

My expectation was that VIOMMU would be 1:1 with physical iommu instances, I think AMD needs this too??

Yes this part is clear now regarding to VCMDQ.

But Nicoline said:

"
One step back, even without VCMDQ feature, a multi-pSMMU setup will have multiple viommus (with our latest design) being added to a viommu list of a single vSMMU's. Yet, vSMMU in this case always traps regular SMMU CMDQ, so it can do viommu selection or even broadcast (if it has to).
"

Quoted 1 line

I don't think there is an arch limitation mandating that?

What I mean is for regular vSMMU. Without VCMDQ, a regular vSMMU on a multi-pSMMU setup will look like (e.g. three devices behind different SMMUs):

|<------ VMM ------->|<------ kernel ------>|
       |-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- viommu1 --|-- pSMMU1 --|--s2_hwpt
       |-- viommu2 --|-- pSMMU2 --|

And device would attach to:

|<---- guest ---->|<--- VMM --->|<- kernel ->|
       |-- dev0 --|-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- dev1 --|-- viommu1 --|-- pSMMU1 --|
       |-- dev2 --|-- viommu2 --|-- pSMMU2 --|

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 4:23 AM

Quoted 28 lines · Kevin

Quoted 9 lines

From: Jason Gunthorpe <[email protected]>
Sent: Friday, May 24, 2024 9:19 PM

Quoted 4 lines · Kevin

I'm curious to learn the real reason of that design. Is it because you want to do certain load-balance between viommu's or due to other reasons in the kernel smmuv3 driver which e.g. cannot support a viommu spanning multiple pSMMU?

Quoted 6 lines

Yeah, there is no concept of support for a SMMUv3 instance where it's command Q's can only work on a subset of devices.

My expectation was that VIOMMU would be 1:1 with physical iommu instances, I think AMD needs this too??

Yes this part is clear now regarding to VCMDQ.

But Nicoline said:

"
One step back, even without VCMDQ feature, a multi-pSMMU setup will have multiple viommus (with our latest design) being added to a viommu list of a single vSMMU's. Yet, vSMMU in this case always traps regular SMMU CMDQ, so it can do viommu selection or even broadcast (if it has to).
"

Quoted 1 line

I don't think there is an arch limitation mandating that?

What I mean is for regular vSMMU. Without VCMDQ, a regular vSMMU on a multi-pSMMU setup will look like (e.g. three devices behind different SMMUs):

|<------ VMM ------->|<------ kernel ------>|
       |-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- viommu1 --|-- pSMMU1 --|--s2_hwpt
       |-- viommu2 --|-- pSMMU2 --|

And device would attach to:

|<---- guest ---->|<--- VMM --->|<- kernel ->|
       |-- dev0 --|-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- dev1 --|-- viommu1 --|-- pSMMU1 --|
       |-- dev2 --|-- viommu2 --|-- pSMMU2 --|

When trapping a device cache invalidation: it is straightforward by deciphering the virtual device ID to pick the viommu that the device is attached to.

When doing iotlb invalidation, a command may or may not contain an ASID (a domain ID, and nested domain in this case):
a) if a command doesn't have an ASID, VMM needs to broadcast the command to all viommus (i.e. pSMMUs)
b) if a command has an ASID, VMM needs to initially maintain an S1 HWPT list by linking an ASID when adding an HWPT entry to the list, by deciphering vSTE and its linked CD. Then it needs to go through the S1 list with the ASID in the command, and to find all corresponding HWPTs to issue/broadcast the command.

Thanks
Nicolin

Quoted 12 lines

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 4:23 AM

What I mean is for regular vSMMU. Without VCMDQ, a regular vSMMU on a multi-pSMMU setup will look like (e.g. three devices behind different SMMUs):

|<------ VMM ------->|<------ kernel ------>|
       |-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- viommu1 --|-- pSMMU1 --|--s2_hwpt
       |-- viommu2 --|-- pSMMU2 --|

And device would attach to:

Quoted 8 lines

|<---- guest ---->|<--- VMM --->|<- kernel ->|
       |-- dev0 --|-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- dev1 --|-- viommu1 --|-- pSMMU1 --|
       |-- dev2 --|-- viommu2 --|-- pSMMU2 --|

When trapping a device cache invalidation: it is straightforward by deciphering the virtual device ID to pick the viommu that the device is attached to.

I understand how above works.

My question is why that option is chosen instead of going with 1:1 mapping between vSMMU and viommu i.e. letting the kernel to figure out which pSMMU should be sent an invalidation cmd to, as how VT-d is virtualized.

I want to know whether doing so is simply to be compatible with what VCMDQ requires, or due to another untold reason.

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 11:21 AM

Quoted 28 lines · Kevin

Quoted 12 lines

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 4:23 AM

What I mean is for regular vSMMU. Without VCMDQ, a regular vSMMU on a multi-pSMMU setup will look like (e.g. three devices behind different SMMUs):

|<------ VMM ------->|<------ kernel ------>|
       |-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- viommu1 --|-- pSMMU1 --|--s2_hwpt
       |-- viommu2 --|-- pSMMU2 --|

And device would attach to:

Quoted 8 lines

|<---- guest ---->|<--- VMM --->|<- kernel ->|
       |-- dev0 --|-- viommu0 --|-- pSMMU0 --|
vSMMU--|-- dev1 --|-- viommu1 --|-- pSMMU1 --|
       |-- dev2 --|-- viommu2 --|-- pSMMU2 --|

When trapping a device cache invalidation: it is straightforward by deciphering the virtual device ID to pick the viommu that the device is attached to.

I understand how above works.

My question is why that option is chosen instead of going with 1:1 mapping between vSMMU and viommu i.e. letting the kernel to figure out which pSMMU should be sent an invalidation cmd to, as how VT-d is virtualized.

Quoted 2 lines

I want to know whether doing so is simply to be compatible with what VCMDQ requires, or due to another untold reason.

Because we use viommu as a VMID holder for SMMU. So a pSMMU must have its own viommu to store its VMID for a shared s2_hwpt:

        |-- viommu0 (VMIDx) --|-- pSMMU0 --|
 vSMMU--|-- viommu1 (VMIDy) --|-- pSMMU1 --|--s2_hwpt
        |-- viommu2 (VMIDz) --|-- pSMMU2 --|

Quoted 8 lines

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 11:21 AM

Quoted 5 lines · Kevin

My question is why that option is chosen instead of going with 1:1 mapping between vSMMU and viommu i.e. letting the kernel to figure out which pSMMU should be sent an invalidation cmd to, as how VT-d is virtualized.

Quoted 9 lines

Quoted 2 lines

I want to know whether doing so is simply to be compatible with what VCMDQ requires, or due to another untold reason.

Because we use viommu as a VMID holder for SMMU. So a pSMMU must have its own viommu to store its VMID for a shared s2_hwpt:

        |-- viommu0 (VMIDx) --|-- pSMMU0 --|
 vSMMU--|-- viommu1 (VMIDy) --|-- pSMMU1 --|--s2_hwpt
        |-- viommu2 (VMIDz) --|-- pSMMU2 --|

there are other options, e.g. you can have one viommu holding multiple VMIDs each associating to a pSMMU.

so it's really an implementation choice that you want a same viommu topology scheme w/ or w/o VCMDQ.

we all know there are some constraints of copying the physical topology, e.g. hotplugging a device or migration. for VCMDQ it's clearly an acceptable tradeoff for performance. w/o VCMDQ I'm not sure whether you want to keep more flexibility for the user. 😊

From: Nicolin Chen <[email protected]>
Sent: Thursday, May 30, 2024 8:59 AM

Quoted 18 lines · Kevin

Quoted 17 lines

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 11:21 AM

Quoted 7 lines · Kevin

My question is why that option is chosen instead of going with 1:1 mapping between vSMMU and viommu i.e. letting the kernel to figure out which pSMMU should be sent an invalidation cmd to, as how VT-d is virtualized.

I want to know whether doing so is simply to be compatible with what VCMDQ requires, or due to another untold reason.

Because we use viommu as a VMID holder for SMMU. So a pSMMU must have its own viommu to store its VMID for a shared s2_hwpt:

        |-- viommu0 (VMIDx) --|-- pSMMU0 --|
 vSMMU--|-- viommu1 (VMIDy) --|-- pSMMU1 --|--s2_hwpt
        |-- viommu2 (VMIDz) --|-- pSMMU2 --|

Quoted 2 lines

there are other options, e.g. you can have one viommu holding multiple VMIDs each associating to a pSMMU.

Well, possibly. But everything previously in a viommu would have to be a list (for number of VMIDs) in the kernel level: not only a VMID list, but also a 2D virtual ID lists, something like:

struct xarray vdev_ids[num_of_vmid]; // per-IOMMU vID to pID lookup

And a driver in this case would be difficult to get a complete concept of a viommu object since it's core object and shared by all kernel-level IOMMU instances. If a driver wants to extend a viommu object for some additional feature, e.g. VINTF in this series, it would likely have to create another per-driver object and again another list of this kind of objects in struct viommu.

At the end of day, we have to duplicate it one way or another for the amount of physical IOMMUs. And it seems to cleaner by doing it with multiple viommu objects.

Quoted 3 lines

so it's really an implementation choice that you want a same viommu topology scheme w/ or w/o VCMDQ.

Quoted 4 lines

we all know there are some constraints of copying the physical topology, e.g. hotplugging a device or migration. for VCMDQ it's clearly an acceptable tradeoff for performance. w/o VCMDQ I'm not sure whether you want to keep more flexibility for the user. 😊

Oh. With regular nested SMMU, there is only one virtual SMMU in the guest VM. No need of copying physical topology. Just the VMM needs to allocate three viommus to add them to a list of its own.

Quoted 28 lines

From: Nicolin Chen <[email protected]>
Sent: Thursday, May 30, 2024 8:59 AM

Quoted 18 lines · Kevin

Quoted 8 lines

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 11:21 AM

Quoted 5 lines · Kevin

My question is why that option is chosen instead of going with 1:1 mapping between vSMMU and viommu i.e. letting the kernel to figure out which pSMMU should be sent an invalidation cmd to, as how VT-d is virtualized.

Quoted 9 lines

Quoted 2 lines

I want to know whether doing so is simply to be compatible with what VCMDQ requires, or due to another untold reason.

Because we use viommu as a VMID holder for SMMU. So a pSMMU must have its own viommu to store its VMID for a shared s2_hwpt:

        |-- viommu0 (VMIDx) --|-- pSMMU0 --|
 vSMMU--|-- viommu1 (VMIDy) --|-- pSMMU1 --|--s2_hwpt
        |-- viommu2 (VMIDz) --|-- pSMMU2 --|

Quoted 2 lines

there are other options, e.g. you can have one viommu holding multiple VMIDs each associating to a pSMMU.

Well, possibly. But everything previously in a viommu would have to be a list (for number of VMIDs) in the kernel level: not only a VMID list, but also a 2D virtual ID lists, something like:

Quoted 1 line

struct xarray vdev_ids[num_of_vmid]; // per-IOMMU vID to pID lookup

ah does it mean that dev ID space on ARM is local to SMMU?

It's not the case on x86 platforms where the RID is platform-wise.

Quoted 19 lines

And a driver in this case would be difficult to get a complete concept of a viommu object since it's core object and shared by all kernel-level IOMMU instances. If a driver wants to extend a viommu object for some additional feature, e.g. VINTF in this series, it would likely have to create another per-driver object and again another list of this kind of objects in struct viommu.

At the end of day, we have to duplicate it one way or another for the amount of physical IOMMUs. And it seems to cleaner by doing it with multiple viommu objects.

Quoted 7 lines

so it's really an implementation choice that you want a same viommu topology scheme w/ or w/o VCMDQ.

we all know there are some constraints of copying the physical topology, e.g. hotplugging a device or migration. for VCMDQ it's clearly an acceptable tradeoff for performance. w/o VCMDQ I'm not sure whether you want to keep more flexibility for the user. 😊

Quoted 4 lines

Oh. With regular nested SMMU, there is only one virtual SMMU in the guest VM. No need of copying physical topology. Just the VMM needs to allocate three viommus to add them to a list of its own.

Okay I missed this point. Then the number of viommus is really about the contract between the vmm and the kernel, which is invisible to the guest or the admin who launches the Qemu.

but wait, isn't there another problem - does the VMM have the
permission to enumerate the topology of physical SMMUs? Or probably
the VMM only needs to know the number of relevant SMMUs for
assigned devices and such info can be indirectly composed by extending GET_HW_INFO...

Quoted 18 lines · Kevin

Quoted 17 lines

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 11:21 AM

Quoted 7 lines · Kevin

My question is why that option is chosen instead of going with 1:1 mapping between vSMMU and viommu i.e. letting the kernel to figure out which pSMMU should be sent an invalidation cmd to, as how VT-d is virtualized.

I want to know whether doing so is simply to be compatible with what VCMDQ requires, or due to another untold reason.

Because we use viommu as a VMID holder for SMMU. So a pSMMU must have its own viommu to store its VMID for a shared s2_hwpt:

        |-- viommu0 (VMIDx) --|-- pSMMU0 --|
 vSMMU--|-- viommu1 (VMIDy) --|-- pSMMU1 --|--s2_hwpt
        |-- viommu2 (VMIDz) --|-- pSMMU2 --|

Quoted 2 lines

there are other options, e.g. you can have one viommu holding multiple VMIDs each associating to a pSMMU.

Well, possibly. But everything previously in a viommu would have to be a list (for number of VMIDs) in the kernel level: not only a VMID list, but also a 2D virtual ID lists, something like:

struct xarray vdev_ids[num_of_vmid]; // per-IOMMU vID to pID lookup

And a driver in this case would be difficult to get a complete concept of a viommu object since it's core object and shared by all kernel-level IOMMU instances. If a driver wants to extend a viommu object for some additional feature, e.g. VINTF in this series, it would likely have to create another per-driver object and again another list of this kind of objects in struct viommu.

Oh. With regular nested SMMU, there is only one virtual SMMU in the guest VM. No need of copying physical topology. Just the VMM needs to allocate three viommus to add them to a list of its own.

Quoted 19 lines · Nicolin Chen

Quoted 18 lines · Kevin

Quoted 17 lines

From: Nicolin Chen <[email protected]>
Sent: Wednesday, May 29, 2024 11:21 AM

Quoted 7 lines · Kevin

My question is why that option is chosen instead of going with 1:1 mapping between vSMMU and viommu i.e. letting the kernel to figure out which pSMMU should be sent an invalidation cmd to, as how VT-d is virtualized.

I want to know whether doing so is simply to be compatible with what VCMDQ requires, or due to another untold reason.

Because we use viommu as a VMID holder for SMMU. So a pSMMU must have its own viommu to store its VMID for a shared s2_hwpt:

        |-- viommu0 (VMIDx) --|-- pSMMU0 --|
 vSMMU--|-- viommu1 (VMIDy) --|-- pSMMU1 --|--s2_hwpt
        |-- viommu2 (VMIDz) --|-- pSMMU2 --|

Quoted 8 lines

Quoted 2 lines

there are other options, e.g. you can have one viommu holding multiple VMIDs each associating to a pSMMU.

Well, possibly. But everything previously in a viommu would have to be a list (for number of VMIDs) in the kernel level: not only a VMID list, but also a 2D virtual ID lists, something like:

struct xarray vdev_ids[num_of_vmid]; // per-IOMMU vID to pID lookup

I feel it makes most sense that ARM (and maybe everyone) just have a viommu per piommu.

The main argument against is we haven't made it efficient for the VMM to support multiple piommus. It has to do a system call per piommu each time it processes the cmdq.

But, on the other hand, if you care about invalidation efficiency it is kind of silly not to expose the piommus to the guest so that the invalidation scope can be appropriately narrowed. Replicating all ASID invalidations to all piommus doesn't make alot of sense if the guest can know that only one piommu actually needs invalidation.