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;
Date:   Tue, 11 Jul 2023 10:26:20 -0700
From:   Jacob Pan <jacob.jun.pan@linux.intel.com>
To:     Lu Baolu <baolu.lu@linux.intel.com>
Cc:     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>,
        Yi Liu <yi.l.liu@intel.com>, iommu@lists.linux.dev,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        jacob.jun.pan@linux.intel.com
Subject: Re: [PATCH 2/9] iommu: Add device parameter to iopf handler
Message-ID: <20230711102620.37b06884@jacob-builder>
In-Reply-To: <20230711010642.19707-3-baolu.lu@linux.intel.com>
References: <20230711010642.19707-1-baolu.lu@linux.intel.com>
        <20230711010642.19707-3-baolu.lu@linux.intel.com>
Organization: OTC
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

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