Re: [PATCH rc v3] iommu: Handle iommu faults for a bad iopf setup

[Baolu Lu <baolu.lu@linux.intel.com>]

On 8/16/24 2:24 AM, Pranjal Shrivastava wrote:
> The iommu_report_device_fault function was updated to return void while
> assuming that drivers only need to call iommu_report_device_fault() for
> reporting an iopf. This implementation causes following problems:
> 
> 1. The drivers rely on the core code to call it's page_reponse,
>     however, when a fault is received and no fault capable domain is
>     attached / iopf_param is NULL, the ops->page_response is NOT called
>     causing the device to stall in case the fault type was PAGE_REQ.
> 
> 2. The arm_smmu_v3 driver relies on the returned value to log errors
>     returning void from iommu_report_device_fault causes these events to
>     be missed while logging.
> 
> Modify the iommu_report_device_fault function to return -EINVAL for
> cases where no fault capable domain is attached or iopf_param was NULL
> and calls back to the driver (ops->page_response) in case the fault type
> was IOMMU_FAULT_PAGE_REQ. The returned value can be used by the drivers
> to log the fault/event as needed.
> 
> Reported-by: Kunkun Jiang<jiangkunkun@huawei.com>
> Closes:https://lore.kernel.org/all/6147caf0-b9a0-30ca-795e-a1aa502a5c51@huawei.com/
> Fixes: 3dfa64aecbaf ("iommu: Make iommu_report_device_fault() return void")
> Signed-off-by: Jason Gunthorpe<jgg@nvidia.com>
> Signed-off-by: Pranjal Shrivastava<praan@google.com>
> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |   2 +-
>   drivers/iommu/io-pgfault.c                  | 116 +++++++++++++-------
>   include/linux/iommu.h                       |   5 +-
>   3 files changed, 83 insertions(+), 40 deletions(-)

Nit: can you please add some words to the comments of
iommu_report_device_fault() to explain what it returns? Right now, it
looks like the return value only matters for certain iommu drivers, so
it would be helpful to add a clear explanation.

Others look good to me,

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Thanks,
baolu