[PATCH] iommu/vt-d fix adding non-PCI devices to Intel IOMMU
Starting with commit fa212a97f3a3 ("iommu/vt-d: Probe DMA-capable ACPI name space devices"), we now probe DMA-capable ACPI name space devices. On Dell XPS 13 9343, which has an Intel LPSS platform device INTL9C60 enumerated via ACPI, this change leads to the following warning: [ cut here ] WARNING: CPU: 1 PID: 1 at pci_device_group+0x11a/0x130 CPU: 1 PID: 1 Comm: swapper/0 Tainted: GT 5.5.0-rc3+ #22 Hardware name: Dell Inc. XPS 13 9343/0310JH, BIOS A20 06/06/2019 RIP: 0010:pci_device_group+0x11a/0x130 Code: f0 ff ff 48 85 c0 49 89 c4 75 c4 48 8d 74 24 10 48 89 ef e8 48 ef ff ff 48 85 c0 49 89 c4 75 af e8 db f7 ff ff 49 89 c4 eb a5 <0f> 0b 49 c7 c4 ea ff ff ff eb 9a e8 96 1e c7 ff 66 0f 1f 44 00 00 RSP: :c0d6c0043cb0 EFLAGS: 00010202 RAX: RBX: a3d1d43dd810 RCX: RDX: a3d1d4fecf80 RSI: a3d12943dcc0 RDI: a3d1d43dd810 RBP: a3d1d43dd810 R08: R09: a3d1d4c04a80 R10: a3d1d4c00880 R11: a3d1d44ba000 R12: R13: a3d1d4383b80 R14: a3d1d4c090d0 R15: a3d1d4324530 FS: () GS:a3d1d670() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 0460a001 CR4: 003606e0 Call Trace: ? iommu_group_get_for_dev+0x81/0x1f0 ? intel_iommu_add_device+0x61/0x170 ? iommu_probe_device+0x43/0xd0 ? intel_iommu_init+0x1fa2/0x2235 ? pci_iommu_init+0x52/0xe7 ? e820__memblock_setup+0x15c/0x15c ? do_one_initcall+0xcc/0x27e ? kernel_init_freeable+0x169/0x259 ? rest_init+0x95/0x95 ? kernel_init+0x5/0xeb ? ret_from_fork+0x35/0x40 ---[ end trace 28473e7abc25b92c ]--- DMAR: ACPI name space devices didn't probe correctly The bug results from the fact that while we now enumerate ACPI devices, we aren't able to handle any non-PCI device when generating the device group. Fix the issue by implementing an Intel-specific callback that returns `pci_device_group` only if the device is a PCI device. Otherwise, it will return a generic device group. Fixes: fa212a97f3a3 ("iommu/vt-d: Probe DMA-capable ACPI name space devices") Signed-off-by: Patrick Steinhardt --- I've recently spotted above warning in v5.5-rc3. The attached fix is rather intended as a discussion starter -- it's quite likely to be wrong as I ain't got much of a clue about the IOMMU subsystem. drivers/iommu/intel-iommu.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 42966611a192..e3696a754fd1 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -5972,6 +5972,13 @@ static bool intel_iommu_is_attach_deferred(struct iommu_domain *domain, return dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO; } +static struct iommu_group *intel_iommu_device_group(struct device *dev) +{ + if (dev_is_pci(dev)) + return pci_device_group(dev); + return generic_device_group(dev); +} + const struct iommu_ops intel_iommu_ops = { .capable= intel_iommu_capable, .domain_alloc = intel_iommu_domain_alloc, @@ -5989,7 +5996,7 @@ const struct iommu_ops intel_iommu_ops = { .get_resv_regions = intel_iommu_get_resv_regions, .put_resv_regions = intel_iommu_put_resv_regions, .apply_resv_region = intel_iommu_apply_resv_region, - .device_group = pci_device_group, + .device_group = intel_iommu_device_group, .dev_has_feat = intel_iommu_dev_has_feat, .dev_feat_enabled = intel_iommu_dev_feat_enabled, .dev_enable_feat= intel_iommu_dev_enable_feat, -- 2.24.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] iommu/arm-smmu: Expose s2cr and smr structs to impl
Move the arm_smmu_s2cr and arm_smmu_smr structs to the internal header file, in order to expose them to the platform specific arm-smmu implementations. Signed-off-by: Bjorn Andersson --- Changes since RFC: - New patch drivers/iommu/arm-smmu.c | 14 -- drivers/iommu/arm-smmu.h | 14 ++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 6ca6a4e072c8..9a9091b9dcc7 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -73,24 +73,10 @@ module_param(disable_bypass, bool, S_IRUGO); MODULE_PARM_DESC(disable_bypass, "Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU."); -struct arm_smmu_s2cr { - struct iommu_group *group; - int count; - enum arm_smmu_s2cr_type type; - enum arm_smmu_s2cr_privcfg privcfg; - u8 cbndx; -}; - #define s2cr_init_val (struct arm_smmu_s2cr){ \ .type = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS,\ } -struct arm_smmu_smr { - u16 mask; - u16 id; - boolvalid; -}; - struct arm_smmu_cb { u64 ttbr[2]; u32 tcr[2]; diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h index 62b9f0cec49b..73f94579b926 100644 --- a/drivers/iommu/arm-smmu.h +++ b/drivers/iommu/arm-smmu.h @@ -224,6 +224,20 @@ enum arm_smmu_implementation { QCOM_SMMUV2, }; +struct arm_smmu_s2cr { + struct iommu_group *group; + int count; + enum arm_smmu_s2cr_type type; + enum arm_smmu_s2cr_privcfg privcfg; + u8 cbndx; +}; + +struct arm_smmu_smr { + u16 mask; + u16 id; + boolvalid; +}; + struct arm_smmu_device { struct device *dev; -- 2.24.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/3] iommu/arm-smmu: Qualcomm bootsplash/efifb
These patches implements the stream mapping inheritance that's necessary in order to not hit a security violation as the display hardware looses its stream mapping during initialization of arm-smmu in various Qualcomm platforms. This was previously posted as an RFC [1], changes since then involves the rebase and migration of the read-back code to the Qualcomm specific implementation, the mapping is maintained indefinitely - to handle probe deferring clients - and rewritten commit messages. [1] https://lore.kernel.org/linux-arm-msm/20190605210856.20677-1-bjorn.anders...@linaro.org/ Bjorn Andersson (3): iommu/arm-smmu: Don't blindly use first SMR to calculate mask iommu/arm-smmu: Expose s2cr and smr structs to impl iommu/arm-smmu: Allow inherting stream mapping from bootloader drivers/iommu/arm-smmu-qcom.c | 35 ++ drivers/iommu/arm-smmu.c | 70 +++ drivers/iommu/arm-smmu.h | 15 3 files changed, 96 insertions(+), 24 deletions(-) -- 2.24.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] iommu/arm-smmu: Allow inherting stream mapping from bootloader
The Qualcomm bootloaders leaves the IOMMU with stream mapping for the display hardware to be able to read the framebuffer memory in DDR, to continuously display a boot splash or to implement EFI framebuffer. This patch implements support for implementations to pin stream mappings and adds the code to the Qualcomm implementation for reading out the stream mapping from the bootloader, with the result of maintaining the display hardware's access to DDR until the context bank is enabled. Heavily based on downstream implementation by Patrick Daly . Signed-off-by: Bjorn Andersson --- Changes since RFC: - Deal with EXIDS - The onetime handoff has been replaced with a "pinned" state, to deal with probe deferring in the display driver - Reads back s2cr for all groups, not only the "valid" ones drivers/iommu/arm-smmu-qcom.c | 35 +++ drivers/iommu/arm-smmu.c | 29 +++-- drivers/iommu/arm-smmu.h | 1 + 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c index 24c071c1d8b0..06e5799dcb87 100644 --- a/drivers/iommu/arm-smmu-qcom.c +++ b/drivers/iommu/arm-smmu-qcom.c @@ -3,6 +3,7 @@ * Copyright (c) 2019, The Linux Foundation. All rights reserved. */ +#include #include #include "arm-smmu.h" @@ -11,6 +12,39 @@ struct qcom_smmu { struct arm_smmu_device smmu; }; +static int qcom_sdm845_smmu500_cfg_probe(struct arm_smmu_device *smmu) +{ + u32 s2cr; + u32 smr; + int i; + + for (i = 0; i < smmu->num_mapping_groups; i++) { + smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i)); + s2cr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_S2CR(i)); + + smmu->smrs[i].mask = FIELD_GET(SMR_MASK, smr); + smmu->smrs[i].id = FIELD_GET(SMR_ID, smr); + if (smmu->features & ARM_SMMU_FEAT_EXIDS) + smmu->smrs[i].valid = FIELD_GET(S2CR_EXIDVALID, s2cr); + else + smmu->smrs[i].valid = FIELD_GET(SMR_VALID, smr); + + smmu->s2crs[i].group = NULL; + smmu->s2crs[i].count = 0; + smmu->s2crs[i].type = FIELD_GET(S2CR_TYPE, s2cr); + smmu->s2crs[i].privcfg = FIELD_GET(S2CR_PRIVCFG, s2cr); + smmu->s2crs[i].cbndx = FIELD_GET(S2CR_CBNDX, s2cr); + + if (!smmu->smrs[i].valid) + continue; + + smmu->s2crs[i].pinned = true; + bitmap_set(smmu->context_map, smmu->s2crs[i].cbndx, 1); + } + + return 0; +} + static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) { int ret; @@ -31,6 +65,7 @@ static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) } static const struct arm_smmu_impl qcom_smmu_impl = { + .cfg_probe = qcom_sdm845_smmu500_cfg_probe, .reset = qcom_sdm845_smmu500_reset, }; diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 9a9091b9dcc7..01f22eff2ec5 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -206,9 +206,19 @@ static int arm_smmu_register_legacy_master(struct device *dev, return err; } -static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end) +static int __arm_smmu_alloc_cb(struct arm_smmu_device *smmu, int start, + struct device *dev) { + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + unsigned long *map = smmu->context_map; + int end = smmu->num_context_banks; int idx; + int i; + + for_each_cfg_sme(fwspec, i, idx) { + if (smmu->s2crs[idx].pinned) + return smmu->s2crs[idx].cbndx; + } do { idx = find_next_zero_bit(map, end, start); @@ -628,7 +638,8 @@ static void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx) } static int arm_smmu_init_domain_context(struct iommu_domain *domain, - struct arm_smmu_device *smmu) + struct arm_smmu_device *smmu, + struct device *dev) { int irq, start, ret = 0; unsigned long ias, oas; @@ -742,8 +753,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain, ret = -EINVAL; goto out_unlock; } - ret = __arm_smmu_alloc_bitmap(smmu->context_map, start, - smmu->num_context_banks); + ret = __arm_smmu_alloc_cb(smmu, start, dev); if (ret < 0) goto out_unlock; @@ -1015,12 +1025,19 @@ static int arm_smmu_find_sme(struct arm_smmu_device *smmu, u16 id, u16 mask) static bool arm_smmu_free_sme(struct arm_smmu_device *smmu, int idx) { + bool pinned = smmu->s2crs[idx].pinned; + u8 cbndx = smmu->s2crs[idx].cbndx;; + if
[PATCH 1/3] iommu/arm-smmu: Don't blindly use first SMR to calculate mask
With the SMRs inherited from the bootloader the first SMR might actually be valid and in use. As such probing the SMR mask using the first SMR might break a stream in use. Search for an unused stream and use this to probe the SMR mask. Signed-off-by: Bjorn Andersson --- Changes since RFC: - Deal with EXIDS - Use arm_smmu_gr0_read/write() drivers/iommu/arm-smmu.c | 27 +++ 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 4f1a350d9529..6ca6a4e072c8 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -945,24 +945,43 @@ static void arm_smmu_write_sme(struct arm_smmu_device *smmu, int idx) */ static void arm_smmu_test_smr_masks(struct arm_smmu_device *smmu) { + u32 s2cr; u32 smr; + int idx; if (!smmu->smrs) return; + for (idx = 0; idx < smmu->num_mapping_groups; idx++) { + if (smmu->features & ARM_SMMU_FEAT_EXIDS) { + s2cr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_S2CR(idx)); + if (!FIELD_GET(S2CR_EXIDVALID, s2cr)) + break; + } else { + smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(idx)); + if (!FIELD_GET(SMR_VALID, smr)) + break; + } + } + + if (idx == smmu->num_mapping_groups) { + dev_err(smmu->dev, "Unable to compute streamid_mask\n"); + return; + } + /* * SMR.ID bits may not be preserved if the corresponding MASK * bits are set, so check each one separately. We can reject * masters later if they try to claim IDs outside these masks. */ smr = FIELD_PREP(SMR_ID, smmu->streamid_mask); - arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_SMR(0), smr); - smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(0)); + arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_SMR(idx), smr); + smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(idx)); smmu->streamid_mask = FIELD_GET(SMR_ID, smr); smr = FIELD_PREP(SMR_MASK, smmu->streamid_mask); - arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_SMR(0), smr); - smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(0)); + arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_SMR(idx), smr); + smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(idx)); smmu->smr_mask_mask = FIELD_GET(SMR_MASK, smr); } -- 2.24.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/arm-smmu-v3: fix resource_size check
This is an off-by-one mistake. resource_size() returns res->end - res->start + 1. Signed-off-by: Masahiro Yamada --- drivers/iommu/arm-smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index d9e0d9c19b4f..b463599559d2 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -3599,7 +3599,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev) /* Base address */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (resource_size(res) + 1 < arm_smmu_resource_size(smmu)) { + if (resource_size(res) < arm_smmu_resource_size(smmu)) { dev_err(dev, "MMIO region too small (%pr)\n", res); return -EINVAL; } -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] iommu/arm-smmu-v3: constify arm_smmu_options[]
This is a read-only array. Signed-off-by: Masahiro Yamada --- drivers/iommu/arm-smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index b463599559d2..ed9933960370 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -673,7 +673,7 @@ struct arm_smmu_option_prop { const char *prop; }; -static struct arm_smmu_option_prop arm_smmu_options[] = { +static const struct arm_smmu_option_prop arm_smmu_options[] = { { ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" }, { ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"}, { 0, NULL}, -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] iommu/arm-smmu-v3: simplify parse_driver_options()
Using ARRAY_SIZE() instead of the sentinel is slightly simpler, IMHO. Signed-off-by: Masahiro Yamada --- drivers/iommu/arm-smmu-v3.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index ed9933960370..b27489b7f9d8 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -676,7 +676,6 @@ struct arm_smmu_option_prop { static const struct arm_smmu_option_prop arm_smmu_options[] = { { ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" }, { ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"}, - { 0, NULL}, }; static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset, @@ -696,16 +695,16 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom) static void parse_driver_options(struct arm_smmu_device *smmu) { - int i = 0; + int i; - do { + for (i = 0; i < ARRAY_SIZE(arm_smmu_options); i++) { if (of_property_read_bool(smmu->dev->of_node, arm_smmu_options[i].prop)) { smmu->options |= arm_smmu_options[i].opt; dev_notice(smmu->dev, "option %s\n", arm_smmu_options[i].prop); } - } while (arm_smmu_options[++i].opt); + }; } /* Low-level queue manipulation functions */ -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu