| Message ID | 20210125110650.3232195-1-jean-philippe@linaro.org |
|---|---|
| Headers | show |
| Series | iommu: I/O page faults for SMMUv3 | expand |
On Mon, Jan 25, 2021 at 01:50:09PM +0000, Jonathan Cameron wrote: > > +static int arm_smmu_master_sva_enable_iopf(struct arm_smmu_master *master) > > +{ > > + int ret; > > + struct device *dev = master->dev; > > + > > + /* > > + * 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)) > > So if we have master->iopf_enabled and this happens. Then I'm not totally sure > what prevents the disable below running its cleanup on stuff that was never > configured. Since arm_smmu_dev_enable_feature() checks that the feature is supported, iopf_enabled can only be true if arm_smmu_master_iopf_supported() is true. What's missing is checking that drivers don't disable IOPF while SVA is enabled - or else the disable below can leak. Another thing I broke in v10 :/ Thanks, Jean > > > + return 0; > > + > > + if (!master->iopf_enabled) > > + return -EINVAL; > > + > > + ret = iopf_queue_add_device(master->smmu->evtq.iopf, dev); > > + if (ret) > > + return ret; > > + > > + ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, dev); > > + if (ret) { > > + iopf_queue_remove_device(master->smmu->evtq.iopf, dev); > > + return ret; > > + } > > + return 0; > > +} > > + > > +static void arm_smmu_master_sva_disable_iopf(struct arm_smmu_master *master) > > +{ > > + struct device *dev = master->dev; > > + > > + if (!master->iopf_enabled) > > + return; > > As above, I think you need a sanity check on > > !arm_smmu_master_iopf_supported(master) before clearing the following. > > I may well be missing something that stops us getting here though. > > Alternative is probably to sanity check iopf_enabled = true is supported > before letting a driver set it. > > > > + > > + iommu_unregister_device_fault_handler(dev); > > + iopf_queue_remove_device(master->smmu->evtq.iopf, dev); > > +}
Add stall support to the SMMUv3, along with a common I/O Page Fault handler. This version fixes a typo introduced in v10 [1] and adds review tags (thanks!) You can find the range diff for v10->v11 below. [1] https://lore.kernel.org/linux-iommu/20210121123623.2060416-1-jean-philippe@linaro.org/ Jean-Philippe Brucker (10): iommu: Fix comment for struct iommu_fwspec iommu/arm-smmu-v3: Use device properties for pasid-num-bits iommu: Separate IOMMU_DEV_FEAT_IOPF from IOMMU_DEV_FEAT_SVA iommu/vt-d: Support IOMMU_DEV_FEAT_IOPF uacce: Enable IOMMU_DEV_FEAT_IOPF iommu: Add a page fault handler iommu/arm-smmu-v3: Maintain a SID->device structure dt-bindings: document stall property for IOMMU masters ACPI/IORT: Enable stall support for platform devices iommu/arm-smmu-v3: Add stall support for platform devices drivers/iommu/Makefile | 1 + .../devicetree/bindings/iommu/iommu.txt | 18 + drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 56 ++- drivers/iommu/iommu-sva-lib.h | 53 ++ include/linux/iommu.h | 26 +- drivers/acpi/arm64/iort.c | 15 +- .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 59 ++- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 349 +++++++++++-- drivers/iommu/intel/iommu.c | 11 +- drivers/iommu/io-pgfault.c | 461 ++++++++++++++++++ drivers/iommu/of_iommu.c | 5 - drivers/misc/uacce/uacce.c | 39 +- 12 files changed, 1019 insertions(+), 74 deletions(-) create mode 100644 drivers/iommu/io-pgfault.c -- @@ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c: static void arm_smmu_release_device(struct device *dev) master = dev_iommu_priv_get(dev); - WARN_ON(arm_smmu_master_sva_enabled(master)); -+ iopf_queue_remove_device(master->smmu->evtq.iopf, dev); +- WARN_ON(arm_smmu_master_sva_enabled(master)); ++ if (WARN_ON(arm_smmu_master_sva_enabled(master))) ++ iopf_queue_remove_device(master->smmu->evtq.iopf, dev); arm_smmu_detach_dev(master); arm_smmu_disable_pasid(master); arm_smmu_remove_master(master); @@ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c: static struct iommu_ops arm_smmu_op .pgsize_bitmap = -1UL, /* Restricted during device attach */ }; -@@ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c: static int arm_smmu_cmdq_init(struct arm_smmu_device *smmu) - static int arm_smmu_init_queues(struct arm_smmu_device *smmu) - { - int ret; -+ bool sva = smmu->features & ARM_SMMU_FEAT_STALLS; - - /* cmdq */ - ret = arm_smmu_init_one_queue(smmu, &smmu->cmdq.q, ARM_SMMU_CMDQ_PROD, @@ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c: static int arm_smmu_init_queues(struct arm_smmu_device *smmu) if (ret) return ret; -+ if (sva && smmu->features & ARM_SMMU_FEAT_STALLS) { ++ if ((smmu->features & ARM_SMMU_FEAT_SVA) && ++ (smmu->features & ARM_SMMU_FEAT_STALLS)) { + smmu->evtq.iopf = iopf_queue_alloc(dev_name(smmu->dev)); + if (!smmu->evtq.iopf) + return -ENOMEM;