[PATCH] iommu/virtio: Report faults via report_iommu_fault()

[PATCH] iommu/virtio: Report faults via report_iommu_fault()

[weimin xiong]

From: xiongweimin <xiongweimin@kylinos.cn>

Fault events currently only print a ratelimited error with the endpoint
ID. Upper layers (VFIO, drivers with domain fault handlers) cannot react.

Track endpoints in an xarray keyed by endpoint ID at probe/release time,
and call report_iommu_fault() when a fault carries an address and the
endpoint is attached to a domain. Keep the existing log as a fallback
when the endpoint is unknown or detached.

Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
Cc: iommu@lists.linux.dev
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Jason Gunthorpe <jgg@nvidia.com>
---

--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -19,6 +19,7 @@
 #include <linux/virtio_config.h>
 #include <linux/virtio_ids.h>
 #include <linux/wait.h>
+#include <linux/xarray.h>
 
 #include <uapi/linux/virtio_iommu.h>
 
@@ -42,6 +43,8 @@
 	spinlock_t			request_lock;
 	struct list_head		requests;
 	void				*evts;
+	/* endpoint ID -> viommu_endpoint, for fault reporting */
+	struct xarray			endpoints;
 
 	/* Device configuration */
 	struct iommu_domain_geometry	geometry;
@@ -593,7 +596,10 @@
 static int viommu_fault_handler(struct viommu_dev *viommu,
 				struct virtio_iommu_fault *fault)
 {
+	struct viommu_endpoint *vdev;
+	struct iommu_domain *domain;
 	char *reason_str;
+	int fault_flags = IOMMU_FAULT_READ;
 
 	u8 reason	= fault->reason;
 	u32 flags	= le32_to_cpu(fault->flags);
@@ -613,16 +619,28 @@
 		break;
 	}
 
-	/* TODO: find EP by ID and report_iommu_fault */
-	if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
-		dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx [%s%s%s]\n",
+	if (flags & VIRTIO_IOMMU_FAULT_F_WRITE)
+		fault_flags = IOMMU_FAULT_WRITE;
+
+	rcu_read_lock();
+	vdev = xa_load(&viommu->endpoints, endpoint);
+	if (vdev && vdev->vdomain && (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)) {
+		domain = &vdev->vdomain->domain;
+		report_iommu_fault(domain, vdev->dev, address, fault_flags);
+		rcu_read_unlock();
+	} else if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS) {
+		rcu_read_unlock();
+		dev_err_ratelimited(viommu->dev,
+				    "%s fault from EP %u at %#llx [%s%s%s]\n",
 				    reason_str, endpoint, address,
 				    flags & VIRTIO_IOMMU_FAULT_F_READ ? "R" : "",
 				    flags & VIRTIO_IOMMU_FAULT_F_WRITE ? "W" : "",
 				    flags & VIRTIO_IOMMU_FAULT_F_EXEC ? "X" : "");
-	else
+	} else {
+		rcu_read_unlock();
 		dev_err_ratelimited(viommu->dev, "%s fault from EP %u\n",
 				    reason_str, endpoint);
+	}
 	return 0;
 }
 
@@ -1037,15 +1055,33 @@
 	INIT_LIST_HEAD(&vdev->resv_regions);
 	dev_iommu_priv_set(dev, vdev);
 
+	for (ret = 0; ret < fwspec->num_ids; ret++) {
+		int err = xa_insert(&viommu->endpoints, fwspec->ids[ret], vdev,
+				   GFP_KERNEL);
+		if (err) {
+			while (ret--)
+				xa_erase(&viommu->endpoints, fwspec->ids[ret]);
+			ret = err;
+			goto err_free_dev;
+		}
+	}
+
 	if (viommu->probe_size) {
 		/* Get additional information for this endpoint */
 		ret = viommu_probe_endpoint(viommu, dev);
 		if (ret)
-			goto err_free_dev;
+			goto err_unreg;
 	}
 
 	return &viommu->iommu;
 
+err_unreg:
+	{
+		int i;
+
+		for (i = 0; i < fwspec->num_ids; i++)
+			xa_erase(&viommu->endpoints, fwspec->ids[i]);
+	}
 err_free_dev:
 	iommu_put_resv_regions(dev, &vdev->resv_regions);
 	kfree(vdev);
@@ -1056,8 +1092,12 @@
 static void viommu_release_device(struct device *dev)
 {
 	struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	int i;
 
 	viommu_detach_dev(vdev);
+	for (i = 0; i < fwspec->num_ids; i++)
+		xa_erase(&vdev->viommu->endpoints, fwspec->ids[i]);
 	iommu_put_resv_regions(dev, &vdev->resv_regions);
 	kfree(vdev);
 }
@@ -1167,6 +1207,7 @@
 
 	spin_lock_init(&viommu->request_lock);
 	ida_init(&viommu->domain_ids);
+	xa_init(&viommu->endpoints);
 	viommu->dev = dev;
 	viommu->vdev = vdev;
 	INIT_LIST_HEAD(&viommu->requests);
@@ -1261,6 +1302,7 @@
 	virtio_reset_device(vdev);
 	vdev->config->del_vqs(vdev);
 
+	xa_destroy(&viommu->endpoints);
 	dev_info(&vdev->dev, "device removed\n");
 }
 

Re: [PATCH] iommu/virtio: Report faults via report_iommu_fault()

[Baolu Lu]

On 7/13/26 09:04, weimin xiong wrote:
> From: xiongweimin <xiongweimin@kylinos.cn>
> 
> Fault events currently only print a ratelimited error with the endpoint
> ID. Upper layers (VFIO, drivers with domain fault handlers) cannot react.
> 
> Track endpoints in an xarray keyed by endpoint ID at probe/release time,
> and call report_iommu_fault() when a fault carries an address and the
> endpoint is attached to a domain. Keep the existing log as a fallback
> when the endpoint is unknown or detached.
> 
> Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
> Cc: iommu@lists.linux.dev
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> ---
> 
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -19,6 +19,7 @@
>   #include <linux/virtio_config.h>
>   #include <linux/virtio_ids.h>
>   #include <linux/wait.h>
> +#include <linux/xarray.h>
>   
>   #include <uapi/linux/virtio_iommu.h>
>   
> @@ -42,6 +43,8 @@
>   	spinlock_t			request_lock;
>   	struct list_head		requests;
>   	void				*evts;
> +	/* endpoint ID -> viommu_endpoint, for fault reporting */
> +	struct xarray			endpoints;
>   
>   	/* Device configuration */
>   	struct iommu_domain_geometry	geometry;
> @@ -593,7 +596,10 @@
>   static int viommu_fault_handler(struct viommu_dev *viommu,
>   				struct virtio_iommu_fault *fault)
>   {
> +	struct viommu_endpoint *vdev;
> +	struct iommu_domain *domain;
>   	char *reason_str;
> +	int fault_flags = IOMMU_FAULT_READ;
>   
>   	u8 reason	= fault->reason;
>   	u32 flags	= le32_to_cpu(fault->flags);
> @@ -613,16 +619,28 @@
>   		break;
>   	}
>   
> -	/* TODO: find EP by ID and report_iommu_fault */
> -	if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
> -		dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx [%s%s%s]\n",
> +	if (flags & VIRTIO_IOMMU_FAULT_F_WRITE)
> +		fault_flags = IOMMU_FAULT_WRITE;
> +
> +	rcu_read_lock();
> +	vdev = xa_load(&viommu->endpoints, endpoint);
> +	if (vdev && vdev->vdomain && (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)) {
> +		domain = &vdev->vdomain->domain;
> +		report_iommu_fault(domain, vdev->dev, address, fault_flags);

Who is the consumer of the fault message reported here?

Thanks,
baolu

Re:Re: [PATCH] iommu/virtio: Report faults via report_iommu_fault()

[Xiong Weimin]

Hi Baolu,


Thanks for the review.


The consumer is whoever registered a fault handler on the IOMMU domain
via iommu_set_fault_handler() — the same path used by other IOMMU
drivers (e.g. ARM SMMU, AMD, MTK) when they call report_iommu_fault().


In virtio-iommu today, viommu_fault_handler() only prints a ratelimited
message and has an explicit TODO to call report_iommu_fault(). This
patch completes that TODO: when a fault carries an address and the
endpoint is attached to a domain, we look up the endpoint and forward
the fault to the domain handler. If no handler is installed,
report_iommu_fault() returns -ENOSYS and we keep the existing log as
fallback.


This is not the VFIO/IOMMUFD IOPF path (iommu_report_device_fault());
it covers virtio-iommu DMA protection faults (domain/mapping faults)
for endpoints behind an IOMMU domain.


Typical consumers would be device drivers or subsystems that install a
domain fault handler — for example remoteproc or GPU drivers in-tree.
The main goal is API completeness and consistency with other IOMMU
drivers, so upper layers can react instead of only seeing a printk.


Happy to add a brief comment in the patch clarifying this if that would
help.


Thanks,
Weimin


At 2026-07-13 10:07:29, "Baolu Lu" <baolu.lu@linux.intel.com> wrote:
>On 7/13/26 09:04, weimin xiong wrote:
>> From: xiongweimin <xiongweimin@kylinos.cn>
>> 
>> Fault events currently only print a ratelimited error with the endpoint
>> ID. Upper layers (VFIO, drivers with domain fault handlers) cannot react.
>> 
>> Track endpoints in an xarray keyed by endpoint ID at probe/release time,
>> and call report_iommu_fault() when a fault carries an address and the
>> endpoint is attached to a domain. Keep the existing log as a fallback
>> when the endpoint is unknown or detached.
>> 
>> Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
>> Cc: iommu@lists.linux.dev
>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
>> Cc: Jason Gunthorpe <jgg@nvidia.com>
>> ---
>> 
>> --- a/drivers/iommu/virtio-iommu.c
>> +++ b/drivers/iommu/virtio-iommu.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/virtio_config.h>
>>   #include <linux/virtio_ids.h>
>>   #include <linux/wait.h>
>> +#include <linux/xarray.h>
>>   
>>   #include <uapi/linux/virtio_iommu.h>
>>   
>> @@ -42,6 +43,8 @@
>>   	spinlock_t			request_lock;
>>   	struct list_head		requests;
>>   	void				*evts;
>> +	/* endpoint ID -> viommu_endpoint, for fault reporting */
>> +	struct xarray			endpoints;
>>   
>>   	/* Device configuration */
>>   	struct iommu_domain_geometry	geometry;
>> @@ -593,7 +596,10 @@
>>   static int viommu_fault_handler(struct viommu_dev *viommu,
>>   				struct virtio_iommu_fault *fault)
>>   {
>> +	struct viommu_endpoint *vdev;
>> +	struct iommu_domain *domain;
>>   	char *reason_str;
>> +	int fault_flags = IOMMU_FAULT_READ;
>>   
>>   	u8 reason	= fault->reason;
>>   	u32 flags	= le32_to_cpu(fault->flags);
>> @@ -613,16 +619,28 @@
>>   		break;
>>   	}
>>   
>> -	/* TODO: find EP by ID and report_iommu_fault */
>> -	if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
>> -		dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx [%s%s%s]\n",
>> +	if (flags & VIRTIO_IOMMU_FAULT_F_WRITE)
>> +		fault_flags = IOMMU_FAULT_WRITE;
>> +
>> +	rcu_read_lock();
>> +	vdev = xa_load(&viommu->endpoints, endpoint);
>> +	if (vdev && vdev->vdomain && (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)) {
>> +		domain = &vdev->vdomain->domain;
>> +		report_iommu_fault(domain, vdev->dev, address, fault_flags);
>
>Who is the consumer of the fault message reported here?
>
>Thanks,
>baolu

Re: Re: [PATCH] iommu/virtio: Report faults via report_iommu_fault()

[Jason Gunthorpe]

On Mon, Jul 13, 2026 at 04:02:03PM +0800, Xiong Weimin wrote:

> The consumer is whoever registered a fault handler on the IOMMU domain
> via iommu_set_fault_handler() — the same path used by other IOMMU
> drivers (e.g. ARM SMMU, AMD, MTK) when they call report_iommu_fault().

I want to remove this old API and posted a series a while back to
remove it from some drivers where it is never used.

If you don't have an actual concrete in-tree use then please don't add
new callers for this stuff.

Jason

Re:Re: Re: [PATCH] iommu/virtio: Report faults via report_iommu_fault()

[Xiong Weimin]

Thanks Jason, understood.


I don't want to add a new caller of an API that is being phased out.
Since there is no concrete in-tree consumer for this driver's fault
handler today, I will drop this patch from the current series.


If fault reporting is needed for this driver later, would the preferred
direction be the newer device fault path, e.g. iommu_report_device_fault()/ 
domain->iopf_handler, or should this simply be left to the IOMMU driver
logging unless there is a real in-tree consumer?


Thanks,
Weimin

At 2026-07-13 19:45:41, "Jason Gunthorpe" <jgg@nvidia.com> wrote:
>On Mon, Jul 13, 2026 at 04:02:03PM +0800, Xiong Weimin wrote:
>
>> The consumer is whoever registered a fault handler on the IOMMU domain
>> via iommu_set_fault_handler() — the same path used by other IOMMU
>> drivers (e.g. ARM SMMU, AMD, MTK) when they call report_iommu_fault().
>
>I want to remove this old API and posted a series a while back to
>remove it from some drivers where it is never used.
>
>If you don't have an actual concrete in-tree use then please don't add
>new callers for this stuff.
>
>Jason

[PATCH v2] iommu/virtio: Report faults via report_iommu_fault()

[weimin xiong]

From: xiongweimin <xiongweimin@kylinos.cn>

Fault events currently only print a ratelimited error with the endpoint
ID. Drivers that registered a domain fault handler with
iommu_set_fault_handler() cannot react.

Track endpoints in an xarray keyed by endpoint ID at probe/release time,
and call report_iommu_fault() when a fault carries an address and the
endpoint is attached to a domain. Keep the existing log as a fallback
when the endpoint is unknown, detached, or no handler is installed
(report_iommu_fault() returns -ENOSYS).

This completes the TODO in viommu_fault_handler() and matches other
IOMMU drivers. It is not the VFIO/IOMMUFD IOPF path
(iommu_report_device_fault()); it covers DMA protection faults for
virtio endpoints.

Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
 drivers/iommu/virtio-iommu.c | 52 ++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 587fc1319..3a46d97f2 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -19,6 +19,7 @@
 #include <linux/virtio_config.h>
 #include <linux/virtio_ids.h>
 #include <linux/wait.h>
+#include <linux/xarray.h>
 
 #include <uapi/linux/virtio_iommu.h>
 
@@ -42,6 +43,8 @@ struct viommu_dev {
 	spinlock_t			request_lock;
 	struct list_head		requests;
 	void				*evts;
+	/* endpoint ID -> viommu_endpoint, for fault reporting */
+	struct xarray			endpoints;
 
 	/* Device configuration */
 	struct iommu_domain_geometry	geometry;
@@ -593,7 +596,10 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
 static int viommu_fault_handler(struct viommu_dev *viommu,
 				struct virtio_iommu_fault *fault)
 {
+	struct viommu_endpoint *vdev;
+	struct iommu_domain *domain;
 	char *reason_str;
+	int fault_flags = IOMMU_FAULT_READ;
 
 	u8 reason	= fault->reason;
 	u32 flags	= le32_to_cpu(fault->flags);
@@ -613,16 +619,28 @@ static int viommu_fault_handler(struct viommu_dev *viommu,
 		break;
 	}
 
-	/* TODO: find EP by ID and report_iommu_fault */
-	if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
-		dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx [%s%s%s]\n",
+	if (flags & VIRTIO_IOMMU_FAULT_F_WRITE)
+		fault_flags = IOMMU_FAULT_WRITE;
+
+	rcu_read_lock();
+	vdev = xa_load(&viommu->endpoints, endpoint);
+	if (vdev && vdev->vdomain && (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)) {
+		domain = &vdev->vdomain->domain;
+		report_iommu_fault(domain, vdev->dev, address, fault_flags);
+		rcu_read_unlock();
+	} else if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS) {
+		rcu_read_unlock();
+		dev_err_ratelimited(viommu->dev,
+				    "%s fault from EP %u at %#llx [%s%s%s]\n",
 				    reason_str, endpoint, address,
 				    flags & VIRTIO_IOMMU_FAULT_F_READ ? "R" : "",
 				    flags & VIRTIO_IOMMU_FAULT_F_WRITE ? "W" : "",
 				    flags & VIRTIO_IOMMU_FAULT_F_EXEC ? "X" : "");
-	else
+	} else {
+		rcu_read_unlock();
 		dev_err_ratelimited(viommu->dev, "%s fault from EP %u\n",
 				    reason_str, endpoint);
+	}
 	return 0;
 }
 
@@ -1037,15 +1055,33 @@ static struct iommu_device *viommu_probe_device(struct device *dev)
 	INIT_LIST_HEAD(&vdev->resv_regions);
 	dev_iommu_priv_set(dev, vdev);
 
+	for (ret = 0; ret < fwspec->num_ids; ret++) {
+		int err = xa_insert(&viommu->endpoints, fwspec->ids[ret], vdev,
+				   GFP_KERNEL);
+		if (err) {
+			while (ret--)
+				xa_erase(&viommu->endpoints, fwspec->ids[ret]);
+			ret = err;
+			goto err_free_dev;
+		}
+	}
+
 	if (viommu->probe_size) {
 		/* Get additional information for this endpoint */
 		ret = viommu_probe_endpoint(viommu, dev);
 		if (ret)
-			goto err_free_dev;
+			goto err_unreg;
 	}
 
 	return &viommu->iommu;
 
+err_unreg:
+	{
+		int i;
+
+		for (i = 0; i < fwspec->num_ids; i++)
+			xa_erase(&viommu->endpoints, fwspec->ids[i]);
+	}
 err_free_dev:
 	iommu_put_resv_regions(dev, &vdev->resv_regions);
 	kfree(vdev);
@@ -1056,8 +1092,12 @@ static struct iommu_device *viommu_probe_device(struct device *dev)
 static void viommu_release_device(struct device *dev)
 {
 	struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	int i;
 
 	viommu_detach_dev(vdev);
+	for (i = 0; i < fwspec->num_ids; i++)
+		xa_erase(&vdev->viommu->endpoints, fwspec->ids[i]);
 	iommu_put_resv_regions(dev, &vdev->resv_regions);
 	kfree(vdev);
 }
@@ -1167,6 +1207,7 @@ static int viommu_probe(struct virtio_device *vdev)
 
 	spin_lock_init(&viommu->request_lock);
 	ida_init(&viommu->domain_ids);
+	xa_init(&viommu->endpoints);
 	viommu->dev = dev;
 	viommu->vdev = vdev;
 	INIT_LIST_HEAD(&viommu->requests);
@@ -1261,6 +1302,7 @@ static void viommu_remove(struct virtio_device *vdev)
 	virtio_reset_device(vdev);
 	vdev->config->del_vqs(vdev);
 
+	xa_destroy(&viommu->endpoints);
 	dev_info(&vdev->dev, "device removed\n");
 }
 
-- 
2.43.0


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH v2] iommu/virtio: Report faults via report_iommu_fault()

[Jason Gunthorpe]

On Tue, Jul 14, 2026 at 10:02:51AM +0800, weimin xiong wrote:
> From: xiongweimin <xiongweimin@kylinos.cn>
> 
> Fault events currently only print a ratelimited error with the endpoint
> ID. Drivers that registered a domain fault handler with
> iommu_set_fault_handler() cannot react.

IDK get it, you said you were going to drop this and you resent it?

Still no.

Jason