diff mbox series

[v6,1/6] iommu: Add cache_invalidate_user op

Message ID 20231117130717.19875-2-yi.l.liu@intel.com
State New
Headers
Series iommufd: Add nesting infrastructure (part 2/2) |

Commit Message

Yi Liu Nov. 17, 2023, 1:07 p.m. UTC 
  From: Lu Baolu <baolu.lu@linux.intel.com>

The updates of the PTEs in the nested page table will be propagated to the
hardware caches on both IOMMU (IOTLB) and devices (DevTLB/ATC).

Add a new domain op cache_invalidate_user for the userspace to flush the
hardware caches for a nested domain through iommufd. No wrapper for it,
as it's only supposed to be used by iommufd. Then, pass in invalidation
requests in form of a user data array conatining a number of invalidation
data entries.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 include/linux/iommu.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

Tian, Kevin Nov. 20, 2023, 7:53 a.m. UTC | #1 
> From: Yi Liu <yi.l.liu@intel.com>
> Sent: Friday, November 17, 2023 9:07 PM
> 
> +/**
> + * struct iommu_user_data_array - iommu driver specific user space data
> array
> + * @type: The data type of all the entries in the user buffer array
> + * @uptr: Pointer to the user buffer array for copy_from_user()

remove "for copy_from_user()"

> + * @entry_len: The fixed-width length of a entry in the array, in bytes

s/a/an/

> + * @entry_num: The number of total entries in the array
> + *
> + * A array having a @entry_num number of @entry_len sized entries, each
> entry is
> + * user space data, an uAPI defined in include/uapi/linux/iommufd.h where
> @type
> + * is also defined as enum iommu_xyz_data_type.

* The user buffer array has @entry_num entries, each with a fixed size
 * @entry_len. The entry format and related type are defined in
 * include/uapi/linux/iommufd.h
Jason Gunthorpe Dec. 6, 2023, 6:32 p.m. UTC | #2 
On Fri, Nov 17, 2023 at 05:07:12AM -0800, Yi Liu wrote:
> +/**
> + * struct iommu_user_data_array - iommu driver specific user space data array
> + * @type: The data type of all the entries in the user buffer array
> + * @uptr: Pointer to the user buffer array for copy_from_user()
> + * @entry_len: The fixed-width length of a entry in the array, in bytes
> + * @entry_num: The number of total entries in the array
> + *
> + * A array having a @entry_num number of @entry_len sized entries, each entry is
> + * user space data, an uAPI defined in include/uapi/linux/iommufd.h where @type
> + * is also defined as enum iommu_xyz_data_type.
> + */
> +struct iommu_user_data_array {
> +	unsigned int type;
> +	void __user *uptr;
> +	size_t entry_len;
> +	int entry_num;

These are u32 in the uapi, they should probably be u32 here
too. Otherwise we have to worry about truncation.

> @@ -465,6 +492,9 @@ struct iommu_domain_ops {
>  			      size_t size);
>  	void (*iotlb_sync)(struct iommu_domain *domain,
>  			   struct iommu_iotlb_gather *iotlb_gather);
> +	int (*cache_invalidate_user)(struct iommu_domain *domain,
> +				     struct iommu_user_data_array *array,
> +				     u32 *error_code);

Regarding the other conversation I worry a u32 error_code is too small.

Unfortunately there is no obvious place to put something better so if
we reach it we will have to add more error_code space via normal
extension.

Maybe expand this to u64? That is 64 bits of error register data and
the consumer index. It should do for SMMUv3 at least?

Jason
Nicolin Chen Dec. 6, 2023, 6:43 p.m. UTC | #3 
On Wed, Dec 06, 2023 at 02:32:09PM -0400, Jason Gunthorpe wrote:
> On Fri, Nov 17, 2023 at 05:07:12AM -0800, Yi Liu wrote:
 
> > @@ -465,6 +492,9 @@ struct iommu_domain_ops {
> >  			      size_t size);
> >  	void (*iotlb_sync)(struct iommu_domain *domain,
> >  			   struct iommu_iotlb_gather *iotlb_gather);
> > +	int (*cache_invalidate_user)(struct iommu_domain *domain,
> > +				     struct iommu_user_data_array *array,
> > +				     u32 *error_code);
> 
> Regarding the other conversation I worry a u32 error_code is too small.
> 
> Unfortunately there is no obvious place to put something better so if
> we reach it we will have to add more error_code space via normal
> extension.
> 
> Maybe expand this to u64? That is 64 bits of error register data and
> the consumer index. It should do for SMMUv3 at least?

I think Yi is moving the error_code to the entry data structure,
where we can even define a list of error_codes as a driver data
needs. So, I assume this u32 pointer would be gone too.

Thanks
Nicolin
Jason Gunthorpe Dec. 6, 2023, 6:50 p.m. UTC | #4 
On Wed, Dec 06, 2023 at 10:43:34AM -0800, Nicolin Chen wrote:
> On Wed, Dec 06, 2023 at 02:32:09PM -0400, Jason Gunthorpe wrote:
> > On Fri, Nov 17, 2023 at 05:07:12AM -0800, Yi Liu wrote:
>  
> > > @@ -465,6 +492,9 @@ struct iommu_domain_ops {
> > >  			      size_t size);
> > >  	void (*iotlb_sync)(struct iommu_domain *domain,
> > >  			   struct iommu_iotlb_gather *iotlb_gather);
> > > +	int (*cache_invalidate_user)(struct iommu_domain *domain,
> > > +				     struct iommu_user_data_array *array,
> > > +				     u32 *error_code);
> > 
> > Regarding the other conversation I worry a u32 error_code is too small.
> > 
> > Unfortunately there is no obvious place to put something better so if
> > we reach it we will have to add more error_code space via normal
> > extension.
> > 
> > Maybe expand this to u64? That is 64 bits of error register data and
> > the consumer index. It should do for SMMUv3 at least?
> 
> I think Yi is moving the error_code to the entry data structure,
> where we can even define a list of error_codes as a driver data
> needs. So, I assume this u32 pointer would be gone too.

Oh, lets see that then..

Jason
Yi Liu Dec. 7, 2023, 6:53 a.m. UTC | #5 
On 2023/12/7 02:50, Jason Gunthorpe wrote:
> On Wed, Dec 06, 2023 at 10:43:34AM -0800, Nicolin Chen wrote:
>> On Wed, Dec 06, 2023 at 02:32:09PM -0400, Jason Gunthorpe wrote:
>>> On Fri, Nov 17, 2023 at 05:07:12AM -0800, Yi Liu wrote:
>>   
>>>> @@ -465,6 +492,9 @@ struct iommu_domain_ops {
>>>>   			      size_t size);
>>>>   	void (*iotlb_sync)(struct iommu_domain *domain,
>>>>   			   struct iommu_iotlb_gather *iotlb_gather);
>>>> +	int (*cache_invalidate_user)(struct iommu_domain *domain,
>>>> +				     struct iommu_user_data_array *array,
>>>> +				     u32 *error_code);
>>>
>>> Regarding the other conversation I worry a u32 error_code is too small.
>>>
>>> Unfortunately there is no obvious place to put something better so if
>>> we reach it we will have to add more error_code space via normal
>>> extension.
>>>
>>> Maybe expand this to u64? That is 64 bits of error register data and
>>> the consumer index. It should do for SMMUv3 at least?
>>
>> I think Yi is moving the error_code to the entry data structure,
>> where we can even define a list of error_codes as a driver data
>> needs. So, I assume this u32 pointer would be gone too.
> 
> Oh, lets see that then..

yes, I'm going to move it.
Binbin Wu Jan. 8, 2024, 7:32 a.m. UTC | #6 
On 11/17/2023 9:07 PM, Yi Liu wrote:
> From: Lu Baolu <baolu.lu@linux.intel.com>
>
> The updates of the PTEs in the nested page table will be propagated to the
> hardware caches on both IOMMU (IOTLB) and devices (DevTLB/ATC).
>
> Add a new domain op cache_invalidate_user for the userspace to flush the
> hardware caches for a nested domain through iommufd. No wrapper for it,
> as it's only supposed to be used by iommufd. Then, pass in invalidation
> requests in form of a user data array conatining a number of invalidation

s/conatining/containing/
> data entries.
>
diff mbox series

Patch

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ec289c1016f5..0c1ff7fe4fa1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -284,6 +284,24 @@  struct iommu_user_data {
 	size_t len;
 };
 
+/**
+ * struct iommu_user_data_array - iommu driver specific user space data array
+ * @type: The data type of all the entries in the user buffer array
+ * @uptr: Pointer to the user buffer array for copy_from_user()
+ * @entry_len: The fixed-width length of a entry in the array, in bytes
+ * @entry_num: The number of total entries in the array
+ *
+ * A array having a @entry_num number of @entry_len sized entries, each entry is
+ * user space data, an uAPI defined in include/uapi/linux/iommufd.h where @type
+ * is also defined as enum iommu_xyz_data_type.
+ */
+struct iommu_user_data_array {
+	unsigned int type;
+	void __user *uptr;
+	size_t entry_len;
+	int entry_num;
+};
+
 /**
  * __iommu_copy_struct_from_user - Copy iommu driver specific user space data
  * @dst_data: Pointer to an iommu driver specific user data that is defined in
@@ -440,6 +458,15 @@  struct iommu_ops {
  * @iotlb_sync_map: Sync mappings created recently using @map to the hardware
  * @iotlb_sync: Flush all queued ranges from the hardware TLBs and empty flush
  *            queue
+ * @cache_invalidate_user: Flush hardware cache for user space IO page table.
+ *                         The @domain must be IOMMU_DOMAIN_NESTED. The @array
+ *                         passes in the cache invalidation requests, in form
+ *                         of a driver data structure. The driver must update
+ *                         array->entry_num to report the number of handled
+ *                         invalidation requests. The 32-bit @error_code can
+ *                         forward a driver specific error code to user space.
+ *                         Both the driver data structure and the error code
+ *                         must be defined in include/uapi/linux/iommufd.h
  * @iova_to_phys: translate iova to physical address
  * @enforce_cache_coherency: Prevent any kind of DMA from bypassing IOMMU_CACHE,
  *                           including no-snoop TLPs on PCIe or other platform
@@ -465,6 +492,9 @@  struct iommu_domain_ops {
 			      size_t size);
 	void (*iotlb_sync)(struct iommu_domain *domain,
 			   struct iommu_iotlb_gather *iotlb_gather);
+	int (*cache_invalidate_user)(struct iommu_domain *domain,
+				     struct iommu_user_data_array *array,
+				     u32 *error_code);
 
 	phys_addr_t (*iova_to_phys)(struct iommu_domain *domain,
 				    dma_addr_t iova);