On 2023/2/6 11:28, Tian, Kevin wrote:
From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
Sent: Friday, February 3, 2023 4:45 PM
Generally enabling IOMMU_DEV_FEAT_SVA requires
IOMMU_DEV_FEAT_IOPF, but
some devices manage I/O Page Faults themselves instead of relying on the
IOMMU. Move IOPF related code from SVA to IOPF enabling path to make
the
driver work for devices that manage IOPF themselves.
For the device drivers that relies on the IOMMU for IOPF through PCI/PRI,
IOMMU_DEV_FEAT_IOPF must be enabled before and disabled after
IOMMU_DEV_FEAT_SVA.
ARM still handles this differently:
arm_smmu_master_enable_sva()
arm_smmu_master_sva_enable_iopf():
{
/*
* Drivers for devices supporting PRI or stall should enable IOPF first.
* Others have device-specific fault handlers and don't need IOPF.
*/
if (!arm_smmu_master_iopf_supported(master))
return 0;
if (!master->iopf_enabled)
return -EINVAL;
}
i.e. device specific IOPF is allowed only when PRI or stall is not supported.
it's different from what this patch does to allow device specific IOPF even
when PRI is supported.
should we make them consistent given SVA/IOPF capabilities are general
iommu definitions or fine to leave each iommu driver with different
restriction?
Good point! I prefer the former. I will add a check in sva enabling path
and return failure if device supports PRI but not enabled (that
implies device has its specific IOPF handling).
- ret = iopf_queue_add_device(iommu->iopf_queue, dev);
- if (!ret)
- ret = iommu_register_device_fault_handler(dev,
iommu_queue_iopf, dev);
-
- return ret;
+ return 0;
}
here and below...
+ ret = iopf_queue_add_device(info->iommu->iopf_queue, dev);
+ if (ret)
+ return ret;
+
+ ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf,
dev);
+ if (ret)
+ iopf_queue_remove_device(info->iommu->iopf_queue, dev);
+
+ return ret;
}
...indicate a bug fix on error handling. better to have the fix as
a separate patch and then move code.
Yes. I will post a fix patch before this move.
Best regards,
baolu