On Sun, Feb 19, 2017 at 10:47:08PM +0800, Lan Tianyu wrote:
> This patch is to add callback to handle fault event reported by
> IOMMU driver. Callback stores fault into an array and notify userspace
> via eventfd to read fault info.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> ---
> drivers/vfio/vfio_iommu_type1.c | 30 ++++++++++++++++++++++++++++++
> include/linux/iommu.h | 7 +++++++
> include/uapi/linux/vfio.h | 7 +++++++
> 3 files changed, 44 insertions(+)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 46674ea..dc434a3 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -56,6 +56,8 @@
> MODULE_PARM_DESC(disable_hugepages,
> "Disable VFIO IOMMU support for IOMMU hugepages.");
>
> +#define NR_IOMMU_FAULT_INFO 10
> +
> struct vfio_iommu {
> struct list_head domain_list;
> struct vfio_domain *external_domain; /* domain for external user */
> @@ -64,6 +66,9 @@ struct vfio_iommu {
> struct blocking_notifier_head notifier;
> struct eventfd_ctx *iommu_fault_fd;
> struct mutex fault_lock;
> + struct vfio_iommu_fault_info fault_info[NR_IOMMU_FAULT_INFO];
What if you run out of this space? Userspace will not
see any more faults. What will cause progress to happen?
> + struct blocking_notifier_head iommu_fault_notifier;
> + u8 fault_count;
> bool v2;
> bool nesting;
> };
> @@ -1456,6 +1461,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> iommu->dma_list = RB_ROOT;
> mutex_init(&iommu->lock);
> mutex_init(&iommu->fault_lock);
> + iommu->fault_count = 0;
> BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier);
>
> return iommu;
> @@ -1516,6 +1522,30 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
> return ret;
> }
>
> +static int vfio_iommu_fault_event_notifier(struct notifier_block *nb,
> + struct iommu_fault_info *fault_info,
> + void *data)
> +{
> + struct vfio_iommu *iommu = data;
> + struct vfio_iommu_fault_info *info;
> +
> + mutex_lock(&iommu->fault_lock);
> +
> + info = &iommu->fault_info[iommu->fault_count];
> + info->addr = fault_info->addr;
> + info->sid = fault_info->sid;
> + info->fault_reason = fault_info->fault_reason;
> + info->is_write = fault_info->is_write;
> +
> + iommu->fault_count++;
Will corrupt memory once array overflows NR_IOMMU_FAULT_INFO.
> +
> + if (iommu->iommu_fault_fd)
> + eventfd_signal(iommu->iommu_fault_fd, 1);
> +
> + mutex_unlock(&iommu->fault_lock);
> + return 0;
> +}
> +
> static long vfio_iommu_type1_ioctl(void *iommu_data,
> unsigned int cmd, unsigned long arg)
> {
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0ff5111..b6a7bdb 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -86,6 +86,13 @@ struct iommu_domain {
> void *iova_cookie;
> };
>
> +struct iommu_fault_info {
> + __u64 addr;
> + __u16 sid;
> + __u8 fault_reason;
> + __u8 is_write:1;
> +};
> +
> enum iommu_cap {
> IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA
> transactions */
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 8616334..da359dd 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -562,6 +562,13 @@ struct vfio_iommu_type1_set_fault_eventfd {
>
> #define VFIO_IOMMU_SET_FAULT_EVENTFD _IO(VFIO_TYPE, VFIO_BASE + 17)
>
> +struct vfio_iommu_fault_info {
> + __u64 addr;
> + __u16 sid;
It's not clear it's userspace's business to know the sid. It normally
does not care once management has bound vfio to a device. You should use
a device identifier that makes sense.
> + __u8 fault_reason;
> + __u8 is_write:1;
> +};
> +
> /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
>
> /*
> --
> 1.8.3.1
