Shared Virtual Addressing (SVA), a.k.a, Shared Virtual Memory (SVM) on
Intel platforms allows address space sharing between device DMA and
applications. SVA can reduce programming complexity and enhance security.
This VFIO series is intended to expose SVA usage to VMs. i.e. Sharing
guest application address space with passthru devices. This is called
vSVA in this series. The whole vSVA enabling requires QEMU/VFIO/IOMMU
changes. For IOMMU and QEMU changes, they are in separate series (listed
in the "Related series").
The high-level architecture for SVA virtualization is as below, the key
design of vSVA support is to utilize the dual-stage IOMMU translation (
also known as IOMMU nesting translation) capability in host IOMMU.
.-------------. .---------------------------.
| vIOMMU | | Guest process CR3, FL only|
| | '---------------------------'
.----------------/
| PASID Entry |--- PASID cache flush -
'-------------' |
| | V
| | CR3 in GPA
'-------------'
Guest
------| Shadow |--------------------------|--------
v v v
Host
.-------------. .----------------------.
| pIOMMU | | Bind FL for GVA-GPA |
| | '----------------------'
.----------------/ |
| PASID Entry | V (Nested xlate)
'----------------\.------------------------------.
| | |SL for GPA-HPA, default domain|
| | '------------------------------'
'-------------'
Where:
- FL = First level/stage one page tables
- SL = Second level/stage two page tables
Patch Overview:
1. a refactor to vfio_iommu_type1 ioctl (patch 0001)
2. reports IOMMU nesting info to userspace ( patch 0002, 0003 and 0014)
3. vfio support for PASID allocation and free for VMs (patch 0004, 0005, 0006)
4. vfio support for binding guest page table to host (patch 0007, 0008, 0009)
5. vfio support for IOMMU cache invalidation from VMs (patch 0010)
6. vfio support for vSVA usage on IOMMU-backed mdevs (patch 0011)
7. expose PASID capability to VM (patch 0012)
8. add doc for VFIO dual stage control (patch 0013)
The complete vSVA kernel upstream patches are divided into three phases:
1. Common APIs and PCI device direct assignment
2. IOMMU-backed Mediated Device assignment
3. Page Request Services (PRS) support
This patchset is aiming for the phase 1 and phase 2, and based on Jacob's
below series.
*) [PATCH v3 0/5] IOMMU user API enhancement - wip
https://lore.kernel.org/linux-iommu/[email protected]/
*) [PATCH 00/10] IOASID extensions for guest SVA - wip
https://lkml.org/lkml/2020/3/25/874
The latest IOASID code added below new interface for itertate all PASIDs of an
ioasid_set. The implementation is not sent out yet as Jacob needs some cleanup,
it can be found in branch vsva-linux-5.8-rc3-v4 on github (mentioned below):
int ioasid_set_for_each_ioasid(int sid, void (*fn)(ioasid_t id, void *data), void *data);
Complete set for current vSVA can be found in below branch.
https://github.com/luxis1999/linux-vsva.git: vsva-linux-5.8-rc3-v4
The corresponding QEMU patch series is included in below branch:
https://github.com/luxis1999/qemu.git: vsva_5.8_rc3_qemu_rfcv7
Regards,
Yi Liu
Changelog:
- Patch v3 -> Patch v4:
a) Address comments against v3
b) Add rb from Stefan on patch 14/15
Patch v3: https://lore.kernel.org/linux-iommu/[email protected]/
- Patch v2 -> Patch v3:
a) Rebase on top of Jacob's v3 iommu uapi patchset
b) Address comments from Kevin and Stefan Hajnoczi
c) Reuse DOMAIN_ATTR_NESTING to get iommu nesting info
d) Drop [PATCH v2 07/15] iommu/uapi: Add iommu_gpasid_unbind_data
Patch v2: https://lore.kernel.org/linux-iommu/[email protected]/#r
- Patch v1 -> Patch v2:
a) Refactor vfio_iommu_type1_ioctl() per suggestion from Christoph
Hellwig.
b) Re-sequence the patch series for better bisect support.
c) Report IOMMU nesting cap info in detail instead of a format in
v1.
d) Enforce one group per nesting type container for vfio iommu type1
driver.
e) Build the vfio_mm related code from vfio.c to be a separate
vfio_pasid.ko.
f) Add PASID ownership check in IOMMU driver.
g) Adopted to latest IOMMU UAPI design. Removed IOMMU UAPI version
check. Added iommu_gpasid_unbind_data for unbind requests from
userspace.
h) Define a single ioctl:VFIO_IOMMU_NESTING_OP for bind/unbind_gtbl
and cahce_invld.
i) Document dual stage control in vfio.rst.
Patch v1: https://lore.kernel.org/linux-iommu/[email protected]/
- RFC v3 -> Patch v1:
a) Address comments to the PASID request(alloc/free) path
b) Report PASID alloc/free availabitiy to user-space
c) Add a vfio_iommu_type1 parameter to support pasid quota tuning
d) Adjusted to latest ioasid code implementation. e.g. remove the
code for tracking the allocated PASIDs as latest ioasid code
will track it, VFIO could use ioasid_free_set() to free all
PASIDs.
RFC v3: https://lore.kernel.org/linux-iommu/[email protected]/
- RFC v2 -> v3:
a) Refine the whole patchset to fit the roughly parts in this series
b) Adds complete vfio PASID management framework. e.g. pasid alloc,
free, reclaim in VM crash/down and per-VM PASID quota to prevent
PASID abuse.
c) Adds IOMMU uAPI version check and page table format check to ensure
version compatibility and hardware compatibility.
d) Adds vSVA vfio support for IOMMU-backed mdevs.
RFC v2: https://lore.kernel.org/linux-iommu/[email protected]/
- RFC v1 -> v2:
Dropped vfio: VFIO_IOMMU_ATTACH/DETACH_PASID_TABLE.
RFC v1: https://lore.kernel.org/linux-iommu/[email protected]/
---
Eric Auger (1):
vfio: Document dual stage control
Liu Yi L (13):
vfio/type1: Refactor vfio_iommu_type1_ioctl()
iommu: Report domain nesting info
iommu/smmu: Report empty domain nesting info
vfio/type1: Report iommu nesting info to userspace
vfio: Add PASID allocation/free support
iommu/vt-d: Support setting ioasid set to domain
vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free)
iommu/vt-d: Check ownership for PASIDs from user-space
vfio/type1: Support binding guest page tables to PASID
vfio/type1: Allow invalidating first-level/stage IOMMU cache
vfio/type1: Add vSVA support for IOMMU-backed mdevs
vfio/pci: Expose PCIe PASID capability to guest
iommu/vt-d: Support reporting nesting capability info
Yi Sun (1):
iommu: Pass domain to sva_unbind_gpasid()
Documentation/driver-api/vfio.rst | 67 +++
drivers/iommu/arm-smmu-v3.c | 29 +-
drivers/iommu/arm-smmu.c | 29 +-
drivers/iommu/intel/iommu.c | 107 ++++-
drivers/iommu/intel/svm.c | 10 +-
drivers/iommu/iommu.c | 2 +-
drivers/vfio/Kconfig | 6 +
drivers/vfio/Makefile | 1 +
drivers/vfio/pci/vfio_pci_config.c | 2 +-
drivers/vfio/vfio_iommu_type1.c | 819 ++++++++++++++++++++++++++++---------
drivers/vfio/vfio_pasid.c | 192 +++++++++
include/linux/intel-iommu.h | 23 +-
include/linux/iommu.h | 4 +-
include/linux/vfio.h | 54 +++
include/uapi/linux/iommu.h | 78 ++++
include/uapi/linux/vfio.h | 85 ++++
16 files changed, 1309 insertions(+), 199 deletions(-)
create mode 100644 drivers/vfio/vfio_pasid.c
--
2.7.4
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Suggested-by: Jean-Philippe Brucker <[email protected]>
Signed-off-by: Liu Yi L <[email protected]>
Signed-off-by: Jacob Pan <[email protected]>
---
drivers/iommu/arm-smmu-v3.c | 29 +++++++++++++++++++++++++++--
drivers/iommu/arm-smmu.c | 29 +++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f578677..0c45d4d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -3019,6 +3019,32 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
return group;
}
+static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
+ void *data)
+{
+ struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
+ u32 size;
+
+ if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ return -ENODEV;
+
+ size = sizeof(struct iommu_nesting_info);
+
+ /*
+ * if provided buffer size is not equal to the size, should
+ * return 0 and also the expected buffer size to caller.
+ */
+ if (info->size != size) {
+ info->size = size;
+ return 0;
+ }
+
+ /* report an empty iommu_nesting_info for now */
+ memset(info, 0x0, size);
+ info->size = size;
+ return 0;
+}
+
static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
enum iommu_attr attr, void *data)
{
@@ -3028,8 +3054,7 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
case IOMMU_DOMAIN_UNMANAGED:
switch (attr) {
case DOMAIN_ATTR_NESTING:
- *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
- return 0;
+ return arm_smmu_domain_nesting_info(smmu_domain, data);
default:
return -ENODEV;
}
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 243bc4c..908607d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1506,6 +1506,32 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
return group;
}
+static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
+ void *data)
+{
+ struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
+ u32 size;
+
+ if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ return -ENODEV;
+
+ size = sizeof(struct iommu_nesting_info);
+
+ /*
+ * if provided buffer size is not equal to the size, should
+ * return 0 and also the expected buffer size to caller.
+ */
+ if (info->size != size) {
+ info->size = size;
+ return 0;
+ }
+
+ /* report an empty iommu_nesting_info for now */
+ memset(info, 0x0, size);
+ info->size = size;
+ return 0;
+}
+
static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
enum iommu_attr attr, void *data)
{
@@ -1515,8 +1541,7 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
case IOMMU_DOMAIN_UNMANAGED:
switch (attr) {
case DOMAIN_ATTR_NESTING:
- *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
- return 0;
+ return arm_smmu_domain_nesting_info(smmu_domain, data);
default:
return -ENODEV;
}
--
2.7.4
Hi Yi,
Please add a commit message: instead of returning a boolean for
DOMAIN_ATTR_NESTING, arm_smmu_domain_get_attr() returns a
iommu_nesting_info handle.
On 7/4/20 1:26 PM, Liu Yi L wrote:
> Cc: Will Deacon <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Suggested-by: Jean-Philippe Brucker <[email protected]>
> Signed-off-by: Liu Yi L <[email protected]>
> Signed-off-by: Jacob Pan <[email protected]>
> ---
> drivers/iommu/arm-smmu-v3.c | 29 +++++++++++++++++++++++++++--
> drivers/iommu/arm-smmu.c | 29 +++++++++++++++++++++++++++--
> 2 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index f578677..0c45d4d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -3019,6 +3019,32 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
> return group;
> }
>
> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
> + void *data)
> +{
> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
> + u32 size;
> +
> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> + return -ENODEV;
> +
> + size = sizeof(struct iommu_nesting_info);
> +
> + /*
> + * if provided buffer size is not equal to the size, should
> + * return 0 and also the expected buffer size to caller.
> + */
> + if (info->size != size) {
< size?
> + info->size = size;
> + return 0;
> + }
> +
> + /* report an empty iommu_nesting_info for now */
> + memset(info, 0x0, size);
> + info->size = size;
For info, the current SMMU NESTED mode is not enabling any nesting. It
just forces the usage of the 2st stage instead of stage1 for single
stage translation.
Thanks
Eric
> + return 0;
> +}
> +
> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> enum iommu_attr attr, void *data)
> {
> @@ -3028,8 +3054,7 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> case IOMMU_DOMAIN_UNMANAGED:
> switch (attr) {
> case DOMAIN_ATTR_NESTING:
> - *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
> - return 0;
> + return arm_smmu_domain_nesting_info(smmu_domain, data);
> default:
> return -ENODEV;
> }
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 243bc4c..908607d 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1506,6 +1506,32 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
> return group;
> }
>
> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
> + void *data)
> +{
> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
> + u32 size;
> +
> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> + return -ENODEV;
> +
> + size = sizeof(struct iommu_nesting_info);
> +
> + /*
> + * if provided buffer size is not equal to the size, should
> + * return 0 and also the expected buffer size to caller.
> + */
> + if (info->size != size) {
> + info->size = size;
> + return 0;
> + }
> +
> + /* report an empty iommu_nesting_info for now */
> + memset(info, 0x0, size);
> + info->size = size;
> + return 0;
> +}
> +
> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> enum iommu_attr attr, void *data)
> {
> @@ -1515,8 +1541,7 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> case IOMMU_DOMAIN_UNMANAGED:
> switch (attr) {
> case DOMAIN_ATTR_NESTING:
> - *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
> - return 0;
> + return arm_smmu_domain_nesting_info(smmu_domain, data);
> default:
> return -ENODEV;
> }
>
Hi Eric,
> From: Auger Eric <[email protected]>
>
> Hi Yi,
>
> Please add a commit message: instead of returning a boolean for
> DOMAIN_ATTR_NESTING, arm_smmu_domain_get_attr() returns a
> iommu_nesting_info handle.
will do. thanks for the suggestion.
>
> On 7/4/20 1:26 PM, Liu Yi L wrote:
> > Cc: Will Deacon <[email protected]>
> > Cc: Robin Murphy <[email protected]>
> > Cc: Eric Auger <[email protected]>
> > Cc: Jean-Philippe Brucker <[email protected]>
> > Suggested-by: Jean-Philippe Brucker <[email protected]>
> > Signed-off-by: Liu Yi L <[email protected]>
> > Signed-off-by: Jacob Pan <[email protected]>
> > ---
> > drivers/iommu/arm-smmu-v3.c | 29 +++++++++++++++++++++++++++--
> > drivers/iommu/arm-smmu.c | 29 +++++++++++++++++++++++++++--
> > 2 files changed, 54 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> > index f578677..0c45d4d 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -3019,6 +3019,32 @@ static struct iommu_group
> *arm_smmu_device_group(struct device *dev)
> > return group;
> > }
> >
> > +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
> *smmu_domain,
> > + void *data)
> > +{
> > + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
> > + u32 size;
> > +
> > + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> > + return -ENODEV;
> > +
> > + size = sizeof(struct iommu_nesting_info);
> > +
> > + /*
> > + * if provided buffer size is not equal to the size, should
> > + * return 0 and also the expected buffer size to caller.
> > + */
> > + if (info->size != size) {
> < size?
< size may work as well. but I'd like the caller provide exact buffer size. not sure
if it is demand in kernel. do you have any suggestion?
> > + info->size = size;
> > + return 0;
> > + }
> > +
> > + /* report an empty iommu_nesting_info for now */
> > + memset(info, 0x0, size);
> > + info->size = size;
> For info, the current SMMU NESTED mode is not enabling any nesting. It just forces
> the usage of the 2st stage instead of stage1 for single stage translation.
yep. The intention is as below:
" However it requires changing the get_attr(NESTING) implementations in both
SMMU drivers as a precursor of this series, to avoid breaking
VFIO_TYPE1_NESTING_IOMMU on Arm. Since we haven't yet defined the
nesting_info structs for SMMUv2 and v3, I suppose we could return an empty
struct iommu_nesting_info for now?"
https://lore.kernel.org/linux-iommu/20200617143909.GA886590@myrica/
do you think any other needs to be done for now?
Regards,
Yi Liu
> Thanks
>
> Eric
> > + return 0;
> > +}
> > +
> > static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> > enum iommu_attr attr, void *data) { @@ -
> 3028,8 +3054,7 @@
> > static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> > case IOMMU_DOMAIN_UNMANAGED:
> > switch (attr) {
> > case DOMAIN_ATTR_NESTING:
> > - *(int *)data = (smmu_domain->stage ==
> ARM_SMMU_DOMAIN_NESTED);
> > - return 0;
> > + return arm_smmu_domain_nesting_info(smmu_domain,
> data);
> > default:
> > return -ENODEV;
> > }
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index
> > 243bc4c..908607d 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -1506,6 +1506,32 @@ static struct iommu_group
> *arm_smmu_device_group(struct device *dev)
> > return group;
> > }
> >
> > +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
> *smmu_domain,
> > + void *data)
> > +{
> > + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
> > + u32 size;
> > +
> > + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> > + return -ENODEV;
> > +
> > + size = sizeof(struct iommu_nesting_info);
> > +
> > + /*
> > + * if provided buffer size is not equal to the size, should
> > + * return 0 and also the expected buffer size to caller.
> > + */
> > + if (info->size != size) {
> > + info->size = size;
> > + return 0;
> > + }
> > +
> > + /* report an empty iommu_nesting_info for now */
> > + memset(info, 0x0, size);
> > + info->size = size;
> > + return 0;
> > +}
> > +
> > static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> > enum iommu_attr attr, void *data) { @@ -
> 1515,8 +1541,7 @@
> > static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> > case IOMMU_DOMAIN_UNMANAGED:
> > switch (attr) {
> > case DOMAIN_ATTR_NESTING:
> > - *(int *)data = (smmu_domain->stage ==
> ARM_SMMU_DOMAIN_NESTED);
> > - return 0;
> > + return arm_smmu_domain_nesting_info(smmu_domain,
> data);
> > default:
> > return -ENODEV;
> > }
> >
Hi Yi,
On 7/6/20 2:46 PM, Liu, Yi L wrote:
> Hi Eric,
>
>> From: Auger Eric <[email protected]>
>>
>> Hi Yi,
>>
>> Please add a commit message: instead of returning a boolean for
>> DOMAIN_ATTR_NESTING, arm_smmu_domain_get_attr() returns a
>> iommu_nesting_info handle.
>
> will do. thanks for the suggestion.
>
>>
>> On 7/4/20 1:26 PM, Liu Yi L wrote:
>>> Cc: Will Deacon <[email protected]>
>>> Cc: Robin Murphy <[email protected]>
>>> Cc: Eric Auger <[email protected]>
>>> Cc: Jean-Philippe Brucker <[email protected]>
>>> Suggested-by: Jean-Philippe Brucker <[email protected]>
>>> Signed-off-by: Liu Yi L <[email protected]>
>>> Signed-off-by: Jacob Pan <[email protected]>
>>> ---
>>> drivers/iommu/arm-smmu-v3.c | 29 +++++++++++++++++++++++++++--
>>> drivers/iommu/arm-smmu.c | 29 +++++++++++++++++++++++++++--
>>> 2 files changed, 54 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>>> index f578677..0c45d4d 100644
>>> --- a/drivers/iommu/arm-smmu-v3.c
>>> +++ b/drivers/iommu/arm-smmu-v3.c
>>> @@ -3019,6 +3019,32 @@ static struct iommu_group
>> *arm_smmu_device_group(struct device *dev)
>>> return group;
>>> }
>>>
>>> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
>> *smmu_domain,
>>> + void *data)
>>> +{
>>> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
>>> + u32 size;
>>> +
>>> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
>>> + return -ENODEV;
>>> +
>>> + size = sizeof(struct iommu_nesting_info);
>>> +
>>> + /*
>>> + * if provided buffer size is not equal to the size, should
>>> + * return 0 and also the expected buffer size to caller.
>>> + */
>>> + if (info->size != size) {
>> < size?
>
> < size may work as well. but I'd like the caller provide exact buffer size. not sure
> if it is demand in kernel. do you have any suggestion?
I just suggested that by analogy with the VFIO argsz
>
>>> + info->size = size;
>>> + return 0;
>>> + }
>>> +
>>> + /* report an empty iommu_nesting_info for now */
>>> + memset(info, 0x0, size);
>>> + info->size = size;
>> For info, the current SMMU NESTED mode is not enabling any nesting. It just forces
>> the usage of the 2st stage instead of stage1 for single stage translation.
>
> yep. The intention is as below:
>
> " However it requires changing the get_attr(NESTING) implementations in both
> SMMU drivers as a precursor of this series, to avoid breaking
> VFIO_TYPE1_NESTING_IOMMU on Arm. Since we haven't yet defined the
> nesting_info structs for SMMUv2 and v3, I suppose we could return an empty
> struct iommu_nesting_info for now?"
> https://lore.kernel.org/linux-iommu/20200617143909.GA886590@myrica/
>
> do you think any other needs to be done for now?
I understand this is a prerequisite. It was more as an information.
Returning a void struct is a bit weird but at the moment I don't have
anything better.
Thanks
Eric
>
> Regards,
> Yi Liu
>
>> Thanks
>>
>> Eric
>>> + return 0;
>>> +}
>>> +
>>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>>> enum iommu_attr attr, void *data) { @@ -
>> 3028,8 +3054,7 @@
>>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>>> case IOMMU_DOMAIN_UNMANAGED:
>>> switch (attr) {
>>> case DOMAIN_ATTR_NESTING:
>>> - *(int *)data = (smmu_domain->stage ==
>> ARM_SMMU_DOMAIN_NESTED);
>>> - return 0;
>>> + return arm_smmu_domain_nesting_info(smmu_domain,
>> data);
>>> default:
>>> return -ENODEV;
>>> }
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index
>>> 243bc4c..908607d 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -1506,6 +1506,32 @@ static struct iommu_group
>> *arm_smmu_device_group(struct device *dev)
>>> return group;
>>> }
>>>
>>> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
>> *smmu_domain,
>>> + void *data)
>>> +{
>>> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
>>> + u32 size;
>>> +
>>> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
>>> + return -ENODEV;
>>> +
>>> + size = sizeof(struct iommu_nesting_info);
>>> +
>>> + /*
>>> + * if provided buffer size is not equal to the size, should
>>> + * return 0 and also the expected buffer size to caller.
>>> + */
>>> + if (info->size != size) {
>>> + info->size = size;
>>> + return 0;
>>> + }
>>> +
>>> + /* report an empty iommu_nesting_info for now */
>>> + memset(info, 0x0, size);
>>> + info->size = size;
>>> + return 0;
>>> +}
>>> +
>>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>>> enum iommu_attr attr, void *data) { @@ -
>> 1515,8 +1541,7 @@
>>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>>> case IOMMU_DOMAIN_UNMANAGED:
>>> switch (attr) {
>>> case DOMAIN_ATTR_NESTING:
>>> - *(int *)data = (smmu_domain->stage ==
>> ARM_SMMU_DOMAIN_NESTED);
>>> - return 0;
>>> + return arm_smmu_domain_nesting_info(smmu_domain,
>> data);
>>> default:
>>> return -ENODEV;
>>> }
>>>
>
Hi Eric,
> From: Auger Eric <[email protected]>
> Sent: Monday, July 6, 2020 9:22 PM
>
> Hi Yi,
>
> On 7/6/20 2:46 PM, Liu, Yi L wrote:
> > Hi Eric,
> >
> >> From: Auger Eric <[email protected]>
> >>
> >> Hi Yi,
> >>
> >> Please add a commit message: instead of returning a boolean for
> >> DOMAIN_ATTR_NESTING, arm_smmu_domain_get_attr() returns a
> >> iommu_nesting_info handle.
> >
> > will do. thanks for the suggestion.
> >
> >>
> >> On 7/4/20 1:26 PM, Liu Yi L wrote:
> >>> Cc: Will Deacon <[email protected]>
> >>> Cc: Robin Murphy <[email protected]>
> >>> Cc: Eric Auger <[email protected]>
> >>> Cc: Jean-Philippe Brucker <[email protected]>
> >>> Suggested-by: Jean-Philippe Brucker <[email protected]>
> >>> Signed-off-by: Liu Yi L <[email protected]>
> >>> Signed-off-by: Jacob Pan <[email protected]>
> >>> ---
> >>> drivers/iommu/arm-smmu-v3.c | 29 +++++++++++++++++++++++++++--
> >>> drivers/iommu/arm-smmu.c | 29 +++++++++++++++++++++++++++--
> >>> 2 files changed, 54 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/arm-smmu-v3.c
> >>> b/drivers/iommu/arm-smmu-v3.c index f578677..0c45d4d 100644
> >>> --- a/drivers/iommu/arm-smmu-v3.c
> >>> +++ b/drivers/iommu/arm-smmu-v3.c
> >>> @@ -3019,6 +3019,32 @@ static struct iommu_group
> >> *arm_smmu_device_group(struct device *dev)
> >>> return group;
> >>> }
> >>>
> >>> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
> >> *smmu_domain,
> >>> + void *data)
> >>> +{
> >>> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
> >>> + u32 size;
> >>> +
> >>> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> >>> + return -ENODEV;
> >>> +
> >>> + size = sizeof(struct iommu_nesting_info);
> >>> +
> >>> + /*
> >>> + * if provided buffer size is not equal to the size, should
> >>> + * return 0 and also the expected buffer size to caller.
> >>> + */
> >>> + if (info->size != size) {
> >> < size?
> >
> > < size may work as well. but I'd like the caller provide exact buffer
> > size. not sure if it is demand in kernel. do you have any suggestion?
>
> I just suggested that by analogy with the VFIO argsz
I see. will change it.
>
> >
> >>> + info->size = size;
> >>> + return 0;
> >>> + }
> >>> +
> >>> + /* report an empty iommu_nesting_info for now */
> >>> + memset(info, 0x0, size);
> >>> + info->size = size;
> >> For info, the current SMMU NESTED mode is not enabling any nesting.
> >> It just forces the usage of the 2st stage instead of stage1 for single stage
> translation.
> >
> > yep. The intention is as below:
> >
> > " However it requires changing the get_attr(NESTING) implementations
> > in both SMMU drivers as a precursor of this series, to avoid breaking
> > VFIO_TYPE1_NESTING_IOMMU on Arm. Since we haven't yet defined the
> > nesting_info structs for SMMUv2 and v3, I suppose we could return an
> > empty struct iommu_nesting_info for now?"
> > https://lore.kernel.org/linux-iommu/20200617143909.GA886590@myrica/
> >
> > do you think any other needs to be done for now?
>
> I understand this is a prerequisite. It was more as an information.
> Returning a void struct is a bit weird but at the moment I don't have anything better.
got you. do you think it is necessary to add your statement as a comment here?
Regards,
Yi Liu
> Thanks
>
> Eric
> >
> > Regards,
> > Yi Liu
> >
> >> Thanks
> >>
> >> Eric
> >>> + return 0;
> >>> +}
> >>> +
> >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> >>> enum iommu_attr attr, void *data) { @@ -
> >> 3028,8 +3054,7 @@
> >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> >>> case IOMMU_DOMAIN_UNMANAGED:
> >>> switch (attr) {
> >>> case DOMAIN_ATTR_NESTING:
> >>> - *(int *)data = (smmu_domain->stage ==
> >> ARM_SMMU_DOMAIN_NESTED);
> >>> - return 0;
> >>> + return arm_smmu_domain_nesting_info(smmu_domain,
> >> data);
> >>> default:
> >>> return -ENODEV;
> >>> }
> >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >>> index 243bc4c..908607d 100644
> >>> --- a/drivers/iommu/arm-smmu.c
> >>> +++ b/drivers/iommu/arm-smmu.c
> >>> @@ -1506,6 +1506,32 @@ static struct iommu_group
> >> *arm_smmu_device_group(struct device *dev)
> >>> return group;
> >>> }
> >>>
> >>> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
> >> *smmu_domain,
> >>> + void *data)
> >>> +{
> >>> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
> >>> + u32 size;
> >>> +
> >>> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> >>> + return -ENODEV;
> >>> +
> >>> + size = sizeof(struct iommu_nesting_info);
> >>> +
> >>> + /*
> >>> + * if provided buffer size is not equal to the size, should
> >>> + * return 0 and also the expected buffer size to caller.
> >>> + */
> >>> + if (info->size != size) {
> >>> + info->size = size;
> >>> + return 0;
> >>> + }
> >>> +
> >>> + /* report an empty iommu_nesting_info for now */
> >>> + memset(info, 0x0, size);
> >>> + info->size = size;
> >>> + return 0;
> >>> +}
> >>> +
> >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> >>> enum iommu_attr attr, void *data) { @@ -
> >> 1515,8 +1541,7 @@
> >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> >>> case IOMMU_DOMAIN_UNMANAGED:
> >>> switch (attr) {
> >>> case DOMAIN_ATTR_NESTING:
> >>> - *(int *)data = (smmu_domain->stage ==
> >> ARM_SMMU_DOMAIN_NESTED);
> >>> - return 0;
> >>> + return arm_smmu_domain_nesting_info(smmu_domain,
> >> data);
> >>> default:
> >>> return -ENODEV;
> >>> }
> >>>
> >