IOMMUs that support nesting translation needs report the capability info
to userspace. It gives information about requirements the userspace needs
to implement plus other features characterizing the physical implementation.
This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can get
nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, it is after
selecting VFIO_TYPE1_NESTING_IOMMU.
Cc: Kevin Tian <[email protected]>
CC: Jacob Pan <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Lu Baolu <[email protected]>
Signed-off-by: Liu Yi L <[email protected]>
Signed-off-by: Jacob Pan <[email protected]>
---
v5 -> v6:
*) rephrase the feature notes per comments from Eric Auger.
*) rename @size of struct iommu_nesting_info to @argsz.
v4 -> v5:
*) address comments from Eric Auger.
v3 -> v4:
*) split the SMMU driver changes to be a separate patch
*) move the @addr_width and @pasid_bits from vendor specific
part to generic part.
*) tweak the description for the @features field of struct
iommu_nesting_info.
*) add description on the @data[] field of struct iommu_nesting_info
v2 -> v3:
*) remvoe cap/ecap_mask in iommu_nesting_info.
*) reuse DOMAIN_ATTR_NESTING to get nesting info.
*) return an empty iommu_nesting_info for SMMU drivers per Jean'
suggestion.
---
include/uapi/linux/iommu.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 7c8e075..5e4745a 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data {
} vendor;
};
+/*
+ * struct iommu_nesting_info - Information for nesting-capable IOMMU.
+ * userspace should check it before using
+ * nesting capability.
+ *
+ * @argsz: size of the whole structure.
+ * @flags: currently reserved for future extension. must set to 0.
+ * @format: PASID table entry format, the same definition as struct
+ * iommu_gpasid_bind_data @format.
+ * @features: supported nesting features.
+ * @addr_width: The output addr width of first level/stage translation
+ * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID
+ * support.
+ * @data: vendor specific cap info. data[] structure type can be deduced
+ * from @format field.
+ *
+ * +===============+======================================================+
+ * | feature | Notes |
+ * +===============+======================================================+
+ * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace |
+ * | | to allocate PASID from kernel. All PASID allocation |
+ * | | free must be mediated through the TBD API. |
+ * +---------------+------------------------------------------------------+
+ * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace |
+ * | | bind the first level/stage page table to associated |
+ * | | PASID (either the one specified in bind request or |
+ * | | the default PASID of iommu domain), through IOMMU |
+ * | | UAPI. |
+ * +---------------+------------------------------------------------------+
+ * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace |
+ * | | explicitly invalidate the IOMMU cache through IOMMU |
+ * | | UAPI according to vendor-specific requirement when |
+ * | | changing the 1st level/stage page table. |
+ * +---------------+------------------------------------------------------+
+ *
+ * @data[] types defined for @format:
+ * +================================+=====================================+
+ * | @format | @data[] |
+ * +================================+=====================================+
+ * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd |
+ * +--------------------------------+-------------------------------------+
+ *
+ */
+struct iommu_nesting_info {
+ __u32 argsz;
+ __u32 flags;
+ __u32 format;
+#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
+#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
+#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
+ __u32 features;
+ __u16 addr_width;
+ __u16 pasid_bits;
+ __u32 padding;
+ __u8 data[];
+};
+
+/*
+ * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info.
+ *
+ * @flags: VT-d specific flags. Currently reserved for future
+ * extension. must be set to 0.
+ * @cap_reg: Describe basic capabilities as defined in VT-d capability
+ * register.
+ * @ecap_reg: Describe the extended capabilities as defined in VT-d
+ * extended capability register.
+ */
+struct iommu_nesting_info_vtd {
+ __u32 flags;
+ __u32 padding;
+ __u64 cap_reg;
+ __u64 ecap_reg;
+};
+
#endif /* _UAPI_IOMMU_H */
--
2.7.4
Yi,
On 7/28/20 8:27 AM, Liu Yi L wrote:
> IOMMUs that support nesting translation needs report the capability info
s/needs/need to
> to userspace. It gives information about requirements the userspace needs
> to implement plus other features characterizing the physical implementation.
>
> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can get
> nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, it is after
> selecting VFIO_TYPE1_NESTING_IOMMU.
This is not what this patch does ;-) It introduces a new IOMMU UAPI
struct that gives information about the nesting capabilities and
features. This struct is supposed to be returned by
iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute parameter,
one a domain whose type has been set to DOMAIN_ATTR_NESTING.
>
> Cc: Kevin Tian <[email protected]>
> CC: Jacob Pan <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Lu Baolu <[email protected]>
> Signed-off-by: Liu Yi L <[email protected]>
> Signed-off-by: Jacob Pan <[email protected]>
> ---
> v5 -> v6:
> *) rephrase the feature notes per comments from Eric Auger.
> *) rename @size of struct iommu_nesting_info to @argsz.
>
> v4 -> v5:
> *) address comments from Eric Auger.
>
> v3 -> v4:
> *) split the SMMU driver changes to be a separate patch
> *) move the @addr_width and @pasid_bits from vendor specific
> part to generic part.
> *) tweak the description for the @features field of struct
> iommu_nesting_info.
> *) add description on the @data[] field of struct iommu_nesting_info
>
> v2 -> v3:
> *) remvoe cap/ecap_mask in iommu_nesting_info.
> *) reuse DOMAIN_ATTR_NESTING to get nesting info.
> *) return an empty iommu_nesting_info for SMMU drivers per Jean'
> suggestion.
> ---
> include/uapi/linux/iommu.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 74 insertions(+)
>
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> index 7c8e075..5e4745a 100644
> --- a/include/uapi/linux/iommu.h
> +++ b/include/uapi/linux/iommu.h
> @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data {
> } vendor;
> };
>
> +/*
> + * struct iommu_nesting_info - Information for nesting-capable IOMMU.
> + * userspace should check it before using
> + * nesting capability.
> + *
> + * @argsz: size of the whole structure.
> + * @flags: currently reserved for future extension. must set to 0.
> + * @format: PASID table entry format, the same definition as struct
> + * iommu_gpasid_bind_data @format.
> + * @features: supported nesting features.
> + * @addr_width: The output addr width of first level/stage translation
> + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID
> + * support.
> + * @data: vendor specific cap info. data[] structure type can be deduced
> + * from @format field.
> + *
> + * +===============+======================================================+
> + * | feature | Notes |
> + * +===============+======================================================+
> + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace |
> + * | | to allocate PASID from kernel. All PASID allocation |
> + * | | free must be mediated through the TBD API. |
s/TBD/IOMMU
> + * +---------------+------------------------------------------------------+
> + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace |
> + * | | bind the first level/stage page table to associated |
s/bind/to bind
> + * | | PASID (either the one specified in bind request or |
> + * | | the default PASID of iommu domain), through IOMMU |
> + * | | UAPI. |
> + * +---------------+------------------------------------------------------+
> + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace |
> + * | | explicitly invalidate the IOMMU cache through IOMMU |
to explicitly
> + * | | UAPI according to vendor-specific requirement when |
> + * | | changing the 1st level/stage page table. |
> + * +---------------+------------------------------------------------------+
> + *
> + * @data[] types defined for @format:
> + * +================================+=====================================+
> + * | @format | @data[] |
> + * +================================+=====================================+
> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd |
> + * +--------------------------------+-------------------------------------+
> + *
> + */
> +struct iommu_nesting_info {
> + __u32 argsz;
> + __u32 flags;
> + __u32 format;
> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
> + __u32 features;
> + __u16 addr_width;
> + __u16 pasid_bits;
> + __u32 padding;
> + __u8 data[];
> +};
As opposed to other IOMMU UAPI structs there is no union member at the
end. Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU
user API. Shouldn't we align.
You may also consider to move this patch in Jacob's series for
consistency, thoughts?
> +
> +/*
> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info.
> + *
> + * @flags: VT-d specific flags. Currently reserved for future
> + * extension. must be set to 0.
> + * @cap_reg: Describe basic capabilities as defined in VT-d capability
> + * register.
> + * @ecap_reg: Describe the extended capabilities as defined in VT-d
> + * extended capability register.
> + */
> +struct iommu_nesting_info_vtd {
> + __u32 flags;
> + __u32 padding;
> + __u64 cap_reg;
> + __u64 ecap_reg;
> +};
> +
> #endif /* _UAPI_IOMMU_H */
>
Thanks
Eric
Hi Eric,
> From: Auger Eric <[email protected]>
> Sent: Thursday, August 13, 2020 8:53 PM
>
> Yi,
> On 7/28/20 8:27 AM, Liu Yi L wrote:
> > IOMMUs that support nesting translation needs report the capability info
> s/needs/need to
> > to userspace. It gives information about requirements the userspace needs
> > to implement plus other features characterizing the physical implementation.
> >
> > This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can get
> > nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, it is after
> > selecting VFIO_TYPE1_NESTING_IOMMU.
> This is not what this patch does ;-) It introduces a new IOMMU UAPI
> struct that gives information about the nesting capabilities and
> features. This struct is supposed to be returned by
> iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute parameter,
> one a domain whose type has been set to DOMAIN_ATTR_NESTING.
got it. let me apply your suggestion. thanks. :-)
> >
> > Cc: Kevin Tian <[email protected]>
> > CC: Jacob Pan <[email protected]>
> > Cc: Alex Williamson <[email protected]>
> > Cc: Eric Auger <[email protected]>
> > Cc: Jean-Philippe Brucker <[email protected]>
> > Cc: Joerg Roedel <[email protected]>
> > Cc: Lu Baolu <[email protected]>
> > Signed-off-by: Liu Yi L <[email protected]>
> > Signed-off-by: Jacob Pan <[email protected]>
> > ---
> > v5 -> v6:
> > *) rephrase the feature notes per comments from Eric Auger.
> > *) rename @size of struct iommu_nesting_info to @argsz.
> >
> > v4 -> v5:
> > *) address comments from Eric Auger.
> >
> > v3 -> v4:
> > *) split the SMMU driver changes to be a separate patch
> > *) move the @addr_width and @pasid_bits from vendor specific
> > part to generic part.
> > *) tweak the description for the @features field of struct
> > iommu_nesting_info.
> > *) add description on the @data[] field of struct iommu_nesting_info
> >
> > v2 -> v3:
> > *) remvoe cap/ecap_mask in iommu_nesting_info.
> > *) reuse DOMAIN_ATTR_NESTING to get nesting info.
> > *) return an empty iommu_nesting_info for SMMU drivers per Jean'
> > suggestion.
> > ---
> > include/uapi/linux/iommu.h | 74
> ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 74 insertions(+)
> >
> > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> > index 7c8e075..5e4745a 100644
> > --- a/include/uapi/linux/iommu.h
> > +++ b/include/uapi/linux/iommu.h
> > @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data {
> > } vendor;
> > };
> >
> > +/*
> > + * struct iommu_nesting_info - Information for nesting-capable IOMMU.
> > + * userspace should check it before using
> > + * nesting capability.
> > + *
> > + * @argsz: size of the whole structure.
> > + * @flags: currently reserved for future extension. must set to 0.
> > + * @format: PASID table entry format, the same definition as struct
> > + * iommu_gpasid_bind_data @format.
> > + * @features: supported nesting features.
> > + * @addr_width: The output addr width of first level/stage translation
> > + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID
> > + * support.
> > + * @data: vendor specific cap info. data[] structure type can be deduced
> > + * from @format field.
> > + *
> > + *
> +===============+===================================================
> ===+
> > + * | feature | Notes |
> > + *
> +===============+===================================================
> ===+
> > + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace |
> > + * | | to allocate PASID from kernel. All PASID allocation |
> > + * | | free must be mediated through the TBD API. |
> s/TBD/IOMMU
got it.
> > + * +---------------+------------------------------------------------------+
> > + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace |
> > + * | | bind the first level/stage page table to associated |
> s/bind/to bind
got it.
> > + * | | PASID (either the one specified in bind request or |
> > + * | | the default PASID of iommu domain), through IOMMU |
> > + * | | UAPI. |
> > + * +---------------+------------------------------------------------------+
> > + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace |
>
> > + * | | explicitly invalidate the IOMMU cache through IOMMU |
> to explicitly
I see.
> > + * | | U
> > API according to vendor-specific requirement when |
> > + * | | changing the 1st level/stage page table. |
> > + * +---------------+------------------------------------------------------+
> > + *
> > + * @data[] types defined for @format:
> > + *
> +================================+==================================
> ===+
> > + * | @format | @data[] |
> > + *
> +================================+==================================
> ===+
> > + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd |
> > + * +--------------------------------+-------------------------------------+
> > + *
> > + */
> > +struct iommu_nesting_info {
> > + __u32 argsz;
> > + __u32 flags;
> > + __u32 format;
> > +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
> > +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
> > +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
> > + __u32 features;
> > + __u16 addr_width;
> > + __u16 pasid_bits;
> > + __u32 padding;
> > + __u8 data[];
> > +};
> As opposed to other IOMMU UAPI structs there is no union member at the
> end.
nice catch. do you think it would be better to adding a union and
put the struct iommu_nesting_info_vtd in it?
> Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU
> user API. Shouldn't we align.
> You may also consider to move this patch in Jacob's series for
> consistency, thoughts?
this was talked one time between Jacob and me. It was put in this
series as the major user of nesting_info is in this series. e.g.
vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's
series if it would make the merge easier.
Thanks,
Yi Liu
> > +
> > +/*
> > + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info.
> > + *
> > + * @flags: VT-d specific flags. Currently reserved for future
> > + * extension. must be set to 0.
> > + * @cap_reg: Describe basic capabilities as defined in VT-d capability
> > + * register.
> > + * @ecap_reg: Describe the extended capabilities as defined in VT-d
> > + * extended capability register.
> > + */
> > +struct iommu_nesting_info_vtd {
> > + __u32 flags;
> > + __u32 padding;
> > + __u64 cap_reg;
> > + __u64 ecap_reg;
> > +};
> > +
> > #endif /* _UAPI_IOMMU_H */
> >
>
> Thanks
>
> Eric
>
Hi Yi,
On 8/14/20 9:15 AM, Liu, Yi L wrote:
> Hi Eric,
>
>> From: Auger Eric <[email protected]>
>> Sent: Thursday, August 13, 2020 8:53 PM
>>
>> Yi,
>> On 7/28/20 8:27 AM, Liu Yi L wrote:
>>> IOMMUs that support nesting translation needs report the capability info
>> s/needs/need to
>>> to userspace. It gives information about requirements the userspace needs
>>> to implement plus other features characterizing the physical implementation.
>>>
>>> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can get
>>> nesting info after setting DOMAIN_ATTR_NESTING. For VFIO, it is after
>>> selecting VFIO_TYPE1_NESTING_IOMMU.
>> This is not what this patch does ;-) It introduces a new IOMMU UAPI
>> struct that gives information about the nesting capabilities and
>> features. This struct is supposed to be returned by
>> iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute parameter,
>> one a domain whose type has been set to DOMAIN_ATTR_NESTING.
>
> got it. let me apply your suggestion. thanks. :-)
>
>>>
>>> Cc: Kevin Tian <[email protected]>
>>> CC: Jacob Pan <[email protected]>
>>> Cc: Alex Williamson <[email protected]>
>>> Cc: Eric Auger <[email protected]>
>>> Cc: Jean-Philippe Brucker <[email protected]>
>>> Cc: Joerg Roedel <[email protected]>
>>> Cc: Lu Baolu <[email protected]>
>>> Signed-off-by: Liu Yi L <[email protected]>
>>> Signed-off-by: Jacob Pan <[email protected]>
>>> ---
>>> v5 -> v6:
>>> *) rephrase the feature notes per comments from Eric Auger.
>>> *) rename @size of struct iommu_nesting_info to @argsz.
>>>
>>> v4 -> v5:
>>> *) address comments from Eric Auger.
>>>
>>> v3 -> v4:
>>> *) split the SMMU driver changes to be a separate patch
>>> *) move the @addr_width and @pasid_bits from vendor specific
>>> part to generic part.
>>> *) tweak the description for the @features field of struct
>>> iommu_nesting_info.
>>> *) add description on the @data[] field of struct iommu_nesting_info
>>>
>>> v2 -> v3:
>>> *) remvoe cap/ecap_mask in iommu_nesting_info.
>>> *) reuse DOMAIN_ATTR_NESTING to get nesting info.
>>> *) return an empty iommu_nesting_info for SMMU drivers per Jean'
>>> suggestion.
>>> ---
>>> include/uapi/linux/iommu.h | 74
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 74 insertions(+)
>>>
>>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
>>> index 7c8e075..5e4745a 100644
>>> --- a/include/uapi/linux/iommu.h
>>> +++ b/include/uapi/linux/iommu.h
>>> @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data {
>>> } vendor;
>>> };
>>>
>>> +/*
>>> + * struct iommu_nesting_info - Information for nesting-capable IOMMU.
>>> + * userspace should check it before using
>>> + * nesting capability.
>>> + *
>>> + * @argsz: size of the whole structure.
>>> + * @flags: currently reserved for future extension. must set to 0.
>>> + * @format: PASID table entry format, the same definition as struct
>>> + * iommu_gpasid_bind_data @format.
>>> + * @features: supported nesting features.
>>> + * @addr_width: The output addr width of first level/stage translation
>>> + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID
>>> + * support.
>>> + * @data: vendor specific cap info. data[] structure type can be deduced
>>> + * from @format field.
>>> + *
>>> + *
>> +===============+===================================================
>> ===+
>>> + * | feature | Notes |
>>> + *
>> +===============+===================================================
>> ===+
>>> + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate userspace |
>>> + * | | to allocate PASID from kernel. All PASID allocation |
>>> + * | | free must be mediated through the TBD API. |
>> s/TBD/IOMMU
>
> got it.
>
>>> + * +---------------+------------------------------------------------------+
>>> + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate userspace |
>>> + * | | bind the first level/stage page table to associated |
>> s/bind/to bind
>
> got it.
>
>>> + * | | PASID (either the one specified in bind request or |
>>> + * | | the default PASID of iommu domain), through IOMMU |
>>> + * | | UAPI. |
>>> + * +---------------+------------------------------------------------------+
>>> + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate userspace |
>>
>>> + * | | explicitly invalidate the IOMMU cache through IOMMU |
>> to explicitly
>
> I see.
>
>>> + * | | U
>>> API according to vendor-specific requirement when |
>>> + * | | changing the 1st level/stage page table. |
>>> + * +---------------+------------------------------------------------------+
>>> + *
>>> + * @data[] types defined for @format:
>>> + *
>> +================================+==================================
>> ===+
>>> + * | @format | @data[] |
>>> + *
>> +================================+==================================
>> ===+
>>> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd |
>>> + * +--------------------------------+-------------------------------------+
>>> + *
>>> + */
>>> +struct iommu_nesting_info {
>>> + __u32 argsz;
>>> + __u32 flags;
>>> + __u32 format;
>>> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
>>> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
>>> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
>>> + __u32 features;
>>> + __u16 addr_width;
>>> + __u16 pasid_bits;
>>> + __u32 padding;
>>> + __u8 data[];
>>> +};
>> As opposed to other IOMMU UAPI structs there is no union member at the
>> end.
>
> nice catch. do you think it would be better to adding a union and
> put the struct iommu_nesting_info_vtd in it?
Yes I think so. At least it would be consistent with the rest of the API
and with the guidelines.
>
>> Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU
>> user API. Shouldn't we align.
>> You may also consider to move this patch in Jacob's series for
>> consistency, thoughts?
>
> this was talked one time between Jacob and me. It was put in this
> series as the major user of nesting_info is in this series. e.g.
> vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's
> series if it would make the merge easier.
Yep I think it would make sense to move in Jacob's series to have a
general understanding of the uapi
Thanks
Eric
>
> Thanks,
> Yi Liu
>
>>> +
>>> +/*
>>> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info.
>>> + *
>>> + * @flags: VT-d specific flags. Currently reserved for future
>>> + * extension. must be set to 0.
>>> + * @cap_reg: Describe basic capabilities as defined in VT-d capability
>>> + * register.
>>> + * @ecap_reg: Describe the extended capabilities as defined in VT-d
>>> + * extended capability register.
>>> + */
>>> +struct iommu_nesting_info_vtd {
>>> + __u32 flags;
>>> + __u32 padding;
>>> + __u64 cap_reg;
>>> + __u64 ecap_reg;
>>> +};
>>> +
>>> #endif /* _UAPI_IOMMU_H */
>>>
>>
>> Thanks
>>
>> Eric
>>
>
On Sun, 16 Aug 2020 14:40:57 +0200
Auger Eric <[email protected]> wrote:
> Hi Yi,
>
> On 8/14/20 9:15 AM, Liu, Yi L wrote:
> > Hi Eric,
> >
> >> From: Auger Eric <[email protected]>
> >> Sent: Thursday, August 13, 2020 8:53 PM
> >>
> >> Yi,
> >> On 7/28/20 8:27 AM, Liu Yi L wrote:
> >>> IOMMUs that support nesting translation needs report the
> >>> capability info
> >> s/needs/need to
> >>> to userspace. It gives information about requirements the
> >>> userspace needs to implement plus other features characterizing
> >>> the physical implementation.
> >>>
> >>> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller
> >>> can get nesting info after setting DOMAIN_ATTR_NESTING. For VFIO,
> >>> it is after selecting VFIO_TYPE1_NESTING_IOMMU.
> >> This is not what this patch does ;-) It introduces a new IOMMU UAPI
> >> struct that gives information about the nesting capabilities and
> >> features. This struct is supposed to be returned by
> >> iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute
> >> parameter, one a domain whose type has been set to
> >> DOMAIN_ATTR_NESTING.
> >
> > got it. let me apply your suggestion. thanks. :-)
> >
> >>>
> >>> Cc: Kevin Tian <[email protected]>
> >>> CC: Jacob Pan <[email protected]>
> >>> Cc: Alex Williamson <[email protected]>
> >>> Cc: Eric Auger <[email protected]>
> >>> Cc: Jean-Philippe Brucker <[email protected]>
> >>> Cc: Joerg Roedel <[email protected]>
> >>> Cc: Lu Baolu <[email protected]>
> >>> Signed-off-by: Liu Yi L <[email protected]>
> >>> Signed-off-by: Jacob Pan <[email protected]>
> >>> ---
> >>> v5 -> v6:
> >>> *) rephrase the feature notes per comments from Eric Auger.
> >>> *) rename @size of struct iommu_nesting_info to @argsz.
> >>>
> >>> v4 -> v5:
> >>> *) address comments from Eric Auger.
> >>>
> >>> v3 -> v4:
> >>> *) split the SMMU driver changes to be a separate patch
> >>> *) move the @addr_width and @pasid_bits from vendor specific
> >>> part to generic part.
> >>> *) tweak the description for the @features field of struct
> >>> iommu_nesting_info.
> >>> *) add description on the @data[] field of struct
> >>> iommu_nesting_info
> >>>
> >>> v2 -> v3:
> >>> *) remvoe cap/ecap_mask in iommu_nesting_info.
> >>> *) reuse DOMAIN_ATTR_NESTING to get nesting info.
> >>> *) return an empty iommu_nesting_info for SMMU drivers per Jean'
> >>> suggestion.
> >>> ---
> >>> include/uapi/linux/iommu.h | 74
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 74 insertions(+)
> >>>
> >>> diff --git a/include/uapi/linux/iommu.h
> >>> b/include/uapi/linux/iommu.h index 7c8e075..5e4745a 100644
> >>> --- a/include/uapi/linux/iommu.h
> >>> +++ b/include/uapi/linux/iommu.h
> >>> @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data {
> >>> } vendor;
> >>> };
> >>>
> >>> +/*
> >>> + * struct iommu_nesting_info - Information for nesting-capable
> >>> IOMMU.
> >>> + * userspace should check it
> >>> before using
> >>> + * nesting capability.
> >>> + *
> >>> + * @argsz: size of the whole structure.
> >>> + * @flags: currently reserved for future extension. must
> >>> set to 0.
> >>> + * @format: PASID table entry format, the same definition
> >>> as struct
> >>> + * iommu_gpasid_bind_data @format.
> >>> + * @features: supported nesting features.
> >>> + * @addr_width: The output addr width of first
> >>> level/stage translation
> >>> + * @pasid_bits: Maximum supported PASID bits, 0
> >>> represents no PASID
> >>> + * support.
> >>> + * @data: vendor specific cap info. data[] structure type
> >>> can be deduced
> >>> + * from @format field.
> >>> + *
> >>> + *
> >> +===============+===================================================
> >> ===+
> >>> + * | feature |
> >>> Notes |
> >>> + *
> >> +===============+===================================================
> >> ===+
> >>> + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate
> >>> userspace |
> >>> + * | | to allocate PASID from kernel. All PASID
> >>> allocation |
> >>> + * | | free must be mediated through the TBD
> >>> API. |
> >> s/TBD/IOMMU
> >
> > got it.
> >
> >>> + *
> >>> +---------------+------------------------------------------------------+
> >>> + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate
> >>> userspace |
> >>> + * | | bind the first level/stage page table to
> >>> associated |
> >> s/bind/to bind
> >
> > got it.
> >
> >>> + * | | PASID (either the one specified in bind
> >>> request or |
> >>> + * | | the default PASID of iommu domain),
> >>> through IOMMU |
> >>> + * | |
> >>> UAPI. |
> >>> + *
> >>> +---------------+------------------------------------------------------+
> >>> + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate
> >>> userspace |
> >>
> >>> + * | | explicitly invalidate the IOMMU cache
> >>> through IOMMU |
> >> to explicitly
> >
> > I see.
> >
> >>> + * | | U
> >>> API according to vendor-specific requirement when |
> >>> + * | | changing the 1st level/stage page
> >>> table. |
> >>> + *
> >>> +---------------+------------------------------------------------------+
> >>> + *
> >>> + * @data[] types defined for @format:
> >>> + *
> >> +================================+==================================
> >> ===+
> >>> + * | @format |
> >>> @data[] |
> >>> + *
> >> +================================+==================================
> >> ===+
> >>> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct
> >>> iommu_nesting_info_vtd |
> >>> + *
> >>> +--------------------------------+-------------------------------------+
> >>> + *
> >>> + */
> >>> +struct iommu_nesting_info {
> >>> + __u32 argsz;
> >>> + __u32 flags;
> >>> + __u32 format;
> >>> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
> >>> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
> >>> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
> >>> + __u32 features;
> >>> + __u16 addr_width;
> >>> + __u16 pasid_bits;
> >>> + __u32 padding;
> >>> + __u8 data[];
> >>> +};
> >> As opposed to other IOMMU UAPI structs there is no union member at
> >> the end.
> >
> > nice catch. do you think it would be better to adding a union and
> > put the struct iommu_nesting_info_vtd in it?
> Yes I think so. At least it would be consistent with the rest of the
> API and with the guidelines.
> >
> >> Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU
> >> user API. Shouldn't we align.
> >> You may also consider to move this patch in Jacob's series for
> >> consistency, thoughts?
> >
> > this was talked one time between Jacob and me. It was put in this
> > series as the major user of nesting_info is in this series. e.g.
> > vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's
> > series if it would make the merge easier.
> Yep I think it would make sense to move in Jacob's series to have a
> general understanding of the uapi
>
I a little reluctant to include this in my UAPI set, the reason is that
there are two dimensions IOMMU UAPI are extended:
1. Define the protocols in interaction with VFIO, sanity checking, and
backward compatibility.
2. Adding more UAPI data structures that are parallel to the existing
ones.
My patchset is to address #1, this patch is for #2. My thinking is that
once we have reached consensus on #1, new UAPI structures such as this
patch can just follow the suit.
If that is OK with you, I would like to keep them separate to avoid
diverging conversations.
Thanks,
Jacob
> Thanks
>
> Eric
> >
> > Thanks,
> > Yi Liu
> >
> >>> +
> >>> +/*
> >>> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting
> >>> info.
> >>> + *
> >>> + * @flags: VT-d specific flags. Currently reserved for
> >>> future
> >>> + * extension. must be set to 0.
> >>> + * @cap_reg: Describe basic capabilities as defined in
> >>> VT-d capability
> >>> + * register.
> >>> + * @ecap_reg: Describe the extended capabilities as
> >>> defined in VT-d
> >>> + * extended capability register.
> >>> + */
> >>> +struct iommu_nesting_info_vtd {
> >>> + __u32 flags;
> >>> + __u32 padding;
> >>> + __u64 cap_reg;
> >>> + __u64 ecap_reg;
> >>> +};
> >>> +
> >>> #endif /* _UAPI_IOMMU_H */
> >>>
> >>
> >> Thanks
> >>
> >> Eric
> >>
> >
>
[Jacob Pan]
Hi Jacob,
On 8/18/20 6:21 AM, Jacob Pan wrote:
> On Sun, 16 Aug 2020 14:40:57 +0200
> Auger Eric <[email protected]> wrote:
>
>> Hi Yi,
>>
>> On 8/14/20 9:15 AM, Liu, Yi L wrote:
>>> Hi Eric,
>>>
>>>> From: Auger Eric <[email protected]>
>>>> Sent: Thursday, August 13, 2020 8:53 PM
>>>>
>>>> Yi,
>>>> On 7/28/20 8:27 AM, Liu Yi L wrote:
>>>>> IOMMUs that support nesting translation needs report the
>>>>> capability info
>>>> s/needs/need to
>>>>> to userspace. It gives information about requirements the
>>>>> userspace needs to implement plus other features characterizing
>>>>> the physical implementation.
>>>>>
>>>>> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller
>>>>> can get nesting info after setting DOMAIN_ATTR_NESTING. For VFIO,
>>>>> it is after selecting VFIO_TYPE1_NESTING_IOMMU.
>>>> This is not what this patch does ;-) It introduces a new IOMMU UAPI
>>>> struct that gives information about the nesting capabilities and
>>>> features. This struct is supposed to be returned by
>>>> iommu_domain_get_attr() with DOMAIN_ATTR_NESTING attribute
>>>> parameter, one a domain whose type has been set to
>>>> DOMAIN_ATTR_NESTING.
>>>
>>> got it. let me apply your suggestion. thanks. :-)
>>>
>>>>>
>>>>> Cc: Kevin Tian <[email protected]>
>>>>> CC: Jacob Pan <[email protected]>
>>>>> Cc: Alex Williamson <[email protected]>
>>>>> Cc: Eric Auger <[email protected]>
>>>>> Cc: Jean-Philippe Brucker <[email protected]>
>>>>> Cc: Joerg Roedel <[email protected]>
>>>>> Cc: Lu Baolu <[email protected]>
>>>>> Signed-off-by: Liu Yi L <[email protected]>
>>>>> Signed-off-by: Jacob Pan <[email protected]>
>>>>> ---
>>>>> v5 -> v6:
>>>>> *) rephrase the feature notes per comments from Eric Auger.
>>>>> *) rename @size of struct iommu_nesting_info to @argsz.
>>>>>
>>>>> v4 -> v5:
>>>>> *) address comments from Eric Auger.
>>>>>
>>>>> v3 -> v4:
>>>>> *) split the SMMU driver changes to be a separate patch
>>>>> *) move the @addr_width and @pasid_bits from vendor specific
>>>>> part to generic part.
>>>>> *) tweak the description for the @features field of struct
>>>>> iommu_nesting_info.
>>>>> *) add description on the @data[] field of struct
>>>>> iommu_nesting_info
>>>>>
>>>>> v2 -> v3:
>>>>> *) remvoe cap/ecap_mask in iommu_nesting_info.
>>>>> *) reuse DOMAIN_ATTR_NESTING to get nesting info.
>>>>> *) return an empty iommu_nesting_info for SMMU drivers per Jean'
>>>>> suggestion.
>>>>> ---
>>>>> include/uapi/linux/iommu.h | 74
>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 74 insertions(+)
>>>>>
>>>>> diff --git a/include/uapi/linux/iommu.h
>>>>> b/include/uapi/linux/iommu.h index 7c8e075..5e4745a 100644
>>>>> --- a/include/uapi/linux/iommu.h
>>>>> +++ b/include/uapi/linux/iommu.h
>>>>> @@ -332,4 +332,78 @@ struct iommu_gpasid_bind_data {
>>>>> } vendor;
>>>>> };
>>>>>
>>>>> +/*
>>>>> + * struct iommu_nesting_info - Information for nesting-capable
>>>>> IOMMU.
>>>>> + * userspace should check it
>>>>> before using
>>>>> + * nesting capability.
>>>>> + *
>>>>> + * @argsz: size of the whole structure.
>>>>> + * @flags: currently reserved for future extension. must
>>>>> set to 0.
>>>>> + * @format: PASID table entry format, the same definition
>>>>> as struct
>>>>> + * iommu_gpasid_bind_data @format.
>>>>> + * @features: supported nesting features.
>>>>> + * @addr_width: The output addr width of first
>>>>> level/stage translation
>>>>> + * @pasid_bits: Maximum supported PASID bits, 0
>>>>> represents no PASID
>>>>> + * support.
>>>>> + * @data: vendor specific cap info. data[] structure type
>>>>> can be deduced
>>>>> + * from @format field.
>>>>> + *
>>>>> + *
>>>> +===============+===================================================
>>>> ===+
>>>>> + * | feature |
>>>>> Notes |
>>>>> + *
>>>> +===============+===================================================
>>>> ===+
>>>>> + * | SYSWIDE_PASID | IOMMU vendor driver sets it to mandate
>>>>> userspace |
>>>>> + * | | to allocate PASID from kernel. All PASID
>>>>> allocation |
>>>>> + * | | free must be mediated through the TBD
>>>>> API. |
>>>> s/TBD/IOMMU
>>>
>>> got it.
>>>
>>>>> + *
>>>>> +---------------+------------------------------------------------------+
>>>>> + * | BIND_PGTBL | IOMMU vendor driver sets it to mandate
>>>>> userspace |
>>>>> + * | | bind the first level/stage page table to
>>>>> associated |
>>>> s/bind/to bind
>>>
>>> got it.
>>>
>>>>> + * | | PASID (either the one specified in bind
>>>>> request or |
>>>>> + * | | the default PASID of iommu domain),
>>>>> through IOMMU |
>>>>> + * | |
>>>>> UAPI. |
>>>>> + *
>>>>> +---------------+------------------------------------------------------+
>>>>> + * | CACHE_INVLD | IOMMU vendor driver sets it to mandate
>>>>> userspace |
>>>>
>>>>> + * | | explicitly invalidate the IOMMU cache
>>>>> through IOMMU |
>>>> to explicitly
>>>
>>> I see.
>>>
>>>>> + * | | U
>>>>> API according to vendor-specific requirement when |
>>>>> + * | | changing the 1st level/stage page
>>>>> table. |
>>>>> + *
>>>>> +---------------+------------------------------------------------------+
>>>>> + *
>>>>> + * @data[] types defined for @format:
>>>>> + *
>>>> +================================+==================================
>>>> ===+
>>>>> + * | @format |
>>>>> @data[] |
>>>>> + *
>>>> +================================+==================================
>>>> ===+
>>>>> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct
>>>>> iommu_nesting_info_vtd |
>>>>> + *
>>>>> +--------------------------------+-------------------------------------+
>>>>> + *
>>>>> + */
>>>>> +struct iommu_nesting_info {
>>>>> + __u32 argsz;
>>>>> + __u32 flags;
>>>>> + __u32 format;
>>>>> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
>>>>> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
>>>>> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
>>>>> + __u32 features;
>>>>> + __u16 addr_width;
>>>>> + __u16 pasid_bits;
>>>>> + __u32 padding;
>>>>> + __u8 data[];
>>>>> +};
>>>> As opposed to other IOMMU UAPI structs there is no union member at
>>>> the end.
>>>
>>> nice catch. do you think it would be better to adding a union and
>>> put the struct iommu_nesting_info_vtd in it?
>> Yes I think so. At least it would be consistent with the rest of the
>> API and with the guidelines.
>>>
>>>> Also this struct is not documented in [PATCH v7 1/7] docs: IOMMU
>>>> user API. Shouldn't we align.
>>>> You may also consider to move this patch in Jacob's series for
>>>> consistency, thoughts?
>>>
>>> this was talked one time between Jacob and me. It was put in this
>>> series as the major user of nesting_info is in this series. e.g.
>>> vfio checks the SYSWIDE_PASID. but I'm open to merge it with Jacob's
>>> series if it would make the merge easier.
>> Yep I think it would make sense to move in Jacob's series to have a
>> general understanding of the uapi
>>
> I a little reluctant to include this in my UAPI set, the reason is that
> there are two dimensions IOMMU UAPI are extended:
> 1. Define the protocols in interaction with VFIO, sanity checking, and
> backward compatibility.
> 2. Adding more UAPI data structures that are parallel to the existing
> ones.
>
> My patchset is to address #1, this patch is for #2. My thinking is that
> once we have reached consensus on #1, new UAPI structures such as this
> patch can just follow the suit.
>
> If that is OK with you, I would like to keep them separate to avoid
> diverging conversations.
OK no problem for me, as long as the new APIs follow the rules &
guidelines introduced in your series.
Thanks
Eric
>
> Thanks,
>
> Jacob
>
>> Thanks
>>
>> Eric
>>>
>>> Thanks,
>>> Yi Liu
>>>
>>>>> +
>>>>> +/*
>>>>> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting
>>>>> info.
>>>>> + *
>>>>> + * @flags: VT-d specific flags. Currently reserved for
>>>>> future
>>>>> + * extension. must be set to 0.
>>>>> + * @cap_reg: Describe basic capabilities as defined in
>>>>> VT-d capability
>>>>> + * register.
>>>>> + * @ecap_reg: Describe the extended capabilities as
>>>>> defined in VT-d
>>>>> + * extended capability register.
>>>>> + */
>>>>> +struct iommu_nesting_info_vtd {
>>>>> + __u32 flags;
>>>>> + __u32 padding;
>>>>> + __u64 cap_reg;
>>>>> + __u64 ecap_reg;
>>>>> +};
>>>>> +
>>>>> #endif /* _UAPI_IOMMU_H */
>>>>>
>>>>
>>>> Thanks
>>>>
>>>> Eric
>>>>
>>>
>>
>
> [Jacob Pan]
>