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

[Pranjal Shrivastava <praan@google.com>]

On Fri, Aug 16, 2024 at 10:17:59AM +0800, Baolu Lu wrote:
> 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.

Alright, will update the comments in v4.

> 
> Others look good to me,
> 
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> 
> Thanks,
> baolu
> 
> 
Thanks,
Pranjal