IOASID extensions for guest SVA

/**
* iommu_sva_alloc_pasid - Allocate a PASID for the mm

drivers/iommu/iommu-sva-lib.c +0 -0

@@ -35,11 +44,11 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm,
 ioasid_t min, ioasid_t max) mutex_lock(&iommu_sva_lock);
 	if (mm->pasid) {

Hi Jean,

Slightly off the title. As we are moving to use cgroup to limit PASID allocations, it would be much simpler if we enforce on the current task.

However, iommu_sva_alloc_pasid() takes an mm_struct pointer as argument which implies it can be something other the the current task mm. So far all kernel callers use current task mm. Is there a use case for doing PASID allocation on behalf of another mm? If not, can we remove the mm argument?

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

Quoted 4 lines · Jean-Philippe Brucker

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

This creates a lot complexity, how do does process A get a secure reference to B? How does it access the memory in B to setup the HW?

Why do we need separation anyhow? SVM devices are supposed to be secure or they shouldn't do SVM.

Quoted 7 lines · Jason Gunthorpe

Quoted 4 lines · Jean-Philippe Brucker

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

Quoted 2 lines

This creates a lot complexity, how do does process A get a secure reference to B? How does it access the memory in B to setup the HW?

mm_access() for example, and passing addresses via IPC

Quoted 12 lines · Jean-Philippe Brucker

Quoted 10 lines · Jason Gunthorpe

Quoted 5 lines · Jean-Philippe Brucker

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

This creates a lot complexity, how do does process A get a secure reference to B? How does it access the memory in B to setup the HW?

Quoted 1 line

mm_access() for example, and passing addresses via IPC

I'd rather the source process establish its own PASID and then pass the rights to use it to some other process via FD passing than try to go the other way. There are lots of security questions with something like mm_access.

Hi Jason,

Quoted 15 lines · Jason Gunthorpe

Quoted 13 lines · Jean-Philippe Brucker

Quoted 10 lines · Jason Gunthorpe

Quoted 5 lines · Jean-Philippe Brucker

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

This creates a lot complexity, how do does process A get a secure reference to B? How does it access the memory in B to setup the HW?

mm_access() for example, and passing addresses via IPC

Quoted 5 lines

I'd rather the source process establish its own PASID and then pass the rights to use it to some other process via FD passing than try to go the other way. There are lots of security questions with something like mm_access.

Thank you all for the input, it sounds like we are OK to remove mm argument from iommu_sva_bind_device() and iommu_sva_alloc_pasid() for now?

Let me try to summarize PASID allocation as below:

Interfaces | Usage | Limit | bind¹ |User visible
--------------------------------------------------------------------
/dev/ioasid² | G-SVA/IOVA | cgroup | No |Yes
--------------------------------------------------------------------
char dev³ | SVA | cgroup | Yes |No
--------------------------------------------------------------------
iommu driver | default PASID| no | No |No

--------------------------------------------------------------------
kernel | super SVA | no | yes |No
--------------------------------------------------------------------

¹ Allocated during SVA bind
² PASIDs allocated via /dev/ioasid are not bound to any mm. But its
ownership is assigned to the process that does the allocation.
³ Include uacce, other private device driver char dev such as idxd

Currently, the proposed /dev/ioasid interface does not map individual PASID with an FD. The FD is at the ioasid_set granularity and bond to the current mm. We could extend the IOCTLs to cover individual PASID-FD passing case when use cases arise. Would this work?

Thanks,

Jacob

Quoted 27 lines · Jacob Pan

Hi Jason,

Quoted 15 lines · Jason Gunthorpe

Quoted 13 lines · Jean-Philippe Brucker

Quoted 11 lines · Jason Gunthorpe

Quoted 5 lines · Jean-Philippe Brucker

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

This creates a lot complexity, how do does process A get a secure reference to B? How does it access the memory in B to setup the HW?

Quoted 7 lines

Quoted 1 line

mm_access() for example, and passing addresses via IPC

I'd rather the source process establish its own PASID and then pass the rights to use it to some other process via FD passing than try to go the other way. There are lots of security questions with something like mm_access.

Quoted 3 lines

Thank you all for the input, it sounds like we are OK to remove mm argument from iommu_sva_bind_device() and iommu_sva_alloc_pasid() for now?

Fine by me. By the way the IDXD currently missues the bind API for supervisor PASID, and the drvdata parameter isn't otherwise used. This would be a good occasion to clean both. The new bind prototype could be:

struct iommu_sva *iommu_sva_bind_device(struct device *dev, int flags)

And a flag IOMMU_SVA_BIND_SUPERVISOR (not that I plan to implement it in the SMMU, but I think we need to clean the current usage)

Quoted 12 lines

Let me try to summarize PASID allocation as below:

Interfaces | Usage | Limit | bind¹ |User visible
--------------------------------------------------------------------
/dev/ioasid² | G-SVA/IOVA | cgroup | No
|Yes
--------------------------------------------------------------------
char dev³ | SVA | cgroup | Yes |No
--------------------------------------------------------------------
iommu driver | default PASID| no | No |No

Is this PASID #0?

Quoted 3 lines

--------------------------------------------------------------------
kernel | super SVA | no | yes |No
--------------------------------------------------------------------

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

Thanks,
Jean

Quoted 14 lines

¹ Allocated during SVA bind
² PASIDs allocated via /dev/ioasid are not bound to any mm. But its
ownership is assigned to the process that does the allocation.
³ Include uacce, other private device driver char dev such as idxd

Currently, the proposed /dev/ioasid interface does not map individual PASID with an FD. The FD is at the ioasid_set granularity and bond to the current mm. We could extend the IOCTLs to cover individual PASID-FD passing case when use cases arise. Would this work?

Thanks,

Jacob

Quoted 4 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID # for device private domains? Maybe I missed your idea?

Quoted 10 lines · Jacob Pan

Quoted 4 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID # for device private domains? Maybe I missed your idea?

There is not much in the kernel that isn't triggered by a process, I would be careful about the idea that there is a class of users that can consume a cgroup controlled resource without being inside the cgroup.

We've got into trouble before overlooking this and with something greenfield like PASID it would be best built in to the API to prevent a mistake. eg accepting a cgroup or process input to the allocator.

Hi Jason,

Quoted 12 lines · Jason Gunthorpe

Quoted 10 lines · Jacob Pan

Quoted 4 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID # for device private domains? Maybe I missed your idea?

Quoted 9 lines

There is not much in the kernel that isn't triggered by a process, I would be careful about the idea that there is a class of users that can consume a cgroup controlled resource without being inside the cgroup.

We've got into trouble before overlooking this and with something greenfield like PASID it would be best built in to the API to prevent a mistake. eg accepting a cgroup or process input to the allocator.

Make sense. But I think we only allow charging the current cgroup, how about I add the following to ioasid_alloc():

	misc_cg = get_current_misc_cg();
	ret = misc_cg_try_charge(MISC_CG_RES_IOASID, misc_cg, 1);
	if (ret) {
		put_misc_cg(misc_cg);
		return ret;
	}

Quoted 29 lines · Jacob Pan

Hi Jason,

Quoted 13 lines · Jason Gunthorpe

Quoted 11 lines · Jacob Pan

Quoted 5 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID

# for device private domains? Maybe I missed your idea?    

Quoted 9 lines

There is not much in the kernel that isn't triggered by a process, I would be careful about the idea that there is a class of users that can consume a cgroup controlled resource without being inside the cgroup.

We've got into trouble before overlooking this and with something greenfield like PASID it would be best built in to the API to prevent a mistake. eg accepting a cgroup or process input to the allocator.

Make sense. But I think we only allow charging the current cgroup, how about I add the following to ioasid_alloc():

Quoted 6 lines

	misc_cg = get_current_misc_cg();
	ret = misc_cg_try_charge(MISC_CG_RES_IOASID, misc_cg, 1);
	if (ret) {
		put_misc_cg(misc_cg);
		return ret;
	}  

Does that allow PASID allocation during driver probe, in kernel_init or modprobe context?

Thanks,
Jean

Hi Jean-Philippe,

Quoted 37 lines · Jean-Philippe Brucker

Quoted 29 lines · Jacob Pan

Hi Jason,

Quoted 13 lines · Jason Gunthorpe

Quoted 11 lines · Jacob Pan

Quoted 5 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID

# for device private domains? Maybe I missed your idea?    

Quoted 9 lines

There is not much in the kernel that isn't triggered by a process, I would be careful about the idea that there is a class of users that can consume a cgroup controlled resource without being inside the cgroup.

We've got into trouble before overlooking this and with something greenfield like PASID it would be best built in to the API to prevent a mistake. eg accepting a cgroup or process input to the allocator.

Make sense. But I think we only allow charging the current cgroup, how about I add the following to ioasid_alloc():

Quoted 6 lines

	misc_cg = get_current_misc_cg();
	ret = misc_cg_try_charge(MISC_CG_RES_IOASID, misc_cg, 1);
	if (ret) {
		put_misc_cg(misc_cg);
		return ret;
	}  

Quoted 3 lines

Does that allow PASID allocation during driver probe, in kernel_init or modprobe context?

Good point. Yes, you can get cgroup subsystem state in kernel_init for charging/uncharging. I would think module_init should work also since it is after kernel_init. I have tried the following:
static int __ref kernel_init(void *unused)

 {
        int ret;

(quoted diff) +2 -0

+       struct cgroup_subsys_state *css;
+       css = task_get_css(current, pids_cgrp_id);

But that would imply:
1. IOASID has to be built-in, not as module
2. IOASIDs charged on PID1/init would not subject to cgroup limit since it will be in the root cgroup and we don't support migration nor will migrate.

Then it comes back to the question of why do we try to limit in-kernel users per cgroup if we can't enforce these cases.

Quoted 61 lines · Jacob Pan

Hi Jean-Philippe,

Quoted 38 lines · Jean-Philippe Brucker

Quoted 30 lines · Jacob Pan

Hi Jason,

Quoted 19 lines · Jason Gunthorpe

Quoted 6 lines · Jacob Pan

Quoted 5 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

Quoted 6 lines

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID # for device private domains? Maybe I missed your idea?

There is not much in the kernel that isn't triggered by a process, I would be careful about the idea that there is a class of users that can consume a cgroup controlled resource without being inside the cgroup.

Quoted 4 lines

We've got into trouble before overlooking this and with something greenfield like PASID it would be best built in to the API to prevent a mistake. eg accepting a cgroup or process input to the allocator.

Make sense. But I think we only allow charging the current cgroup, how about I add the following to ioasid_alloc():

Quoted 6 lines

	misc_cg = get_current_misc_cg();
	ret = misc_cg_try_charge(MISC_CG_RES_IOASID, misc_cg, 1);
	if (ret) {
		put_misc_cg(misc_cg);
		return ret;
	}    

Quoted 3 lines

Does that allow PASID allocation during driver probe, in kernel_init or modprobe context?

Good point. Yes, you can get cgroup subsystem state in kernel_init for charging/uncharging. I would think module_init should work also since it is after kernel_init. I have tried the following:
static int __ref kernel_init(void *unused)

 {
        int ret;

(quoted diff) +2 -0

+       struct cgroup_subsys_state *css;
+       css = task_get_css(current, pids_cgrp_id);

But that would imply:
1. IOASID has to be built-in, not as module
2. IOASIDs charged on PID1/init would not subject to cgroup limit since it will be in the root cgroup and we don't support migration nor will migrate.

Quoted 2 lines

Then it comes back to the question of why do we try to limit in-kernel users per cgroup if we can't enforce these cases.

Are these real use cases? Why would a driver binding to a device create a single kernel pasid at bind time? Why wouldn't it use untagged DMA?

When someone needs it they can rework it and explain why they are doing something sane.

Jason

Quoted 54 lines · Jacob Pan

Hi Jean-Philippe,

Quoted 37 lines · Jean-Philippe Brucker

Quoted 29 lines · Jacob Pan

Hi Jason,

Quoted 13 lines · Jason Gunthorpe

Quoted 11 lines · Jacob Pan

Quoted 5 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID

# for device private domains? Maybe I missed your idea?    

Quoted 9 lines

There is not much in the kernel that isn't triggered by a process, I would be careful about the idea that there is a class of users that can consume a cgroup controlled resource without being inside the cgroup.

We've got into trouble before overlooking this and with something greenfield like PASID it would be best built in to the API to prevent a mistake. eg accepting a cgroup or process input to the allocator.

Make sense. But I think we only allow charging the current cgroup, how about I add the following to ioasid_alloc():

Quoted 6 lines

	misc_cg = get_current_misc_cg();
	ret = misc_cg_try_charge(MISC_CG_RES_IOASID, misc_cg, 1);
	if (ret) {
		put_misc_cg(misc_cg);
		return ret;
	}  

Quoted 3 lines

Does that allow PASID allocation during driver probe, in kernel_init or modprobe context?

Good point. Yes, you can get cgroup subsystem state in kernel_init for charging/uncharging. I would think module_init should work also since it is after kernel_init. I have tried the following:
static int __ref kernel_init(void *unused)

 {
        int ret;

(quoted diff) +2 -0

+       struct cgroup_subsys_state *css;
+       css = task_get_css(current, pids_cgrp_id);

Quoted 2 lines

But that would imply:
1. IOASID has to be built-in, not as module

Quoted 5 lines

2. IOASIDs charged on PID1/init would not subject to cgroup limit since it will be in the root cgroup and we don't support migration nor will migrate.

Then it comes back to the question of why do we try to limit in-kernel users per cgroup if we can't enforce these cases.

Are these real use cases? Why would a driver binding to a device create a single kernel pasid at bind time? Why wouldn't it use untagged DMA?

It's not inconceivable to have a control queue doing DMA tagged with PASID. The devices I know either use untagged DMA, or have a choice to use a PASID.

Quoted 3 lines · Jean-Philippe Brucker

It's not inconceivable to have a control queue doing DMA tagged with PASID. The devices I know either use untagged DMA, or have a choice to use a PASID.

I don't think we should encourage that. A PASID and all the related is so expensive compared to just doing normal untagged kernel DMA.

I assume HW has these features because virtualization use cases might use them, eg by using mdev to assign a command queue - then it would need be be contained by a PASID.

Jason

Quoted 6 lines · Jason Gunthorpe

Quoted 3 lines · Jean-Philippe Brucker

It's not inconceivable to have a control queue doing DMA tagged with PASID. The devices I know either use untagged DMA, or have a choice to use a PASID.

Quoted 2 lines

I don't think we should encourage that. A PASID and all the related is so expensive compared to just doing normal untagged kernel DMA.

How is it expensive? Low number of PASIDs, or slowing down DMA transactions? PASIDs aren't a scarce resource on Arm systems, they have almost 1M unused PASIDs per VM.

Quoted 3 lines

And a flag IOMMU_SVA_BIND_SUPERVISOR (not that I plan to implement it in the SMMU, but I think we need to clean the current usage)

You mean move #define SVM_FLAG_SUPERVISOR_MODE out of Intel code to be a generic flag in iommu-sva-lib.h called IOMMU_SVA_BIND_SUPERVISOR?

Quoted 5 lines

Also wondering about device driver allocating auxiliary domains for their private use, to do iommu_map/unmap on private PASIDs (a clean replacement to super SVA, for example). Would that go through the same path as /dev/ioasid and use the cgroup of current task?

For the in-kernel private use, I don't think we should restrict based on cgroup, since there is no affinity to user processes. I also think the PASID allocation should just use kernel API instead of /dev/ioasid. Why would user space need to know the actual PASID # for device private domains? Maybe I missed your idea?

Hi Jason,

Quoted 13 lines · Jason Gunthorpe

Quoted 12 lines · Jean-Philippe Brucker

Quoted 10 lines · Jason Gunthorpe

Quoted 5 lines · Jean-Philippe Brucker

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

This creates a lot complexity, how do does process A get a secure reference to B? How does it access the memory in B to setup the HW?

Quoted 7 lines

Quoted 1 line

mm_access() for example, and passing addresses via IPC

I'd rather the source process establish its own PASID and then pass the rights to use it to some other process via FD passing than try to go the other way. There are lots of security questions with something like mm_access.

Thank you all for the input, it sounds like we are OK to remove mm argument from iommu_sva_bind_device() and iommu_sva_alloc_pasid() for now?

Let me try to summarize PASID allocation as below:

Interfaces | Usage | Limit | bind¹ |User visible
/dev/ioasid² | G-SVA/IOVA | cgroup | No |Yes
char dev³ | SVA | cgroup | Yes |No
iommu driver | default PASID| no | No |No
kernel | super SVA | no | yes |No

¹ Allocated during SVA bind
² PASIDs allocated via /dev/ioasid are not bound to any mm. But its
ownership is assigned to the process that does the allocation.

Currently, the proposed /dev/ioasid interface does not map individual PASID with an FD. The FD is at the ioasid_set granularity and bond to the current mm. We could extend the IOCTLs to cover individual PASID-FD passing case when use cases arise. Would this work?

Quoted 40 lines · Jacob Pan

Hi Jason,

Quoted 15 lines · Jason Gunthorpe

Quoted 13 lines · Jean-Philippe Brucker

Quoted 11 lines · Jason Gunthorpe

Quoted 5 lines · Jean-Philippe Brucker

Although there is no use for it at the moment (only two upstream users and it looks like amdkfd always uses current too), I quite like the client-server model where the privileged process does bind() and programs the hardware queue on behalf of the client process.

This creates a lot complexity, how do does process A get a secure reference to B? How does it access the memory in B to setup the HW?

Quoted 7 lines

Quoted 1 line

mm_access() for example, and passing addresses via IPC

I'd rather the source process establish its own PASID and then pass the rights to use it to some other process via FD passing than try to go the other way. There are lots of security questions with something like mm_access.

Thank you all for the input, it sounds like we are OK to remove mm argument from iommu_sva_bind_device() and iommu_sva_alloc_pasid() for now?

Let me try to summarize PASID allocation as below:

Interfaces | Usage | Limit | bind¹ |User visible
/dev/ioasid² | G-SVA/IOVA | cgroup | No

|Yes char dev³	| SVA		|  cgroup	|
Yes	|No iommu driver	| default PASID|  no
| No	|No kernel		| super SVA	| no
	| yes   |No

Quoted 3 lines

¹ Allocated during SVA bind
² PASIDs allocated via /dev/ioasid are not bound to any mm. But its
ownership is assigned to the process that does the allocation.

What does "not bound to a mm" mean?

IMHO a use created PASID is either bound to a mm (current) at creation time, or it will never be bound to a mm and its page table is under user control via /dev/ioasid.

I thought the whole point of something like a /dev/ioasid was to get away from each and every device creating its own PASID interface?

It maybe somewhat reasonable that some devices could have some easy 'make a SVA PASID on current' interface built in,

but anything more
complicated should use /dev/ioasid, and anything consuming PASID should also have an API to import and attach a PASID from /dev/ioasid.

Quoted 4 lines

Currently, the proposed /dev/ioasid interface does not map individual PASID with an FD. The FD is at the ioasid_set granularity and bond to the current mm. We could extend the IOCTLs to cover individual PASID-FD passing case when use cases arise. Would this work?

Is it a good idea that the FD is per ioasid_set ?

What is the set used
for?

Usually kernel interfaces work nicer with a one fd/one object model.

But even if it is a set, you could pass the set between co-operating processes and the PASID can be created in the correct 'current'. But there is all kinds of security questsions as soon as you start doing anything like this - is there really a use case?

Jason

Quoted 4 lines

IMHO a use created PASID is either bound to a mm (current) at creation time, or it will never be bound to a mm and its page table is under user control via /dev/ioasid.

True for PASID used in native SVA bind. But for binding with a guest mm, PASID is allocated first (VT-d virtual cmd interface Spec 10.4.44), the bind with the host IOMMU when vIOMMU PASID cache is invalidated.

Our intention is to have two separate interfaces:
1. /dev/ioasid (allocation/free only)
2. /dev/sva (handles all SVA related activities including page tables)

Quoted 3 lines

I thought the whole point of something like a /dev/ioasid was to get away from each and every device creating its own PASID interface?

yes, but only for the use cases that need to expose PASID to the userspace.

Quoted 2 lines

It maybe somewhat reasonable that some devices could have some easy 'make a SVA PASID on current' interface built in,

I agree, this is the case PASID is hidden from the userspace, right? e.g. uacce.

Quoted 4 lines

but anything more
complicated should use /dev/ioasid, and anything consuming PASID should also have an API to import and attach a PASID from /dev/ioasid.

Would the above two use cases constitute the "complicated" criteria? Or we should say anything that need the explicit PASID value has to through /dev/ioasid?

In terms of usage for guest SVA, an ioasid_set is mostly tied to a host mm, the use case is as the following:

1. Identify a pool of PASIDs for permission checking (below to the same VM), e.g. only allow SVA binding for PASIDs allocated from the same set.

2. Allow different PASID-aware kernel subsystems to associate, e.g. KVM, device drivers, and IOMMU driver. i.e. each KVM instance only cares about the ioasid_set associated with the VM. Events notifications are also within the ioasid_set to synchronize PASID states.

3. Guest-Host PASID look up (each set has its own XArray to store the mapping)

4. Quota control (going away once we have cgroup)

Quoted 11 lines · Jacob Pan

Quoted 4 lines

IMHO a use created PASID is either bound to a mm (current) at creation time, or it will never be bound to a mm and its page table is under user control via /dev/ioasid.

True for PASID used in native SVA bind. But for binding with a guest mm, PASID is allocated first (VT-d virtual cmd interface Spec 10.4.44), the bind with the host IOMMU when vIOMMU PASID cache is invalidated.

Our intention is to have two separate interfaces:
1. /dev/ioasid (allocation/free only)
2. /dev/sva (handles all SVA related activities including page tables)

I'm not sure I understand why you'd want to have two things. Doesn't that just complicate everything?

Manipulating the ioasid, including filling it with page tables, seems an integral inseperable part of the whole interface. Why have two ?

Quoted 5 lines

Quoted 3 lines

I thought the whole point of something like a /dev/ioasid was to get away from each and every device creating its own PASID interface?

yes, but only for the use cases that need to expose PASID to the userspace.

Why "but only"? This thing should reach for a higher generality, not just be contained to solve some problem within qemu.

Quoted 4 lines

Quoted 2 lines

It maybe somewhat reasonable that some devices could have some easy 'make a SVA PASID on current' interface built in,

I agree, this is the case PASID is hidden from the userspace, right? e.g. uacce.

"hidden", I guess, but does it matter so much?

The PASID would still consume a cgroup credit

Quoted 7 lines

Quoted 4 lines

but anything more
complicated should use /dev/ioasid, and anything consuming PASID should also have an API to import and attach a PASID from /dev/ioasid.

Would the above two use cases constitute the "complicated" criteria? Or we should say anything that need the explicit PASID value has to through /dev/ioasid?

Anything that needs more that creating a hidden PASID link'd to current should use the full interface.

Quoted 2 lines

In terms of usage for guest SVA, an ioasid_set is mostly tied to a host mm, the use case is as the following:

From that doc:

It is imperative to enforce
VM-IOASID ownership such that a malicious guest cannot target DMA traffic outside its own IOASIDs, or free an active IOASID that belongs to another VM.

Huh?

Security in a PASID world comes from the IOMMU blocking access to the PASID except from approved PCI-ID's. If a VF/PF is assigned to a guest then that guest can cause the device to issue any PASID by having complete control and the vIOMMU is supposed to tell the real IOMMU what PASID's the device is alowed to access.

If a device is sharing a single PCI function with different security contexts (eg vfio mdev) then the device itself is responsible to ensure that only the secure interface can program a PASID and a less secure context can never self-enroll.

Here the mdev driver would have to consule with the vIOMMU to ensure the mdev device is allowed to access the PASID - is that what this set stuff is about?

If yes, it is backwards. The MDEV is the thing doing the security, the MDEV should have the list of allowed PASID's and a single PASID created under /dev/ioasid should be loaded into MDEV with some 'Ok you can use PASID xyz from FD abc' command.

Because you absolutely don't want to have a generic 'set' that all the mdevs are sharing as that violates the basic security principle at the start - each and every device must have a unique list of what PASID's it can talk to.

Quoted 8 lines

1. Identify a pool of PASIDs for permission checking (below to the same VM), e.g. only allow SVA binding for PASIDs allocated from the same set.

2. Allow different PASID-aware kernel subsystems to associate, e.g. KVM, device drivers, and IOMMU driver. i.e. each KVM instance only cares about the ioasid_set associated with the VM. Events notifications are also within the ioasid_set to synchronize PASID states.

Quoted 4 lines

3. Guest-Host PASID look up (each set has its own XArray to store the mapping)

4. Quota control (going away once we have cgroup)

It sounds worrysome things have gone this way.

I'd say you shoul have a single /dev/ioasid per VM and KVM should attach to that - it should get all the global events/etc that are not device specific.

permission checking *must* be done on a per-device level, either inside the mdev driver, or inside the IOMMU at a per-PCI device level.

Not sure what guest-host PASID means, these have to be 1:1 for device assignment to work - why would use something else for mdev?

In one of the earlier discussions, I was made aware of some use cases (by AMD, iirc) where PASID can be used w/o IOMMU. That is why I tried to keep ioasid a separate subsystem. Other than that, I don't see an issue combining the two.

it matters when it comes to which interface to choose. Use /dev/ioasid to allocate if PASID value cannot be hidden. Use some other interface for bind current and allocate if a PASID is not visible to the user.

Yes, I think we are on the same page. For example, today's uacce or idxd driver creates a hidden PASID when user does open(), where a new WQ is provisioned and bound to current mm. This is the case where /dev/ioasid is not needed.

Sorry, I am not following. In the doc, I have an example to show the ioasid_set to VM/mm mapping. We use mm as the ioasid_set token to identify who the owner of an IOASID is. i.e. who allocated the IOASID. Non-owner cannot perform bind page table or free operations.

Yes, each PF/VF has its own PASID table. The device can do whatever it wants as long as the PASID is present in the table. Programming of the pIOMMU PASID table entry, however, is controlled by the host.

IMHO, there are two levels of security here:
1. A PASID can only be used by a secure context
2. A device can only use allowed PASIDs (PASID namespace is system-wide but PASID table storage is per PF/VF)

IOASID set is designed for #1.

Quoted 4 lines

If a device is sharing a single PCI function with different security contexts (eg vfio mdev) then the device itself is responsible to ensure that only the secure interface can program a PASID and a less secure context can never self-enroll.

If two mdevs from the same PF dev are assigned to two VMs, the PASID table will be shared. IOASID set ensures one VM cannot program another VM's PASIDs. I assume 'secure context' is per VM when it comes to host PASID.

No. the mdev driver consults with IOASID core When the guest programs a guest PASID on to he mdev. VDCM driver does a lookup:
host_pasid = ioasid_find_by_spid(ioasid_set, guest_pasid);

Quoted 4 lines

I'd say you shoul have a single /dev/ioasid per VM and KVM should attach to that - it should get all the global events/etc that are not device specific.

You mean a single /dev/ioasid FD per VM and KVM? I think that is what we are doing in this set. A VM process can only open /dev/ioasid once, then use the FD for allocation and pass the PASID for bind page table etc.

Quoted 3 lines

Not sure what guest-host PASID means, these have to be 1:1 for device assignment to work - why would use something else for mdev?

We have G-H PASID translation. They don't have to be 1:1.
IOASID Set Private ID (SPID) is intended as a generic solution for guest PASID. Could you review the secion Section: IOASID Set Private ID (SPID) in the doc patch?

We also had some slides from last year. Slide 3s-6 mostly.
https://static.sched.com/hosted_files/kvmforum2020/9f/KVM_forum_2020_PASID_MGMT_Yi_Jacob_final.pdf

Quoted 4 lines

If two mdevs from the same PF dev are assigned to two VMs, the PASID table will be shared. IOASID set ensures one VM cannot program another VM's PASIDs. I assume 'secure context' is per VM when it comes to host PASID.

No, the mdev device driver must enforce this directly. It is the one that programms the physical shared HW, it is the one that needs a list of PASID's it is allowed to program *for each mdev*

ioasid_set doesn't seem to help at all, certainly not as a concept tied to /dev/ioasid.

Quoted 3 lines

No. the mdev driver consults with IOASID core When the guest programs a guest PASID on to he mdev. VDCM driver does a lookup:
host_pasid = ioasid_find_by_spid(ioasid_set, guest_pasid);

This is the wrong layering. Tell the mdev device directly what it is allowed to do. Do not pollute the ioasid core with security stuff.

Quoted 7 lines

Quoted 4 lines

I'd say you shoul have a single /dev/ioasid per VM and KVM should attach to that - it should get all the global events/etc that are not device specific.

You mean a single /dev/ioasid FD per VM and KVM? I think that is what we are doing in this set. A VM process can only open /dev/ioasid once, then use the FD for allocation and pass the PASID for bind page table etc.

Yes, I think that is reasonable.

Tag all the IOCTL's with the IOASID number.

Quoted 3 lines

Quoted 3 lines

Not sure what guest-host PASID means, these have to be 1:1 for device assignment to work - why would use something else for mdev?

Quoted 4 lines

We have G-H PASID translation. They don't have to be 1:1. IOASID Set Private ID (SPID) is intended as a generic solution for guest PASID. Could you review the secion Section: IOASID Set Private ID (SPID) in the doc patch?

Again this only works for MDEV? How would you do translation for a real PF/VF?

So when you 'allow' a mdev to access a PASID you want to say:
Allow Guest PASID A, map it to host PASID B on this /dev/ioasid FD

?

That seems like a good helper library to provide for drivers to use, but it should be a construct entirely contained in the driver.

Hi Jason,

Quoted 9 lines · Jason Gunthorpe

Quoted 4 lines

If two mdevs from the same PF dev are assigned to two VMs, the PASID table will be shared. IOASID set ensures one VM cannot program another VM's PASIDs. I assume 'secure context' is per VM when it comes to host PASID.

No, the mdev device driver must enforce this directly. It is the one that programms the physical shared HW, it is the one that needs a list of PASID's it is allowed to program *for each mdev*

This requires the mdev driver to obtain a list of allowed PASIDs(possibly during PASID bind time) prior to do enforcement. IMHO, the PASID enforcement points are:
1. During WQ configuration (e.g.program MSI)
2. During work submission

For VT-d shared workqueue, there is no way to enforce #2 in mdev driver in that the PASID is obtained from PASID MSR from the CPU and submitted w/o driver involvement.

The enforcement for #2 is in the KVM PASID translation table, which is per VM.

For our current VFIO mdev model, bind guest page table does not involve mdev driver. So this is a gap we must fill, i.e. include a callback from mdev driver?

Quoted 3 lines

ioasid_set doesn't seem to help at all, certainly not as a concept tied to /dev/ioasid.

Yes, we can take the security role off ioasid_set once we have per mdev list. However, ioasid_set being a per VM/mm entity also bridge communications among kernel subsystems that don't have direct call path. e.g. KVM, VDCM and IOMMU.

Quoted 3 lines

So when you 'allow' a mdev to access a PASID you want to say:
Allow Guest PASID A, map it to host PASID B on this /dev/ioasid FD

Host and guest PASID value, as well as device info are available through iommu_uapi_sva_bind_gpasid(), we just need to feed that info to mdev driver.

Quoted 2 lines

That seems like a good helper library to provide for drivers to use, but it should be a construct entirely contained in the driver.

why? would it be cleaner if it is in the common code?

On Tue, Mar 30, 2021 at 05:10:41PM -0700, Jacob Pan wrote:
[...]
[...]
[...]

Quoted 6 lines

This requires the mdev driver to obtain a list of allowed PASIDs(possibly during PASID bind time) prior to do enforcement. IMHO, the PASID enforcement points are:
1. During WQ configuration (e.g.program MSI)
2. During work submission

Quoted 3 lines

For VT-d shared workqueue, there is no way to enforce #2 in mdev driver in that the PASID is obtained from PASID MSR from the CPU and submitted w/o driver involvement.

I assume that the PASID MSR is privileged and only qemu can program it? Otherwise this seems like a security problem.

If qemu controls it then the idxd userspace driver in qemu must ensure it is only ever programmed to an authorized PASID.

Quoted 2 lines

The enforcement for #2 is in the KVM PASID translation table, which is per VM.

I don't understand why KVM gets involved in PASID??

Doesn't work submission go either to the mdev driver or through the secure PASID of #1?

Quoted 3 lines

For our current VFIO mdev model, bind guest page table does not involve mdev driver. So this is a gap we must fill, i.e. include a callback from mdev driver?

No not a callback, tell the mdev driver with a VFIO IOCTL that it is authorized to use a specific PASID because the vIOMMU was told to allow it by the guest kernel. Simple and straightforward.

Quoted 7 lines

Quoted 3 lines

ioasid_set doesn't seem to help at all, certainly not as a concept tied to /dev/ioasid.

Yes, we can take the security role off ioasid_set once we have per mdev list. However, ioasid_set being a per VM/mm entity also bridge communications among kernel subsystems that don't have direct call path. e.g. KVM, VDCM and IOMMU.

Everything should revolve around the /dev/ioasid FD. qemu should pass it to all places that need to know about PASID's in the VM.

We should try to avoid hidden behind the scenes kernel interconnections between subsystems.

Quoted 4 lines

Quoted 4 lines

So when you 'allow' a mdev to access a PASID you want to say:
Allow Guest PASID A, map it to host PASID B on this /dev/ioasid FD

Quoted 3 lines

Host and guest PASID value, as well as device info are available through iommu_uapi_sva_bind_gpasid(), we just need to feed that info to mdev driver.

You need that IOCTL to exist on the *mdev driver*. It is a VFIO ioctl, not a iommu or ioasid or sva IOCTL.

Quoted 3 lines

Quoted 2 lines

That seems like a good helper library to provide for drivers to use, but it should be a construct entirely contained in the driver.

why? would it be cleaner if it is in the common code?

No, it is the "mid layer" problematic design.

Having the iommu layer store driver-specific data on behalf of a driver will just make a mess. Use the natural layering we have and store driver specific data in the driver structs.

Add a library to help build the datastructure if it necessary.

Jason

"3.3 PASID translation
To support PASID isolation for Shared Work Queues used by VMs, the CPU must provide a way for the PASID to be communicated to the device in the DMWr transaction. On Intel CPUs, the CPU provides a PASID translation table in the vCPUs virtual machine control structures. During ENQCMD/ENQCMDS instruction execution in a VM, the PASID translation table is used by the CPU to replace the guest PASID in the work descriptor with a host PASID before the descriptor is sent to the device.3.3 PASID translation"

Quoted 2 lines

Doesn't work submission go either to the mdev driver or through the secure PASID of #1?

No, once a PASID is bound with IOMMU, KVM, and the mdev, work submission is all done in HW. But I don't think this will change for either uAPI design.

Quoted 2 lines

Everything should revolve around the /dev/ioasid FD. qemu should pass it to all places that need to know about PASID's in the VM.

I guess we need to extend KVM interface to support PASIDs. Our original intention was to avoid introducing new interfaces.

Quoted 3 lines

We should try to avoid hidden behind the scenes kernel interconnections between subsystems.

Can we? in case of exception. Since all these IOCTLs are coming from the unreliable user space, we must deal all exceptions.

For example, when user closes /dev/ioasid FD before (or w/o) unbind IOCTL for VFIO, KVM, kernel must do cleanup and coordinate among subsystems. In this patchset, we have a per mm(ioasid_set) notifier to inform mdev, KVM to clean up and drop its refcount. Do you have any suggestion on this?

Let me try to paraphrase, you are suggesting common helper code and data format but still driver specific storage of the mapping, correct?

Quoted 6 lines

Quoted 3 lines

We should try to avoid hidden behind the scenes kernel interconnections between subsystems.

Can we? in case of exception. Since all these IOCTLs are coming from the unreliable user space, we must deal all exceptions.

Quoted 5 lines

For example, when user closes /dev/ioasid FD before (or w/o) unbind IOCTL for VFIO, KVM, kernel must do cleanup and coordinate among subsystems. In this patchset, we have a per mm(ioasid_set) notifier to inform mdev, KVM to clean up and drop its refcount. Do you have any suggestion on this?

The ioasid should be a reference counted object.

When the FD is closed, or the ioasid is "destroyed" it just blocks DMA and parks the PASID until *all* places release it. Upon a zero refcount the PASID is recycled for future use.

The duration between unmapping the ioasid and releasing all HW access will have HW see PCIE TLP errors due to the blocked access. If userspace messes up the order it is fine to cause this. We already had this dicussion when talking about how to deal with process exit in the simple SVA case.

Hi Jason,

Quoted 6 lines · Jason Gunthorpe

Quoted 6 lines

Quoted 3 lines

We should try to avoid hidden behind the scenes kernel interconnections between subsystems.

Can we? in case of exception. Since all these IOCTLs are coming from the unreliable user space, we must deal all exceptions.

Quoted 8 lines

Quoted 5 lines

For example, when user closes /dev/ioasid FD before (or w/o) unbind IOCTL for VFIO, KVM, kernel must do cleanup and coordinate among subsystems. In this patchset, we have a per mm(ioasid_set) notifier to inform mdev, KVM to clean up and drop its refcount. Do you have any suggestion on this?

The ioasid should be a reference counted object.

yes, this is done in this patchset.

Quoted 4 lines

When the FD is closed, or the ioasid is "destroyed" it just blocks DMA and parks the PASID until *all* places release it. Upon a zero refcount the PASID is recycled for future use.

Just to clarify, you are saying (when FREE happens before proper teardown) there is no need to proactively notify all users of the IOASID to drop their reference. Instead, just wait for the other parties to naturally close and drop their references. Am I understanding you correctly?

I feel having the notifications can add two values:
1. Shorten the duration of errors (as you mentioned below), FD close can take a long and unpredictable time. e.g. FD shared.

2. Provide teardown ordering among PASID users. i.e. vCPU, IOMMU, mdev.

Quoted 5 lines

The duration between unmapping the ioasid and releasing all HW access will have HW see PCIE TLP errors due to the blocked access. If userspace messes up the order it is fine to cause this. We already had this dicussion when talking about how to deal with process exit in the simple SVA case.

Yes, we have disabled fault reporting during this period. The slight differences vs. the simple SVA case is that KVM is also involved and there might be an ordering requirement to stop vCPU first.

Quoted 7 lines · Jacob Pan

Hi Jason,

Quoted 3 lines · Jason Gunthorpe

Quoted 3 lines

Quoted 3 lines

We should try to avoid hidden behind the scenes kernel interconnections between subsystems.

[...]
[...]

Quoted 2 lines

yes, this is done in this patchset.

[...]

Quoted 5 lines

Just to clarify, you are saying (when FREE happens before proper teardown) there is no need to proactively notify all users of the IOASID to drop their reference. Instead, just wait for the other parties to naturally close and drop their references. Am I understanding you correctly?

Yes. What are receivers going to do when you notify them anyhow? What will a mdev do? This is how you get into they crazy locking problems.

It is an error for userspace to shutdown like this, recover sensibly and don't crash the kernel. PCIe error TLPs are expected, supress them. That is what we decided on the mmu notifier discussion.

Quoted 3 lines

I feel having the notifications can add two values:
1. Shorten the duration of errors (as you mentioned below), FD close can take a long and unpredictable time. e.g. FD shared.

Only if userspace exits in some uncontrolled way. In a controlled exit it can close all the FDs in the right order.

It is OK if userspace does something weird and ends up with disabled IOASIDs. It shouldn't do that if it cares.

Quoted 2 lines

2. Provide teardown ordering among PASID users. i.e. vCPU, IOMMU, mdev.

This is a hard ask too, there is no natural ordering here I can see, obviously we want vcpu, mdev, iommu for qemu but that doesn't seem to fall out unless we explicitly hard wire it into the kernel.

Doesn't kvm always kill the vCPU first based on the mmu notifier shooting down all the memory? IIRC this happens before FD close?

Quoted 8 lines

Quoted 5 lines

The duration between unmapping the ioasid and releasing all HW access will have HW see PCIE TLP errors due to the blocked access. If userspace messes up the order it is fine to cause this. We already had this dicussion when talking about how to deal with process exit in the simple SVA case.

Yes, we have disabled fault reporting during this period. The slight differences vs. the simple SVA case is that KVM is also involved and there might be an ordering requirement to stop vCPU first.

KVM can continue to use the PASIDs, they are parked and DMA is permanently blocked. When KVM reaches a natural point in its teardown it can release them.

If you have to stop the vcpu from a iommu notifier you are in the crazy locking world I mentioned. IMHO don't create exciting locking problems just to avoid PCI errors in uncontrolled shutdown.

Suppress the errors instead.

Jason

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, March 30, 2021 9:43 PM

No, the mdev device driver must enforce this directly. It is the one that programms the physical shared HW, it is the one that needs a list of PASID's it is allowed to program *for each mdev*

ioasid_set doesn't seem to help at all, certainly not as a concept tied to /dev/ioasid.

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, March 30, 2021 12:32 AM

Quoted 11 lines · Jacob Pan

Quoted 4 lines

IMHO a use created PASID is either bound to a mm (current) at creation time, or it will never be bound to a mm and its page table is under user control via /dev/ioasid.

True for PASID used in native SVA bind. But for binding with a guest mm, PASID is allocated first (VT-d virtual cmd interface Spec 10.4.44), the bind with the host IOMMU when vIOMMU PASID cache is invalidated.

Our intention is to have two separate interfaces:
1. /dev/ioasid (allocation/free only)
2. /dev/sva (handles all SVA related activities including page tables)

I'm not sure I understand why you'd want to have two things. Doesn't that just complicate everything?

Manipulating the ioasid, including filling it with page tables, seems an integral inseperable part of the whole interface. Why have two ?

Quoted 17 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, March 30, 2021 12:32 AM

Quoted 11 lines · Jacob Pan

Quoted 4 lines

IMHO a use created PASID is either bound to a mm (current) at creation time, or it will never be bound to a mm and its page table is under user control via /dev/ioasid.

True for PASID used in native SVA bind. But for binding with a guest mm, PASID is allocated first (VT-d virtual cmd interface Spec 10.4.44), the bind with the host IOMMU when vIOMMU PASID cache is invalidated.

Our intention is to have two separate interfaces:
1. /dev/ioasid (allocation/free only)
2. /dev/sva (handles all SVA related activities including page tables)

Quoted 5 lines

I'm not sure I understand why you'd want to have two things. Doesn't that just complicate everything?

Manipulating the ioasid, including filling it with page tables, seems an integral inseperable part of the whole interface. Why have two ?

Hi, Jason,

Actually above is a major open while we are refactoring vSVA uAPI toward this direction. We have two concerns about merging /dev/ioasid with /dev/sva, and would like to hear your thought whether they are valid.

First, userspace may use ioasid in a non-SVA scenario where ioasid is bound to specific security context (e.g. a control vq in vDPA) instead of tying to mm. In this case there is no pgtable binding initiated from user space. Instead, ioasid is allocated from /dev/ioasid and then programmed to the intended security context through specific passthrough framework which manages that context.

Second, ioasid is managed per process/VM while pgtable binding is a device-wise operation. The userspace flow looks like below for an integral /dev/ioasid interface:

- ioctl(container->fd, VFIO_SET_IOMMU, VFIO_TYPE1_NESTING_IOMMU)
- ioasid_fd = open(/dev/ioasid)
- ioctl(ioasid_fd, IOASID_GET_USVA_FD, &sva_fd) //an empty context
- ioctl(device->fd, VFIO_DEVICE_SET_SVA, &sva_fd); //sva_fd ties to device
- ioctl(sva_fd, USVA_GET_INFO, &sva_info);
- ioctl(ioasid_fd, IOMMU_ALLOC_IOASID, &ioasid);
- ioctl(sva_fd, USVA_BIND_PGTBL, &bind_data);
- ioctl(sva_fd, USVA_FLUSH_CACHE, &inv_info);
- ioctl(sva_fd, USVA_UNBIND_PGTBL, &unbind_data);
- ioctl(device->fd, VFIO_DEVICE_UNSET_SVA, &sva_fd);
- close(sva_fd)
- close(ioasid_fd)

Our hesitation here is based on one of your earlier comments that you are not a fan of constructing fd's through ioctl. Are you OK with above flow or have a better idea of handling it?

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, March 30, 2021 9:29 PM

On Tue, Mar 30, 2021 at 01:37:05AM +0000, Tian, Kevin wrote:

Quoted 6 lines

Hi, Jason,

Actually above is a major open while we are refactoring vSVA uAPI toward this direction. We have two concerns about merging /dev/ioasid with /dev/sva, and would like to hear your thought whether they are valid.

Quoted 6 lines

First, userspace may use ioasid in a non-SVA scenario where ioasid is bound to specific security context (e.g. a control vq in vDPA) instead of tying to mm. In this case there is no pgtable binding initiated from user space. Instead, ioasid is allocated from /dev/ioasid and then programmed to the intended security context through specific passthrough framework which manages that context.

This sounds like the exact opposite of what I'd like to see.

I do not want to see every subsystem gaining APIs to program a PASID. All of that should be consolidated in *one place*.

I do not want to see VDPA and VFIO have two nearly identical sets of APIs to control the PASID.

Drivers consuming a PASID, like VDPA, should consume the PASID and do nothing more than authorize the HW to use it.

quemu should have general code under the viommu driver that drives /dev/ioasid to create PASID's and manage the IO mapping according to the guest's needs.

Drivers like VDPA and VFIO should simply accept that PASID and configure/authorize their HW to do DMA's with its tag.

Quoted 8 lines

Second, ioasid is managed per process/VM while pgtable binding is a device-wise operation. The userspace flow looks like below for an integral /dev/ioasid interface:

- ioctl(container->fd, VFIO_SET_IOMMU, VFIO_TYPE1_NESTING_IOMMU)
- ioasid_fd = open(/dev/ioasid)
- ioctl(ioasid_fd, IOASID_GET_USVA_FD, &sva_fd) //an empty context
- ioctl(device->fd, VFIO_DEVICE_SET_SVA, &sva_fd); //sva_fd ties to

device

Quoted 9 lines

- ioctl(sva_fd, USVA_GET_INFO, &sva_info);
- ioctl(ioasid_fd, IOMMU_ALLOC_IOASID, &ioasid);
- ioctl(sva_fd, USVA_BIND_PGTBL, &bind_data);
- ioctl(sva_fd, USVA_FLUSH_CACHE, &inv_info);
- ioctl(sva_fd, USVA_UNBIND_PGTBL, &unbind_data);
- ioctl(device->fd, VFIO_DEVICE_UNSET_SVA, &sva_fd);
- close(sva_fd)
- close(ioasid_fd)

Quoted 3 lines

Our hesitation here is based on one of your earlier comments that you are not a fan of constructing fd's through ioctl. Are you OK with above flow or have a better idea of handling it?

My reaction is to squash 'sva' and ioasid fds together, I can't see why you'd need two fds to manipulate a PASID.

The reason is /dev/ioasid FD is per-VM since the ioasid allocated to the VM should be able to be shared by all assigned device for the VM. But the SVA operations (bind/unbind page table, cache_invalidate) should be per-device.

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, March 31, 2021 8:41 PM

Quoted 4 lines · Yi L

The reason is /dev/ioasid FD is per-VM since the ioasid allocated to the VM should be able to be shared by all assigned device for the VM. But the SVA operations (bind/unbind page table, cache_invalidate) should be per-device.

It is not *per-device* it is *per-ioasid*

And as /dev/ioasid is an interface for controlling multiple ioasid's there is no issue to also multiplex the page table manipulation for multiple ioasids as well.

What you should do next is sketch out in some RFC the exactl ioctls each FD would have and show how the parts I outlined would work and point out any remaining gaps.

The device FD is something like the vfio_device FD from VFIO, it has *nothing* to do with PASID beyond having a single ioctl to authorize the device to use the PASID. All control of the PASID is in /dev/ioasid.

From: Liu, Yi L <[email protected]>
Sent: Thursday, April 1, 2021 12:39 PM

Quoted 8 lines

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, March 31, 2021 8:41 PM

Quoted 3 lines · Yi L

The reason is /dev/ioasid FD is per-VM since the ioasid allocated to the VM should be able to be shared by all assigned device for the VM. But the SVA operations (bind/unbind page table, cache_invalidate)

should

Quoted 8 lines

Quoted 1 line

be per-device.

It is not *per-device* it is *per-ioasid*

And as /dev/ioasid is an interface for controlling multiple ioasid's there is no issue to also multiplex the page table manipulation for multiple ioasids as well.

Quoted 8 lines

What you should do next is sketch out in some RFC the exactl ioctls each FD would have and show how the parts I outlined would work and point out any remaining gaps.

The device FD is something like the vfio_device FD from VFIO, it has *nothing* to do with PASID beyond having a single ioctl to authorize the device to use the PASID. All control of the PASID is in /dev/ioasid.

good to see this reply. Your idea is much clearer to me now. If I'm getting you correctly. I think the skeleton is something like below:
f
1) userspace opens a /dev/ioasid, meanwhile there will be an ioasid allocated and a per-ioasid context which can be used to do bind page table and cache invalidate, an ioasid FD returned to userspace.
2) userspace passes the ioasid FD to VFIO, let it associated with a device FD (like vfio_device FD).
3) userspace binds page table on the ioasid FD with the page table info.
4) userspace unbinds the page table on the ioasid FD
5) userspace de-associates the ioasid FD and device FD

Does above suit your outline?

If yes, I still have below concern and wish to see your opinion.
- the ioasid FD and device association will happen at runtime instead of just happen in the setup phase.
- how about AMD and ARM's vSVA support? Their PASID allocation and page table
happens within guest. They only need to bind the guest PASID table to
host.
Above model seems unable to fit them. (Jean, Eric, Jacob please feel free to correct me)
- this per-ioasid SVA operations is not aligned with the native SVA usage model. Native SVA bind is per-device.

After reading your reply in https://lore.kernel.org/linux-iommu/[email protected]/#t
So you mean /dev/ioasid FD is per-VM instead of per-ioasid, so above skeleton
doesn't suit your idea.

+-----------------------------+-----------------------------------------------+
|      userspace              |               kernel space                    |
+-----------------------------+-----------------------------------------------+
| ioasid_fd =                 | /dev/ioasid does below:                       |
| open("/dev/ioasid", O_RDWR);|   struct ioasid_fd_ctx {                      |
|                             |       struct list_head ioasid_list;           |
|                             |       ...                                     |
|                             |   } ifd_ctx; // ifd_ctx is per ioasid_fd      |

+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       ALLOC, &ioasid);      |   struct ioasid_data {                        |
|                             |       ioasid_t ioasid;                        |

|                             |       struct list_head device_list;           |
|                             |       struct list_head next;                  |
|                             |       ...                                     |
|                             |   } id_data; // id_data is per ioasid         |
|                             |                                               |
|                             |   list_add(&id_data.next,                     |
|                             |            &ifd_ctx.ioasid_list);
|

+-----------------------------+-----------------------------------------------+
| ioctl(device_fd,            | VFIO does below:                              |
|       DEVICE_ALLOW_IOASID,  | 1) get ioasid_fd, check if ioasid_fd is valid |
|       ioasid_fd,            | 2) check if ioasid is allocated from ioasid_fd|
|       ioasid);              | 3) register device/domain info to /dev/ioasid |
|                             |    tracked in id_data.device_list             |
|                             | 4) record the ioasid in VFIO's per-device     |
|                             |    ioasid list for future security check      |

+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       BIND_PGTBL,           | 1) find ioasid's id_data                      |
|       pgtbl_data,           | 2) loop the id_data.device_list and tell iommu|
|       ioasid);              |    give ioasid access to the devices
|

+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       UNBIND_PGTBL,         | 1) find ioasid's id_data                      |
|       ioasid);              | 2) loop the id_data.device_list and tell iommu|
|                             |    clear ioasid access to the devices         |

+-----------------------------+-----------------------------------------------+
| ioctl(device_fd,            | VFIO does below:                              |
|      DEVICE_DISALLOW_IOASID,| 1) check if ioasid is associated in VFIO's    |
|       ioasid_fd,            |    device ioasid list.                        |
|       ioasid);              | 2) unregister device/domain info from         |
|                             |    /dev/ioasid, clear in id_data.device_list  |

+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       FREE, ioasid);        |  list_del(&id_data.next);                     |
+-----------------------------+-----------------------------------------------+

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, April 1, 2021 7:54 PM

Quoted 1 line · Yi L

After reading your reply in https://lore.kernel.org/linux-

iommu/[email protected]/#t

Quoted 1 line

So you mean /dev/ioasid FD is per-VM instead of per-ioasid, so above

skeleton

Quoted 1 line

doesn't suit your idea.

You can do it one PASID per FD or multiple PASID's per FD. Most likely we will have high numbers of PASID's in a qemu process so I assume that number of FDs will start to be a contraining factor, thus multiplexing is reasonable.

It doesn't really change anything about the basic flow.

digging deeply into it either seems like a reasonable choice.

Quoted 8 lines

+-----------------------------+-----------------------------------------------+
|      userspace              |               kernel space                    |
+-----------------------------+-----------------------------------------------+
| ioasid_fd =                 | /dev/ioasid does below:                       |
| open("/dev/ioasid", O_RDWR);|   struct ioasid_fd_ctx {                      |
|                             |       struct list_head ioasid_list;           |
|                             |       ...                                     |
|                             |   } ifd_ctx; // ifd_ctx is per ioasid_fd      |

Sure, possibly an xarray not a list

Quoted 4 lines

+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       ALLOC, &ioasid);      |   struct ioasid_data {                        |
|                             |       ioasid_t ioasid;                        |

Quoted 8 lines

|                             |       struct list_head device_list;           |
|                             |       struct list_head next;                  |
|                             |       ...                                     |
|                             |   } id_data; // id_data is per ioasid         |
|                             |                                               |
|                             |   list_add(&id_data.next,                     |
|                             |            &ifd_ctx.ioasid_list);
|

Yes, this should have a kref in it too

Quoted 8 lines

+-----------------------------+-----------------------------------------------+
| ioctl(device_fd,            | VFIO does below:                              |
|       DEVICE_ALLOW_IOASID,  | 1) get ioasid_fd, check if ioasid_fd is valid |
|       ioasid_fd,            | 2) check if ioasid is allocated from ioasid_fd|
|       ioasid);              | 3) register device/domain info to /dev/ioasid |
|                             |    tracked in id_data.device_list             |
|                             | 4) record the ioasid in VFIO's per-device     |
|                             |    ioasid list for future security check      |

You would provide a function that does steps 1&2 look at eventfd for instance.

I'm not sure we need to register the device with the ioasid. device should incr the kref on the ioasid_data at this point.

Quoted 6 lines

+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       BIND_PGTBL,           | 1) find ioasid's id_data                      |
|       pgtbl_data,           | 2) loop the id_data.device_list and tell iommu|
|       ioasid);              |    give ioasid access to the devices
|

This seems backwards, DEVICE_ALLOW_IOASID should tell the iommu to give the ioasid to the device.

Here the ioctl should be about assigning a memory map from the the current
mm_struct to the pasid

Quoted 5 lines

+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       UNBIND_PGTBL,         | 1) find ioasid's id_data                      |
|       ioasid);              | 2) loop the id_data.device_list and tell iommu|
|                             |    clear ioasid access to the devices         |

Also seems backwards. The ioctl here should be 'destroy ioasid' which wipes out the page table, halts DMA access and parks the PASID until all users are done.

Quoted 6 lines

+-----------------------------+-----------------------------------------------+
| ioctl(device_fd,            | VFIO does below:                              |
|      DEVICE_DISALLOW_IOASID,| 1) check if ioasid is associated in VFIO's    |
|       ioasid_fd,            |    device ioasid list.                        |
|       ioasid);              | 2) unregister device/domain info from         |
|                             |    /dev/ioasid, clear in id_data.device_list  |

This should disconnect the iommu and kref_put the ioasid_data

Include a sequence showing how the kvm FD is used to program the vPASID to pPASID table that ENQCMD uses.

Show how dynamic authorization works based on requests from the guest's vIOMMU

Jason

Quoted 4 lines

- how about AMD and ARM's vSVA support? Their PASID allocation and page table
happens within guest. They only need to bind the guest PASID table to
host.

Quoted 4 lines

Above model seems unable to fit them. (Jean, Eric, Jacob please feel free to correct me)
- this per-ioasid SVA operations is not aligned with the native SVA usage model. Native SVA bind is per-device.

After reading your reply in https://lore.kernel.org/linux-iommu/[email protected]/#t
So you mean /dev/ioasid FD is per-VM instead of per-ioasid, so above skeleton
doesn't suit your idea. I draft below skeleton to see if our mind is the
same. But I still believe there is an open on how to fit ARM and AMD's
vSVA support in this the per-ioasid SVA operation model. thoughts?

+-----------------------------+-----------------------------------------------+
|      userspace              |               kernel space                    |
+-----------------------------+-----------------------------------------------+
| ioasid_fd =                 | /dev/ioasid does below:                       |
| open("/dev/ioasid", O_RDWR);|   struct ioasid_fd_ctx {                      |
|                             |       struct list_head ioasid_list;           |
|                             |       ...                                     |
|                             |   } ifd_ctx; // ifd_ctx is per ioasid_fd      |
+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       ALLOC, &ioasid);      |   struct ioasid_data {                        |
|                             |       ioasid_t ioasid;                        |
|                             |       struct list_head device_list;           |
|                             |       struct list_head next;                  |
|                             |       ...                                     |
|                             |   } id_data; // id_data is per ioasid         |
|                             |                                               |
|                             |   list_add(&id_data.next,                     |
|                             |            &ifd_ctx.ioasid_list);             |
+-----------------------------+-----------------------------------------------+
| ioctl(device_fd,            | VFIO does below:                              |
|       DEVICE_ALLOW_IOASID,  | 1) get ioasid_fd, check if ioasid_fd is valid |
|       ioasid_fd,            | 2) check if ioasid is allocated from ioasid_fd|
|       ioasid);              | 3) register device/domain info to /dev/ioasid |
|                             |    tracked in id_data.device_list             |
|                             | 4) record the ioasid in VFIO's per-device     |
|                             |    ioasid list for future security check      |
+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       BIND_PGTBL,           | 1) find ioasid's id_data                      |
|       pgtbl_data,           | 2) loop the id_data.device_list and tell iommu|
|       ioasid);              |    give ioasid access to the devices          |
+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       UNBIND_PGTBL,         | 1) find ioasid's id_data                      |
|       ioasid);              | 2) loop the id_data.device_list and tell iommu|
|                             |    clear ioasid access to the devices         |
+-----------------------------+-----------------------------------------------+
| ioctl(device_fd,            | VFIO does below:                              |

|      DEVICE_DISALLOW_IOASID,| 1) check if ioasid is associated in VFIO's    |
|       ioasid_fd,            |    device ioasid list.                        |
|       ioasid);              | 2) unregister device/domain info from         |
|                             |    /dev/ioasid, clear in id_data.device_list  |
+-----------------------------+-----------------------------------------------+
| ioctl(ioasid_fd,            | /dev/ioasid does below:                       |
|       FREE, ioasid);        |  list_del(&id_data.next);                     |
+-----------------------------+-----------------------------------------------+

Quoted 4 lines · Yi L

Quoted 4 lines

- how about AMD and ARM's vSVA support? Their PASID allocation and page table
happens within guest. They only need to bind the guest PASID table to
host.

In this case each VM has its own IOASID space, and the host IOASID allocator doesn't participate. Plus this only makes sense when assigning a whole VF to a guest, and VFIO is the tool for this. So I wouldn't shoehorn those ops into /dev/ioasid, though we do need a transport for invalidate commands.

Quoted 4 lines

Quoted 4 lines

Above model seems unable to fit them. (Jean, Eric, Jacob please feel free to correct me)
- this per-ioasid SVA operations is not aligned with the native SVA usage model. Native SVA bind is per-device.

Bare-metal SVA doesn't need /dev/ioasid either.

A program uses a device handle to either ask whether SVA is enabled, or to enable it explicitly. With or without /dev/ioasid, that step is required. OpenCL uses the first method - automatically enable "fine-grain system SVM" if available, and provide a flag to userspace.

So userspace does not need to know about PASID. It's only one method for doing SVA (some GPUs are context-switching page tables instead).

* Page faults, page response. From and to devices, and don't necessarily have a PASID. But needed by vdpa as well, so that's also going through /dev/ioasid?

From: Jean-Philippe Brucker <[email protected]>
Sent: Thursday, April 1, 2021 8:05 PM

Also wondering about:

* Querying IOMMU nesting capabilities before binding page tables (which page table formats are supported?). We were planning to have a VFIO cap, but I'm guessing we need to go back to the sysfs solution?

* Invalidation, probably an ioasid_fd ioctl?

* Page faults, page response. From and to devices, and don't necessarily have a PASID. But needed by vdpa as well, so that's also going through /dev/ioasid?

Thanks,
Jean

Quoted 2 lines

From: Jean-Philippe Brucker <[email protected]>
Sent: Thursday, April 1, 2021 8:05 PM

[...]

Quoted 6 lines

Also wondering about:

* Querying IOMMU nesting capabilities before binding page tables (which page table formats are supported?). We were planning to have a VFIO cap, but I'm guessing we need to go back to the sysfs solution?

I think it can also be with /dev/ioasid.

Quoted 1 line

* Invalidation, probably an ioasid_fd ioctl?

yeah, if we are doing bind/unbind_pagtable via ioasid_fd, then yes, invalidation should go this way as well. This is why I worried it may fail to meet the requirement from you and Eric.

Quoted 3 lines

* Page faults, page response. From and to devices, and don't necessarily have a PASID. But needed by vdpa as well, so that's also going through /dev/ioasid?

page faults should still be per-device, but the fault event fd may be stored in /dev/ioasid. page response would be in /dev/ioasid just like invalidation.

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, April 1, 2021 9:43 PM

Quoted 7 lines · Yi L

Quoted 2 lines

From: Jean-Philippe Brucker <[email protected]>
Sent: Thursday, April 1, 2021 8:05 PM

[...]

Quoted 4 lines

Also wondering about:

* Querying IOMMU nesting capabilities before binding page tables

(which

Quoted 1 line

Quoted 1 line

page table formats are supported?). We were planning to have a VFIO

cap,

Quoted 3 lines

Quoted 1 line

but I'm guessing we need to go back to the sysfs solution?

I think it can also be with /dev/ioasid.

Sure, anything to do with page table formats and setting page tables should go through ioasid.

Quoted 5 lines

Quoted 1 line

* Invalidation, probably an ioasid_fd ioctl?

yeah, if we are doing bind/unbind_pagtable via ioasid_fd, then yes, invalidation should go this way as well. This is why I worried it may fail to meet the requirement from you and Eric.

Yes, all manipulation of page tables, including removing memory ranges, or setting memory ranges to trigger a page fault behavior should go through here.

Quoted 4 lines

Quoted 3 lines

* Page faults, page response. From and to devices, and don't necessarily have a PASID. But needed by vdpa as well, so that's also going through /dev/ioasid?

Quoted 2 lines

page faults should still be per-device, but the fault event fd may be stored in /dev/ioasid. page response would be in /dev/ioasid just like invalidation.

Here you mean non-SVA page faults that are delegated to userspace to handle?

Why would that be per-device?

Can you show the flow you imagine?

Jason

DMA page faults are delivered to root-complex via page request message and it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID, faulted page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

4) device driver's fault handler signals an event FD to notify userspace to fetch the information about the page fault. If it's VM case, inject the page fault to VM and let guest to solve it.

Eric has sent below series for the page fault reporting for VM with passthru device.
https://lore.kernel.org/kvm/20210223210625.604517-5-eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org/

From: Jason Gunthorpe <[email protected]>
Sent: Friday, April 2, 2021 12:04 AM

Quoted 1 line · Yi L

DMA page faults are delivered to root-complex via page request message

and

Quoted 4 lines

it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID,

faulted

Quoted 3 lines

page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

This seems confused.

The PASID should define how to handle the page fault, not the driver.

I don't remember any device specific actions in ATS, so what is the driver supposed to do?

Quoted 3 lines

4) device driver's fault handler signals an event FD to notify userspace to fetch the information about the page fault. If it's VM case, inject the page fault to VM and let guest to solve it.

If the PASID is set to 'report page fault to userspace' then some event should come out of /dev/ioasid, or be reported to a linked eventfd, or whatever.

If the PASID is set to 'SVM' then the fault should be passed to handle_mm_fault

And so on.

Userspace chooses what happens based on how they configure the PASID through /dev/ioasid.

Why would a device driver get involved here?

Quoted 3 lines

Eric has sent below series for the page fault reporting for VM with passthru device.
https://lore.kernel.org/kvm/20210223210625.604517-5-

[email protected]/

It certainly should not be in vfio pci. Everything using a PASID needs this infrastructure, VDPA, mdev, PCI, CXL, etc.

Quoted 41 lines

From: Jason Gunthorpe <[email protected]>
Sent: Friday, April 2, 2021 12:04 AM

Quoted 1 line · Yi L

DMA page faults are delivered to root-complex via page request message

and

Quoted 4 lines

it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID,

faulted

Quoted 3 lines

page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

This seems confused.

The PASID should define how to handle the page fault, not the driver.

I don't remember any device specific actions in ATS, so what is the driver supposed to do?

Quoted 3 lines

4) device driver's fault handler signals an event FD to notify userspace to fetch the information about the page fault. If it's VM case, inject the page fault to VM and let guest to solve it.

If the PASID is set to 'report page fault to userspace' then some event should come out of /dev/ioasid, or be reported to a linked eventfd, or whatever.

If the PASID is set to 'SVM' then the fault should be passed to handle_mm_fault

And so on.

Userspace chooses what happens based on how they configure the PASID through /dev/ioasid.

Why would a device driver get involved here?

Quoted 8 lines

Quoted 3 lines

Eric has sent below series for the page fault reporting for VM with passthru device.
https://lore.kernel.org/kvm/20210223210625.604517-5-

[email protected]/

It certainly should not be in vfio pci. Everything using a PASID needs this infrastructure, VDPA, mdev, PCI, CXL, etc.

This touches an interesting fact:

The fault may be triggered in either 1st-level or 2nd-level page table, when nested translation is enabled (in vSVA case). The 1st-level is bound by the user space, which therefore needs to receive the fault event. The 2nd-level is managed by VFIO (or vDPA), which needs to fix the fault in kernel (e.g. find HVA per faulting GPA, call handle_mm_fault and map GPA->HPA to IOMMU). Yi's current proposal lets VFIO to register the device fault handler, which then forward the event through /dev/ioasid to userspace only if it is a 1st-level fault. Are you suggesting a pgtable- centric fault reporting mechanism to separate handlers in each level, i.e. letting VFIO register handler only for 2nd-level fault and then /dev/ ioasid register handler for 1st-level fault?

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, April 6, 2021 7:35 AM

Quoted 6 lines · Kevin

Quoted 6 lines

From: Jason Gunthorpe <[email protected]>
Sent: Friday, April 2, 2021 12:04 AM

Quoted 1 line · Yi L

DMA page faults are delivered to root-complex via page request

message

Quoted 10 lines

Quoted 10 lines

and

Quoted 4 lines

it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID,

faulted

Quoted 3 lines

page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

Quoted 8 lines

Quoted 8 lines

This seems confused.

The PASID should define how to handle the page fault, not the driver.

I don't remember any device specific actions in ATS, so what is the driver supposed to do?

Quoted 1 line

4) device driver's fault handler signals an event FD to notify userspace

to

Quoted 10 lines

Quoted 10 lines

Quoted 2 lines

fetch the information about the page fault. If it's VM case, inject the page fault to VM and let guest to solve it.

If the PASID is set to 'report page fault to userspace' then some event should come out of /dev/ioasid, or be reported to a linked eventfd, or whatever.

If the PASID is set to 'SVM' then the fault should be passed to handle_mm_fault

Quoted 8 lines

Quoted 8 lines

And so on.

Userspace chooses what happens based on how they configure the PASID through /dev/ioasid.

Why would a device driver get involved here?

Quoted 1 line

Eric has sent below series for the page fault reporting for VM with

passthru

Quoted 10 lines

Quoted 7 lines

Quoted 2 lines

device.
https://lore.kernel.org/kvm/20210223210625.604517-5-

[email protected]/

It certainly should not be in vfio pci. Everything using a PASID needs this infrastructure, VDPA, mdev, PCI, CXL, etc.

This touches an interesting fact:

Quoted 11 lines

The fault may be triggered in either 1st-level or 2nd-level page table, when nested translation is enabled (in vSVA case). The 1st-level is bound by the user space, which therefore needs to receive the fault event. The 2nd-level is managed by VFIO (or vDPA), which needs to fix the fault in kernel (e.g. find HVA per faulting GPA, call handle_mm_fault and map GPA->HPA to IOMMU). Yi's current proposal lets VFIO to register the device fault handler, which then forward the event through /dev/ioasid to userspace only if it is a 1st-level fault. Are you suggesting a pgtable- centric fault reporting mechanism to separate handlers in each level, i.e. letting VFIO register handler only for 2nd-level fault and then /dev/ ioasid register handler for 1st-level fault?

This I'm struggling to understand. /dev/ioasid should handle all the faults cases, why would VFIO ever get involved in a fault? What would it even do?

If the fault needs to be fixed in the hypervisor then it is a kernel fault and it does handle_mm_fault. This absolutely should not be in VFIO or VDPA

If the fault needs to be fixed in the guest, then it needs to be delivered over /dev/ioasid in some way and injected into the vIOMMU. VFIO and VDPA have nothing to do with vIOMMU driver in quemu.

You need to have an interface under /dev/ioasid to create both page table levels and part of that will be to tell the kernel what VA is mapped and how to handle faults.

With nested translation it is GVA->GPA->HPA. The kernel needs to fix fault related to GPA->HPA (managed by VFIO/VDPA) while handle_mm_fault only handles HVA->HPA. In this case, the 2nd-level page fault is expected to be delivered to VFIO/VDPA first which then find HVA related to GPA, call handle_mm_fault to fix HVA->HPA, and then call iommu_map to fix GPA->HPA in the IOMMU page table. This is exactly like how CPU EPT violation is handled.

Quoted 7 lines

If the fault needs to be fixed in the guest, then it needs to be delivered over /dev/ioasid in some way and injected into the vIOMMU. VFIO and VDPA have nothing to do with vIOMMU driver in quemu.

You need to have an interface under /dev/ioasid to create both page table levels and part of that will be to tell the kernel what VA is mapped and how to handle faults.

VFIO/VDPA already have their own interface to manage GPA->HPA mappings. Why do we want to duplicate it in /dev/ioasid?

Quoted 8 lines · Yi L

DMA page faults are delivered to root-complex via page request message and it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID, faulted page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

This seems confused.

The PASID should define how to handle the page fault, not the driver.

I don't remember any device specific actions in ATS, so what is the driver supposed to do?

Quoted 3 lines

4) device driver's fault handler signals an event FD to notify userspace to fetch the information about the page fault. If it's VM case, inject the page fault to VM and let guest to solve it.

If the PASID is set to 'report page fault to userspace' then some event should come out of /dev/ioasid, or be reported to a linked eventfd, or whatever.

If the PASID is set to 'SVM' then the fault should be passed to handle_mm_fault

And so on.

Userspace chooses what happens based on how they configure the PASID through /dev/ioasid.

Why would a device driver get involved here?

Quoted 3 lines

Eric has sent below series for the page fault reporting for VM with passthru device.
https://lore.kernel.org/kvm/[email protected]/

It certainly should not be in vfio pci. Everything using a PASID needs this infrastructure, VDPA, mdev, PCI, CXL, etc.

Jason

Hi Jason,

Quoted 11 lines · Jason Gunthorpe

Quoted 8 lines · Yi L

DMA page faults are delivered to root-complex via page request message and it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID, faulted page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

Quoted 3 lines

This seems confused.

The PASID should define how to handle the page fault, not the driver.

In my series I don't use PASID at all. I am just enabling nested stage and the guest uses a single context. I don't allocate any user PASID at any point.

When there is a fault at physical level (a stage 1 fault that concerns the guest), this latter needs to be reported and injected into the guest. The vfio pci driver registers a fault handler to the iommu layer and in that fault handler it fills a circ bugger and triggers an eventfd that is listened to by the VFIO-PCI QEMU device. this latter retrives the faault from the mmapped circ buffer, it knowns which vIOMMU it is attached to, and passes the fault to the vIOMMU.
Then the vIOMMU triggers and IRQ in the guest.

We are reusing the existing concepts from VFIO, region, IRQ to do that.

For that use case, would you also use /dev/ioasid?

Quoted 33 lines · Auger Eric

Hi Jason,

Quoted 12 lines · Jason Gunthorpe

Quoted 9 lines · Yi L

DMA page faults are delivered to root-complex via page request message and it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID, faulted page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

Quoted 4 lines

This seems confused.

The PASID should define how to handle the page fault, not the driver.

In my series I don't use PASID at all. I am just enabling nested stage and the guest uses a single context. I don't allocate any user PASID at any point.

When there is a fault at physical level (a stage 1 fault that concerns the guest), this latter needs to be reported and injected into the guest. The vfio pci driver registers a fault handler to the iommu layer and in that fault handler it fills a circ bugger and triggers an eventfd that is listened to by the VFIO-PCI QEMU device. this latter retrives the faault from the mmapped circ buffer, it knowns which vIOMMU it is attached to, and passes the fault to the vIOMMU.
Then the vIOMMU triggers and IRQ in the guest.

Quoted 3 lines

We are reusing the existing concepts from VFIO, region, IRQ to do that.

For that use case, would you also use /dev/ioasid?

/dev/ioasid could do all the things you described vfio-pci as doing, it can even do them the same way you just described.

Stated another way, do you plan to duplicate all of this code someday for vfio-cxl? What about for vfio-platform? ARM SMMU can be hooked to platform devices, right?

I feel what you guys are struggling with is some choice in the iommu kernel APIs that cause the events to be delivered to the pci_device owner, not the PASID owner.

That feels solvable.

Jason

Hi Jason,

Quoted 49 lines · Jason Gunthorpe

Quoted 33 lines · Auger Eric

Hi Jason,

Quoted 12 lines · Jason Gunthorpe

Quoted 9 lines · Yi L

DMA page faults are delivered to root-complex via page request message and it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID, faulted page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

Quoted 4 lines

This seems confused.

The PASID should define how to handle the page fault, not the driver.

In my series I don't use PASID at all. I am just enabling nested stage and the guest uses a single context. I don't allocate any user PASID at any point.

When there is a fault at physical level (a stage 1 fault that concerns the guest), this latter needs to be reported and injected into the guest. The vfio pci driver registers a fault handler to the iommu layer and in that fault handler it fills a circ bugger and triggers an eventfd that is listened to by the VFIO-PCI QEMU device. this latter retrives the faault from the mmapped circ buffer, it knowns which vIOMMU it is attached to, and passes the fault to the vIOMMU.
Then the vIOMMU triggers and IRQ in the guest.

Quoted 3 lines

We are reusing the existing concepts from VFIO, region, IRQ to do that.

For that use case, would you also use /dev/ioasid?

/dev/ioasid could do all the things you described vfio-pci as doing, it can even do them the same way you just described.

Stated another way, do you plan to duplicate all of this code someday for vfio-cxl? What about for vfio-platform? ARM SMMU can be hooked to platform devices, right?

I feel what you guys are struggling with is some choice in the iommu kernel APIs that cause the events to be delivered to the pci_device owner, not the PASID owner.

Quoted 2 lines

That feels solvable.

Perhaps more of a philosophical question for you and Alex. There is no doubt that the direction you guided for /dev/ioasid is a much cleaner one, especially after VDPA emerged as another IOMMU backed framework.

The question is what do we do with the nested translation features that have been targeting the existing VFIO-IOMMU for the last three years? That predates VDPA. Shall we put a stop marker *after* nested support and say no more extensions for VFIO-IOMMU, new features must be built on this new interface?

If we were to close a checkout line for some unforeseen reasons, should we honor the customers already in line for a long time?

This is not a tactic or excuse for not working on the new /dev/ioasid interface. In fact, I believe we can benefit from the lessons learned while completing the existing. This will give confidence to the new interface. Thoughts?

Quoted 54 lines · On Fri, 16 Apr 2021 06:12:58 -0700 Jacob Pan <[email protected]>…

Hi Jason,

Quoted 50 lines · Jason Gunthorpe

Quoted 33 lines · Auger Eric

Hi Jason,

Quoted 12 lines · Jason Gunthorpe

Quoted 9 lines · Yi L

DMA page faults are delivered to root-complex via page request message and it is per-device according to PCIe spec. Page request handling flow is:

1) iommu driver receives a page request from device
2) iommu driver parses the page request message. Get the RID,PASID, faulted page and requested permissions etc.
3) iommu driver triggers fault handler registered by device driver with iommu_report_device_fault()

Quoted 4 lines

This seems confused.

The PASID should define how to handle the page fault, not the driver.

In my series I don't use PASID at all. I am just enabling nested stage and the guest uses a single context. I don't allocate any user PASID at any point.

When there is a fault at physical level (a stage 1 fault that concerns the guest), this latter needs to be reported and injected into the guest. The vfio pci driver registers a fault handler to the iommu layer and in that fault handler it fills a circ bugger and triggers an eventfd that is listened to by the VFIO-PCI QEMU device. this latter retrives the faault from the mmapped circ buffer, it knowns which vIOMMU it is attached to, and passes the fault to the vIOMMU. Then the vIOMMU triggers and IRQ in the guest.

Quoted 4 lines

We are reusing the existing concepts from VFIO, region, IRQ to do that.

For that use case, would you also use /dev/ioasid?

/dev/ioasid could do all the things you described vfio-pci as doing, it can even do them the same way you just described.

Stated another way, do you plan to duplicate all of this code someday for vfio-cxl? What about for vfio-platform? ARM SMMU can be hooked to platform devices, right?

I feel what you guys are struggling with is some choice in the iommu kernel APIs that cause the events to be delivered to the pci_device owner, not the PASID owner.

Quoted 5 lines

Quoted 2 lines

That feels solvable.

Perhaps more of a philosophical question for you and Alex. There is no doubt that the direction you guided for /dev/ioasid is a much cleaner one, especially after VDPA emerged as another IOMMU backed framework.

I think this statement answers all your remaining questions ;)

Quoted 6 lines

The question is what do we do with the nested translation features that have been targeting the existing VFIO-IOMMU for the last three years? That predates VDPA. Shall we put a stop marker *after* nested support and say no more extensions for VFIO-IOMMU, new features must be built on this new interface?

Quoted 7 lines

If we were to close a checkout line for some unforeseen reasons, should we honor the customers already in line for a long time?

This is not a tactic or excuse for not working on the new /dev/ioasid interface. In fact, I believe we can benefit from the lessons learned while completing the existing. This will give confidence to the new interface. Thoughts?

I understand a big part of Jason's argument is that we shouldn't be in the habit of creating duplicate interfaces, we should create one, well designed interfaces to share among multiple subsystems. As new users have emerged, our solution needs to change to a common one rather than a VFIO specific one. The IOMMU uAPI provides an abstraction, but at the wrong level, requiring userspace interfaces for each subsystem.

Luckily the IOMMU uAPI is not really exposed as an actual uAPI, but that changes if we proceed to enable the interfaces to tunnel it through VFIO.

The logical answer would therefore be that we don't make that commitment to the IOMMU uAPI if we believe now that it's fundamentally flawed.

Ideally this new /dev/ioasid interface, and making use of it as a VFIO IOMMU backend, should replace type1. Type1 will live on until that interface gets to parity, at which point we may deprecate type1, but it wouldn't make sense to continue to expand type1 in the same direction as we intend /dev/ioasid to take over in the meantime, especially if it means maintaining an otherwise dead uAPI. Thanks,

Alex

Perhaps similar to cgroup v1 vs v2, it took a long time and with known limitations in v1.

From: Alex Williamson <[email protected]>
Sent: Friday, April 16, 2021 11:46 PM

Quoted 4 lines

This is not a tactic or excuse for not working on the new /dev/ioasid interface. In fact, I believe we can benefit from the lessons learned while completing the existing. This will give confidence to the new interface. Thoughts?

I understand a big part of Jason's argument is that we shouldn't be in the habit of creating duplicate interfaces, we should create one, well designed interfaces to share among multiple subsystems. As new users have emerged, our solution needs to change to a common one rather than a VFIO specific one. The IOMMU uAPI provides an abstraction, but at the wrong level, requiring userspace interfaces for each subsystem.

Luckily the IOMMU uAPI is not really exposed as an actual uAPI, but that changes if we proceed to enable the interfaces to tunnel it through VFIO.

The logical answer would therefore be that we don't make that commitment to the IOMMU uAPI if we believe now that it's fundamentally flawed.

Ideally this new /dev/ioasid interface, and making use of it as a VFIO IOMMU backend, should replace type1.

Type1 will live on until that
interface gets to parity, at which point we may deprecate type1, but it wouldn't make sense to continue to expand type1 in the same direction as we intend /dev/ioasid to take over in the meantime, especially if it means maintaining an otherwise dead uAPI. Thanks,

Quoted 2 lines

Ideally this new /dev/ioasid interface, and making use of it as a VFIO IOMMU backend, should replace type1.

yeah, just a double check, I think this also requires a new set of uAPIs (e.g. new MAP/UNMAP), which means the current VFIO IOMMU type1 related ioctls would be deprecated in future. right?

Quoted 6 lines · Yi L

Quoted 2 lines

Ideally this new /dev/ioasid interface, and making use of it as a VFIO IOMMU backend, should replace type1.

yeah, just a double check, I think this also requires a new set of uAPIs (e.g. new MAP/UNMAP), which means the current VFIO IOMMU type1 related ioctls would be deprecated in future. right?

This is something to think about, it might make sense to run the current ioctls in some "compat" mode under /dev/ioasid just to make migration easier

In this sense /dev/ioasid would be a container that holds multiple IOASIDs and every new format ioctl specifies the IOASID to operate on. The legacy ioctls would use some default IOASID but otherwise act the same.

I'm assuming here there is nothing especially wrong with the /dev/vfio interface beyond being in the wrong place in the kernel and not supporting multiple IOASIDs?

Then there may be a fairly simple approch to just make /dev/vfio == /dev/ioasid, at least for type 1.

By this I mean we could have the new /dev/ioasid code take over the /dev/vfio char dev and present both interfaces, but with the same fops.

That's essentially replacing vfio-core, where I think we're more

The interface of making that selection might change to accept an external /dev/ioasid file descriptor, of course. Maybe you can elaborate on how the vfio device and group uAPI live (or not) in this new scheme were /dev/ioasid is the primary interface. Thanks,

Quoted 1 line · Alex Williamson

That's essentially replacing vfio-core, where I think we're more

I am only talking about /dev/vfio here which is basically the IOMMU interface part.

I still expect that VFIO_GROUP_SET_CONTAINER will be used to connect /dev/{ioasid,vfio} to the VFIO group and all the group and device logic stays inside VFIO.

The appeal of unifying /dev/{ioasid,vfio} to a single fops is that it cleans up vfio a lot - we don't have to have two different code paths where one handles a vfio_container and the other a ioasid_container and the all the related different iommu ops and so on.

Everything can be switched to ioasid_container all down the line. If it wasn't for PPC this looks fairly simple.

Since getting rid of PPC looks a bit hard, we'd be stuck with accepting a /dev/ioasid and then immediately wrappering it in a vfio_container an shimming it through a vfio_iommu_ops. It is not ideal at all, but in my look around I don't see a major problem if type1 implementation is moved to live under /dev/ioasid.

For concreteness if we look at the set container flow with ioasid I'd say something like:

vfio_group_fops_unl_ioctl()
VFIO_GROUP_SET_CONTAINER
vfio_group_set_container()

     if (f.file->f_op == &vfio_fops) {
          // Use a real vfio_container and vfio_iommu_driver
          driver->ops->attach_group()
             tce_iommu_attach_group()
     }

     if (ioasid_container = ioasid_get_from_fd(container_fd)) {
         // create a dummy vfio_container and use the ioasid driver
	 container = kzalloc()
         container->iommu_driver = ioasid_shim
         driver->ops->attach_group()
             ioasid_shim_attach_group(ioasid_container, ...)
                 ioasid_attach_group()
                     // What used to be vfio_iommu_attach_group()

Broadly all the ops vfio need go through the ioasid_shim which relays them to the generic ioasid API.

We end up with a ioasid.h that basically has the vfio_iommu_type1 code lightly recast into some 'struct iommu_container' and a set of ioasid_* function entry points that follow vfio_iommu_driver_ops_type1:
ioasid_attach_group
ioasid_detatch_group
ioasid_<something about user pages> ioasid_read/ioasid_write

If we have this, and /dev/ioasid implements the legacy IOCTLs, then /dev/vfio == /dev/ioasid and we can compile out vfio_fops and related from vfio.c and tell ioasid.c to create /dev/vfio instead using the ops it owns.

This is a very long winded way of saying ideally we'd do approximately:
git mv drivers/vfio/vfio_iommu_type1.c drivers/ioasid/ioasid.c

As the first step. Essentially we declare that what is type1 is really the user interface to the internal kernel IOMMU kAPI, which has been steadily evolving since type1 was created 10 years ago.

Quoted 4 lines

The interface of making that selection might change to accept an external /dev/ioasid file descriptor, of course. Maybe you can elaborate on how the vfio device and group uAPI live (or not) in this new scheme were /dev/ioasid is the primary interface. Thanks,

They say in vfio. You'd still open a group and you'd still pass in either /dev/vfio or /dev/ioasid to define the container

Though, completely as an unrelated aside, I admit to not entirely understanding why the group is the central element of the uAPI.

It is weird that the vfio "drivers" all work on the struct vfio_device (at least after my series), and it has a file_operations presence via vfio_device_fops, but instead of struct vfio_device directly having a 'struct device' and cdev to access the FD we get it through a group FD and agroup chardev via VFIO_GROUP_GET_DEVICE_FD

If we were to revise this, and I don't see a huge reason to do so, I would put a struct device and cdev in struct vfio_device, attach the vfio_device directly to the ioasid and then forget about the group, at least as uapi, completely.

Or at least I don't see where that gets into trouble, but I'm not too familiar with the multi-vfio in a process scenario..

Quoted 3 lines

I still expect that VFIO_GROUP_SET_CONTAINER will be used to connect /dev/{ioasid,vfio} to the VFIO group and all the group and device logic stays inside VFIO.

But that group and device logic is also tied to the container, where the IOMMU backend is the interchangeable thing that provides the IOMMU manipulation for that container.

If you're using VFIO_GROUP_SET_CONTAINER to associate a group to a /dev/ioasid, then you're really either taking that group outside of vfio or you're re-implementing group management in /dev/ioasid.

Quoted 2 lines

Everything can be switched to ioasid_container all down the line. If it wasn't for PPC this looks fairly simple.

At what point is it no longer vfio? I'd venture to say that replacing the container rather than invoking a different IOMMU backend is that point.

Quoted 5 lines

Since getting rid of PPC looks a bit hard, we'd be stuck with accepting a /dev/ioasid and then immediately wrappering it in a vfio_container an shimming it through a vfio_iommu_ops. It is not ideal at all, but in my look around I don't see a major problem if type1 implementation is moved to live under /dev/ioasid.

But type1 is \just\ an IOMMU backend, not "/dev/vfio". Given that nobody flinched at removing NVLink support, maybe just deprecate SPAPR now and see if anyone objects ;)

Quoted 7 lines

We end up with a ioasid.h that basically has the vfio_iommu_type1 code lightly recast into some 'struct iommu_container' and a set of ioasid_* function entry points that follow vfio_iommu_driver_ops_type1:
ioasid_attach_group
ioasid_detatch_group
ioasid_<something about user pages> ioasid_read/ioasid_write

Again, this looks like a vfio IOMMU backend. What are we accomplishing by replacing /dev/vfio with /dev/ioasid versus some manipulation of VFIO_SET_IOMMU accepting a /dev/ioasid fd?

Quoted 4 lines

If we have this, and /dev/ioasid implements the legacy IOCTLs, then /dev/vfio == /dev/ioasid and we can compile out vfio_fops and related from vfio.c and tell ioasid.c to create /dev/vfio instead using the ops it owns.

Why would we want /dev/ioasid to implement legacy ioctls instead of simply implementing an interface to allow /dev/ioasid to be used as a vfio IOMMU backend?

The pseudo code above really suggests you do want to remove /dev/vfio/vfio, but this is only one of the IOMMU backends for vfio, so I can't quite figure out if we're talking past each other.

As I expressed in another thread, type1 has a lot of shortcomings. The mapping interface leaves userspace trying desperately to use statically mapped buffers because the map/unmap latency is too high. We have horrible issues with duplicate locked page accounting across containers. It suffers pretty hard from feature creep in various areas. A new IOMMU backend is an opportunity to redesign some of these things.

The IOMMU group also abstracts isolation and visibility relative to DMA. For example, in a PCIe topology a multi-function device may not have isolation between functions, but each requester ID is visible to the IOMMU.

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, April 22, 2021 7:03 AM

Quoted 3 lines

The pseudo code above really suggests you do want to remove /dev/vfio/vfio, but this is only one of the IOMMU backends for vfio, so I can't quite figure out if we're talking past each other.

I'm not quite sure what you mean by "one of the IOMMU backends?" You mean type1, right?

Quoted 8 lines

As I expressed in another thread, type1 has a lot of shortcomings. The mapping interface leaves userspace trying desperately to use statically mapped buffers because the map/unmap latency is too high. We have horrible issues with duplicate locked page accounting across containers. It suffers pretty hard from feature creep in various areas. A new IOMMU backend is an opportunity to redesign some of these things.

Sure, but also those kinds of transformational things go alot better if you can smoothly go from the old to the new and have technical co-existance in side the kernel. Having a shim that maps the old APIs to new APIs internally to Linux helps keep the implementation from becoming too bogged down with compatibility.

Quoted 5 lines

The IOMMU group also abstracts isolation and visibility relative to DMA. For example, in a PCIe topology a multi-function device may not have isolation between functions, but each requester ID is visible to the IOMMU.

Okay, I'm glad I have this all right in my head, as I was pretty sure this was what the group was about.

My next question is why do we have three things as a FD: group, device and container (aka IOMMU interface)?

Do we have container because the /dev/vfio/vfio can hold only a single page table so we need to swap containers sometimes?

If we start from a clean sheet and make a sketch..

/dev/ioasid is the IOMMU control interface. It can create multiple IOASIDs that have page tables and it can manipulate those page tables. Each IOASID is identified by some number.

struct vfio_device/vdpa_device/etc are consumers of /dev/ioasid

When a device attaches to an ioasid userspace gives VFIO/VDPA the ioasid FD and the ioasid # in the FD.

The security rule for isolation is that once a device is attached to a /dev/ioasid fd then all other devices in that security group must be attached to the same ioasid FD or left unused.

Thus /dev/ioasid also becomes the unit of security and the IOMMU subsystem level becomes aware of and enforces the group security rules. Userspace does not need to "see" the group

In sketch it would be like
ioasid_fd = open("/dev/ioasid");
vfio_device_fd = open("/dev/vfio/device0")
vdpa_device_fd = open("/dev/vdpa/device0")
ioctl(vifo_device_fd, JOIN_IOASID_FD, ioasifd)
ioctl(vdpa_device_fd, JOIN_IOASID_FD, ioasifd)

gpa_ioasid_id = ioctl(ioasid_fd, CREATE_IOASID, ..) ioctl(ioasid_fd, SET_IOASID_PAGE_TABLES, ..)

ioctl(vfio_device, ATTACH_IOASID, gpa_ioasid_id) ioctl(vpda_device, ATTACH_IOASID, gpa_ioasid_id)

.. both VDPA and VFIO see the guest physical map and the kernel has
enough info that both could use the same IOMMU page table structure ..

// Guest viommu turns off bypass mode for the vfio device ioctl(vfio_device, DETATCH_IOASID)

// Guest viommu creates a new page table rid_ioasid_id = ioctl(ioasid_fd, CREATE_IOASID, ..) ioctl(ioasid_fd, SET_IOASID_PAGE_TABLES, ..)

// Guest viommu links the new page table to the RID ioctl(vfio_device, ATTACH_IOASID, rid_ioasid_id)

The group security concept becomes implicit and hidden from the uAPI. JOIN_IOASID_FD implicitly finds the device's group inside the kernel and requires that all members of the group be joined only to this ioasid_fd.

Essentially we discover the group from the device instead of the device from the group.

Where does it fall down compared to the three FD version we have today?

The shim layer could be considered as a new iommu backend in VFIO, which connects VFIO iommu ops to the internal helpers in drivers/ioasid.

Another tricky thing is that a container may be linked to multiple iommu domains in VFIO, as devices in the container may locate behind different IOMMUs with inconsistent capability (commit 1ef3e2bc).

Just to confirm. Above flow is for current map/unmap flavor as what VFIO/vDPA do today. Later when nested translation is supported, there is no need to detach gpa_ioasid_fd. Instead, a new cmd will be introduced to nest rid_ioasid_fd on top of gpa_ioasid_fd:

I also feel hiding group from uAPI is a good thing and is interested in the rationale behind for explicitly managing group in vfio (which is essentially the same boundary as provided by iommu group), e.g. for better user experience when group security is broken?

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, April 22, 2021 8:10 PM

On Thu, Apr 22, 2021 at 08:34:32AM +0000, Tian, Kevin wrote:

Quoted 3 lines

Another tricky thing is that a container may be linked to multiple iommu domains in VFIO, as devices in the container may locate behind different IOMMUs with inconsistent capability (commit 1ef3e2bc).

Frankly this sounds over complicated. I would think /dev/ioasid should select the IOMMU when the first device is joined, and all future joins must be compatible with the original IOMMU - ie there is only one set of IOMMU capabilities in a /dev/ioasid.

This means qemue might have multiple /dev/ioasid's if the system has multiple incompatible IOMMUs (is this actually a thing?) The platform

should design its IOMMU domains to minimize the number of /dev/ioasid's required.

Is there a reason we need to share IOASID'd between completely divergance IOMMU implementations? I don't expect the HW should be able to physically share page tables??

That decision point alone might be the thing that just says we can't ever have /dev/vfio/vfio == /dev/ioasid

Quoted 5 lines

Just to confirm. Above flow is for current map/unmap flavor as what VFIO/vDPA do today. Later when nested translation is supported, there is no need to detach gpa_ioasid_fd. Instead, a new cmd will be introduced to nest rid_ioasid_fd on top of gpa_ioasid_fd:

Sure.. The tricky bit will be to define both of the common nested operating modes.

  nested_ioasid = ioctl(ioasid_fd, CREATE_NESTED_IOASID,  gpa_ioasid_id);
  ioctl(ioasid_fd, SET_NESTED_IOASID_PAGE_TABLES, nested_ioasid, ..)

   // IOMMU will match on the device RID, no PASID:

ioctl(vfio_device, ATTACH_IOASID, nested_ioasid);

   // IOMMU will match on the device RID and PASID:

ioctl(vfio_device, ATTACH_IOASID_PASID, pasid, nested_ioasid);

Notice that ATTACH (or bind, whatever) is always done on the vfio_device FD. ATTACH tells the IOMMU HW to link the PCI BDF&PASID to a specific page table defined by an IOASID.

I expect we have many flavours of IOASID tables, eg we have normal, and 'nested with table controlled by hypervisor'. ARM has 'nested with table controlled by guest' right? So like this?

nested_ioasid = ioctl(ioasid_fd, CREATE_DELGATED_IOASID,
gpa_ioasid_id, <some kind of viommu_id>)
// PASID now goes to <viommu_id>
ioctl(vfio_device, ATTACH_IOASID_PASID, pasid, nested_ioasid);

Where <viommu_id> is some internal to the guest handle of the viommu page table scoped within gpa_ioasid_id? Like maybe it is GPA of the base of the page table?

The guest can't select its own PASIDs without telling the hypervisor, right?

Or could we still have just one /dev/ioasid but allow userspace to create multiple gpa_ioasid_id's each associated to a different iommu domain? Then the compatibility check will be done at ATTACH_IOASID instead of JOIN_IOASID_FD.

This does impose one burden to userspace though, to understand the IOMMU compatibilities and figure out which incompatible features may affect the page table management (while such knowledge is IOMMU vendor specific) and then explicitly manage multiple /dev/ioasid's or multiple gpa_ioasid_id's.

Alternatively is it a good design by having the kernel return error at attach/join time to indicate that incompatibility is detected then the userspace should open a new /dev/ioasid or creates a new gpa_ioasid_id for the failing device upon such failure, w/o constructing its own compatibility knowledge?

Quoted 2 lines

This means qemue might have multiple /dev/ioasid's if the system has multiple incompatible IOMMUs (is this actually a thing?) The platform

One example is Intel platform with igd. Typically there is one IOMMU dedicated for igd and the other IOMMU serving all the remaining devices. The igd IOMMU may not support IOMMU_CACHE while the other one does.

yes, e.g. in vSVA both devices (behind divergence IOMMUs) are bound to a single guest process which has an unique PASID and 1st-level page table. Earlier incompatibility example is only for 2nd-level.

Quoted 9 lines

Sure.. The tricky bit will be to define both of the common nested operating modes.

  nested_ioasid = ioctl(ioasid_fd, CREATE_NESTED_IOASID,  gpa_ioasid_id);
  ioctl(ioasid_fd, SET_NESTED_IOASID_PAGE_TABLES, nested_ioasid, ..)

   // IOMMU will match on the device RID, no PASID:

ioctl(vfio_device, ATTACH_IOASID, nested_ioasid);

Quoted 2 lines

   // IOMMU will match on the device RID and PASID:

ioctl(vfio_device, ATTACH_IOASID_PASID, pasid, nested_ioasid);

I'm a bit confused here why we have both pasid and ioasid notations together. Why not use nested_ioasid as pasid directly (i.e. every pasid in nested mode is created by CREATE_NESTED_IOASID)?

Below I list different scenarios for ATTACH_IOASID in my view. Here vfio_device could be a real PCI function (RID), or a subfunction device (RID+def_ioasid).

If the whole PASID table is delegated to the guest in ARM case, the guest can select its own PASIDs w/o telling the hypervisor.

From: Jason Gunthorpe <[email protected]>
Sent: Friday, April 23, 2021 7:50 PM

Quoted 4 lines · Kevin

Or could we still have just one /dev/ioasid but allow userspace to create multiple gpa_ioasid_id's each associated to a different iommu domain? Then the compatibility check will be done at ATTACH_IOASID instead of JOIN_IOASID_FD.

To my mind what makes sense that that /dev/ioasid presents a single IOMMU behavior that is basically the same. This may ultimately not be what we call a domain today.

We may end up with a middle object which is a group of domains that all have the same capabilities, and we define capabilities in a way that most platforms have a single group of domains.

The key capability of a group of domains is they can all share the HW page table representation, so if an IOASID instantiates a page table it can be assigned to any device on any domain in the gruop of domains.

If you try to say that /dev/ioasid has many domains and they can't have their HW page tables shared then I think the implementation complexity will explode.

Quoted 6 lines

This does impose one burden to userspace though, to understand the IOMMU compatibilities and figure out which incompatible features may affect the page table management (while such knowledge is IOMMU vendor specific) and then explicitly manage multiple /dev/ioasid's or multiple gpa_ioasid_id's.

Right, this seems very hard in the general case..

Quoted 5 lines

Alternatively is it a good design by having the kernel return error at attach/join time to indicate that incompatibility is detected then the userspace should open a new /dev/ioasid or creates a new gpa_ioasid_id for the failing device upon such failure, w/o constructing its own compatibility knowledge?

Yes, this feels workable too

Quoted 7 lines

Quoted 2 lines

This means qemue might have multiple /dev/ioasid's if the system has multiple incompatible IOMMUs (is this actually a thing?) The platform

One example is Intel platform with igd. Typically there is one IOMMU dedicated for igd and the other IOMMU serving all the remaining devices. The igd IOMMU may not support IOMMU_CACHE while the other one does.

If we can do as above the two domains may be in the same group of domains and the IOMMU_CACHE is not exposed at the /dev/ioasid level.

For instance the API could specifiy IOMMU_CACHE during attach, not during IOASID creation.

Getting all the data model right in the API is going to be trickiest part of this.

Quoted 3 lines

yes, e.g. in vSVA both devices (behind divergence IOMMUs) are bound to a single guest process which has an unique PASID and 1st-level page table. Earlier incompatibility example is only for 2nd-level.

Because when we get to here, things become inscrutable as an API if you are trying to say two different IOMMU presentations can actually be nested.

Quoted 4 lines

Quoted 4 lines

Sure.. The tricky bit will be to define both of the common nested operating modes.

nested_ioasid = ioctl(ioasid_fd, CREATE_NESTED_IOASID,

gpa_ioasid_id);

Quoted 9 lines

Quoted 7 lines

ioctl(ioasid_fd, SET_NESTED_IOASID_PAGE_TABLES, nested_ioasid, ..)

   // IOMMU will match on the device RID, no PASID:

ioctl(vfio_device, ATTACH_IOASID, nested_ioasid);

   // IOMMU will match on the device RID and PASID:

ioctl(vfio_device, ATTACH_IOASID_PASID, pasid, nested_ioasid);

I'm a bit confused here why we have both pasid and ioasid notations

together.

Quoted 1 line

Why not use nested_ioasid as pasid directly (i.e. every pasid in nested

mode

Quoted 1 line

is created by CREATE_NESTED_IOASID)?

The IOASID is not a PASID, it is just a page table.

A generic IOMMU matches on either RID or (RID,PASID), so you should specify the PASID when establishing the match.

IOASID only specifies the page table.

So you read the above as configuring the path

PCI_DEVICE -> (RID,PASID) -> nested_ioasid -> gpa_ioasid_id -> physical

Where (RID,PASID) indicate values taken from the PCI packet.

In principle the IOMMU could also be commanded to reuse the same ioasid page table with a different PASID:

PCI_DEVICE_B -> (RID_B,PASID_B) -> nested_ioasid -> gpa_ioasid_id ->
physical

This is impossible if the ioasid == PASID in the API.

Quoted 4 lines

Below I list different scenarios for ATTACH_IOASID in my view. Here vfio_device could be a real PCI function (RID), or a subfunction device (RID+def_ioasid).

What is RID+def_ioasid? The IOMMU does not match on IOASID's.

A subfunction device always need to use PASID, or an internal IOMMU, confused what you are trying to explain?

Quoted 3 lines

If the whole PASID table is delegated to the guest in ARM case, the guest can select its own PASIDs w/o telling the hypervisor.

The hypervisor has to route the PASID's to the guest at some point - a guest can't just claim a PASID unilaterally, that would not be secure.

If it is not done with per-PASID hypercalls then the hypervisor has to route all PASID's for a RID to the guest and /dev/ioasid needs to have a nested IOASID object that represents this connection - ie it points to the PASID table of the guest vIOMMU or something.

Remember this all has to be compatible with mdev's too and without hypercalls to create PASIDs that will be hard: mdev sharing a RID and slicing the physical PASIDs can't support a 'send all PASIDs to the guest' model, or even a 'the guest gets to pick the PASID' option.

Quoted 18 lines

From: Jason Gunthorpe <[email protected]>
Sent: Friday, April 23, 2021 7:50 PM

Quoted 4 lines · Kevin

Or could we still have just one /dev/ioasid but allow userspace to create multiple gpa_ioasid_id's each associated to a different iommu domain? Then the compatibility check will be done at ATTACH_IOASID instead of JOIN_IOASID_FD.

To my mind what makes sense that that /dev/ioasid presents a single IOMMU behavior that is basically the same. This may ultimately not be what we call a domain today.

We may end up with a middle object which is a group of domains that all have the same capabilities, and we define capabilities in a way that most platforms have a single group of domains.

Quoted 3 lines

The key capability of a group of domains is they can all share the HW page table representation, so if an IOASID instantiates a page table it can be assigned to any device on any domain in the gruop of domains.

Sorry that I didn't quite get it. If a group of domains can share the same page table then why not just attaching all devices under those domains into a single domain?

to describe the HW page table. Ideally a new iommu domain should be created only when it's impossible to share an existing page table. Otherwise you'll get bad iotlb efficiency because each domain has its unique domain id (tagged in iotlb) then duplicated iotlb entries may exist even when a single page table is shared by those domains.

Or, can you elaborate what is the targeted usage by having a group of domains which all share the same page table?

Want to hear your opinion for one open here. There is no doubt that an ioasid represents a HW page table when the table is constructed by userspace and then linked to the IOMMU through the bind/unbind API. But I'm not very sure about whether an ioasid should represent the exact pgtable or the mapping metadata when the underlying pgtable is indirectly constructed through map/unmap API. VFIO does the latter way, which is why it allows multiple incompatible domains in a single container which all share the same mapping metadata.

OK, now I see where the disconnection comes from. In my context ioasid is the identifier that is actually used in the wire, but seems you treat it as a sw-defined namespace purely for representing page tables. We should clear this concept first before further discussing other details. 😊

Then following your proposal, does it mean that we need another interface for allocating PASID? and since ioasid means different thing in uAPI and in-kernel API, possibly a new name is required to avoid confusion?

From: Jason Gunthorpe <[email protected]>
Sent: Monday, April 26, 2021 8:38 PM

Quoted 8 lines

Want to hear your opinion for one open here. There is no doubt that an ioasid represents a HW page table when the table is constructed by userspace and then linked to the IOMMU through the bind/unbind API. But I'm not very sure about whether an ioasid should represent the exact pgtable or the mapping metadata when the underlying pgtable is indirectly constructed through map/unmap API. VFIO does the latter way, which is why it allows multiple incompatible domains in a single container which all share the same mapping metadata.

I think VFIO's map/unmap is way too complex and we know it has bad performance problems.

If /dev/ioasid is single HW page table only then I would focus on that implementation and leave it to userspace to span different /dev/ioasids if needed.

Quoted 4 lines

OK, now I see where the disconnection comes from. In my context ioasid is the identifier that is actually used in the wire, but seems you treat it as a sw-defined namespace purely for representing page tables. We should clear this concept first before further discussing other details. 😊

There is no general HW requirement that every IO page table be referred to by the same PASID and this API would have to support

non-PASID IO page tables as well. So I'd keep the two things separated in the uAPI - even though the kernel today has a global PASID pool.

Quoted 5 lines

Then following your proposal, does it mean that we need another interface for allocating PASID? and since ioasid means different thing in uAPI and in-kernel API, possibly a new name is required to avoid confusion?

I would suggest have two ways to control the PASID

1) Over /dev/ioasid allocate a PASID for an IOASID. All future PASID based usages of the IOASID will use that global PASID

2) Over the device FD, when the IOASID is bound return the PASID that was selected. If the IOASID does not have a global PASID then the kernel is free to make something up. In this mode a single IOASID can have multiple PASIDs.

Simple things like DPDK can use #2 and potentially have better PASID limits. hypervisors will most likely have to use #1, but it depends on how their vIOMMU interface works.

I think the name IOASID is fine for the uAPI, the kernel version can be called ioasid_id or something.

(also looking at ioasid.c, why do we need such a thin and odd wrapper around xarray?)

Quoted 3 lines

From: Jason Gunthorpe <[email protected]>
Sent: Monday, April 26, 2021 8:38 PM

[...]

Quoted 9 lines

Quoted 8 lines

Want to hear your opinion for one open here. There is no doubt that an ioasid represents a HW page table when the table is constructed by userspace and then linked to the IOMMU through the bind/unbind API. But I'm not very sure about whether an ioasid should represent the exact pgtable or the mapping metadata when the underlying pgtable is indirectly constructed through map/unmap API. VFIO does the latter way, which is why it allows multiple incompatible domains in a single container which all share the same mapping metadata.

Quoted 2 lines

I think VFIO's map/unmap is way too complex and we know it has bad performance problems.

Can you or Alex elaborate where the complexity and performance problem locate in VFIO map/umap? We'd like to understand more detail and see how to avoid it in the new interface.

From: Alex Williamson <[email protected]>
Sent: Wednesday, April 28, 2021 11:06 PM

Quoted 13 lines · Kevin

Quoted 3 lines

From: Jason Gunthorpe <[email protected]>
Sent: Monday, April 26, 2021 8:38 PM

[...]

Quoted 9 lines

Quoted 8 lines

Want to hear your opinion for one open here. There is no doubt that an ioasid represents a HW page table when the table is constructed by userspace and then linked to the IOMMU through the bind/unbind API. But I'm not very sure about whether an ioasid should represent the exact pgtable or the mapping metadata when the underlying pgtable is indirectly constructed through map/unmap API. VFIO does the latter way, which is why it allows multiple incompatible domains in a single container which all share the same mapping metadata.

Quoted 5 lines

Quoted 2 lines

I think VFIO's map/unmap is way too complex and we know it has bad performance problems.

Can you or Alex elaborate where the complexity and performance problem locate in VFIO map/umap? We'd like to understand more detail and see

how

Quoted 1 line

to avoid it in the new interface.

The map/unmap interface is really only good for long lived mappings, the overhead is too high for things like vIOMMU use cases or any case where the mapping is intended to be dynamic. Userspace drivers must make use of a long lived buffer mapping in order to achieve performance.

The mapping and unmapping granularity has been a problem as well, type1v1 allowed arbitrary unmaps to bisect the original mapping, with the massive caveat that the caller relies on the return value of the unmap to determine what was actually unmapped because the IOMMU use of
superpages is transparent to the caller. This led to type1v2 that simply restricts the user to avoid ever bisecting mappings. That still leaves us with problems for things like virtio-mem support where we need to create initial mappings with a granularity that allows us to later remove entries, which can prevent effective use of IOMMU superpages.

Locked page accounting has been another constant issue. We perform locked page accounting at the container level, where each container accounts independently. A user may require multiple containers, the containers may pin the same physical memory, but be accounted against the user once per container.

Those are the main ones I can think of. It is nice to have a simple map/unmap interface, I'd hope that a new /dev/ioasid interface wouldn't raise the barrier to entry too high, but the user needs to have the ability to have more control of their mappings and locked page accounting should probably be offloaded somewhere. Thanks,

for /dev/ioasid there is still an open whether an process is allowed to open /dev/ioasid once or multiple times. If there is only one ioasid_fd per process, the accounting can be made accurately. otherwise the same problem still exists as each ioasid_fd is akin to the container, then we need find a better solution.

Quoted 8 lines

From: Alex Williamson <[email protected]>
Sent: Wednesday, April 28, 2021 11:06 PM

Quoted 3 lines · Kevin

Can you or Alex elaborate where the complexity and performance problem locate in VFIO map/umap? We'd like to understand more detail and see

Quoted 8 lines

how

Quoted 1 line

to avoid it in the new interface.

The map/unmap interface is really only good for long lived mappings, the overhead is too high for things like vIOMMU use cases or any case where the mapping is intended to be dynamic. Userspace drivers must make use of a long lived buffer mapping in order to achieve performance.

This is not a limitation of VFIO map/unmap. It's the limitation of any map/unmap semantics since the fact of long-lived vs. short-lived is imposed by userspace. Nested translation is the only viable optimization allowing 2nd-level to be a long-lived mapping even w/ vIOMMU. From this angle I'm not sure how a new map/unmap implementation could address this perf limitation alone.

Quoted 11 lines

The mapping and unmapping granularity has been a problem as well, type1v1 allowed arbitrary unmaps to bisect the original mapping, with the massive caveat that the caller relies on the return value of the unmap to determine what was actually unmapped because the IOMMU use of
superpages is transparent to the caller. This led to type1v2 that simply restricts the user to avoid ever bisecting mappings. That still leaves us with problems for things like virtio-mem support where we need to create initial mappings with a granularity that allows us to later remove entries, which can prevent effective use of IOMMU superpages.

We could start with a semantics similar to type1v2.

btw why does virtio-mem require a smaller granularity? Can we split superpages in-the-fly when removal actually happens (just similar to page split in VM live migration for efficient dirty page tracking)?

and isn't it another problem imposed by userspace? How could a new map/unmap implementation mitigate this problem if the userspace insists on a smaller granularity for initial mappings?

Quoted 5 lines

Locked page accounting has been another constant issue. We perform locked page accounting at the container level, where each container accounts independently. A user may require multiple containers, the containers may pin the same physical memory, but be accounted against the user once per container.

for /dev/ioasid there is still an open whether an process is allowed to open /dev/ioasid once or multiple times. If there is only one ioasid_fd per process, the accounting can be made accurately. otherwise the same problem still exists as each ioasid_fd is akin to the container, then we need find a better solution.

Quoted 6 lines

Those are the main ones I can think of. It is nice to have a simple map/unmap interface, I'd hope that a new /dev/ioasid interface wouldn't raise the barrier to entry too high, but the user needs to have the ability to have more control of their mappings and locked page accounting should probably be offloaded somewhere. Thanks,

Based on your feedbacks I feel it's probably reasonable to start with a type1v2 semantics for the new interface. Locked accounting could also start with the same VFIO restriction and then improve it incrementally, if a cleaner way is intrusive (if not affecting uAPI). But I didn't get the suggestion on "more control of their mappings". Can you elaborate?

We had tossed around an idea of a super-container with vfio, it's maybe something we'd want to incorporate into this design. For instance, if memory could be pre-registered with a super container, which would handle the locked memory accounting for that memory, then sub-containers could all handle the IOMMU context of their sets of devices relative to that common memory pool.

From: Jason Gunthorpe
Sent: Saturday, May 8, 2021 1:11 AM

Quoted 6 lines · Alex Williamson

We had tossed around an idea of a super-container with vfio, it's maybe something we'd want to incorporate into this design. For instance, if memory could be pre-registered with a super container, which would handle the locked memory accounting for that memory, then sub-containers could all handle the IOMMU context of their sets of devices relative to that common memory pool.

This is where I suggested to David to use nesting of IOASIDs.

Without HW support for nesting a SW nest is really just re-using the memory registration information stored in the parent when constructing the children

From: Alex Williamson <[email protected]>
Sent: Saturday, May 8, 2021 1:06 AM

Quoted 7 lines

Quoted 6 lines

Those are the main ones I can think of. It is nice to have a simple map/unmap interface, I'd hope that a new /dev/ioasid interface wouldn't raise the barrier to entry too high, but the user needs to have the ability to have more control of their mappings and locked page accounting should probably be offloaded somewhere. Thanks,

Quoted 6 lines

Based on your feedbacks I feel it's probably reasonable to start with a type1v2 semantics for the new interface. Locked accounting could also start with the same VFIO restriction and then improve it incrementally, if a cleaner way is intrusive (if not affecting uAPI). But I didn't get the suggestion on "more control of their mappings". Can you elaborate?

Things like I note above, userspace cannot currently specify mapping granularity nor has any visibility to the granularity they get from the IOMMU. What actually happens in the IOMMU is pretty opaque to the user currently. Thanks,

Quoted 15 lines

From: Alex Williamson<[email protected]>
Sent: Saturday, May 8, 2021 1:06 AM

Quoted 6 lines

Quoted 6 lines

Those are the main ones I can think of. It is nice to have a simple map/unmap interface, I'd hope that a new /dev/ioasid interface wouldn't raise the barrier to entry too high, but the user needs to have the ability to have more control of their mappings and locked page accounting should probably be offloaded somewhere. Thanks,

Quoted 6 lines

Based on your feedbacks I feel it's probably reasonable to start with a type1v2 semantics for the new interface. Locked accounting could also start with the same VFIO restriction and then improve it incrementally, if a cleaner way is intrusive (if not affecting uAPI). But I didn't get the suggestion on "more control of their mappings". Can you elaborate?

Quoted 5 lines

Things like I note above, userspace cannot currently specify mapping granularity nor has any visibility to the granularity they get from the IOMMU. What actually happens in the IOMMU is pretty opaque to the user currently. Thanks,

It's much clearer. Based on all the discussions so far I'm thinking about a staging approach when building the new interface, basically following the model that Jason pointed out - generic stuff first, then platform specific extension:

Phase 1: /dev/ioasid with core ingredients and vfio type1v2 semantics
- ioasid is the software handle representing an I/O page table

Quoted 9 lines

If /dev/ioasid is single HW page table only then I would focus on that implementation and leave it to userspace to span different /dev/ioasids if needed.

Quoted 4 lines

OK, now I see where the disconnection comes from. In my context ioasid is the identifier that is actually used in the wire, but seems you treat it as a sw-defined namespace purely for representing page tables. We should clear this concept first before further discussing other details. 😊

Quoted 2 lines

There is no general HW requirement that every IO page table be referred to by the same PASID and this API would have to support

Yes, but what is the value of allowing multiple PASIDs referring to the the same I/O page table (except the nesting pgtable case)? Doesn't it lead to poor iotlb efficiency issue similar to multiple iommu domains referring to the same page table?

Quoted 3 lines

non-PASID IO page tables as well. So I'd keep the two things separated in the uAPI - even though the kernel today has a global PASID pool.

for non-PASID usages the allocated PASID will be wasted if we don't separate ioasid from pasid. But it may be worthwhile given 1m available pasids and the simplification in the uAPI which only needs to care about one id space then.

Quoted 3 lines

Simple things like DPDK can use #2 and potentially have better PASID limits. hypervisors will most likely have to use #1, but it depends on how their vIOMMU interface works.

Can you elaborate why DPDK wants to use #2 i.e. not using a global PASID?

Quoted 2 lines

I think the name IOASID is fine for the uAPI, the kernel version can be called ioasid_id or something.

ioasid is already an id and then ioasid_id just adds confusion. Another point is that ioasid is currently used to represent both PCI PASID and ARM substream ID in the kernel. It implies that if we want to separate ioasid and pasid in the uAPI the 'pasid' also needs to be replaced with another general term usable for substream ID. Are we making the terms too confusing here?

Quoted 9 lines

Quoted 2 lines

I think the name IOASID is fine for the uAPI, the kernel version can be called ioasid_id or something.

ioasid is already an id and then ioasid_id just adds confusion. Another point is that ioasid is currently used to represent both PCI PASID and ARM substream ID in the kernel. It implies that if we want to separate ioasid and pasid in the uAPI the 'pasid' also needs to be replaced with another general term usable for substream ID. Are we making the terms too confusing here?

This is why I also am not so sure about exposing the PASID in the API because it is ultimately a HW specific item.

As I said to David, one avenue is to have some generic uAPI that is very general and keep all this deeply detailed stuff, that really only matters for qemu, as part of a more HW specific vIOMMU driver interface.

Hi Jason,

Quoted 13 lines · Jason Gunthorpe

Quoted 9 lines

Quoted 2 lines

I think the name IOASID is fine for the uAPI, the kernel version can be called ioasid_id or something.

ioasid is already an id and then ioasid_id just adds confusion. Another point is that ioasid is currently used to represent both PCI PASID and ARM substream ID in the kernel. It implies that if we want to separate ioasid and pasid in the uAPI the 'pasid' also needs to be replaced with another general term usable for substream ID. Are we making the terms too confusing here?

This is why I also am not so sure about exposing the PASID in the API because it is ultimately a HW specific item.

Quoted 4 lines

As I said to David, one avenue is to have some generic uAPI that is very general and keep all this deeply detailed stuff, that really only matters for qemu, as part of a more HW specific vIOMMU driver interface.

I think it is not just for QEMU. I am assuming you meant PASID is needed for guest driver to program assigned but not mediated devices.

User space drivers may also need to get the real HW PASID to program it on to the HW. So this uAPI need to provide some lookup functionality. Perhaps the kernel generic version can be called ioasid_hw_id?

So we have the following per my understanding:
- IOASID: a userspace logical number which identifies a page table, this can be a first level (GVA-GPA), or a second level (GPA->HPA) page table.
- PASID: strictly defined in PCIe term
- Substream ID: strictly defined in ARM SMMUv3 spec.
- IOASID_HW_ID: a generic ID backed by PASID, Substream ID, or any other HW IDs used to tag DMA

Is that right?

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, April 29, 2021 4:46 AM

Quoted 9 lines

Quoted 2 lines

I think the name IOASID is fine for the uAPI, the kernel version can be called ioasid_id or something.

ioasid is already an id and then ioasid_id just adds confusion. Another point is that ioasid is currently used to represent both PCI PASID and ARM substream ID in the kernel. It implies that if we want to separate ioasid and pasid in the uAPI the 'pasid' also needs to be replaced with another general term usable for substream ID. Are we making the terms too confusing here?

This is why I also am not so sure about exposing the PASID in the API because it is ultimately a HW specific item.

As I said to David, one avenue is to have some generic uAPI that is very general and keep all this deeply detailed stuff, that really only matters for qemu, as part of a more HW specific vIOMMU driver interface.

Quoted 4 lines

(also looking at ioasid.c, why do we need such a thin and odd wrapper around xarray?)

I'll leave it to Jean and Jacob.

Quoted 6 lines

Quoted 4 lines

(also looking at ioasid.c, why do we need such a thin and odd wrapper around xarray?)

I'll leave it to Jean and Jacob.

Could you elaborate?

Quoted 8 lines · Jacob Pan

Quoted 6 lines

Quoted 4 lines

(also looking at ioasid.c, why do we need such a thin and odd wrapper around xarray?)

I'll leave it to Jean and Jacob.

Could you elaborate?

I mean stuff like this:

int ioasid_set_data(ioasid_t ioasid, void *data)

{
        struct ioasid_data *ioasid_data;
        int ret = 0;

        spin_lock(&ioasid_allocator_lock);
        ioasid_data = xa_load(&active_allocator->xa, ioasid);
        if (ioasid_data)
                rcu_assign_pointer(ioasid_data->private, data);
        else
                ret = -ENOENT;
        spin_unlock(&ioasid_allocator_lock);

        /*
         * Wait for readers to stop accessing the old private data, so the
         * caller can free it.
         */
        if (!ret)
                synchronize_rcu();

        return ret;
}

EXPORT_SYMBOL_GPL(ioasid_set_data);

It is a weird way to use xarray to have a structure which itself is just a wrapper around another RCU protected structure.

Make the caller supply the ioasid_data memory, embedded in its own element, get rid of the void * and rely on XA_ZERO_ENTRY to hold allocated but not active entries.

Make the synchronize_rcu() the caller responsiblity, and callers should really be able to use call_rcu()

Jason

Quoted 7 lines

It is a weird way to use xarray to have a structure which itself is just a wrapper around another RCU protected structure.

Make the caller supply the ioasid_data memory, embedded in its own element, get rid of the void * and rely on XA_ZERO_ENTRY to hold allocated but not active entries.

Let me try to paraphrase to make sure I understand. Currently struct ioasid_data is private to the iasid core, its memory is allocated by the ioasid core.

You are suggesting the following:
1. make struct ioasid_data public
2. caller allocates memory for ioasid_data, initialize it then pass it to ioasid_alloc to store in the xarray
3. caller will be responsible for setting private data inside ioasid_data and do call_rcu after update if needed.

Quoted 11 lines · Jacob Pan

Quoted 7 lines

It is a weird way to use xarray to have a structure which itself is just a wrapper around another RCU protected structure.

Make the caller supply the ioasid_data memory, embedded in its own element, get rid of the void * and rely on XA_ZERO_ENTRY to hold allocated but not active entries.

Let me try to paraphrase to make sure I understand. Currently struct ioasid_data is private to the iasid core, its memory is allocated by the ioasid core.

Quoted 6 lines

You are suggesting the following:
1. make struct ioasid_data public
2. caller allocates memory for ioasid_data, initialize it then pass it to ioasid_alloc to store in the xarray
3. caller will be responsible for setting private data inside ioasid_data and do call_rcu after update if needed.