Skip to contentSkip to conversation

iommu/arm-smmu-v3: Adopt the crashed kernel's stream table for kdump

PATCH Superseded Nicolin Chen 6 participants 36 messages Aug 30 – Oct 5

Subsystems (3)

v10 of 12 Superseded Latest is v12 →

lore
Share & tools
lore
#0 lore raw

v10 cover iommu/arm-smmu-v3: Adopt the crashed kernel's stream table for kdump

When transitioning to a kdump kernel, the primary kernel might have crashed while endpoint devices were actively bus-mastering DMA. Currently, the SMMU driver aggressively resets the hardware during probe by clearing CR0_SMMUEN and setting the Global Bypass Attribute (GBPA) to ABORT.

In a kdump scenario, this aggressive reset is highly destructive:
a) If GBPA is set to ABORT, in-flight DMA will be aborted, generating fatal PCIe AER or SErrors that may panic the kdump kernel
b) If GBPA is set to BYPASS, in-flight DMA targeting some IOVAs will bypass the SMMU and corrupt the physical memory at those 1:1 mapped IOVAs.

To safely absorb in-flight DMA, the kdump kernel must leave SMMUEN=1 intact and avoid modifying STRTAB_BASE. This allows HW to continue translating in- flight DMA using the crashed kernel's page tables until the endpoint device drivers probe and quiesce their respective hardware.

However, the ARM SMMUv3 architecture specification states that updating the SMMU_STRTAB_BASE register while SMMUEN == 1 is UNPREDICTABLE or ignored.

This leaves a kdump kernel no choice but to adopt the stream table from the crashed kernel.

In this series:
- Introduce an ARM_SMMU_OPT_KDUMP_ADOPT
- Skip SMMUEN and STRTAB_BASE resets in arm_smmu_device_reset()
- Skip EVENTQ/PRIQ setup including interrupts and their handlers
- Memremap the crashed kernel's stream tables into the kdump kernel [*]
- Reserve the crashed kernel's in-use ASIDs and VMIDs, preventing any TLB aliasing with the kdump kernel's own domains
- Defer any default domain attachment to retain STEs until device drivers explicitly request it.
Most of the new code is added to two new files: arm-smmu-v3-kexec.c holds the read-only helpers that parse and walk the crashed kernel's stream/CD tables and reserve its in-use IDs, guarded by a hidden config symbol named ARM_SMMU_V3_KEXEC (def_bool CRASH_DUMP); arm-smmu-v3-kdump.c then builds the kdump adoption on top of those helpers, which are meant to be shared with the proposed SMMUv3 Live Update support:
https://lore.kernel.org/all/[email protected]/

[*] For verification reasons, this series only fixes coherent SMMUs.

For non-ARM_SMMU_OPT_KDUMP_ADOPT cases, keep a status quo since the commit 3f54c447df34f ("iommu/arm-smmu-v3: Don't disable SMMU in kdump kernel"):
full reset followed by driver-initiated reattach, potentially rejecting any in-flight DMA.

Note that this series is no longer treated as a bug fix, since it has grown fairly big and most of the kdump code now resides in separate files. For folks interested in back-porting the change: a v6.12+ kernel (since commit 85196f54743d ("iommu/arm-smmu-v3: Reorganize struct arm_smmu_strtab_cfg")) would be still compatible with this series.

This is on Github:
https://github.com/nicolinc/iommufd/commits/smmuv3_kdump-v10

Changelog
v10
* Rebase on v7.3-rc1
* Bound the 2-level entry count before the shift
* Drop the vmid_map devres action that landed upstream
* Validate the CD table base alignments during the scan
* New prep patch: make the ASID space per SMMU instance
* Gate the EVTQ/PRIQ on feature bits, so kdump skips both entirely
v9
https://lore.kernel.org/all/[email protected]/
* Reject valid CD L1 descriptors carrying a null L2 pointer
* Move the ida devres prep before the stream table adoption
* Reject a 2-level CD table on hardware without FEAT_2_LVL_CDTAB
* Cap the linear table log2size by sid_bits on 2-level capable HW
* Add a hidden ARM_SMMU_V3_KEXEC config for Live Update to extend
* Factor the table walkers and ID reservation into arm-smmu-v3-kexec.c
v8
https://lore.kernel.org/all/[email protected]/
* Move the kdump code into a new arm-smmu-v3-kdump.c
* Move the EVTQ/PRIQ patches to the front of the series
* Prefix "kdump: " to prints via dev_fmt in the new file
* Add a prep patch destroying the vmid_map ida via devres
* Reject valid-span L1 descriptors with a null L2 pointer
* Document notes/limitations at the top of arm-smmu-v3-kdump.c
* Rename arm_smmu_kdump_adopt_l2_strtab() to a deferred variant
* Add a new patch reserving the crashed kernel's ASIDs and VMIDs
* Make arm_smmu_get_step_for_sid() a static inline in the header
* Validate alignments of the adopted stream table base addresses
* Retarget to the merge window; drop the Fixes and Cc-stable tags
* Rename the kdump probe function to arm_smmu_device_kdump_probe()
* Document that a disabled event queue discards new events silently
* Add a common arm_smmu_is_attach_deferred() calling a kdump helper
* Document that acking SFM_ERR is defined but does not exit the SFM
* Document that CR0 queue enables can be cleared while SMMUEN is set
* Clear only the CR0 queue enables in kdump reset, keeping other fields
v7
https://lore.kernel.org/all/[email protected]/
* Rebase v7.2-rc1
* Add Reviewed-by from Pranjal
* Reword the linear stream table adoption comment
* Use dev_dbg for the stream table adoption message
* Document why the lazy L2 adoption uses devm_memremap()
* Drop redundant FEAT_COHERENCY checks in the adopt functions
* Use feature bit instead of STRTAB_BASE_CFG in adopt cleanup
* Skip CR0_ATSCHK update in adopt mode to retain the crashed policy
* Restore FEAT_2_LVL_STRTAB if the cleanup action fails to register
v6
https://lore.kernel.org/all/[email protected]/
* Rebase v7.1-rc3
* Add Reviewed-by from Jason
* Replace dma_addr_t with phys_addr_t
* Drop arm_smmu_kdump_phys_is_corrupted()
* Skip threaded IRQ handlers for EVTQ and PRIQ
* Bypass arm_smmu_rmr_install_bypass_ste() in kdump case
* Drop devm_ for adopt-time allocations; set up cleanup function via devm_add_action_or_reset()
v5
https://lore.kernel.org/all/[email protected]/
* Add Reviewed-by from Kevin
* Drop READ_ONCE on lazy-attach L1 read
* Split "Skip EVTQ/PRIQ setup" into two patches
* Tighten kdump probe comment and dev_warn message
* Use MEM + BUSY in arm_smmu_kdump_phys_is_corrupted
v4
https://lore.kernel.org/all/[email protected]/
* Rebase v7.1-rc1
* s/arm_smmu_adopt/arm_smmu_kdump_adopt
* Revert alloc/memremap/fmt on fallback
* Reorder patches to avoid bisect regression
* Use IRQ_NONE for spurious evtq/priq entries
* Cap linear log2size by kdump's allocation bound
* Defer clearing FEAT_2_LVL_STRTAB on linear adopt
* Add arm_smmu_kdump_phys_is_corrupted() validation
* Defer l2 stream table memremap till master inserts
* Re-validate L1 desc on master insert with READ_ONCE
v3
https://lore.kernel.org/all/[email protected]/
* s/OPT_KDUMP/OPT_KDUMP_ADOPT
* Do not adopt if GERROR_SFM_ERR
* Retain CR0_ATSCHK beside CR0_SMMUEN
* Clear latched GERROR bits (e.g. CMDQ_ERR)
* Assert ARM_SMMU_FEAT_COHERENCY in adopt functions
* Add STE.Cfg check in arm_smmu_is_attach_deferred()
* Fix validations on return codes from devm_memremap()
* Sanitize crashed kernel register values in adopt functions
* Drop unnecessary l2ptrs guard in arm_smmu_is_attach_deferred()
* Don't enable PRIQ/EVTQ irqs and guard the irq functions for combined irq cases
v2
https://lore.kernel.org/all/[email protected]/
* Add warning in non-coherent SMMU cases
* Keep eventq/priq disabled vs. enabling-and-disabling-later
* Check KDUMP option in the beginning of arm_smmu_device_reset()
* Validate STRTAB format matches HW capability instead of forcing flags
v1:
https://lore.kernel.org/all/[email protected]/

Nicolin Chen (13):
iommu/arm-smmu-v3: Init the vmid_map ida before the stream table setup iommu/arm-smmu-v3: Make the ASID space per SMMU instance iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue iommu/arm-smmu-v3: Disable the EVTQ and the PRIQ in a kdump kernel iommu/arm-smmu-v3: Add strtab parse helpers to a new

    arm-smmu-v3-kexec.c

iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel iommu/arm-smmu-v3-kexec: Add a CD table parse helper iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers iommu/arm-smmu-v3-kdump: Reserve crashed kernel's ASIDs and VMIDs iommu/arm-smmu-v3-kdump: Implement is_attach_deferred() iommu/arm-smmu-v3: Retain CR0_SMMUEN during kdump device reset iommu/arm-smmu-v3: Skip RMR bypass for kdump adoption iommu/arm-smmu-v3: Detect ARM_SMMU_OPT_KDUMP_ADOPT in probe()

 drivers/iommu/arm/Kconfig                     |   4 +
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   2 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  68 ++-
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 285 +++++++++++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c | 458 ++++++++++++++++++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |   6 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 247 +++++++---
 7 files changed, 999 insertions(+), 71 deletions(-)
 create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c
 create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935

Signature · 1 line
2.43.0
#1 lore raw

PATCH v10 1/13 iommu/arm-smmu-v3: Init the vmid_map ida before the stream table setup

The vmid_map ida is initialized after the stream table setup, along with the devres action that destroys it.

An upcoming change will reserve the crashed kernel's in-use VMIDs in this ida, from a kdump kernel's stream table adoption that runs in place of the regular setup and returns early.

Move the ida_init() and its devres registration to the entry of the whole function, so that the ida is ready by the time the adoption path runs.

Assisted-by Claude:claude-opus-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +7 -7
index 5732f3ba0122d..1c4322d87d8df 100644
@@ -4584,17 +4584,17 @@ static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
 {
 	int ret;
 
+	ida_init(&smmu->vmid_map);
+	ret = devm_add_action_or_reset(smmu->dev, arm_smmu_destroy_vmid_map,
+				       &smmu->vmid_map);
+	if (ret)
+		return ret;
+
 	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
 		ret = arm_smmu_init_strtab_2lvl(smmu);
 	else
 		ret = arm_smmu_init_strtab_linear(smmu);
-	if (ret)
-		return ret;
-
-	ida_init(&smmu->vmid_map);
-
-	return devm_add_action_or_reset(smmu->dev, arm_smmu_destroy_vmid_map,
-					&smmu->vmid_map);
+	return ret;
 }
 
 static int arm_smmu_init_structures(struct arm_smmu_device *smmu)
Signature · 1 line
2.43.0
#2 lore raw

PATCH v10 2/13 iommu/arm-smmu-v3: Make the ASID space per SMMU instance

In reply to Nicolin Chen

An ASID tags the TLB entries within one SMMU, so two instances can use the same ASID without ever aliasing each other. Yet the driver allocates them out of a single global xarray, which makes the instances share a space that the hardware keeps apart, and lets one instance exhaust the IDs of another.

That global space is a leftover from the BTM support that shared ASIDs with the CPU. Now ARM_SMMU_FEAT_BTM is never set, nothing looks a domain up by its ASID, and a domain is pinned to one SMMU at the attach.

Give each SMMU its own asid_map, mirroring the per-SMMU vmid_map, and clean it up with devres, so that both of the ID maps get the same lifetime as the SMMU device structure that holds them. An upcoming change will need this, to reserve the crashed kernel's in-use ASIDs in the new map during a kdump kernel's stream table adoption.

Note that arm_smmu_asid_lock stays global, as it serializes the STE and CD updates against any ASID change rather than guarding the map itself, which does its own locking. Giving each SMMU its own lock looks possible now, but that would touch every attach path and belongs to a separate change.

Assisted-by Claude:claude-opus-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h    |  2 +-
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c    |  6 +++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c    | 18 +++++++++++++++---
 3 files changed, 19 insertions(+), 7 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1 -1
index 50f8321e979ce..fd6f489cfedf2 100644
@@ -952,6 +952,7 @@ struct arm_smmu_device {
 #define ARM_SMMU_MAX_VMIDS		(1 << 16)
 	unsigned int			vmid_bits;
 	struct ida			vmid_map;
+	struct xarray			asid_map;
 
 	unsigned int			ssid_bits;
 	unsigned int			sid_bits;
@@ -1128,7 +1129,6 @@ to_smmu_nested_domain(struct iommu_domain *dom)
 	return container_of(dom, struct arm_smmu_nested_domain, domain);
 }
 
-extern struct xarray arm_smmu_asid_xa;
 extern struct mutex arm_smmu_asid_lock;
 
 struct arm_smmu_domain *arm_smmu_domain_alloc(void);
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +3 -3
index 0a429c64fbf3e..2433ec61f5336 100644
@@ -301,7 +301,7 @@ static void arm_smmu_sva_domain_free(struct iommu_domain *domain)
 	 * reused, and if there is a race then it just suffers harmless
 	 * unnecessary invalidation.
 	 */
-	xa_erase(&arm_smmu_asid_xa, smmu_domain->cd.asid);
+	xa_erase(&smmu_domain->smmu->asid_map, smmu_domain->cd.asid);
 
 	/*
 	 * Actual free is defered to the SRCU callback
@@ -341,7 +341,7 @@ struct iommu_domain *arm_smmu_sva_domain_alloc(struct device *dev,
 	smmu_domain->stage = ARM_SMMU_DOMAIN_SVA;
 	smmu_domain->smmu = smmu;
 
-	ret = xa_alloc(&arm_smmu_asid_xa, &asid, smmu_domain,
+	ret = xa_alloc(&smmu->asid_map, &asid, smmu_domain,
 		       XA_LIMIT(1, (1 << smmu->asid_bits) - 1), GFP_KERNEL);
 	if (ret)
 		goto err_free;
@@ -355,7 +355,7 @@ struct iommu_domain *arm_smmu_sva_domain_alloc(struct device *dev,
 	return &smmu_domain->domain;
 
 err_asid:
-	xa_erase(&arm_smmu_asid_xa, smmu_domain->cd.asid);
+	xa_erase(&smmu_domain->smmu->asid_map, smmu_domain->cd.asid);
 err_free:
 	arm_smmu_domain_free(smmu_domain);
 	return ERR_PTR(ret);
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +15 -3
index 1c4322d87d8df..c1d04f84870f6 100644
@@ -85,7 +85,6 @@ struct arm_smmu_option_prop {
 	const char *prop;
 };
 
-DEFINE_XARRAY_ALLOC1(arm_smmu_asid_xa);
 DEFINE_MUTEX(arm_smmu_asid_lock);
 
 static struct arm_smmu_option_prop arm_smmu_options[] = {
@@ -2839,7 +2838,7 @@ static void arm_smmu_domain_free_paging(struct iommu_domain *domain)
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
 		/* Prevent SVA from touching the CD while we're freeing it */
 		mutex_lock(&arm_smmu_asid_lock);
-		xa_erase(&arm_smmu_asid_xa, smmu_domain->cd.asid);
+		xa_erase(&smmu->asid_map, smmu_domain->cd.asid);
 		mutex_unlock(&arm_smmu_asid_lock);
 	} else {
 		struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
@@ -2859,7 +2858,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_device *smmu,
 
 	/* Prevent SVA from modifying the ASID until it is written to the CD */
 	mutex_lock(&arm_smmu_asid_lock);
-	ret = xa_alloc(&arm_smmu_asid_xa, &asid, smmu_domain,
+	ret = xa_alloc(&smmu->asid_map, &asid, smmu_domain,
 		       XA_LIMIT(1, (1 << smmu->asid_bits) - 1), GFP_KERNEL);
 	cd->asid	= (u16)asid;
 	mutex_unlock(&arm_smmu_asid_lock);
@@ -4483,6 +4482,13 @@ static void arm_smmu_destroy_vmid_map(void *data)
 	ida_destroy(ida);
 }
 
+static void arm_smmu_destroy_asid_map(void *data)
+{
+	struct xarray *xa = data;
+
+	xa_destroy(xa);
+}
+
 static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 {
 	int ret;
@@ -4590,6 +4596,12 @@ static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
 	if (ret)
 		return ret;
 
+	xa_init_flags(&smmu->asid_map, XA_FLAGS_ALLOC1);
+	ret = devm_add_action_or_reset(smmu->dev, arm_smmu_destroy_asid_map,
+				       &smmu->asid_map);
+	if (ret)
+		return ret;
+
 	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
 		ret = arm_smmu_init_strtab_2lvl(smmu);
 	else
Signature · 1 line
2.43.0
Reviewed-by on v10 2/13
#3 lore raw

Re: [PATCH v10 02/13] iommu/arm-smmu-v3: Make the ASID space per SMMU instance

Quoted 10 lines · Nicolin Chen

An ASID tags the TLB entries within one SMMU, so two instances can use the same ASID without ever aliasing each other. Yet the driver allocates them out of a single global xarray, which makes the instances share a space that the hardware keeps apart, and lets one instance exhaust the IDs of another.

That global space is a leftover from the BTM support that shared ASIDs with the CPU. Now ARM_SMMU_FEAT_BTM is never set, nothing looks a domain up by its ASID, and a domain is pinned to one SMMU at the attach.

[...]

Signature · 1 line
Jason
maintainer
#4 lore raw

Re: [PATCH v10 02/13] iommu/arm-smmu-v3: Make the ASID space per SMMU instance

In reply to Nicolin Chen
Quoted 8 lines · Nicolin Chen

An ASID tags the TLB entries within one SMMU, so two instances can use the same ASID without ever aliasing each other. Yet the driver allocates them out of a single global xarray, which makes the instances share a space that the hardware keeps apart, and lets one instance exhaust the IDs of another.

That global space is a leftover from the BTM support that shared ASIDs with the CPU. Now ARM_SMMU_FEAT_BTM is never set, nothing looks a domain up by its ASID, and a domain is pinned to one SMMU at the attach.

We probably need to figure out what we're doing with BTM. Over at:

https://lore.kernel.org/r/[email protected]

Jason was talking about bringing it back once the remaining invs stuff has landed. If that happens, what is the correct sequence to allocate the same ASID on every SMMU instance after these patches?

Will

#5 lore raw

Re: [PATCH v10 02/13] iommu/arm-smmu-v3: Make the ASID space per SMMU instance

Quoted 10 lines · Will Deacon
Quoted 8 lines · Nicolin Chen

An ASID tags the TLB entries within one SMMU, so two instances can use the same ASID without ever aliasing each other. Yet the driver allocates them out of a single global xarray, which makes the instances share a space that the hardware keeps apart, and lets one instance exhaust the IDs of another.

That global space is a leftover from the BTM support that shared ASIDs with the CPU. Now ARM_SMMU_FEAT_BTM is never set, nothing looks a domain up by its ASID, and a domain is pinned to one SMMU at the attach.

Quoted 7 lines

We probably need to figure out what we're doing with BTM. Over at:

https://lore.kernel.org/r/[email protected]

Jason was talking about bringing it back once the remaining invs stuff has landed. If that happens, what is the correct sequence to allocate the same ASID on every SMMU instance after these patches?

The last attempt I had at this already relied on the per-instance ASID, it made it simpler.

The flow upon SVA domain attach to a device is broadly
- Check if the asid is free in the per instance asid xarray
- If there is a conflict
* Reserve a new empty asid
* Allocate two invs list, first with both ASIDs, second with only new
* Set the first invs list. Barrier
* Update the CD entries for the conflicted domain to the new ASID
* Flush CDs and IOTLB
* Set the second invs list. Barrier
* Set the SVA as the owner of the replaced asid

This cycle repeats every time the SVA is connected to a domain, we don't need to do a 'global' operation. That is why the per-instance approach was simpler

BTM has a per-CD flag to indicate if the CD uses a 'BTM' ASID or a normal ASID. It is fine for normal ASID's to alias the MM as they won't participate in DVM, so only SVA needs to do this.

It was the STE that was broken and lacked that bit so you always had to be careful with the vmid.

So the simplificaiton to do BTM is to only support guest BTM, where the instance has no S2 support in the IDR. Leave figuring out the vmid with kvm to another project.

IIRC..

Jason

#6 lore raw

PATCH v10 3/13 iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue

In reply to Nicolin Chen

The driver programs and enables the event queue unconditionally, while the PRI queue has an ARM_SMMU_FEAT_PRI gating each of its touch points. Yet a kdump kernel wants to leave both of the queues alone, which would take an is_kdump_kernel() test at every one of those places.

Add an ARM_SMMU_FEAT_EVTQ that the probe always sets, as the event queue is architecturally mandatory, and gate the queue's allocation, its interrupt and its CR0 and IRQ_CTRL enables on it, so that a later change can turn the queue off in a single place.

No functional change intended.

Assisted-by Claude:claude-opus-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 79 +++++++++++++--------
 2 files changed, 52 insertions(+), 28 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1 -0
index fd6f489cfedf2..75e768b3d66d8 100644
@@ -927,6 +927,7 @@ struct arm_smmu_device {
 #define ARM_SMMU_FEAT_BBML2		(1 << 24)
 #define ARM_SMMU_FEAT_HAFT		(1 << 25)
 #define ARM_SMMU_FEAT_DS		(1 << 26)
+#define ARM_SMMU_FEAT_EVTQ		(1 << 27)
 	u32				features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +51 -28

127 lines not on this page · raw message

Signature · 1 line
2.43.0
Acked-by on v10 3/13
#7 lore raw

Re: [PATCH v10 03/13] iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue

Quoted 10 lines · Nicolin Chen

The driver programs and enables the event queue unconditionally, while the PRI queue has an ARM_SMMU_FEAT_PRI gating each of its touch points. Yet a kdump kernel wants to leave both of the queues alone, which would take an is_kdump_kernel() test at every one of those places.

Add an ARM_SMMU_FEAT_EVTQ that the probe always sets, as the event queue is architecturally mandatory, and gate the queue's allocation, its interrupt and its CR0 and IRQ_CTRL enables on it, so that a later change can turn the queue off in a single place.

Quoted 6 lines

No functional change intended.

Assisted-by Claude:claude-opus-5
Signature · 1 line
  Kiryl Shutsemau / Kirill A. Shutemov
Reviewed-by on v10 3/13
#8 lore raw

Re: [PATCH v10 03/13] iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue

In reply to Nicolin Chen
Quoted 11 lines · Nicolin Chen

The driver programs and enables the event queue unconditionally, while the PRI queue has an ARM_SMMU_FEAT_PRI gating each of its touch points. Yet a kdump kernel wants to leave both of the queues alone, which would take an is_kdump_kernel() test at every one of those places.

Add an ARM_SMMU_FEAT_EVTQ that the probe always sets, as the event queue is architecturally mandatory, and gate the queue's allocation, its interrupt and its CR0 and IRQ_CTRL enables on it, so that a later change can turn the queue off in a single place.

[...]

Signature · 1 line
Jason
maintainer
#9 lore raw

Re: [PATCH v10 03/13] iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue

In reply to Nicolin Chen
Quoted 12 lines · Nicolin Chen

The driver programs and enables the event queue unconditionally, while the PRI queue has an ARM_SMMU_FEAT_PRI gating each of its touch points. Yet a kdump kernel wants to leave both of the queues alone, which would take an is_kdump_kernel() test at every one of those places.

Add an ARM_SMMU_FEAT_EVTQ that the probe always sets, as the event queue is architecturally mandatory, and gate the queue's allocation, its interrupt and its CR0 and IRQ_CTRL enables on it, so that a later change can turn the queue off in a single place.

No functional change intended.

Quoted 8 lines
Assisted-by Claude:claude-opus-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 79 +++++++++++++--------
 2 files changed, 52 insertions(+), 28 deletions(-)

Sashiko has comments about the event queue MSI configuration for this patch. Please can you have a look?

Will

#10 lore raw

Re: [PATCH v10 03/13] iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue

Quoted 22 lines · Will Deacon
Quoted 12 lines · Nicolin Chen

The driver programs and enables the event queue unconditionally, while the PRI queue has an ARM_SMMU_FEAT_PRI gating each of its touch points. Yet a kdump kernel wants to leave both of the queues alone, which would take an is_kdump_kernel() test at every one of those places.

Add an ARM_SMMU_FEAT_EVTQ that the probe always sets, as the event queue is architecturally mandatory, and gate the queue's allocation, its interrupt and its CR0 and IRQ_CTRL enables on it, so that a later change can turn the queue off in a single place.

No functional change intended.

Quoted 8 lines
Assisted-by Claude:claude-opus-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 79 +++++++++++++--------
 2 files changed, 52 insertions(+), 28 deletions(-)
Quoted 2 lines

Sashiko has comments about the event queue MSI configuration for this patch. Please can you have a look?

Sashiko raised those because it doesn't read subsequent patches.

And I left them as-is because I don't think they worth a change:

1. IRQ_CTRL_EVTQ_IRQEN is never enabled in the kdump mode, so that ARM_SMMU_EVTQ_IRQ_CFG0 line shouldn't matter.

2. arm_smmu_write_msi_msg() is never called in the kdump mode since devm_request_threaded_irq() is guarded with ARM_SMMU_FEAT_EVTQ.

Sashiko is only right about the small waste of the MSI vector since it's unused. But, that nvec decrement trick only works for PRIQ, as its MSI index is at top of the list:

enum arm_smmu_msi_index {
EVTQ_MSI_INDEX,
GERROR_MSI_INDEX,
PRIQ_MSI_INDEX,
ARM_SMMU_MAX_MSIS, // "nvec--" shifts up this, skipping PRIQ
};

On the other hand, EVTQ_MSI_INDEX is at 0. Skipping it would need to shift the entire index list; struct arm_smmu_msi_cfg would not work straightforwardly as today.

Nicolin

#11 lore raw

Re: [PATCH v10 03/13] iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue

Quoted 11 lines · Nicolin Chen

Sashiko is only right about the small waste of the MSI vector since it's unused. But, that nvec decrement trick only works for PRIQ, as its MSI index is at top of the list:

   enum arm_smmu_msi_index {
	EVTQ_MSI_INDEX,
	GERROR_MSI_INDEX,
	PRIQ_MSI_INDEX,
	ARM_SMMU_MAX_MSIS, // "nvec--" shifts up this, skipping PRIQ
   };
Quoted 3 lines

On the other hand, EVTQ_MSI_INDEX is at 0. Skipping it would need to shift the entire index list; struct arm_smmu_msi_cfg would not work straightforwardly as today.

I made a small change swapping those two indexes. Though it cannot break the dependency on the vectors, I think we should be fine as there should be no use case for !FEAT_EVTQ && FEAT_PRI.

-----------------------prep patch--------------------------
@@ -59,4 +59,4 @@ static DEFINE_STATIC_KEY_FALSE(arm_smmu_erratum_repeat_tlbi_cfgi_key);
enum arm_smmu_msi_index {
- EVTQ_MSI_INDEX,
GERROR_MSI_INDEX,
+ EVTQ_MSI_INDEX,
PRIQ_MSI_INDEX,
@@ -70,7 +70,2 @@ static_assert(sizeof(struct arm_smmu_cd) == NUM_ENTRY_QWORDS * sizeof(u64));
static phys_addr_t arm_smmu_msi_cfg[ARM_SMMU_MAX_MSIS][3] = {
- [EVTQ_MSI_INDEX] = {
- ARM_SMMU_EVTQ_IRQ_CFG0,
- ARM_SMMU_EVTQ_IRQ_CFG1,
- ARM_SMMU_EVTQ_IRQ_CFG2,
- },
[GERROR_MSI_INDEX] = {
@@ -80,2 +75,7 @@ static phys_addr_t arm_smmu_msi_cfg[ARM_SMMU_MAX_MSIS][3] = {
},
+ [EVTQ_MSI_INDEX] = {
+ ARM_SMMU_EVTQ_IRQ_CFG0,
+ ARM_SMMU_EVTQ_IRQ_CFG1,
+ ARM_SMMU_EVTQ_IRQ_CFG2,
+ },
[PRIQ_MSI_INDEX] = {
@@ -4967,3 +4967,3 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)

- /* Allocate MSIs for evtq, gerror and priq. Ignore cmdq */
+ /* Allocate MSIs for gerror, evtq and priq. Ignore cmdq */
ret = platform_device_msi_init_and_alloc_irqs(dev, nvec, arm_smmu_write_msi_msg);
@@ -4974,4 +4974,4 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)

- smmu->evtq.q.irq = msi_get_virq(dev, EVTQ_MSI_INDEX);
smmu->gerr_irq = msi_get_virq(dev, GERROR_MSI_INDEX);
+ smmu->evtq.q.irq = msi_get_virq(dev, EVTQ_MSI_INDEX);
smmu->priq.q.irq = msi_get_virq(dev, PRIQ_MSI_INDEX);
-----------------------------------------------------------

-----------------------nvec patch--------------------------
@@ -4952,3 +4961,11 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
- writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
+
+ /* PRIQ's vector needs EVTQ's to be kept */
+ WARN_ON_ONCE(!(smmu->features & ARM_SMMU_FEAT_EVTQ) &&
+ (smmu->features & ARM_SMMU_FEAT_PRI));
+
+ if (smmu->features & ARM_SMMU_FEAT_EVTQ)
+ writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
+ else
+ nvec--;

-----------------------------------------------------------

I also did a rebase on arm/smmu/updates, resolving merge conflict.

I can quickly respin a v11 once we finalize the discussions in the other thread.

Thanks
Nicolin

#12 lore raw

PATCH v10 4/13 iommu/arm-smmu-v3: Disable the EVTQ and the PRIQ in a kdump kernel

In reply to Nicolin Chen

A kdump kernel cannot use either queue. The crashed kernel's CDs and page tables might be corrupted, so events would spam the EVTQ, and there is no way to serve the page requests that would arrive at the PRIQ.

The reset routine still enables both of the queues and then masks the two enable bits back out, having already programmed the queue bases and taken the interrupts of both.

Clear ARM_SMMU_FEAT_EVTQ and ARM_SMMU_FEAT_PRI in the probe instead, so all of the queue handling, i.e. the interrupts, the IRQ_CTRL bits and the CR0 enables, keeps away from a kdump kernel via the feature tests.

Both queues also go unallocated now, as the two allocations test the same features. Each of them is sized from the maxima that IDR1 advertises, so up to 4MB apiece on a 4K-page kernel, which a kdump kernel would otherwise pay for every SMMU instance out of a small crashkernel reservation.

The IOPF workqueue of the event queue stays allocated here, as untangling it from the SVA and the stall features takes a pair of changes carried by the PRI series. It costs a struct and a workqueue rather than the megabytes that the two queues take, so leave it to that series.

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +9 -3
index c776fdd43325d..7b9ef1ad1f688 100644
@@ -4981,9 +4981,6 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
 		return ret;
 	}
 
-	if (is_kdump_kernel())
-		enables &= ~(CR0_EVTQEN | CR0_PRIQEN);
-
 	/* Enable the SMMU interface */
 	enables |= CR0_SMMUEN;
 	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
@@ -5296,6 +5293,15 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 	if (arm_smmu_sva_supported(smmu))
 		smmu->features |= ARM_SMMU_FEAT_SVA;
 
+	/*
+	 * A kdump kernel wants neither queue: the crashed kernel's CDs and page
+	 * tables might be corrupted, spamming events, and page requests cannot
+	 * be served. A disabled queue discards new records without raising any
+	 * global error.
+	 */
+	if (is_kdump_kernel())
+		smmu->features &= ~(ARM_SMMU_FEAT_EVTQ | ARM_SMMU_FEAT_PRI);
+
 	dev_info(smmu->dev, "oas %lu-bit (features 0x%08x)\n",
 		 smmu->oas, smmu->features);
 	return 0;
Signature · 1 line
2.43.0
Acked-by on v10 4/13
#13 lore raw

Re: [PATCH v10 04/13] iommu/arm-smmu-v3: Disable the EVTQ and the PRIQ in a kdump kernel

Quoted 22 lines · Nicolin Chen

A kdump kernel cannot use either queue. The crashed kernel's CDs and page tables might be corrupted, so events would spam the EVTQ, and there is no way to serve the page requests that would arrive at the PRIQ.

The reset routine still enables both of the queues and then masks the two enable bits back out, having already programmed the queue bases and taken the interrupts of both.

Clear ARM_SMMU_FEAT_EVTQ and ARM_SMMU_FEAT_PRI in the probe instead, so all of the queue handling, i.e. the interrupts, the IRQ_CTRL bits and the CR0 enables, keeps away from a kdump kernel via the feature tests.

Both queues also go unallocated now, as the two allocations test the same features. Each of them is sized from the maxima that IDR1 advertises, so up to 4MB apiece on a 4K-page kernel, which a kdump kernel would otherwise pay for every SMMU instance out of a small crashkernel reservation.

The IOPF workqueue of the event queue stays allocated here, as untangling it from the SVA and the stall features takes a pair of changes carried by the PRI series. It costs a struct and a workqueue rather than the megabytes that the two queues take, so leave it to that series.

Quoted 7 lines
Signature · 1 line
  Kiryl Shutsemau / Kirill A. Shutemov
Reviewed-by on v10 4/13
#14 lore raw

Re: [PATCH v10 04/13] iommu/arm-smmu-v3: Disable the EVTQ and the PRIQ in a kdump kernel

In reply to Nicolin Chen
Quoted 9 lines · Nicolin Chen

A kdump kernel cannot use either queue. The crashed kernel's CDs and page tables might be corrupted, so events would spam the EVTQ, and there is no way to serve the page requests that would arrive at the PRIQ.

The reset routine still enables both of the queues and then masks the two enable bits back out, having already programmed the queue bases and taken the interrupts of both.

[...]

Signature · 1 line
Jason
#15 lore raw

PATCH v10 5/13 iommu/arm-smmu-v3: Add strtab parse helpers to a new arm-smmu-v3-kexec.c

In reply to Nicolin Chen

When booting after a kexec, both the kdump adoption and also the in-flight SMMUv3 live-update restoration built on the Kexec Handover (KHO) framework need to parse the previous kernel's STRTAB_BASE/STRTAB_BASE_CFG registers, in order to retain the stream table translating any in-flight DMAs.

Add a new arm-smmu-v3-kexec.c holding the common read-only helpers:
- arm_smmu_kexec_parse_strtab_2lvl()
- arm_smmu_kexec_parse_strtab_linear()
- arm_smmu_kexec_check_strtab_l1_desc()

These helpers only validate the previous kernel's values against this very kernel's own limits, taking no ownership of the tables: a kdump kernel will adopt the tables via memremaps, while a live-update kernel will claim them using the KHO restore API.

Build this new file under a hidden ARM_SMMU_V3_KEXEC Kconfig symbol, which is, for now, merely a CRASH_DUMP. Then, the coming live-update series would need to append its own user to this symbol, e.g. "|| IOMMU_LIVEUPDATE".

---
 drivers/iommu/arm/Kconfig                     |   4 +
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  11 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c | 166 ++++++++++++++++++
 4 files changed, 182 insertions(+)
 create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c
drivers/iommu/arm/Kconfig +4 -0
index 5fac08b89deea..04b05d49abf76 100644
@@ -109,6 +109,10 @@ config ARM_SMMU_V3_IOMMUFD
 
 	  Say Y here if you are doing development and testing on this feature.
 
+# Common helpers for a kexec'd kernel, e.g. kdump adoption and live update
+config ARM_SMMU_V3_KEXEC
+	def_bool CRASH_DUMP
+
 config ARM_SMMU_V3_KUNIT_TEST
 	tristate "KUnit tests for arm-smmu-v3 driver"  if !KUNIT_ALL_TESTS
 	depends on KUNIT
drivers/iommu/arm/arm-smmu-v3/Makefile +1 -0
index 493a659cc66bb..89a27de8dd2f0 100644
@@ -3,6 +3,7 @@ obj-$(CONFIG_ARM_SMMU_V3) += arm_smmu_v3.o
 arm_smmu_v3-y := arm-smmu-v3.o
 arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_IOMMUFD) += arm-smmu-v3-iommufd.o
 arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_SVA) += arm-smmu-v3-sva.o
+arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_KEXEC) += arm-smmu-v3-kexec.o
 arm_smmu_v3-$(CONFIG_TEGRA241_CMDQV) += tegra241-cmdqv.o
 
 obj-$(CONFIG_ARM_SMMU_V3_KUNIT_TEST) += arm-smmu-v3-test.o
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +11 -0
index 75e768b3d66d8..dd71d758defd3 100644
@@ -1251,6 +1251,17 @@ tegra241_cmdqv_probe(struct arm_smmu_device *smmu)
 }
 #endif /* CONFIG_TEGRA241_CMDQV */
 
+#ifdef CONFIG_ARM_SMMU_V3_KEXEC
+int arm_smmu_kexec_parse_strtab_2lvl(struct arm_smmu_device *smmu, u32 cfg_reg,
+				     phys_addr_t base, u32 *num_l1_ents);
+int arm_smmu_kexec_parse_strtab_linear(struct arm_smmu_device *smmu,
+				       u32 cfg_reg, phys_addr_t base,
+				       u32 *num_ents);
+int arm_smmu_kexec_check_strtab_l1_desc(struct arm_smmu_device *smmu,
+					u64 l1_desc, u32 idx,
+					phys_addr_t *l2_base);
+#endif /* CONFIG_ARM_SMMU_V3_KEXEC */
+
 struct arm_vsmmu {
 	struct iommufd_viommu core;
 	struct arm_smmu_device *smmu;
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c new file +166 -0

166 lines not on this page · raw message

Signature · 1 line
2.43.0
#16 lore raw

PATCH v10 6/13 iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

In reply to Nicolin Chen

When transitioning to a kdump kernel, the primary kernel might have crashed while endpoint devices were actively bus-mastering DMA. Currently, the SMMU driver aggressively resets the hardware during probe by clearing CR0_SMMUEN and setting the Global Bypass Attribute (GBPA) to ABORT.

In a kdump scenario, this aggressive reset is highly destructive:
a) If GBPA is set to ABORT, in-flight DMA will be aborted, generating fatal PCIe AER or SErrors that may panic the kdump kernel
b) If GBPA is set to BYPASS, in-flight DMA targeting some IOVAs will bypass the SMMU and corrupt the physical memory at those 1:1 mapped IOVAs.

To safely absorb in-flight DMAs, a kdump kernel will have to leave SMMUEN=1 intact and avoid modifying STRTAB_BASE, allowing HW to continue translating in-flight DMAs reusing the crashed kernel's page tables until the endpoint device drivers probe and quiesce their respective hardware.

However, the ARM SMMUv3 architecture specification states that updating the SMMU_STRTAB_BASE register while SMMUEN == 1 is UNPREDICTABLE or ignored.

This leaves a kdump kernel no choice but to adopt the stream table from the crashed kernel.

Introduce ARM_SMMU_OPT_KDUMP_ADOPT and adopt functions memremapping all the stream tables extracted from STRTAB_BASE and STRTAB_BASE_CFG. Add them in a new arm-smmu-v3-kdump.c, which is only built when CONFIG_CRASH_DUMP=y.

Note that the adoption of the crashed kernel's stream table follows certain strict rules, since the old stream table might be compromised. Thus, apply some basic validations against the values read from the registers. If tests fail, it means the stream table cannot be trusted, so toss it entirely. To avoid OOM due to a potentially corrupted stream table, the memremap for l2 tables is done lazily on the kdump kernel's demand.

The new option will be set in a following change, once the device reset and the RMR setup are reworked not to overwrite the adopted stream table, and the crashed kernel's in-use ASIDs and VMIDs are reserved.

Assisted-by Claude:claude-fable-5
---
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 229 ++++++++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  21 +-
 4 files changed, 269 insertions(+), 3 deletions(-)
 create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c
drivers/iommu/arm/arm-smmu-v3/Makefile +1 -0
index 89a27de8dd2f0..2bc52473d960e 100644
@@ -4,6 +4,7 @@ arm_smmu_v3-y := arm-smmu-v3.o
 arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_IOMMUFD) += arm-smmu-v3-iommufd.o
 arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_SVA) += arm-smmu-v3-sva.o
 arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_KEXEC) += arm-smmu-v3-kexec.o
+arm_smmu_v3-$(CONFIG_CRASH_DUMP) += arm-smmu-v3-kdump.o
 arm_smmu_v3-$(CONFIG_TEGRA241_CMDQV) += tegra241-cmdqv.o
 
 obj-$(CONFIG_ARM_SMMU_V3_KUNIT_TEST) += arm-smmu-v3-test.o
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +21 -0
index dd71d758defd3..41d7a907b9ba2 100644
@@ -935,6 +935,7 @@ struct arm_smmu_device {
 #define ARM_SMMU_OPT_MSIPOLL		(1 << 2)
 #define ARM_SMMU_OPT_CMDQ_FORCE_SYNC	(1 << 3)
 #define ARM_SMMU_OPT_TEGRA241_CMDQV	(1 << 4)
+#define ARM_SMMU_OPT_KDUMP_ADOPT	(1 << 5)
 	u32				options;
 
 	struct arm_smmu_cmdq		cmdq;
@@ -1262,6 +1263,26 @@ int arm_smmu_kexec_check_strtab_l1_desc(struct arm_smmu_device *smmu,
 					phys_addr_t *l2_base);
 #endif /* CONFIG_ARM_SMMU_V3_KEXEC */
 
+#ifdef CONFIG_CRASH_DUMP
+int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu);
+int arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu,
+					    u32 sid, phys_addr_t base, u32 span,
+					    struct arm_smmu_strtab_l2 **l2table);
+#else /* CONFIG_CRASH_DUMP */
+static inline int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int
+arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid,
+					phys_addr_t base, u32 span,
+					struct arm_smmu_strtab_l2 **l2table)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_CRASH_DUMP */
+
 struct arm_vsmmu {
 	struct iommufd_viommu core;
 	struct arm_smmu_device *smmu;
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c new file +229 -0

229 lines not on this page · raw message

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +18 -3
index 7b9ef1ad1f688..971b12517941f 100644
@@ -2002,11 +2002,23 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 	dma_addr_t l2ptr_dma;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
 	struct arm_smmu_strtab_l2 **l2table;
+	u32 l1_idx = arm_smmu_strtab_l1_idx(sid);
 
-	l2table = &cfg->l2.l2ptrs[arm_smmu_strtab_l1_idx(sid)];
+	l2table = &cfg->l2.l2ptrs[l1_idx];
 	if (*l2table)
 		return 0;
 
+	/* Deferred adoption of the crashed kernel's L2 table */
+	if (smmu->options & ARM_SMMU_OPT_KDUMP_ADOPT) {
+		u64 l2ptr = le64_to_cpu(cfg->l2.l1tab[l1_idx].l2ptr);
+		phys_addr_t base = l2ptr & STRTAB_L1_DESC_L2PTR_MASK;
+		u32 span = FIELD_GET(STRTAB_L1_DESC_SPAN, l2ptr);
+
+		if (span)
+			return arm_smmu_kdump_adopt_deferred_l2_strtab(
+				smmu, sid, base, span, l2table);
+	}
+
 	*l2table = dmam_alloc_coherent(smmu->dev, sizeof(**l2table),
 				       &l2ptr_dma, GFP_KERNEL);
 	if (!*l2table) {
@@ -2018,8 +2030,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 
 	arm_smmu_init_initial_stes((*l2table)->stes,
 				   ARRAY_SIZE((*l2table)->stes));
-	arm_smmu_write_strtab_l1_desc(&cfg->l2.l1tab[arm_smmu_strtab_l1_idx(sid)],
-				      l2ptr_dma);
+	arm_smmu_write_strtab_l1_desc(&cfg->l2.l1tab[l1_idx], l2ptr_dma);
 	return 0;
 }
 
@@ -4611,6 +4622,10 @@ static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
 	if (ret)
 		return ret;
 
+	if ((smmu->options & ARM_SMMU_OPT_KDUMP_ADOPT) &&
+	    !arm_smmu_kdump_adopt_strtab(smmu))
+		return 0;
+
 	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
 		ret = arm_smmu_init_strtab_2lvl(smmu);
 	else
Signature · 1 line
2.43.0
maintainer
#17 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 37 lines · Nicolin Chen

When transitioning to a kdump kernel, the primary kernel might have crashed while endpoint devices were actively bus-mastering DMA. Currently, the SMMU driver aggressively resets the hardware during probe by clearing CR0_SMMUEN and setting the Global Bypass Attribute (GBPA) to ABORT.

In a kdump scenario, this aggressive reset is highly destructive:
a) If GBPA is set to ABORT, in-flight DMA will be aborted, generating fatal PCIe AER or SErrors that may panic the kdump kernel
b) If GBPA is set to BYPASS, in-flight DMA targeting some IOVAs will bypass the SMMU and corrupt the physical memory at those 1:1 mapped IOVAs.

To safely absorb in-flight DMAs, a kdump kernel will have to leave SMMUEN=1 intact and avoid modifying STRTAB_BASE, allowing HW to continue translating in-flight DMAs reusing the crashed kernel's page tables until the endpoint device drivers probe and quiesce their respective hardware.

However, the ARM SMMUv3 architecture specification states that updating the SMMU_STRTAB_BASE register while SMMUEN == 1 is UNPREDICTABLE or ignored.

This leaves a kdump kernel no choice but to adopt the stream table from the crashed kernel.

Introduce ARM_SMMU_OPT_KDUMP_ADOPT and adopt functions memremapping all the stream tables extracted from STRTAB_BASE and STRTAB_BASE_CFG. Add them in a new arm-smmu-v3-kdump.c, which is only built when CONFIG_CRASH_DUMP=y.

Note that the adoption of the crashed kernel's stream table follows certain strict rules, since the old stream table might be compromised. Thus, apply some basic validations against the values read from the registers. If tests fail, it means the stream table cannot be trusted, so toss it entirely. To avoid OOM due to a potentially corrupted stream table, the memremap for l2 tables is done lazily on the kdump kernel's demand.

The new option will be set in a following change, once the device reset and the RMR setup are reworked not to overwrite the adopted stream table, and the crashed kernel's in-use ASIDs and VMIDs are reserved.

Quoted 8 lines
Assisted-by Claude:claude-fable-5
---
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 229 ++++++++++++++++++

Please don't put this here. The live update / handover stuff is going to need very similar logic (see the RFC from Pranjal) and I don't fancy having to rename or resplit this file when that comes along.

Maybe just stick all of arm-smmu-v3-kexec.c and arm-smmu-v3-kdump.c into arm-smmu-v3-handover.c or something?

Quoted 12 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c new file +6 -0
new file mode 100644
index 0000000000000..a074d59ce3445
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Implementation of the kdump stream table adoption for ARM SMMUv3
+ *
+ * When the crashed kernel left the SMMU enabled with in-flight DMAs, the kdump
+ * kernel adopts the crashed kernel's stream tables, instead of doing a regular

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c line 16

Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c new file +10 -0
+ * reset, to keep in-flight DMAs translating until the endpoint device drivers
+ * re-probe and quiesce their devices.
+ *
+ * Note:
+ *  - Adoption only starts on an SMMU that the crashed kernel left enabled, as a
+ *    disabled SMMU (CR0_SMMUEN=0) could hold meaningless register values.
+ *  - Values read from the crashed kernel's registers get structural validation
+ *    only (format, size, span, alignment, and ID range); the physical addresses
+ *    are not vetted, as the kdump kernel has no record of which pages held the
+ *    tables.

Can we at least check that they don't point at the kdump region?

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c line 25

Quoted 9 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c new file +9 -0
+ *  - A structural inconsistency at adoption time tosses the entire adoption and
+ *    makes the SMMU fall back to a full reset blocking in-flight DMAs.
+ *  - L2 stream tables are adopted lazily at master-inserting time, to bound the
+ *    peak memory use against a corrupted L1 table; any lazy L2 adoption failure
+ *    rejects that device alone, as its blast radius is bounded to the bus.
+ *  - Only a coherent SMMU (ARM_SMMU_FEAT_COHERENCY) is supported, as the stream
+ *    table adoption is done by memremap with MEMREMAP_WB, which is verified on
+ *    the real hardware. Callers of these functions are responsible for gating
+ *    ARM_SMMU_FEAT_COHERENCY once during the probe.

This is an artificial restriction and not one that I'm wild about for kdump:
we should be able to support this for non-coherent SMMUs as well. Is there anything more to it than using MEMREMAP_WC in that case?

Will

#18 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 7 lines · Will Deacon
Quoted 3 lines
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 229 ++++++++++++++++++

Please don't put this here. The live update / handover stuff is going to need very similar logic (see the RFC from Pranjal) and I don't fancy having to rename or resplit this file when that comes along.

Ah, I was thinking we'd put most of that stuff in here, but a kexec name would probably be nicer.

Quoted 9 lines
Quoted 7 lines

+ * Note:
+ * - Adoption only starts on an SMMU that the crashed kernel left enabled, as a
+ * disabled SMMU (CR0_SMMUEN=0) could hold meaningless register values.
+ * - Values read from the crashed kernel's registers get structural validation

+ *    only (format, size, span, alignment, and ID range); the physical addresses
+ *    are not vetted, as the kdump kernel has no record of which pages held the
+ *    tables.

Can we at least check that they don't point at the kdump region?

I don't think it is necessary, I was on the fence about all this exisitng validation even in the first place.

Is there any easy way to do it?

Quoted 10 lines
Quoted 9 lines

+ * - A structural inconsistency at adoption time tosses the entire adoption and
+ * makes the SMMU fall back to a full reset blocking in-flight DMAs.
+ * - L2 stream tables are adopted lazily at master-inserting time, to bound the
+ * peak memory use against a corrupted L1 table; any lazy L2 adoption failure
+ * rejects that device alone, as its blast radius is bounded to the bus.
+ * - Only a coherent SMMU (ARM_SMMU_FEAT_COHERENCY) is supported, as the stream

+ *    table adoption is done by memremap with MEMREMAP_WB, which is verified on
+ *    the real hardware. Callers of these functions are responsible for gating
+ *    ARM_SMMU_FEAT_COHERENCY once during the probe.
Quoted 3 lines

This is an artificial restriction and not one that I'm wild about for kdump:
we should be able to support this for non-coherent SMMUs as well. Is there anything more to it than using MEMREMAP_WC in that case?

I've forgotten why it ended up like this, it was some complication that seemed hard.. MEMREMAP_WC is not the same attribute dma coherent would have used, I'm not sure we have the right stuff to be able to flush any write combining buffer? I'm nervous about that at least.

Remember this is not just reading it but it has to operate like this after the fact. In handover you wanted to use the dma cohernent preservation.

So, I think this restriction is mostly a lack of the right MEMREMAP flag? It is not insovlable just work outside the scope of this series.

Jason

#19 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 20 lines · Jason Gunthorpe
Quoted 10 lines · Will Deacon
Quoted 9 lines

+ * - A structural inconsistency at adoption time tosses the entire adoption and
+ * makes the SMMU fall back to a full reset blocking in-flight DMAs.
+ * - L2 stream tables are adopted lazily at master-inserting time, to bound the
+ * peak memory use against a corrupted L1 table; any lazy L2 adoption failure
+ * rejects that device alone, as its blast radius is bounded to the bus.
+ * - Only a coherent SMMU (ARM_SMMU_FEAT_COHERENCY) is supported, as the stream

+ *    table adoption is done by memremap with MEMREMAP_WB, which is verified on
+ *    the real hardware. Callers of these functions are responsible for gating
+ *    ARM_SMMU_FEAT_COHERENCY once during the probe.
Quoted 3 lines

This is an artificial restriction and not one that I'm wild about for kdump:
we should be able to support this for non-coherent SMMUs as well. Is there anything more to it than using MEMREMAP_WC in that case?

I've forgotten why it ended up like this, it was some complication that seemed hard.. MEMREMAP_WC is not the same attribute dma coherent would have used, I'm not sure we have the right stuff to be able to flush any write combining buffer? I'm nervous about that at least.

Quoted 6 lines

Remember this is not just reading it but it has to operate like this after the fact. In handover you wanted to use the dma cohernent preservation.

So, I think this restriction is mostly a lack of the right MEMREMAP flag? It is not insovlable just work outside the scope of this series.

I agree that it's safer to defer that to a followup series. It also needs someone who has a non-coherent HW for a full test.

On the other hand, this series is tested by folks from multiple organizations, so it's really a needed and verified one.

Thanks
Nicolin

maintainer
#20 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 25 lines · Nicolin Chen
Quoted 24 lines · Jason Gunthorpe
Quoted 10 lines · Will Deacon
Quoted 9 lines

+ * - A structural inconsistency at adoption time tosses the entire adoption and
+ * makes the SMMU fall back to a full reset blocking in-flight DMAs.
+ * - L2 stream tables are adopted lazily at master-inserting time, to bound the
+ * peak memory use against a corrupted L1 table; any lazy L2 adoption failure
+ * rejects that device alone, as its blast radius is bounded to the bus.
+ * - Only a coherent SMMU (ARM_SMMU_FEAT_COHERENCY) is supported, as the stream

+ *    table adoption is done by memremap with MEMREMAP_WB, which is verified on
+ *    the real hardware. Callers of these functions are responsible for gating
+ *    ARM_SMMU_FEAT_COHERENCY once during the probe.
Quoted 3 lines

This is an artificial restriction and not one that I'm wild about for kdump:
we should be able to support this for non-coherent SMMUs as well. Is there anything more to it than using MEMREMAP_WC in that case?

I've forgotten why it ended up like this, it was some complication that seemed hard.. MEMREMAP_WC is not the same attribute dma coherent would have used, I'm not sure we have the right stuff to be able to flush any write combining buffer? I'm nervous about that at least.

Remember this is not just reading it but it has to operate like this after the fact. In handover you wanted to use the dma cohernent preservation.

Quoted 8 lines
Quoted 2 lines

So, I think this restriction is mostly a lack of the right MEMREMAP flag? It is not insovlable just work outside the scope of this series.

I agree that it's safer to defer that to a followup series. It also needs someone who has a non-coherent HW for a full test.

On the other hand, this series is tested by folks from multiple organizations, so it's really a needed and verified one.

I wasn't disputing that, though. I'm saying that it's half finished without the non-coherent part, so I'd like to understand what's needed to add that. Can you please explain what is needed beyond using MEMREMAP_WC? That maps to Normal-NC on arm64, just like the DMA API does.

Will

#21 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 32 lines · Will Deacon
Quoted 31 lines · Nicolin Chen
Quoted 20 lines · Jason Gunthorpe
Quoted 10 lines · Will Deacon
Quoted 9 lines

+ * - A structural inconsistency at adoption time tosses the entire adoption and
+ * makes the SMMU fall back to a full reset blocking in-flight DMAs.
+ * - L2 stream tables are adopted lazily at master-inserting time, to bound the
+ * peak memory use against a corrupted L1 table; any lazy L2 adoption failure
+ * rejects that device alone, as its blast radius is bounded to the bus.
+ * - Only a coherent SMMU (ARM_SMMU_FEAT_COHERENCY) is supported, as the stream

+ *    table adoption is done by memremap with MEMREMAP_WB, which is verified on
+ *    the real hardware. Callers of these functions are responsible for gating
+ *    ARM_SMMU_FEAT_COHERENCY once during the probe.
Quoted 3 lines

This is an artificial restriction and not one that I'm wild about for kdump:
we should be able to support this for non-coherent SMMUs as well. Is there anything more to it than using MEMREMAP_WC in that case?

I've forgotten why it ended up like this, it was some complication that seemed hard.. MEMREMAP_WC is not the same attribute dma coherent would have used, I'm not sure we have the right stuff to be able to flush any write combining buffer? I'm nervous about that at least.

Quoted 6 lines

Remember this is not just reading it but it has to operate like this after the fact. In handover you wanted to use the dma cohernent preservation.

So, I think this restriction is mostly a lack of the right MEMREMAP flag? It is not insovlable just work outside the scope of this series.

I agree that it's safer to defer that to a followup series. It also needs someone who has a non-coherent HW for a full test.

Quoted 7 lines
Quoted 2 lines

On the other hand, this series is tested by folks from multiple organizations, so it's really a needed and verified one.

I wasn't disputing that, though. I'm saying that it's half finished without the non-coherent part, so I'd like to understand what's needed to add that. Can you please explain what is needed beyond using MEMREMAP_WC? That maps to Normal-NC on arm64, just like the DMA API does.

I am not aware of anything beyond MEMREMAP_WC.

It should likely need a helper to select flag:
ARM_SMMU_FEAT_COHERENCY ? MEMREMAP_WB : MEMREMAP_WC
and a verification on a non-coherent SMMU.

Thanks
Nicolin

#22 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

In reply to Will Deacon
Quoted 11 lines · Will Deacon
Quoted 3 lines · Nicolin Chen
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 229 ++++++++++++++++++

Please don't put this here. The live update / handover stuff is going to need very similar logic (see the RFC from Pranjal) and I don't fancy having to rename or resplit this file when that comes along.

Maybe just stick all of arm-smmu-v3-kexec.c and arm-smmu-v3-kdump.c into arm-smmu-v3-handover.c or something?

Hmm, Pranjal and I already aligned that cleanly. And there is no further rename/resplit in his series.

arm-smmu-v3-kexec: common helpers for both
arm-smmu-v3-kdump: kdump specific (my series)
arm-smmu-v3-liveupdate: liveupdate specific (Pranj's)

Each of them is built on a separate config:
arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_KEXEC) += arm-smmu-v3-kexec.o arm_smmu_v3-$(CONFIG_IOMMU_LIVEUPDATE) += arm-smmu-v3-liveupdate.o arm_smmu_v3-$(CONFIG_CRASH_DUMP) += arm-smmu-v3-kdump.o

If we put everything into one big file, we'd add the two individual config guards internally, which isn't as clean as currently IMHO.

Note that the kdump file has a very long notes (kdump specific).

Thanks
Nicolin

maintainer
#23 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 25 lines · Nicolin Chen
Quoted 11 lines · Will Deacon
Quoted 3 lines · Nicolin Chen
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 229 ++++++++++++++++++

Please don't put this here. The live update / handover stuff is going to need very similar logic (see the RFC from Pranjal) and I don't fancy having to rename or resplit this file when that comes along.

Maybe just stick all of arm-smmu-v3-kexec.c and arm-smmu-v3-kdump.c into arm-smmu-v3-handover.c or something?

Hmm, Pranjal and I already aligned that cleanly. And there is no further rename/resplit in his series.

arm-smmu-v3-kexec: common helpers for both
arm-smmu-v3-kdump: kdump specific (my series)
arm-smmu-v3-liveupdate: liveupdate specific (Pranj's)

Each of them is built on a separate config:
arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_KEXEC) += arm-smmu-v3-kexec.o arm_smmu_v3-$(CONFIG_IOMMU_LIVEUPDATE) += arm-smmu-v3-liveupdate.o arm_smmu_v3-$(CONFIG_CRASH_DUMP) += arm-smmu-v3-kdump.o

Quoted 4 lines

If we put everything into one big file, we'd add the two individual config guards internally, which isn't as clean as currently IMHO.

Note that the kdump file has a very long notes (kdump specific).

I dunno, three files seems way over the top for this imo. I'd rather have one (even if it uses preprocessor guards internally) otherwise it just seems inevitable that we'll have random internals exposed from one file to the other, or duplication of logic or both. Even if we end up splitting it eventually, I'd much rather start from a position where everything is in one place.

Will

#24 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 29 lines · Will Deacon
Quoted 28 lines · Nicolin Chen
Quoted 11 lines · Will Deacon
Quoted 3 lines · Nicolin Chen
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 229 ++++++++++++++++++

Please don't put this here. The live update / handover stuff is going to need very similar logic (see the RFC from Pranjal) and I don't fancy having to rename or resplit this file when that comes along.

Maybe just stick all of arm-smmu-v3-kexec.c and arm-smmu-v3-kdump.c into arm-smmu-v3-handover.c or something?

Hmm, Pranjal and I already aligned that cleanly. And there is no further rename/resplit in his series.

arm-smmu-v3-kexec: common helpers for both
arm-smmu-v3-kdump: kdump specific (my series)
arm-smmu-v3-liveupdate: liveupdate specific (Pranj's)

Each of them is built on a separate config:
arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_KEXEC) += arm-smmu-v3-kexec.o arm_smmu_v3-$(CONFIG_IOMMU_LIVEUPDATE) += arm-smmu-v3-liveupdate.o arm_smmu_v3-$(CONFIG_CRASH_DUMP) += arm-smmu-v3-kdump.o

If we put everything into one big file, we'd add the two individual config guards internally, which isn't as clean as currently IMHO.

Quoted 8 lines
Quoted 1 line

Note that the kdump file has a very long notes (kdump specific).

I dunno, three files seems way over the top for this imo. I'd rather have one (even if it uses preprocessor guards internally) otherwise it just seems inevitable that we'll have random internals exposed from one file to the other, or duplication of logic or both. Even if we end up splitting it eventually, I'd much rather start from a position where everything is in one place.

OK. I'll merge them, maybe into arm-smmu-v3-kexec as Jason suggested?

Or do you still prefer handover?

Thanks
Nicolin

maintainer
#25 lore raw

Re: [PATCH v10 06/13] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

Quoted 39 lines · Nicolin Chen
Quoted 29 lines · Will Deacon
Quoted 28 lines · Nicolin Chen
Quoted 11 lines · Will Deacon
Quoted 3 lines · Nicolin Chen
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 229 ++++++++++++++++++

Please don't put this here. The live update / handover stuff is going to need very similar logic (see the RFC from Pranjal) and I don't fancy having to rename or resplit this file when that comes along.

Maybe just stick all of arm-smmu-v3-kexec.c and arm-smmu-v3-kdump.c into arm-smmu-v3-handover.c or something?

Hmm, Pranjal and I already aligned that cleanly. And there is no further rename/resplit in his series.

arm-smmu-v3-kexec: common helpers for both
arm-smmu-v3-kdump: kdump specific (my series)
arm-smmu-v3-liveupdate: liveupdate specific (Pranj's)

Each of them is built on a separate config:
arm_smmu_v3-$(CONFIG_ARM_SMMU_V3_KEXEC) += arm-smmu-v3-kexec.o arm_smmu_v3-$(CONFIG_IOMMU_LIVEUPDATE) += arm-smmu-v3-liveupdate.o arm_smmu_v3-$(CONFIG_CRASH_DUMP) += arm-smmu-v3-kdump.o

If we put everything into one big file, we'd add the two individual config guards internally, which isn't as clean as currently IMHO.

Quoted 8 lines
Quoted 1 line

Note that the kdump file has a very long notes (kdump specific).

I dunno, three files seems way over the top for this imo. I'd rather have one (even if it uses preprocessor guards internally) otherwise it just seems inevitable that we'll have random internals exposed from one file to the other, or duplication of logic or both. Even if we end up splitting it eventually, I'd much rather start from a position where everything is in one place.

Quoted 3 lines

OK. I'll merge them, maybe into arm-smmu-v3-kexec as Jason suggested?

Or do you still prefer handover?

I'm terrible at naming, so I don't mind :)

Will

#26 lore raw

PATCH v10 7/13 iommu/arm-smmu-v3-kexec: Add a CD table parse helper

In reply to Nicolin Chen

An S1 STE points to a CD table that both of the kexec flavors decode: the kdump adoption scans the CD table to reserve all the in-use ASIDs, and the live-update restoration claims the CD table via the KHO restore API.

Add another read-only helper to the arm-smmu-v3-kexec.c:
- arm_smmu_kexec_check_ste_cdtab()

It validates the CD table geometry in an S1 STE against this kernel's own ssid_bits and the 2-level HW capability. And it accepts a linear CD table on the 2-level capable HW too, since a previous kernel might have used one, like the linear stream table.

The CD table base gets validated against the table size as well, since the spec aligns a CD table to its own size, where an unaligned base would be a CONSTRAINED UNPREDICTABLE case: HW may zero its low bits or may fetch any CD in the table, so a scan reading such a base could miss the CDs in use.

Assisted-by Claude:claude-fable-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  3 +
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c | 57 +++++++++++++++++++
 2 files changed, 60 insertions(+)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +3 -0
index 41d7a907b9ba2..1bcf6cb2ebb3c 100644
@@ -1261,6 +1261,9 @@ int arm_smmu_kexec_parse_strtab_linear(struct arm_smmu_device *smmu,
 int arm_smmu_kexec_check_strtab_l1_desc(struct arm_smmu_device *smmu,
 					u64 l1_desc, u32 idx,
 					phys_addr_t *l2_base);
+int arm_smmu_kexec_check_ste_cdtab(struct arm_smmu_device *smmu, u64 ste0,
+				   phys_addr_t *cdtab, u32 *s1fmt,
+				   u32 *max_contexts);
 #endif /* CONFIG_ARM_SMMU_V3_KEXEC */
 
 #ifdef CONFIG_CRASH_DUMP
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c +57 -0

60 lines not on this page · raw message

Signature · 1 line
2.43.0
#27 lore raw

PATCH v10 8/13 iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers

In reply to Nicolin Chen

An adopted stream table keeps translating in-flight DMAs, so the SMMU also keeps caching the TLB entries tagged with the previous kernel's ASIDs and VMIDs. Any kexec flavor retaining those translations must reserve all the in-use IDs, so that this kernel cannot ever give any of its own domains an overlapping ID that would alias the previous kernel's cached TLB entries.

Add the reservation helpers to arm-smmu-v3-kexec.c:
- arm_smmu_kexec_scan_and_resv_ids()
- arm_smmu_kexec_unresv_ids()

At adoption time, arm_smmu_kexec_scan_and_resv_ids() runs a one-off scan:
walking the stream table set up in the strtab_cfg and every CD table behind an S1 STE, it reserves the ASIDs in the asid xarray and the S2VMIDs in the vmid ida. The scan memremaps each table transiently, which is a necessity even for a live update: the ID reservation must be complete by the end of the SMMU probe itself, before this kernel starts allocating any ID for its own domains, yet the CD tables are only claimed when their masters get to re-probe, long after that point. Thus, the scan cannot rely on any claimed mapping and must map each table by itself.

The scan walks untrusted tables, yet every loop is strictly index-bounded:
the iteration counts derive from the log2size and s1cdmax fields, which are validated against this kernel's own sid_bits and ssid_bits, so a corrupted table cannot extend the walk.

The ASID space is per SMMU instance and the scan runs before its own SMMU registers with the IOMMU core, i.e. before any domain of that SMMU can hold an ASID. So it reserves via xa_reserve(), and a failing scan just empties the xarray and the ida, since they hold nothing else at that point.

Assisted-by Claude:claude-fable-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   2 +
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c | 235 ++++++++++++++++++
 2 files changed, 237 insertions(+)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +2 -0
index 1bcf6cb2ebb3c..64927c54f8b6b 100644
@@ -1264,6 +1264,8 @@ int arm_smmu_kexec_check_strtab_l1_desc(struct arm_smmu_device *smmu,
 int arm_smmu_kexec_check_ste_cdtab(struct arm_smmu_device *smmu, u64 ste0,
 				   phys_addr_t *cdtab, u32 *s1fmt,
 				   u32 *max_contexts);
+int arm_smmu_kexec_scan_and_resv_ids(struct arm_smmu_device *smmu);
+void arm_smmu_kexec_unresv_ids(struct arm_smmu_device *smmu);
 #endif /* CONFIG_ARM_SMMU_V3_KEXEC */
 
 #ifdef CONFIG_CRASH_DUMP
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c +235 -0

238 lines not on this page · raw message

Signature · 1 line
2.43.0
Reviewed-by on v10 8/13
#28 lore raw

Re: [PATCH v10 08/13] iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers

Quoted 11 lines · Nicolin Chen

An adopted stream table keeps translating in-flight DMAs, so the SMMU also keeps caching the TLB entries tagged with the previous kernel's ASIDs and VMIDs. Any kexec flavor retaining those translations must reserve all the in-use IDs, so that this kernel cannot ever give any of its own domains an overlapping ID that would alias the previous kernel's cached TLB entries.

Add the reservation helpers to arm-smmu-v3-kexec.c:
- arm_smmu_kexec_scan_and_resv_ids()
- arm_smmu_kexec_unresv_ids()

[...]

Signature · 1 line
Jason
#29 lore raw

PATCH v10 9/13 iommu/arm-smmu-v3-kdump: Reserve crashed kernel's ASIDs and VMIDs

In reply to Nicolin Chen

The adopted stream table keeps translating in-flight DMA, so the SMMU keeps caching TLB entries tagged with the crashed kernel's ASIDs and VMIDs. If this kernel handed one of those IDs to its own domain, the new domain's DMA could hit the crashed kernel's cached translations, e.g. a stale entry left behind by an invalidation that the crash cut short.

Scan the adopted stream table at adoption time, reserving every ID in use via arm_smmu_kexec_scan_and_resv_ids(), and roll all of them back through arm_smmu_kexec_unresv_ids() should the scan fail.

A nested STE's guest-owned CD table is left alone, since its ASIDs live in a space of their own under that STE's VMID. Reserving that VMID covers all of them, so there is no reason for the scan to walk the (VMID, ASID) pairs behind it. The only ASIDs needing a reservation are those in the space that this kernel uses for its own domains.

Note that, on an E2H/VHE host, the kernel's stage-1 domains are tagged by the EL2 ASID, and the TLBI_EL2_* commands take no VMID. So isolating this kernel by a reserved VMID alone would not work. Reserving the ASIDs covers both the E2H and the NSEL1 cases.

Reservations are never released: a kdump kernel reboots after it saves the vmcore, and the full-reset fallback flushes the entire TLB, which turns any stale reservation into a merely unused ID.

If the scan finds any inconsistent structure, toss the entire adoption and fall back to the full reset.

Assisted-by Claude:claude-fable-5
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 12 +++++++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c       |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c +11 -1
index a074d59ce3445..6740cc7e671cb 100644
@@ -205,16 +205,26 @@ int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu)
 		goto err;
 	}
 
+	ret = arm_smmu_kexec_scan_and_resv_ids(smmu);
+	if (ret) {
+		dev_warn(smmu->dev, "failed to reserve in-use ASIDs/VMIDs\n");
+		arm_smmu_kdump_adopt_cleanup(smmu);
+		goto err_unresv;
+	}
+
 	ret = devm_add_action_or_reset(smmu->dev, arm_smmu_kdump_adopt_cleanup,
 				       smmu);
 	/* devm_add_action_or_reset ran the cleanup upon failure */
 	if (ret) {
 		dev_warn(smmu->dev, "failed to set up cleanup action\n");
-		goto err;
+		goto err_unresv;
 	}
 
 	return 0;
 
+err_unresv:
+	/* The full reset will flush the entire TLB, so release everything */
+	arm_smmu_kexec_unresv_ids(smmu);
 err:
 	dev_warn(smmu->dev, "falling back to full reset\n");
 	/*
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +1 -0
index 971b12517941f..1d4fb6c4f0a91 100644
@@ -4610,6 +4610,7 @@ static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
 {
 	int ret;
 
+	/* Init both first, as a kdump adoption reserves in-use ASIDs/VMIDs */
 	ida_init(&smmu->vmid_map);
 	ret = devm_add_action_or_reset(smmu->dev, arm_smmu_destroy_vmid_map,
 				       &smmu->vmid_map);
Signature · 1 line
2.43.0
#30 lore raw

PATCH v10 10/13 iommu/arm-smmu-v3-kdump: Implement is_attach_deferred()

In reply to Nicolin Chen

Though the kdump kernel adopts the crashed kernel's stream table, the iommu core will still try to attach each probed device to a default domain, which overwrites the adopted STE and breaks in-flight DMA from that device.

Implement an is_attach_deferred() callback to prevent this. For each device that has STE.V=1 and STE.Cfg!=Abort in the adopted table, defer the default domain attachment, until the device driver explicitly requests it.

Also, move arm_smmu_get_step_for_sid() to the header for the kdump function to use.

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   | 23 +++++++++++++++++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 19 ++++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 25 ++++++++-----------
 3 files changed, 52 insertions(+), 15 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +23 -0
index 64927c54f8b6b..3f2ebdb99e8f7 100644
@@ -1219,6 +1219,22 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
 				  struct arm_smmu_cmdq *cmdq,
 				  struct arm_smmu_cmd *cmds, int n,
 				  bool sync);
+
+static inline struct arm_smmu_ste *
+arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid)
+{
+	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+
+	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
+		/* Two-level walk */
+		return &cfg->l2.l2ptrs[arm_smmu_strtab_l1_idx(sid)]
+				->stes[arm_smmu_strtab_l2_idx(sid)];
+	} else {
+		/* Simple linear lookup */
+		return &cfg->linear.table[sid];
+	}
+}
+
 int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
 				struct arm_smmu_cmdq *cmdq,
 				struct arm_smmu_cmd *cmds, int n,
@@ -1273,6 +1289,7 @@ int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu);
 int arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu,
 					    u32 sid, phys_addr_t base, u32 span,
 					    struct arm_smmu_strtab_l2 **l2table);
+bool arm_smmu_kdump_is_attach_deferred(struct arm_smmu_master *master);
 #else /* CONFIG_CRASH_DUMP */
 static inline int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu)
 {
@@ -1286,6 +1303,12 @@ arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid,
 {
 	return -EOPNOTSUPP;
 }
+
+static inline bool
+arm_smmu_kdump_is_attach_deferred(struct arm_smmu_master *master)
+{
+	return false;
+}
 #endif /* CONFIG_CRASH_DUMP */
 
 struct arm_vsmmu {
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c +19 -0
index 6740cc7e671cb..5bc938901b65d 100644
@@ -237,3 +237,22 @@ int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu)
 	smmu->options &= ~ARM_SMMU_OPT_KDUMP_ADOPT;
 	return ret;
 }
+
+bool arm_smmu_kdump_is_attach_deferred(struct arm_smmu_master *master)
+{
+	struct arm_smmu_device *smmu = master->smmu;
+	int i;
+
+	for (i = 0; i < master->num_streams; i++) {
+		struct arm_smmu_ste *ste =
+			arm_smmu_get_step_for_sid(smmu, master->streams[i].id);
+		u64 ent0 = le64_to_cpu(ste->data[0]);
+
+		/* Defer only when there might be in-flight DMAs */
+		if ((ent0 & STRTAB_STE_0_V) &&
+		    FIELD_GET(STRTAB_STE_0_CFG, ent0) != STRTAB_STE_0_CFG_ABORT)
+			return true;
+	}
+
+	return false;
+}
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +10 -15
index 1d4fb6c4f0a91..ef1ef4d452949 100644
@@ -2965,21 +2965,6 @@ static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
 	return 0;
 }
 
-static struct arm_smmu_ste *
-arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid)
-{
-	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
-
-	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
-		/* Two-level walk */
-		return &cfg->l2.l2ptrs[arm_smmu_strtab_l1_idx(sid)]
-				->stes[arm_smmu_strtab_l2_idx(sid)];
-	} else {
-		/* Simple linear lookup */
-		return &cfg->linear.table[sid];
-	}
-}
-
 void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master,
 				  const struct arm_smmu_ste *target)
 {
@@ -4390,6 +4375,15 @@ static int arm_smmu_def_domain_type(struct device *dev)
 	return 0;
 }
 
+static bool arm_smmu_is_attach_deferred(struct device *dev)
+{
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+
+	if (master->smmu->options & ARM_SMMU_OPT_KDUMP_ADOPT)
+		return arm_smmu_kdump_is_attach_deferred(master);
+	return false;
+}
+
 static const struct iommu_ops arm_smmu_ops = {
 	.identity_domain	= &arm_smmu_identity_domain,
 	.blocked_domain		= &arm_smmu_blocked_domain,
@@ -4398,6 +4392,7 @@ static const struct iommu_ops arm_smmu_ops = {
 	.hw_info		= arm_smmu_hw_info,
 	.domain_alloc_sva       = arm_smmu_sva_domain_alloc,
 	.domain_alloc_paging_flags = arm_smmu_domain_alloc_paging_flags,
+	.is_attach_deferred	= arm_smmu_is_attach_deferred,
 	.probe_device		= arm_smmu_probe_device,
 	.release_device		= arm_smmu_release_device,
 	.device_group		= arm_smmu_device_group,
Signature · 1 line
2.43.0
#31 lore raw

PATCH v10 11/13 iommu/arm-smmu-v3: Retain CR0_SMMUEN during kdump device reset

In reply to Nicolin Chen

When ARM_SMMU_OPT_KDUMP_ADOPT is detected, do not disable SMMUEN and skip the CR1/CR2/STRTAB_BASE update sequence in arm_smmu_device_reset(). Those register writes are all CONSTRAINED UNPREDICTABLE while CR0_SMMUEN==1, so leaving them intact lets in-flight DMAs continue to be translated by the adopted stream table.

Initialize 'enables' to 0, so it can carry the retained CR0 fields in the kdump case, clearing only the queue enable bits. Then, preserve them when enabling the command queue.

The retained CR0 keeps the crashed kernel's ATSCHK too, which selects the fast (0) or the safe (1) mode for the ATS translated traffic. Switching to the safe mode would start checking the in-flight traffic against STE.EATS fields that the crashed kernel never set up for a check, aborting the very DMAs being carried.

Clear latched gerror bits if necessary.

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 60 +++++++++++++++++++--
 1 file changed, 56 insertions(+), 4 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +56 -4
index ef1ef4d452949..0fb8c60526699 100644
@@ -4882,10 +4882,28 @@ static void arm_smmu_write_strtab(struct arm_smmu_device *smmu)
 static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
 {
 	int ret;
-	u32 reg, enables;
+	u32 reg, enables = 0;
 
-	/* Clear CR0 and sync (disables SMMU and queue processing) */
 	reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
+
+	/*
+	 * In a kdump case (set when CR0_SMMUEN=1 and !GERROR_SFM_ERR), retain
+	 * all the live CR0 fields, e.g. CR0_SMMUEN to avoid aborting in-flight
+	 * DMA and CR0_ATSCHK to carry on the ATS-check policy, while clearing
+	 * only the queue enable bits for this kernel to take over the queues.
+	 *
+	 * According to spec, updating STRTAB_BASE/CR1/CR2 when CR0_SMMUEN=1 is
+	 * CONSTRAINED UNPREDICTABLE. So, skip those register updates and rely
+	 * on the adopted stream table from the crashed kernel.
+	 */
+	if (smmu->options & ARM_SMMU_OPT_KDUMP_ADOPT) {
+		dev_info(smmu->dev,
+			 "kdump: retaining SMMUEN for in-flight DMA\n");
+		enables = reg & ~(CR0_CMDQEN | CR0_EVTQEN | CR0_PRIQEN);
+		goto reset_queues;
+	}
+
+	/* Clear CR0 and sync (disables SMMU and queue processing) */
 	if (reg & CR0_SMMUEN) {
 		dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
 		arm_smmu_update_gbpa(smmu, GBPA_ABORT, 0);
@@ -4915,12 +4933,41 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
 	/* Stream table */
 	arm_smmu_write_strtab(smmu);
 
+reset_queues:
+	if (smmu->options & ARM_SMMU_OPT_KDUMP_ADOPT) {
+		/*
+		 * Disable queues since arm_smmu_device_disable() was skipped.
+		 * CR0 fields are independent per spec, so the queue enable bits
+		 * can be cleared while retaining SMMUEN=1.
+		 */
+		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+					      ARM_SMMU_CR0ACK);
+		if (ret) {
+			dev_err(smmu->dev, "failed to disable queues\n");
+			return ret;
+		}
+	}
+
+	/*
+	 * GERROR bits are latched. Read after queue disabling so that unhandled
+	 * errors would be visible. Ack everything prior to re-enabling the CMDQ
+	 * as a stale CMDQ_ERR would halt the CMDQ and new command will timeout.
+	 * Acking SFM_ERR is defined too, although it would not exit the SFM.
+	 */
+	if (is_kdump_kernel()) {
+		u32 gerror = readl_relaxed(smmu->base + ARM_SMMU_GERROR);
+		u32 gerrorn = readl_relaxed(smmu->base + ARM_SMMU_GERRORN);
+
+		if ((gerror ^ gerrorn) & GERROR_ERR_MASK)
+			writel(gerror, smmu->base + ARM_SMMU_GERRORN);
+	}
+
 	/* Command queue */
 	writeq_relaxed(smmu->cmdq.q.q_base, smmu->base + ARM_SMMU_CMDQ_BASE);
 	writel_relaxed(smmu->cmdq.q.llq.prod, smmu->base + ARM_SMMU_CMDQ_PROD);
 	writel_relaxed(smmu->cmdq.q.llq.cons, smmu->base + ARM_SMMU_CMDQ_CONS);
 
-	enables = CR0_CMDQEN;
+	enables |= CR0_CMDQEN;
 	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
 				      ARM_SMMU_CR0ACK);
 	if (ret) {
@@ -4976,7 +5023,12 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
 		}
 	}
 
-	if (smmu->features & ARM_SMMU_FEAT_ATS) {
+	/*
+	 * In a kdump adopt case, retain the crashed kernel's ATS-check policy
+	 * captured above rather than forcing it on.
+	 */
+	if (!(smmu->options & ARM_SMMU_OPT_KDUMP_ADOPT) &&
+	    (smmu->features & ARM_SMMU_FEAT_ATS)) {
 		enables |= CR0_ATSCHK;
 		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
 					      ARM_SMMU_CR0ACK);
Signature · 1 line
2.43.0
#32 lore raw

PATCH v10 12/13 iommu/arm-smmu-v3: Skip RMR bypass for kdump adoption

In reply to Nicolin Chen

RMR bypass STEs are installed during SMMUv3 probe for StreamIDs listed by IORT RMR nodes. A normal boot switches the driver to a fresh stream table whose initial STEs abort, so those RMR SIDs need bypass entries before it becomes live. This preserves firmware/guest-owned traffic, including vSMMU guest MSI cases built around RMR-described SIDs.

ARM_SMMU_OPT_KDUMP_ADOPT is the opposite case: the driver keeps SMMUEN set and adopts the crashed kernel's stream table, so RMR SIDs already have the only translation state known to be safe for active in-flight DMA. Replacing an adopted STE with bypass can turn translated DMA into physical DMA, then point it at the wrong memory.

arm_smmu_make_bypass_ste() also rewrites the STE in place after clearing it first. While the table is live, a concurrent hardware STE fetch can observe V=0 or mixed old/new state.

Leaving the adopted STE unmodified keeps the kdump kernel using the crashed kernel's translation. That gives the endpoint driver a chance to probe and quiesce the device.

If the old STE was already abort or invalid, installing bypass would create new DMA permission; leaving it alone is a safer failure mode. Later domain setup still gets the RMR direct mappings through the reserved-region path.

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +9 -4
index 0fb8c60526699..525cdc2e7d495 100644
@@ -5536,6 +5536,14 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
 	struct list_head rmr_list;
 	struct iommu_resv_region *e;
 
+	/*
+	 * Kdump adoption keeps the crashed kernel's table live. Rewriting the
+	 * adopted STE here could expose an in-flight fetch to a transient V=0
+	 * entry, or change Cfg=translate to Cfg=bypass. Must skip here.
+	 */
+	if (smmu->options & ARM_SMMU_OPT_KDUMP_ADOPT)
+		return;
+
 	INIT_LIST_HEAD(&rmr_list);
 	iort_get_rmr_sids(dev_fwnode(smmu->dev), &rmr_list);
 
@@ -5552,10 +5560,7 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
 				continue;
 			}
 
-			/*
-			 * STE table is not programmed to HW, see
-			 * arm_smmu_initial_bypass_stes()
-			 */
+			/* The fresh stream table is not yet live. */
 			arm_smmu_make_bypass_ste(smmu,
 				arm_smmu_get_step_for_sid(smmu, rmr->sids[i]));
 		}
Signature · 1 line
2.43.0
#33 lore raw

PATCH v10 13/13 iommu/arm-smmu-v3: Detect ARM_SMMU_OPT_KDUMP_ADOPT in probe()

In reply to Nicolin Chen

arm_smmu_device_hw_probe() runs before arm_smmu_init_structures(), so it's natural to decide whether the kdump kernel must adopt the crashed kernel's stream table.

Given that memremap is used to adopt the old stream table, set this option only on a coherent SMMU.

And make sure SMMU isn't in Service Failure Mode.

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  5 ++++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 27 +++++++++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  4 +++
 3 files changed, 36 insertions(+)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +5 -0
index 3f2ebdb99e8f7..e0f16f8d13842 100644
@@ -1290,6 +1290,7 @@ int arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu,
 					    u32 sid, phys_addr_t base, u32 span,
 					    struct arm_smmu_strtab_l2 **l2table);
 bool arm_smmu_kdump_is_attach_deferred(struct arm_smmu_master *master);
+void arm_smmu_device_kdump_probe(struct arm_smmu_device *smmu);
 #else /* CONFIG_CRASH_DUMP */
 static inline int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu)
 {
@@ -1309,6 +1310,10 @@ arm_smmu_kdump_is_attach_deferred(struct arm_smmu_master *master)
 {
 	return false;
 }
+
+static inline void arm_smmu_device_kdump_probe(struct arm_smmu_device *smmu)
+{
+}
 #endif /* CONFIG_CRASH_DUMP */
 
 struct arm_vsmmu {
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c +27 -0
index 5bc938901b65d..e7c7927b1ff79 100644
@@ -256,3 +256,30 @@ bool arm_smmu_kdump_is_attach_deferred(struct arm_smmu_master *master)
 
 	return false;
 }
+
+void arm_smmu_device_kdump_probe(struct arm_smmu_device *smmu)
+{
+	u32 gerror, gerrorn, active;
+
+	/* No adoption if SMMU is disabled (i.e., there is no in-flight DMA) */
+	if (!(readl_relaxed(smmu->base + ARM_SMMU_CR0) & CR0_SMMUEN))
+		return;
+
+	/* For now, only support a coherent SMMU that works with MEMREMAP_WB */
+	if (!(smmu->features & ARM_SMMU_FEAT_COHERENCY)) {
+		dev_warn(smmu->dev,
+			 "non-coherent SMMU unsupported; reset to block all DMAs\n");
+		return;
+	}
+
+	gerror = readl_relaxed(smmu->base + ARM_SMMU_GERROR);
+	gerrorn = readl_relaxed(smmu->base + ARM_SMMU_GERRORN);
+	active = gerror ^ gerrorn;
+	if (active & GERROR_SFM_ERR) {
+		dev_warn(smmu->dev,
+			 "SMMU in Service Failure Mode, must reset\n");
+		return;
+	}
+
+	smmu->options |= ARM_SMMU_OPT_KDUMP_ADOPT;
+}
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +4 -0
index 525cdc2e7d495..2c5befcf64456 100644
@@ -5367,6 +5367,10 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 
 	dev_info(smmu->dev, "oas %lu-bit (features 0x%08x)\n",
 		 smmu->oas, smmu->features);
+
+	if (is_kdump_kernel())
+		arm_smmu_device_kdump_probe(smmu);
+
 	return 0;
 }
 
Signature · 1 line
2.43.0
Tested-by on the v10 series
#34 lore raw
In reply to Nicolin Chen
Quoted 4 lines · Nicolin Chen

To safely absorb in-flight DMA, the kdump kernel must leave SMMUEN=1 intact and avoid modifying STRTAB_BASE. This allows HW to continue translating in- flight DMA using the crashed kernel's page tables until the endpoint device drivers probe and quiesce their respective hardware.

For the whole series:

Tested-by on the v10 series
#35 lore raw
In reply to Nicolin Chen
Quoted 4 lines · Nicolin Chen

To safely absorb in-flight DMA, the kdump kernel must leave SMMUEN=1 intact and avoid modifying STRTAB_BASE. This allows HW to continue translating in- flight DMA using the crashed kernel's page tables until the endpoint device drivers probe and quiesce their respective hardware.

Tested v10 on AWS Graviton4 bare metal (m8g.metal-24xl: Neoverse V2, single SMMUv3 with a 2-level stream table). On this platform, if the SMMU aborts a NIC DMA read, firmware resets the host. An unpatched kernel fails kdump when the NIC is still fetching from memory as the SMMU probe installs a fresh stream table.

Reproducer: sysrq-c while the NIC receives ~100 kpps of UDP over 64 flows, spread across most RX queues. The kdump initramfs omits the NIC driver, so nothing quiesces the device.

Results using Amazon Linux 2023 with kdump kernel 6.18.39:

- Unpatched: 2/2 failures. The SMMU logs read and write translation aborts ~2.5 s after the panic, and the host resets before the vmcore is written.

- v7.3-rc1 + this series: 3/3 complete vmcores, no translation aborts. The kdump kernel logs "kdump: adopting crashed kernel's stream table" and "kdump: retaining SMMUEN for in-flight DMA". In one run the NIC driver was present, so the device was also re-probed and reset.