Hi Baolu,
On Wed, 12 Jul 2023 10:16:11 +0800, Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxx>
> >> ---
> >> 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
