diff mbox series

[2/9] iommu: Add device parameter to iopf handler

Message ID	20230711010642.19707-3-baolu.lu@linux.intel.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Lu Baolu <baolu.lu@linux.intel.com> To: Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, Jason Gunthorpe <jgg@ziepe.ca>, Kevin Tian <kevin.tian@intel.com>, Jean-Philippe Brucker <jean-philippe@linaro.org>, Nicolin Chen <nicolinc@nvidia.com> Cc: Yi Liu <yi.l.liu@intel.com>, Jacob Pan <jacob.jun.pan@linux.intel.com>, iommu@lists.linux.dev, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Lu Baolu <baolu.lu@linux.intel.com> Subject: [PATCH 2/9] iommu: Add device parameter to iopf handler Date: Tue, 11 Jul 2023 09:06:35 +0800 Message-Id: <20230711010642.19707-3-baolu.lu@linux.intel.com> In-Reply-To: <20230711010642.19707-1-baolu.lu@linux.intel.com> References: <20230711010642.19707-1-baolu.lu@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	iommu: Prepare to deliver page faults to user space \| [0/9] iommu: Prepare to deliver page faults to user space [1/9] iommu: Move iommu fault data to linux/iommu.h [2/9] iommu: Add device parameter to iopf handler [3/9] iommu: Add common code to handle IO page faults [4/9] iommu: Change the return value of dev_iommu_get() [5/9] iommu: Make fault_param generic [6/9] iommu: Remove iommu_[un]register_device_fault_handler() [7/9] iommu: Add helper to set iopf handler for domain [8/9] iommu: Add iommu page fault cookie helpers [9/9] iommu: Use fault cookie to store iopf_param

Commit Message

Baolu Lu July 11, 2023, 1:06 a.m. UTC

  Add the device parameter to the iopf handler so that it can know which
device this fault was generated.

This is necessary for use cases such as delivering IO page faults to user
space. The IOMMUFD layer needs to be able to lookup the device id of a
fault and route it together with the fault message to the user space.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 include/linux/iommu.h      | 1 +
 drivers/iommu/iommu-sva.h  | 4 ++--
 drivers/iommu/io-pgfault.c | 2 +-
 drivers/iommu/iommu-sva.c  | 2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)

Comments

Jacob Pan July 11, 2023, 5:26 p.m. UTC | #1

Hi BaoLu,

On Tue, 11 Jul 2023 09:06:35 +0800, Lu Baolu <baolu.lu@linux.intel.com>
wrote:

> Add the device parameter to the iopf handler so that it can know which
> device this fault was generated.
> 
> This is necessary for use cases such as delivering IO page faults to user
> space. The IOMMUFD layer needs to be able to lookup the device id of a
> fault and route it together with the fault message to the user space.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  include/linux/iommu.h      | 1 +
>  drivers/iommu/iommu-sva.h  | 4 ++--
>  drivers/iommu/io-pgfault.c | 2 +-
>  drivers/iommu/iommu-sva.c  | 2 +-
>  4 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0eb0fb852020..a00fb43b5e73 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -249,6 +249,7 @@ struct iommu_domain {
>  	struct iommu_domain_geometry geometry;
>  	struct iommu_dma_cookie *iova_cookie;
>  	enum iommu_page_response_code (*iopf_handler)(struct iommu_fault
> *fault,
> +						      struct device *dev,
>  						      void *data);
>  	void *fault_data;
>  	union {
> diff --git a/drivers/iommu/iommu-sva.h b/drivers/iommu/iommu-sva.h
> index 54946b5a7caf..c848661c4e20 100644
> --- a/drivers/iommu/iommu-sva.h
> +++ b/drivers/iommu/iommu-sva.h
> @@ -23,7 +23,7 @@ struct iopf_queue *iopf_queue_alloc(const char *name);
>  void iopf_queue_free(struct iopf_queue *queue);
>  int iopf_queue_discard_partial(struct iopf_queue *queue);
>  enum iommu_page_response_code
> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data);
> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
> void *data); 
>  #else /* CONFIG_IOMMU_SVA */
>  static inline int iommu_queue_iopf(struct iommu_fault *fault, void
> *cookie) @@ -63,7 +63,7 @@ static inline int
> iopf_queue_discard_partial(struct iopf_queue *queue) }
>  
>  static inline enum iommu_page_response_code
> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
> void *data) {
>  	return IOMMU_PAGE_RESP_INVALID;
>  }
> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
> index e5b8b9110c13..fa604e1b5c5c 100644
> --- a/drivers/iommu/io-pgfault.c
> +++ b/drivers/iommu/io-pgfault.c
> @@ -88,7 +88,7 @@ static void iopf_handler(struct work_struct *work)
>  		 * faults in the group if there is an error.
>  		 */
>  		if (status == IOMMU_PAGE_RESP_SUCCESS)
> -			status = domain->iopf_handler(&iopf->fault,
> +			status = domain->iopf_handler(&iopf->fault,
> group->dev, domain->fault_data);
>  
>  		if (!(iopf->fault.prm.flags &
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index 3ebd4b6586b3..14766a2b61af 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -157,7 +157,7 @@ EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
>   * I/O page fault handler for SVA
>   */
>  enum iommu_page_response_code
> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
dev has no use for sva handler, right? mark them __always_unused?

> void *data) {
>  	vm_fault_t ret;
>  	struct vm_area_struct *vma;


Thanks,

Jacob

Baolu Lu July 12, 2023, 2:16 a.m. UTC | #2

On 2023/7/12 1:26, Jacob Pan wrote:
> Hi BaoLu,

Hi Jacob,

> 
> On Tue, 11 Jul 2023 09:06:35 +0800, Lu Baolu<baolu.lu@linux.intel.com>
> wrote:
> 
>> Add the device parameter to the iopf handler so that it can know which
>> device this fault was generated.
>>
>> This is necessary for use cases such as delivering IO page faults to user
>> space. The IOMMUFD layer needs to be able to lookup the device id of a
>> fault and route it together with the fault message to the user space.
>>
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>> ---
>>   include/linux/iommu.h      | 1 +
>>   drivers/iommu/iommu-sva.h  | 4 ++--
>>   drivers/iommu/io-pgfault.c | 2 +-
>>   drivers/iommu/iommu-sva.c  | 2 +-
>>   4 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 0eb0fb852020..a00fb43b5e73 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -249,6 +249,7 @@ struct iommu_domain {
>>   	struct iommu_domain_geometry geometry;
>>   	struct iommu_dma_cookie *iova_cookie;
>>   	enum iommu_page_response_code (*iopf_handler)(struct iommu_fault
>> *fault,
>> +						      struct device *dev,
>>   						      void *data);
>>   	void *fault_data;
>>   	union {
>> diff --git a/drivers/iommu/iommu-sva.h b/drivers/iommu/iommu-sva.h
>> index 54946b5a7caf..c848661c4e20 100644
>> --- a/drivers/iommu/iommu-sva.h
>> +++ b/drivers/iommu/iommu-sva.h
>> @@ -23,7 +23,7 @@ struct iopf_queue *iopf_queue_alloc(const char *name);
>>   void iopf_queue_free(struct iopf_queue *queue);
>>   int iopf_queue_discard_partial(struct iopf_queue *queue);
>>   enum iommu_page_response_code
>> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data);
>> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
>> void *data);
>>   #else /* CONFIG_IOMMU_SVA */
>>   static inline int iommu_queue_iopf(struct iommu_fault *fault, void
>> *cookie) @@ -63,7 +63,7 @@ static inline int
>> iopf_queue_discard_partial(struct iopf_queue *queue) }
>>   
>>   static inline enum iommu_page_response_code
>> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
>> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
>> void *data) {
>>   	return IOMMU_PAGE_RESP_INVALID;
>>   }
>> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
>> index e5b8b9110c13..fa604e1b5c5c 100644
>> --- a/drivers/iommu/io-pgfault.c
>> +++ b/drivers/iommu/io-pgfault.c
>> @@ -88,7 +88,7 @@ static void iopf_handler(struct work_struct *work)
>>   		 * faults in the group if there is an error.
>>   		 */
>>   		if (status == IOMMU_PAGE_RESP_SUCCESS)
>> -			status = domain->iopf_handler(&iopf->fault,
>> +			status = domain->iopf_handler(&iopf->fault,
>> group->dev, domain->fault_data);
>>   
>>   		if (!(iopf->fault.prm.flags &
>> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
>> index 3ebd4b6586b3..14766a2b61af 100644
>> --- a/drivers/iommu/iommu-sva.c
>> +++ b/drivers/iommu/iommu-sva.c
>> @@ -157,7 +157,7 @@ EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
>>    * I/O page fault handler for SVA
>>    */
>>   enum iommu_page_response_code
>> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
>> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
> dev has no use for sva handler, right? mark them __always_unused?

My understanding is that __always_unused attribute in Linux kernel code
marks a variable or function as unused. It implies that the compiler is
free to optimize the variable or function away.

If my understanding is correct, then it may not be suitable here.

Best regards,
baolu

Jacob Pan July 12, 2023, 5:46 a.m. UTC | #3

Hi Baolu,

On Wed, 12 Jul 2023 10:16:11 +0800, Baolu Lu <baolu.lu@linux.intel.com>
wrote:

> On 2023/7/12 1:26, Jacob Pan wrote:
> > Hi BaoLu,  
> 
> Hi Jacob,
> 
> > 
> > On Tue, 11 Jul 2023 09:06:35 +0800, Lu Baolu<baolu.lu@linux.intel.com>
> > wrote:
> >   
> >> Add the device parameter to the iopf handler so that it can know which
> >> device this fault was generated.
> >>
> >> This is necessary for use cases such as delivering IO page faults to
> >> user space. The IOMMUFD layer needs to be able to lookup the device id
> >> of a fault and route it together with the fault message to the user
> >> space.
> >>
> >> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
> >> ---
> >>   include/linux/iommu.h      | 1 +
> >>   drivers/iommu/iommu-sva.h  | 4 ++--
> >>   drivers/iommu/io-pgfault.c | 2 +-
> >>   drivers/iommu/iommu-sva.c  | 2 +-
> >>   4 files changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >> index 0eb0fb852020..a00fb43b5e73 100644
> >> --- a/include/linux/iommu.h
> >> +++ b/include/linux/iommu.h
> >> @@ -249,6 +249,7 @@ struct iommu_domain {
> >>   	struct iommu_domain_geometry geometry;
> >>   	struct iommu_dma_cookie *iova_cookie;
> >>   	enum iommu_page_response_code (*iopf_handler)(struct
> >> iommu_fault *fault,
> >> +						      struct device
> >> *dev, void *data);
> >>   	void *fault_data;
> >>   	union {
> >> diff --git a/drivers/iommu/iommu-sva.h b/drivers/iommu/iommu-sva.h
> >> index 54946b5a7caf..c848661c4e20 100644
> >> --- a/drivers/iommu/iommu-sva.h
> >> +++ b/drivers/iommu/iommu-sva.h
> >> @@ -23,7 +23,7 @@ struct iopf_queue *iopf_queue_alloc(const char
> >> *name); void iopf_queue_free(struct iopf_queue *queue);
> >>   int iopf_queue_discard_partial(struct iopf_queue *queue);
> >>   enum iommu_page_response_code
> >> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data);
> >> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
> >> void *data);
> >>   #else /* CONFIG_IOMMU_SVA */
> >>   static inline int iommu_queue_iopf(struct iommu_fault *fault, void
> >> *cookie) @@ -63,7 +63,7 @@ static inline int
> >> iopf_queue_discard_partial(struct iopf_queue *queue) }
> >>   
> >>   static inline enum iommu_page_response_code
> >> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
> >> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,
> >> void *data) {
> >>   	return IOMMU_PAGE_RESP_INVALID;
> >>   }
> >> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
> >> index e5b8b9110c13..fa604e1b5c5c 100644
> >> --- a/drivers/iommu/io-pgfault.c
> >> +++ b/drivers/iommu/io-pgfault.c
> >> @@ -88,7 +88,7 @@ static void iopf_handler(struct work_struct *work)
> >>   		 * faults in the group if there is an error.
> >>   		 */
> >>   		if (status == IOMMU_PAGE_RESP_SUCCESS)
> >> -			status = domain->iopf_handler(&iopf->fault,
> >> +			status = domain->iopf_handler(&iopf->fault,
> >> group->dev, domain->fault_data);
> >>   
> >>   		if (!(iopf->fault.prm.flags &
> >> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> >> index 3ebd4b6586b3..14766a2b61af 100644
> >> --- a/drivers/iommu/iommu-sva.c
> >> +++ b/drivers/iommu/iommu-sva.c
> >> @@ -157,7 +157,7 @@ EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
> >>    * I/O page fault handler for SVA
> >>    */
> >>   enum iommu_page_response_code
> >> -iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
> >> +iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev,  
> > dev has no use for sva handler, right? mark them __always_unused?  
> 
> My understanding is that __always_unused attribute in Linux kernel code
> marks a variable or function as unused. It implies that the compiler is
> free to optimize the variable or function away.
that is my understanding as well, I meant
iommu_sva_handle_iopf(struct iommu_fault *fault, struct device
__always_unused *dev,  

I tested compile w/ and w/o __always_unused, seems no difference but it
makes the code clear that dev is not used here.


Thanks,

Jacob

diff mbox series

Patch

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0eb0fb852020..a00fb43b5e73 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -249,6 +249,7 @@  struct iommu_domain {
 	struct iommu_domain_geometry geometry;
 	struct iommu_dma_cookie *iova_cookie;
 	enum iommu_page_response_code (*iopf_handler)(struct iommu_fault *fault,
+						      struct device *dev,
 						      void *data);
 	void *fault_data;
 	union {
diff --git a/drivers/iommu/iommu-sva.h b/drivers/iommu/iommu-sva.h
index 54946b5a7caf..c848661c4e20 100644
--- a/drivers/iommu/iommu-sva.h
+++ b/drivers/iommu/iommu-sva.h
@@ -23,7 +23,7 @@  struct iopf_queue *iopf_queue_alloc(const char *name);
 void iopf_queue_free(struct iopf_queue *queue);
 int iopf_queue_discard_partial(struct iopf_queue *queue);
 enum iommu_page_response_code
-iommu_sva_handle_iopf(struct iommu_fault *fault, void *data);
+iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev, void *data);
 
 #else /* CONFIG_IOMMU_SVA */
 static inline int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
@@ -63,7 +63,7 @@  static inline int iopf_queue_discard_partial(struct iopf_queue *queue)
 }
 
 static inline enum iommu_page_response_code
-iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
+iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev, void *data)
 {
 	return IOMMU_PAGE_RESP_INVALID;
 }
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index e5b8b9110c13..fa604e1b5c5c 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -88,7 +88,7 @@  static void iopf_handler(struct work_struct *work)
 		 * faults in the group if there is an error.
 		 */
 		if (status == IOMMU_PAGE_RESP_SUCCESS)
-			status = domain->iopf_handler(&iopf->fault,
+			status = domain->iopf_handler(&iopf->fault, group->dev,
 						      domain->fault_data);
 
 		if (!(iopf->fault.prm.flags &
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 3ebd4b6586b3..14766a2b61af 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -157,7 +157,7 @@  EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
  * I/O page fault handler for SVA
  */
 enum iommu_page_response_code
-iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
+iommu_sva_handle_iopf(struct iommu_fault *fault, struct device *dev, void *data)
 {
 	vm_fault_t ret;
 	struct vm_area_struct *vma;