Re: [PATCH v5 3/5] iommu/uapi: Use named union for user data
Hi Jacob,
On 7/16/20 8:45 PM, Jacob Pan wrote:
> IOMMU UAPI data size is filled by the user space which must be validated
> by ther kernel. To ensure backward compatibility, user data can only be
s/ther/the
> extended by either re-purpose padding bytes or extend the variable sized
> union at the end. No size change is allowed before the union. Therefore,
> the minimum size is the offset of the union.
>
> To use offsetof() on the union, we must make it named.
>
> Link: https://lore.kernel.org/linux-iommu/20200611145518.0c281...@x1.home/
> Signed-off-by: Jacob Pan
> Reviewed-by: Lu Baolu
Reviewed-by: Eric Auger
Thanks
Eric
> ---
> drivers/iommu/intel/iommu.c | 24
> drivers/iommu/intel/svm.c | 2 +-
> include/uapi/linux/iommu.h | 4 ++--
> 3 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 6ad8b6f20235..f3a6ca88cf95 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -5409,8 +5409,8 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
> struct device *dev,
>
> /* Size is only valid in address selective invalidation */
> if (inv_info->granularity == IOMMU_INV_GRANU_ADDR)
> - size = to_vtd_size(inv_info->addr_info.granule_size,
> -inv_info->addr_info.nb_granules);
> + size = to_vtd_size(inv_info->granu.addr_info.granule_size,
> +inv_info->granu.addr_info.nb_granules);
>
> for_each_set_bit(cache_type,
>(unsigned long *)&inv_info->cache,
> @@ -5432,20 +5432,20 @@ intel_iommu_sva_invalidate(struct iommu_domain
> *domain, struct device *dev,
>* granularity.
>*/
> if (inv_info->granularity == IOMMU_INV_GRANU_PASID &&
> - (inv_info->pasid_info.flags & IOMMU_INV_PASID_FLAGS_PASID))
> - pasid = inv_info->pasid_info.pasid;
> + (inv_info->granu.pasid_info.flags &
> IOMMU_INV_PASID_FLAGS_PASID))
> + pasid = inv_info->granu.pasid_info.pasid;
> else if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
> - (inv_info->addr_info.flags &
> IOMMU_INV_ADDR_FLAGS_PASID))
> - pasid = inv_info->addr_info.pasid;
> + (inv_info->granu.addr_info.flags &
> IOMMU_INV_ADDR_FLAGS_PASID))
> + pasid = inv_info->granu.addr_info.pasid;
>
> switch (BIT(cache_type)) {
> case IOMMU_CACHE_INV_TYPE_IOTLB:
> /* HW will ignore LSB bits based on address mask */
> if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
> size &&
> - (inv_info->addr_info.addr & ((BIT(VTD_PAGE_SHIFT +
> size)) - 1))) {
> + (inv_info->granu.addr_info.addr &
> ((BIT(VTD_PAGE_SHIFT + size)) - 1))) {
> WARN_ONCE(1, "Address out of range, 0x%llx,
> size order %llu\n",
> - inv_info->addr_info.addr, size);
> + inv_info->granu.addr_info.addr, size);
> }
>
> /*
> @@ -5453,9 +5453,9 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
> struct device *dev,
>* We use npages = -1 to indicate that.
>*/
> qi_flush_piotlb(iommu, did, pasid,
> - mm_to_dma_pfn(inv_info->addr_info.addr),
> +
> mm_to_dma_pfn(inv_info->granu.addr_info.addr),
> (granu == QI_GRAN_NONG_PASID) ? -1 : 1
> << size,
> - inv_info->addr_info.flags &
> IOMMU_INV_ADDR_FLAGS_LEAF);
> + inv_info->granu.addr_info.flags &
> IOMMU_INV_ADDR_FLAGS_LEAF);
>
> if (!info->ats_enabled)
> break;
> @@ -5476,13 +5476,13 @@ intel_iommu_sva_invalidate(struct iommu_domain
> *domain, struct device *dev,
> size = 64 - VTD_PAGE_SHIFT;
> addr = 0;
> } else if (inv_info->granularity ==
> IOMMU_INV_GRANU_ADDR)
> - addr = inv_info->addr_info.addr;
> + addr = inv_info->granu.addr_info.addr;
>
> if (info->ats_enabled)
> qi_flush_dev_iotlb_pasid(iommu, sid,
> info->pfsid, pasid,
> info->ats_qdep,
> - inv_info->addr_info.addr,
> + inv_info->granu.addr_info.addr,
>
[PATCH v5 3/5] iommu/uapi: Use named union for user data
IOMMU UAPI data size is filled by the user space which must be validated
by ther kernel. To ensure backward compatibility, user data can only be
extended by either re-purpose padding bytes or extend the variable sized
union at the end. No size change is allowed before the union. Therefore,
the minimum size is the offset of the union.
To use offsetof() on the union, we must make it named.
Link: https://lore.kernel.org/linux-iommu/20200611145518.0c281...@x1.home/
Signed-off-by: Jacob Pan
Reviewed-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 24
drivers/iommu/intel/svm.c | 2 +-
include/uapi/linux/iommu.h | 4 ++--
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 6ad8b6f20235..f3a6ca88cf95 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5409,8 +5409,8 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
/* Size is only valid in address selective invalidation */
if (inv_info->granularity == IOMMU_INV_GRANU_ADDR)
- size = to_vtd_size(inv_info->addr_info.granule_size,
- inv_info->addr_info.nb_granules);
+ size = to_vtd_size(inv_info->granu.addr_info.granule_size,
+ inv_info->granu.addr_info.nb_granules);
for_each_set_bit(cache_type,
(unsigned long *)&inv_info->cache,
@@ -5432,20 +5432,20 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
* granularity.
*/
if (inv_info->granularity == IOMMU_INV_GRANU_PASID &&
- (inv_info->pasid_info.flags & IOMMU_INV_PASID_FLAGS_PASID))
- pasid = inv_info->pasid_info.pasid;
+ (inv_info->granu.pasid_info.flags &
IOMMU_INV_PASID_FLAGS_PASID))
+ pasid = inv_info->granu.pasid_info.pasid;
else if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
-(inv_info->addr_info.flags &
IOMMU_INV_ADDR_FLAGS_PASID))
- pasid = inv_info->addr_info.pasid;
+(inv_info->granu.addr_info.flags &
IOMMU_INV_ADDR_FLAGS_PASID))
+ pasid = inv_info->granu.addr_info.pasid;
switch (BIT(cache_type)) {
case IOMMU_CACHE_INV_TYPE_IOTLB:
/* HW will ignore LSB bits based on address mask */
if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
size &&
- (inv_info->addr_info.addr & ((BIT(VTD_PAGE_SHIFT +
size)) - 1))) {
+ (inv_info->granu.addr_info.addr &
((BIT(VTD_PAGE_SHIFT + size)) - 1))) {
WARN_ONCE(1, "Address out of range, 0x%llx,
size order %llu\n",
- inv_info->addr_info.addr, size);
+ inv_info->granu.addr_info.addr, size);
}
/*
@@ -5453,9 +5453,9 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
* We use npages = -1 to indicate that.
*/
qi_flush_piotlb(iommu, did, pasid,
- mm_to_dma_pfn(inv_info->addr_info.addr),
+
mm_to_dma_pfn(inv_info->granu.addr_info.addr),
(granu == QI_GRAN_NONG_PASID) ? -1 : 1
<< size,
- inv_info->addr_info.flags &
IOMMU_INV_ADDR_FLAGS_LEAF);
+ inv_info->granu.addr_info.flags &
IOMMU_INV_ADDR_FLAGS_LEAF);
if (!info->ats_enabled)
break;
@@ -5476,13 +5476,13 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
size = 64 - VTD_PAGE_SHIFT;
addr = 0;
} else if (inv_info->granularity ==
IOMMU_INV_GRANU_ADDR)
- addr = inv_info->addr_info.addr;
+ addr = inv_info->granu.addr_info.addr;
if (info->ats_enabled)
qi_flush_dev_iotlb_pasid(iommu, sid,
info->pfsid, pasid,
info->ats_qdep,
- inv_info->addr_info.addr,
+ inv_info->granu.addr_info.addr,
size);
else
pr_warn_ratelimited("Passdown device IOTLB
flush w/o ATS!\n");
diff --git a/driv
