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

[Robin Murphy <robin.murphy@arm.com>]

On 15/08/2024 1:46 pm, Pranjal Shrivastava wrote:
[...]> @@ -155,22 +208,37 @@ static struct iopf_group 
*iopf_group_alloc(struct iommu_fault_param *iopf_param,
>    * hardware has been set to block the page faults) and the pending page faults
>    * have been flushed.
>    */
> -void iommu_report_device_fault(struct device *dev, struct iopf_fault *evt)
> +int iommu_report_device_fault(struct device *dev, struct iopf_fault *evt)
>   {
> +	struct iommu_attach_handle *attach_handle;
>   	struct iommu_fault *fault = &evt->fault;
>   	struct iommu_fault_param *iopf_param;
>   	struct iopf_group abort_group = {};
>   	struct iopf_group *group;
> +	int ret = 0;
>   
> +	attach_handle = find_fault_handler(dev, evt);
> +	if (!attach_handle) {
> +		ret = -EINVAL;
> +		goto err_bad_iopf;
> +	}
> +
> +	/*
> +	 * Something has gone wrong if a fault capable domain is attached but no
> +	 * iopf_param is setup
> +	 */
>   	iopf_param = iopf_get_dev_fault_param(dev);
> -	if (WARN_ON(!iopf_param))
> -		return;
> +	if (WARN_ON(!iopf_param)) {
> +		ret = -EINVAL;
> +		goto err_bad_iopf;

Nit: the err_bad_iopf path is only ever returning -EINVAL, so you could 
simply hard-code that there...

> +	}
>   
>   	if (!(fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE)) {
> -		report_partial_fault(iopf_param, fault);
> +		ret = report_partial_fault(iopf_param, fault);

...and declare "ret" here in the one scope it's actually needed.

Thanks,
Robin.

>   		iopf_put_dev_fault_param(iopf_param);
>   		/* A request that is not the last does not need to be ack'd */
> -		return;
> +
> +		return ret;
>   	}
>   
>   	/*