tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

iommu/arm-smmu-v3: Add update_safe bits to fix STE update sequence

C_BAD_STE was observed when updating nested STE from an S1-bypass mode to
an S1DSS-bypass mode. As both modes enabled S2, the used bit is slightly
different than the normal S1-bypass and S1DSS-bypass modes. As a result,
fields like MEV and EATS in S2's used list marked the word1 as a critical
word that requested a STE.V=0. This breaks a hitless update.

However, both MEV and EATS aren't critical in terms of STE update. One
controls the merge of the events and the other controls the ATS that is
managed by the driver at the same time via pci_enable_ats().

Add an arm_smmu_get_ste_update_safe() to allow STE update algorithm to
relax those fields, avoiding the STE update breakages.

After this change, entry_set has no caller checking its return value, so
change it to void.

Note that this change is required by both MEV and EATS fields, which were
introduced in different kernel versions. So add get_update_safe() first.
MEV and EATS will be added to arm_smmu_get_ste_update_safe() separately.

Fixes: 1e8be08d1c91 ("iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED")
Cc: stable@vger.kernel.org
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>

authored by

Jason Gunthorpe and committed by
Will Deacon 2781f2a9 ea69dc4e

+53 -10
+28 -3
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c
··· 38 38 static bool arm_smmu_entry_differs_in_used_bits(const __le64 *entry, 39 39 const __le64 *used_bits, 40 40 const __le64 *target, 41 + const __le64 *safe, 41 42 unsigned int length) 42 43 { 43 44 bool differs = false; 44 45 unsigned int i; 45 46 46 47 for (i = 0; i < length; i++) { 47 - if ((entry[i] & used_bits[i]) != target[i]) 48 + __le64 used = used_bits[i] & ~safe[i]; 49 + 50 + if ((entry[i] & used) != (target[i] & used)) 48 51 differs = true; 49 52 } 50 53 return differs; ··· 59 56 struct arm_smmu_test_writer *test_writer = 60 57 container_of(writer, struct arm_smmu_test_writer, writer); 61 58 __le64 *entry_used_bits; 59 + __le64 *safe_target; 60 + __le64 *safe_init; 62 61 63 62 entry_used_bits = kunit_kzalloc( 64 63 test_writer->test, sizeof(*entry_used_bits) * NUM_ENTRY_QWORDS, 65 64 GFP_KERNEL); 66 65 KUNIT_ASSERT_NOT_NULL(test_writer->test, entry_used_bits); 66 + 67 + safe_target = kunit_kzalloc(test_writer->test, 68 + sizeof(*safe_target) * NUM_ENTRY_QWORDS, 69 + GFP_KERNEL); 70 + KUNIT_ASSERT_NOT_NULL(test_writer->test, safe_target); 71 + 72 + safe_init = kunit_kzalloc(test_writer->test, 73 + sizeof(*safe_init) * NUM_ENTRY_QWORDS, 74 + GFP_KERNEL); 75 + KUNIT_ASSERT_NOT_NULL(test_writer->test, safe_init); 67 76 68 77 pr_debug("STE value is now set to: "); 69 78 print_hex_dump_debug(" ", DUMP_PREFIX_NONE, 16, 8, ··· 94 79 * configuration. 95 80 */ 96 81 writer->ops->get_used(test_writer->entry, entry_used_bits); 82 + if (writer->ops->get_update_safe) 83 + writer->ops->get_update_safe(test_writer->entry, 84 + test_writer->init_entry, 85 + safe_init); 86 + if (writer->ops->get_update_safe) 87 + writer->ops->get_update_safe(test_writer->entry, 88 + test_writer->target_entry, 89 + safe_target); 97 90 KUNIT_EXPECT_FALSE( 98 91 test_writer->test, 99 92 arm_smmu_entry_differs_in_used_bits( 100 93 test_writer->entry, entry_used_bits, 101 - test_writer->init_entry, NUM_ENTRY_QWORDS) && 94 + test_writer->init_entry, safe_init, 95 + NUM_ENTRY_QWORDS) && 102 96 arm_smmu_entry_differs_in_used_bits( 103 97 test_writer->entry, entry_used_bits, 104 - test_writer->target_entry, 98 + test_writer->target_entry, safe_target, 105 99 NUM_ENTRY_QWORDS)); 106 100 } 107 101 } ··· 130 106 static const struct arm_smmu_entry_writer_ops test_ste_ops = { 131 107 .sync = arm_smmu_test_writer_record_syncs, 132 108 .get_used = arm_smmu_get_ste_used, 109 + .get_update_safe = arm_smmu_get_ste_update_safe, 133 110 }; 134 111 135 112 static const struct arm_smmu_entry_writer_ops test_cd_ops = {
+21 -7
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
··· 1093 1093 } 1094 1094 EXPORT_SYMBOL_IF_KUNIT(arm_smmu_get_ste_used); 1095 1095 1096 + VISIBLE_IF_KUNIT 1097 + void arm_smmu_get_ste_update_safe(const __le64 *cur, const __le64 *target, 1098 + __le64 *safe_bits) 1099 + { 1100 + } 1101 + EXPORT_SYMBOL_IF_KUNIT(arm_smmu_get_ste_update_safe); 1102 + 1096 1103 /* 1097 1104 * Figure out if we can do a hitless update of entry to become target. Returns a 1098 1105 * bit mask where 1 indicates that qword needs to be set disruptively. ··· 1112 1105 { 1113 1106 __le64 target_used[NUM_ENTRY_QWORDS] = {}; 1114 1107 __le64 cur_used[NUM_ENTRY_QWORDS] = {}; 1108 + __le64 safe[NUM_ENTRY_QWORDS] = {}; 1115 1109 u8 used_qword_diff = 0; 1116 1110 unsigned int i; 1117 1111 1118 1112 writer->ops->get_used(entry, cur_used); 1119 1113 writer->ops->get_used(target, target_used); 1114 + if (writer->ops->get_update_safe) 1115 + writer->ops->get_update_safe(entry, target, safe); 1120 1116 1121 1117 for (i = 0; i != NUM_ENTRY_QWORDS; i++) { 1118 + /* 1119 + * Safe is only used for bits that are used by both entries, 1120 + * otherwise it is sequenced according to the unused entry. 1121 + */ 1122 + safe[i] &= target_used[i] & cur_used[i]; 1123 + 1122 1124 /* 1123 1125 * Check that masks are up to date, the make functions are not 1124 1126 * allowed to set a bit to 1 if the used function doesn't say it ··· 1136 1120 WARN_ON_ONCE(target[i] & ~target_used[i]); 1137 1121 1138 1122 /* Bits can change because they are not currently being used */ 1123 + cur_used[i] &= ~safe[i]; 1139 1124 unused_update[i] = (entry[i] & cur_used[i]) | 1140 1125 (target[i] & ~cur_used[i]); 1141 1126 /* ··· 1149 1132 return used_qword_diff; 1150 1133 } 1151 1134 1152 - static bool entry_set(struct arm_smmu_entry_writer *writer, __le64 *entry, 1135 + static void entry_set(struct arm_smmu_entry_writer *writer, __le64 *entry, 1153 1136 const __le64 *target, unsigned int start, 1154 1137 unsigned int len) 1155 1138 { ··· 1165 1148 1166 1149 if (changed) 1167 1150 writer->ops->sync(writer); 1168 - return changed; 1169 1151 } 1170 1152 1171 1153 /* ··· 1234 1218 entry_set(writer, entry, target, 0, 1); 1235 1219 } else { 1236 1220 /* 1237 - * No inuse bit changed. Sanity check that all unused bits are 0 1238 - * in the entry. The target was already sanity checked by 1239 - * compute_qword_diff(). 1221 + * No inuse bit changed, though safe bits may have changed. 1240 1222 */ 1241 - WARN_ON_ONCE( 1242 - entry_set(writer, entry, target, 0, NUM_ENTRY_QWORDS)); 1223 + entry_set(writer, entry, target, 0, NUM_ENTRY_QWORDS); 1243 1224 } 1244 1225 } 1245 1226 EXPORT_SYMBOL_IF_KUNIT(arm_smmu_write_entry); ··· 1567 1554 static const struct arm_smmu_entry_writer_ops arm_smmu_ste_writer_ops = { 1568 1555 .sync = arm_smmu_ste_writer_sync_entry, 1569 1556 .get_used = arm_smmu_get_ste_used, 1557 + .get_update_safe = arm_smmu_get_ste_update_safe, 1570 1558 }; 1571 1559 1572 1560 static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid,
+4
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
··· 898 898 899 899 struct arm_smmu_entry_writer_ops { 900 900 void (*get_used)(const __le64 *entry, __le64 *used); 901 + void (*get_update_safe)(const __le64 *cur, const __le64 *target, 902 + __le64 *safe_bits); 901 903 void (*sync)(struct arm_smmu_entry_writer *writer); 902 904 }; 903 905 ··· 911 909 912 910 #if IS_ENABLED(CONFIG_KUNIT) 913 911 void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits); 912 + void arm_smmu_get_ste_update_safe(const __le64 *cur, const __le64 *target, 913 + __le64 *safe_bits); 914 914 void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer, __le64 *cur, 915 915 const __le64 *target); 916 916 void arm_smmu_get_cd_used(const __le64 *ent, __le64 *used_bits);