* [PATCH AUTOSEL 6.11 120/244] iommu/arm-smmu-qcom: hide last LPASS SMMU context bank from linux
[not found] <20240925113641.1297102-1-sashal@kernel.org>
@ 2024-09-25 11:25 ` Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 121/244] iommu/arm-smmu-qcom: Work around SDM845 Adreno SMMU w/ 16K pages Sasha Levin
` (5 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-09-25 11:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Marc Gonzalez, Caleb Connolly, Bjorn Andersson, Will Deacon,
Sasha Levin, robdclark, joro, iommu, linux-arm-msm,
linux-arm-kernel
From: Marc Gonzalez <mgonzalez@freebox.fr>
[ Upstream commit 3a8990b8a778219327c5f8ecf10b5d81377b925a ]
On qcom msm8998, writing to the last context bank of lpass_q6_smmu
(base address 0x05100000) produces a system freeze & reboot.
The hardware/hypervisor reports 13 context banks for the LPASS SMMU
on msm8998, but only the first 12 are accessible...
Override the number of context banks
[ 2.546101] arm-smmu 5100000.iommu: probing hardware configuration...
[ 2.552439] arm-smmu 5100000.iommu: SMMUv2 with:
[ 2.558945] arm-smmu 5100000.iommu: stage 1 translation
[ 2.563627] arm-smmu 5100000.iommu: address translation ops
[ 2.568923] arm-smmu 5100000.iommu: non-coherent table walk
[ 2.574566] arm-smmu 5100000.iommu: (IDR0.CTTW overridden by FW configuration)
[ 2.580220] arm-smmu 5100000.iommu: stream matching with 12 register groups
[ 2.587263] arm-smmu 5100000.iommu: 13 context banks (0 stage-2 only)
[ 2.614447] arm-smmu 5100000.iommu: Supported page sizes: 0x63315000
[ 2.621358] arm-smmu 5100000.iommu: Stage-1: 36-bit VA -> 36-bit IPA
[ 2.627772] arm-smmu 5100000.iommu: preserved 0 boot mappings
Specifically, the crashes occur here:
qsmmu->bypass_cbndx = smmu->num_context_banks - 1;
arm_smmu_cb_write(smmu, qsmmu->bypass_cbndx, ARM_SMMU_CB_SCTLR, 0);
and here:
arm_smmu_write_context_bank(smmu, i);
arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_FSR, ARM_SMMU_CB_FSR_FAULT);
It is likely that FW reserves the last context bank for its own use,
thus a simple work-around is: DON'T USE IT in Linux.
If we decrease the number of context banks, last one will be "hidden".
Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20240820-smmu-v3-1-2f71483b00ec@freebox.fr
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 36c6b36ad4ff7..b981ff25a983d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -282,6 +282,13 @@ static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
u32 smr;
int i;
+ /*
+ * MSM8998 LPASS SMMU reports 13 context banks, but accessing
+ * the last context bank crashes the system.
+ */
+ if (of_device_is_compatible(smmu->dev->of_node, "qcom,msm8998-smmu-v2") && smmu->num_context_banks == 13)
+ smmu->num_context_banks = 12;
+
/*
* Some platforms support more than the Arm SMMU architected maximum of
* 128 stream matching groups. For unknown reasons, the additional
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.11 121/244] iommu/arm-smmu-qcom: Work around SDM845 Adreno SMMU w/ 16K pages
[not found] <20240925113641.1297102-1-sashal@kernel.org>
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 120/244] iommu/arm-smmu-qcom: hide last LPASS SMMU context bank from linux Sasha Levin
@ 2024-09-25 11:25 ` Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 123/244] iommu/arm-smmu-v3: Match Stall behaviour for S2 Sasha Levin
` (4 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-09-25 11:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Konrad Dybcio, Sumit Semwal, Will Deacon, Sasha Levin, robdclark,
joro, iommu, linux-arm-msm, linux-arm-kernel
From: Konrad Dybcio <konrad.dybcio@linaro.org>
[ Upstream commit 2d42d3ba443706c9164fa0bef4e5fd1c36bc1bd9 ]
SDM845's Adreno SMMU is unique in that it actually advertizes support
for 16K (and 32M) pages, which doesn't hold for newer SoCs.
This however, seems either broken in the hardware implementation, the
hypervisor middleware that abstracts the SMMU, or there's a bug in the
Linux kernel somewhere down the line that nobody managed to track down.
Booting SDM845 with 16K page sizes and drm/msm results in:
*** gpu fault: ttbr0=0000000000000000 iova=000100000000c000 dir=READ
type=TRANSLATION source=CP (0,0,0,0)
right after loading the firmware. The GPU then starts spitting out
illegal intstruction errors, as it's quite obvious that it got a
bogus pointer.
Moreover, it seems like this issue also concerns other implementations
of SMMUv2 on Qualcomm SoCs, such as the one on SC7180.
Hide 16K support on such instances to work around this.
Reported-by: Sumit Semwal <sumit.semwal@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Link: https://lore.kernel.org/r/20240824-topic-845_gpu_smmu-v2-1-a302b8acc052@quicinc.com
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index b981ff25a983d..087fb4f6f4d3d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -345,6 +345,14 @@ static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
return 0;
}
+static int qcom_adreno_smmuv2_cfg_probe(struct arm_smmu_device *smmu)
+{
+ /* Support for 16K pages is advertised on some SoCs, but it doesn't seem to work */
+ smmu->features &= ~ARM_SMMU_FEAT_FMT_AARCH64_16K;
+
+ return 0;
+}
+
static void qcom_smmu_write_s2cr(struct arm_smmu_device *smmu, int idx)
{
struct arm_smmu_s2cr *s2cr = smmu->s2crs + idx;
@@ -443,6 +451,7 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
.init_context = qcom_adreno_smmu_init_context,
+ .cfg_probe = qcom_adreno_smmuv2_cfg_probe,
.def_domain_type = qcom_smmu_def_domain_type,
.alloc_context_bank = qcom_adreno_smmu_alloc_context_bank,
.write_sctlr = qcom_adreno_smmu_write_sctlr,
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.11 123/244] iommu/arm-smmu-v3: Match Stall behaviour for S2
[not found] <20240925113641.1297102-1-sashal@kernel.org>
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 120/244] iommu/arm-smmu-qcom: hide last LPASS SMMU context bank from linux Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 121/244] iommu/arm-smmu-qcom: Work around SDM845 Adreno SMMU w/ 16K pages Sasha Levin
@ 2024-09-25 11:25 ` Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 124/244] iommu/vt-d: Always reserve a domain ID for identity setup Sasha Levin
` (3 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-09-25 11:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mostafa Saleh, Will Deacon, Sasha Levin, joro, jgg, nicolinc,
mshavit, baolu.lu, jsnitsel, linux-arm-kernel, iommu
From: Mostafa Saleh <smostafa@google.com>
[ Upstream commit ce7cb08e22e09f43649b025c849a3ae3b80833c4 ]
According to the spec (ARM IHI 0070 F.b), in
"5.5 Fault configuration (A, R, S bits)":
A STE with stage 2 translation enabled and STE.S2S == 0 is
considered ILLEGAL if SMMU_IDR0.STALL_MODEL == 0b10.
Also described in the pseudocode “SteIllegal()”
if STE.Config == '11x' then
[..]
if eff_idr0_stall_model == '10' && STE.S2S == '0' then
// stall_model forcing stall, but S2S == 0
return TRUE;
Which means, S2S must be set when stall model is
"ARM_SMMU_FEAT_STALL_FORCE", but currently the driver ignores that.
Although, the driver can do the minimum and only set S2S for
“ARM_SMMU_FEAT_STALL_FORCE”, it is more consistent to match S1
behaviour, which also sets it for “ARM_SMMU_FEAT_STALL” if the
master has requested stalls.
Also, since S2 stalls are enabled now, report them to the IOMMU layer
and for VFIO devices it will fail anyway as VFIO doesn’t register an
iopf handler.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Link: https://lore.kernel.org/r/20240830110349.797399-2-smostafa@google.com
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +++-----
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 +
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index ed2b106e02dd1..a7d2a6b8a4980 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1012,7 +1012,8 @@ void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits)
used_bits[2] |=
cpu_to_le64(STRTAB_STE_2_S2VMID | STRTAB_STE_2_VTCR |
STRTAB_STE_2_S2AA64 | STRTAB_STE_2_S2ENDI |
- STRTAB_STE_2_S2PTW | STRTAB_STE_2_S2R);
+ STRTAB_STE_2_S2PTW | STRTAB_STE_2_S2S |
+ STRTAB_STE_2_S2R);
used_bits[3] |= cpu_to_le64(STRTAB_STE_3_S2TTB_MASK);
}
@@ -1646,6 +1647,7 @@ void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
STRTAB_STE_2_S2ENDI |
#endif
STRTAB_STE_2_S2PTW |
+ (master->stall_enabled ? STRTAB_STE_2_S2S : 0) |
STRTAB_STE_2_S2R);
target->data[3] = cpu_to_le64(pgtbl_cfg->arm_lpae_s2_cfg.vttbr &
@@ -1739,10 +1741,6 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
return -EOPNOTSUPP;
}
- /* Stage-2 is always pinned at the moment */
- if (evt[1] & EVTQ_1_S2)
- return -EFAULT;
-
if (!(evt[1] & EVTQ_1_STALL))
return -EOPNOTSUPP;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 14bca41a981b4..0dc7ad43c64c0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -267,6 +267,7 @@ struct arm_smmu_ste {
#define STRTAB_STE_2_S2AA64 (1UL << 51)
#define STRTAB_STE_2_S2ENDI (1UL << 52)
#define STRTAB_STE_2_S2PTW (1UL << 54)
+#define STRTAB_STE_2_S2S (1UL << 57)
#define STRTAB_STE_2_S2R (1UL << 58)
#define STRTAB_STE_3_S2TTB_MASK GENMASK_ULL(51, 4)
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.11 124/244] iommu/vt-d: Always reserve a domain ID for identity setup
[not found] <20240925113641.1297102-1-sashal@kernel.org>
` (2 preceding siblings ...)
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 123/244] iommu/arm-smmu-v3: Match Stall behaviour for S2 Sasha Levin
@ 2024-09-25 11:25 ` Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 125/244] iommu/vt-d: Fix potential lockup if qi_submit_sync called with 0 count Sasha Levin
` (2 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-09-25 11:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Lu Baolu, Jason Gunthorpe, Kevin Tian, Jerry Snitselaar,
Joerg Roedel, Sasha Levin, dwmw2, joro, will, iommu
From: Lu Baolu <baolu.lu@linux.intel.com>
[ Upstream commit 2c13012e09190174614fd6901857a1b8c199e17d ]
We will use a global static identity domain. Reserve a static domain ID
for it.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Link: https://lore.kernel.org/r/20240809055431.36513-4-baolu.lu@linux.intel.com
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iommu/intel/iommu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 4aa070cf56e70..e3e513cabc86a 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1447,10 +1447,10 @@ static int iommu_init_domains(struct intel_iommu *iommu)
* entry for first-level or pass-through translation modes should
* be programmed with a domain id different from those used for
* second-level or nested translation. We reserve a domain id for
- * this purpose.
+ * this purpose. This domain id is also used for identity domain
+ * in legacy mode.
*/
- if (sm_supported(iommu))
- set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
+ set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.11 125/244] iommu/vt-d: Fix potential lockup if qi_submit_sync called with 0 count
[not found] <20240925113641.1297102-1-sashal@kernel.org>
` (3 preceding siblings ...)
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 124/244] iommu/vt-d: Always reserve a domain ID for identity setup Sasha Levin
@ 2024-09-25 11:25 ` Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 126/244] iommu/vt-d: Unconditionally flush device TLB for pasid table updates Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 127/244] iommu/arm-smmu-v3: Do not use devm for the cd table allocations Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-09-25 11:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sanjay K Kumar, Jacob Pan, Kevin Tian, Lu Baolu, Joerg Roedel,
Sasha Levin, dwmw2, joro, will, iommu
From: Sanjay K Kumar <sanjay.k.kumar@intel.com>
[ Upstream commit 3cf74230c139f208b7fb313ae0054386eee31a81 ]
If qi_submit_sync() is invoked with 0 invalidation descriptors (for
instance, for DMA draining purposes), we can run into a bug where a
submitting thread fails to detect the completion of invalidation_wait.
Subsequently, this led to a soft lockup. Currently, there is no impact
by this bug on the existing users because no callers are submitting
invalidations with 0 descriptors. This fix will enable future users
(such as DMA drain) calling qi_submit_sync() with 0 count.
Suppose thread T1 invokes qi_submit_sync() with non-zero descriptors, while
concurrently, thread T2 calls qi_submit_sync() with zero descriptors. Both
threads then enter a while loop, waiting for their respective descriptors
to complete. T1 detects its completion (i.e., T1's invalidation_wait status
changes to QI_DONE by HW) and proceeds to call reclaim_free_desc() to
reclaim all descriptors, potentially including adjacent ones of other
threads that are also marked as QI_DONE.
During this time, while T2 is waiting to acquire the qi->q_lock, the IOMMU
hardware may complete the invalidation for T2, setting its status to
QI_DONE. However, if T1's execution of reclaim_free_desc() frees T2's
invalidation_wait descriptor and changes its status to QI_FREE, T2 will
not observe the QI_DONE status for its invalidation_wait and will
indefinitely remain stuck.
This soft lockup does not occur when only non-zero descriptors are
submitted.In such cases, invalidation descriptors are interspersed among
wait descriptors with the status QI_IN_USE, acting as barriers. These
barriers prevent the reclaim code from mistakenly freeing descriptors
belonging to other submitters.
Considered the following example timeline:
T1 T2
========================================
ID1
WD1
while(WD1!=QI_DONE)
unlock
lock
WD1=QI_DONE* WD2
while(WD2!=QI_DONE)
unlock
lock
WD1==QI_DONE?
ID1=QI_DONE WD2=DONE*
reclaim()
ID1=FREE
WD1=FREE
WD2=FREE
unlock
soft lockup! T2 never sees QI_DONE in WD2
Where:
ID = invalidation descriptor
WD = wait descriptor
* Written by hardware
The root of the problem is that the descriptor status QI_DONE flag is used
for two conflicting purposes:
1. signal a descriptor is ready for reclaim (to be freed)
2. signal by the hardware that a wait descriptor is complete
The solution (in this patch) is state separation by using QI_FREE flag
for #1.
Once a thread's invalidation descriptors are complete, their status would
be set to QI_FREE. The reclaim_free_desc() function would then only
free descriptors marked as QI_FREE instead of those marked as
QI_DONE. This change ensures that T2 (from the previous example) will
correctly observe the completion of its invalidation_wait (marked as
QI_DONE).
Signed-off-by: Sanjay K Kumar <sanjay.k.kumar@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Link: https://lore.kernel.org/r/20240728210059.1964602-1-jacob.jun.pan@linux.intel.com
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iommu/intel/dmar.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 1c8d3141cb55c..01e157d89a163 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1204,9 +1204,7 @@ static void free_iommu(struct intel_iommu *iommu)
*/
static inline void reclaim_free_desc(struct q_inval *qi)
{
- while (qi->desc_status[qi->free_tail] == QI_DONE ||
- qi->desc_status[qi->free_tail] == QI_ABORT) {
- qi->desc_status[qi->free_tail] = QI_FREE;
+ while (qi->desc_status[qi->free_tail] == QI_FREE && qi->free_tail != qi->free_head) {
qi->free_tail = (qi->free_tail + 1) % QI_LENGTH;
qi->free_cnt++;
}
@@ -1463,8 +1461,16 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
raw_spin_lock(&qi->q_lock);
}
- for (i = 0; i < count; i++)
- qi->desc_status[(index + i) % QI_LENGTH] = QI_DONE;
+ /*
+ * The reclaim code can free descriptors from multiple submissions
+ * starting from the tail of the queue. When count == 0, the
+ * status of the standalone wait descriptor at the tail of the queue
+ * must be set to QI_FREE to allow the reclaim code to proceed.
+ * It is also possible that descriptors from one of the previous
+ * submissions has to be reclaimed by a subsequent submission.
+ */
+ for (i = 0; i <= count; i++)
+ qi->desc_status[(index + i) % QI_LENGTH] = QI_FREE;
reclaim_free_desc(qi);
raw_spin_unlock_irqrestore(&qi->q_lock, flags);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.11 126/244] iommu/vt-d: Unconditionally flush device TLB for pasid table updates
[not found] <20240925113641.1297102-1-sashal@kernel.org>
` (4 preceding siblings ...)
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 125/244] iommu/vt-d: Fix potential lockup if qi_submit_sync called with 0 count Sasha Levin
@ 2024-09-25 11:25 ` Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 127/244] iommu/arm-smmu-v3: Do not use devm for the cd table allocations Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-09-25 11:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Lu Baolu, Yi Liu, Joerg Roedel, Sasha Levin, dwmw2, joro, will,
iommu
From: Lu Baolu <baolu.lu@linux.intel.com>
[ Upstream commit 1f5e307ca16c0c19186cbd56ac460a687e6daba0 ]
The caching mode of an IOMMU is irrelevant to the behavior of the device
TLB. Previously, commit <304b3bde24b5> ("iommu/vt-d: Remove caching mode
check before device TLB flush") removed this redundant check in the
domain unmap path.
Checking the caching mode before flushing the device TLB after a pasid
table entry is updated is unnecessary and can lead to inconsistent
behavior.
Extends this consistency by removing the caching mode check in the pasid
table update path.
Suggested-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Link: https://lore.kernel.org/r/20240820030208.20020-1-baolu.lu@linux.intel.com
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iommu/intel/pasid.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index b51fc268dc845..2e5fa0a232999 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -264,9 +264,7 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev,
else
iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
- /* Device IOTLB doesn't need to be flushed in caching mode. */
- if (!cap_caching_mode(iommu->cap))
- devtlb_invalidation_with_pasid(iommu, dev, pasid);
+ devtlb_invalidation_with_pasid(iommu, dev, pasid);
}
/*
@@ -493,9 +491,7 @@ int intel_pasid_setup_dirty_tracking(struct intel_iommu *iommu,
iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
- /* Device IOTLB doesn't need to be flushed in caching mode. */
- if (!cap_caching_mode(iommu->cap))
- devtlb_invalidation_with_pasid(iommu, dev, pasid);
+ devtlb_invalidation_with_pasid(iommu, dev, pasid);
return 0;
}
@@ -572,9 +568,7 @@ void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
pasid_cache_invalidation_with_pasid(iommu, did, pasid);
qi_flush_piotlb(iommu, did, pasid, 0, -1, 0);
- /* Device IOTLB doesn't need to be flushed in caching mode. */
- if (!cap_caching_mode(iommu->cap))
- devtlb_invalidation_with_pasid(iommu, dev, pasid);
+ devtlb_invalidation_with_pasid(iommu, dev, pasid);
}
/**
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.11 127/244] iommu/arm-smmu-v3: Do not use devm for the cd table allocations
[not found] <20240925113641.1297102-1-sashal@kernel.org>
` (5 preceding siblings ...)
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 126/244] iommu/vt-d: Unconditionally flush device TLB for pasid table updates Sasha Levin
@ 2024-09-25 11:25 ` Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-09-25 11:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jason Gunthorpe, Nicolin Chen, Will Deacon, Sasha Levin, joro,
jgg, mshavit, smostafa, baolu.lu, linux-arm-kernel, iommu
From: Jason Gunthorpe <jgg@nvidia.com>
[ Upstream commit 47b2de35cab2b683f69d03515c2658c2d8515323 ]
The master->cd_table is entirely contained within the struct
arm_smmu_master which is guaranteed to be freed by the core code under
arm_smmu_release_device().
There is no reason to use devm here, arm_smmu_free_cd_tables() is reliably
called to free the CD related memory. Remove it and save some memory.
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/5-v4-6416877274e1+1af-smmuv3_tidy_jgg@nvidia.com
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 29 +++++++++------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index a7d2a6b8a4980..8a419e88a920e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1185,8 +1185,8 @@ static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
{
size_t size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
- l1_desc->l2ptr = dmam_alloc_coherent(smmu->dev, size,
- &l1_desc->l2ptr_dma, GFP_KERNEL);
+ l1_desc->l2ptr = dma_alloc_coherent(smmu->dev, size,
+ &l1_desc->l2ptr_dma, GFP_KERNEL);
if (!l1_desc->l2ptr) {
dev_warn(smmu->dev,
"failed to allocate context descriptor table\n");
@@ -1400,17 +1400,17 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
cd_table->num_l1_ents = DIV_ROUND_UP(max_contexts,
CTXDESC_L2_ENTRIES);
- cd_table->l1_desc = devm_kcalloc(smmu->dev, cd_table->num_l1_ents,
- sizeof(*cd_table->l1_desc),
- GFP_KERNEL);
+ cd_table->l1_desc = kcalloc(cd_table->num_l1_ents,
+ sizeof(*cd_table->l1_desc),
+ GFP_KERNEL);
if (!cd_table->l1_desc)
return -ENOMEM;
l1size = cd_table->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
}
- cd_table->cdtab = dmam_alloc_coherent(smmu->dev, l1size, &cd_table->cdtab_dma,
- GFP_KERNEL);
+ cd_table->cdtab = dma_alloc_coherent(smmu->dev, l1size,
+ &cd_table->cdtab_dma, GFP_KERNEL);
if (!cd_table->cdtab) {
dev_warn(smmu->dev, "failed to allocate context descriptor\n");
ret = -ENOMEM;
@@ -1421,7 +1421,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
err_free_l1:
if (cd_table->l1_desc) {
- devm_kfree(smmu->dev, cd_table->l1_desc);
+ kfree(cd_table->l1_desc);
cd_table->l1_desc = NULL;
}
return ret;
@@ -1441,21 +1441,18 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_master *master)
if (!cd_table->l1_desc[i].l2ptr)
continue;
- dmam_free_coherent(smmu->dev, size,
- cd_table->l1_desc[i].l2ptr,
- cd_table->l1_desc[i].l2ptr_dma);
+ dma_free_coherent(smmu->dev, size,
+ cd_table->l1_desc[i].l2ptr,
+ cd_table->l1_desc[i].l2ptr_dma);
}
- devm_kfree(smmu->dev, cd_table->l1_desc);
- cd_table->l1_desc = NULL;
+ kfree(cd_table->l1_desc);
l1size = cd_table->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
} else {
l1size = cd_table->num_l1_ents * (CTXDESC_CD_DWORDS << 3);
}
- dmam_free_coherent(smmu->dev, l1size, cd_table->cdtab, cd_table->cdtab_dma);
- cd_table->cdtab_dma = 0;
- cd_table->cdtab = NULL;
+ dma_free_coherent(smmu->dev, l1size, cd_table->cdtab, cd_table->cdtab_dma);
}
/* Stream table manipulation functions */
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread