On 2017年02月21日 04:53, Alex Williamson wrote:
> On Sun, 19 Feb 2017 22:47:08 +0800
> Lan Tianyu <tianyu.lan@xxxxxxxxx> 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
>
> How is this value determined?
The length of fault info array is defined by manually.
It's hard to estimate how many fault info entry will be queued in the
array.
>
>> +
>> 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];
>> + 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++;
>> +
>> + if (iommu->iommu_fault_fd)
>> + eventfd_signal(iommu->iommu_fault_fd, 1);
>> +
>> + mutex_unlock(&iommu->fault_lock);
>> + return 0;
>> +}
>
>
> This notifier is never registered as any sort of IOMMU fault reporting
> infrastructure is currently imaginary, and the iommu development list
> isn't cc'd.
Yes, I should cc IOMM maillist. I thought this patchset was to determine
new VFIO API and so...
>
>> +
>> 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;
>> +};
>
>
> A common fault reporting structure would need to be agreed upon by all
> parties, ie. everyone that current supporting the IOMMU API. As used
> in QEMU, the fault_reason here appears to be very VT-d specific.
Fault reason number will be platform specific and it'd hard to use a
common definition to show fault reason number of all platforms. I wonder
whether we can define a vendor field in the fault info and
consumer can use the vendor field to deal with platform specific info.
--
Best regards
Tianyu Lan
