iommu/arm-smmu-v3: Adopt the crashed kernel's stream table for kdump
Subsystems (2)
v9 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-v9
Changelog
v9
* 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 (12):
iommu/arm-smmu-v3: Do not enable EVTQ/PRIQ interrupts in kdump kernel iommu/arm-smmu-v3: Skip EVTQ/PRIQ setup in kdump kernel iommu/arm-smmu-v3: Add strtab parse helpers to a new
arm-smmu-v3-kexec.c
iommu/arm-smmu-v3: Destroy vmid_map ida via devres 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 | 66 +++ .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 288 +++++++++++ .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c | 461 ++++++++++++++++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 246 +++++++--- 6 files changed, 999 insertions(+), 68 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
Signature · 1 line
2.43.0
PATCH v9 1/12 iommu/arm-smmu-v3: Do not enable EVTQ/PRIQ interrupts in kdump kernel
In kdump cases, the crashed kernel's CDs and page tables can be corrupted, which could trigger event spamming. Also, we cannot serve page requests.
Skip the IRQ setup for EVTQ/PRIQ in arm_smmu_setup_irqs().
Skip their IRQ handler registration in unique-IRQ and combined-IRQ cases.
Note that arm_smmu_device_reset() at this point still enables EVTQEN/PRIQEN transiently before clearing them. A following change will skip the entire queue setup, removing that transient enable window.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 58 ++++++++++++++------- 1 file changed, 39 insertions(+), 19 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +39 -19
@@ -2294,7 +2294,11 @@ static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)static irqreturn_t arm_smmu_combined_irq_handler (int irq,void *dev) { -arm_smmu_gerror_handler (irq, dev); +irqreturn_t ret =arm_smmu_gerror_handler (irq, dev); + +/* In kdump, EVTQ/PRIQ are disabled and there is no thread to wake */ +if (is_kdump_kernel ()) +return ret;return IRQ_WAKE_THREAD ; }
@@ -4625,6 +4629,21 @@ static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)arm_smmu_setup_msis (smmu);/* Request interrupt lines */ + irq = smmu->gerr_irq; +if (irq) { + ret =devm_request_irq (smmu->dev, irq, arm_smmu_gerror_handler, +0 ,"arm-smmu-v3-gerror" , smmu); +if (ret <0 ) +dev_warn (smmu->dev,"failed to enable gerror irq\n" ); + }else { +dev_warn (smmu->dev, +"no gerr irq - errors will not be reported!\n" ); + } + +/* No EVTQ/PRIQ interrupts in kdump -- queues are disabled */ +if (is_kdump_kernel ()) +return ; + irq = smmu->evtq.q.irq;if (irq) { ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
@@ -4637,16 +4656,6 @@ static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)dev_warn (smmu->dev,"no evtq irq - events will not be reported!\n" ); } - irq = smmu->gerr_irq; -if (irq) { - ret =devm_request_irq (smmu->dev, irq, arm_smmu_gerror_handler, -0 ,"arm-smmu-v3-gerror" , smmu); -if (ret <0 ) -dev_warn (smmu->dev,"failed to enable gerror irq\n" ); - }else { -dev_warn (smmu->dev,"no gerr irq - errors will not be reported!\n" ); - } -if (smmu->features &ARM_SMMU_FEAT_PRI ) { irq = smmu->priq.q.irq;if (irq) {
@@ -4667,7 +4676,7 @@ static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)static int arm_smmu_setup_irqs (struct arm_smmu_device *smmu) {int ret, irq; -u32 irqen_flags =IRQ_CTRL_EVTQ_IRQEN |IRQ_CTRL_GERROR_IRQEN ; +u32 irqen_flags =IRQ_CTRL_GERROR_IRQEN ;/* Disable IRQs first */ ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
@@ -4682,19 +4691,30 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)/* * Cavium ThunderX2 implementation doesn't support unique irq * lines. Use a single irq line for all the SMMUv3 interrupts. +* +* In kdump, EVTQ/PRIQ are disabled, so no threaded handling. */ - ret =devm_request_threaded_irq (smmu->dev, irq, - arm_smmu_combined_irq_handler, - arm_smmu_combined_irq_thread, -IRQF_ONESHOT , -"arm-smmu-v3-combined-irq" , smmu); +if (is_kdump_kernel ()) + ret =devm_request_irq (smmu->dev, irq, + arm_smmu_combined_irq_handler,0 , +"arm-smmu-v3-combined-irq" , + smmu); +else + ret =devm_request_threaded_irq ( + smmu->dev, irq, arm_smmu_combined_irq_handler, + arm_smmu_combined_irq_thread,IRQF_ONESHOT , +"arm-smmu-v3-combined-irq" , smmu);if (ret <0 )dev_warn (smmu->dev,"failed to enable combined irq\n" ); }else arm_smmu_setup_unique_irqs (smmu); -if (smmu->features &ARM_SMMU_FEAT_PRI ) - irqen_flags |=IRQ_CTRL_PRIQ_IRQEN ; +/* No EVTQ/PRIQ IRQ generation in kdump -- queues are disabled */ +if (!is_kdump_kernel ()) { + irqen_flags |=IRQ_CTRL_EVTQ_IRQEN ; +if (smmu->features &ARM_SMMU_FEAT_PRI ) + irqen_flags |=IRQ_CTRL_PRIQ_IRQEN ; + }/* Enable interrupt generation on the SMMU */ ret = arm_smmu_write_reg_sync(smmu, irqen_flags,
Signature · 1 line
2.43.0
Re: [PATCH v9 01/12] iommu/arm-smmu-v3: Do not enable EVTQ/PRIQ interrupts in kdump kernel
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev) · line 2302
Quoted 9 lines
static
irqreturn_tarm_smmu_combined_irq_handler(int irq, void *dev){drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +5
-1-arm_smmu_gerror_handler (irq, dev); +irqreturn_t ret =arm_smmu_gerror_handler (irq, dev); + +/* In kdump, EVTQ/PRIQ are disabled and there is no thread to wake */ +if (is_kdump_kernel ()) +return ret;return IRQ_WAKE_THREAD ;
To Robin's earlier point, how about some ARM_SMMU_FEAT_EVTQ ? We already have ARM_SMMU_FEAT_PRI. Unset these by kdump instead of sprinkling kdump things everywhere?
I'm not especially kean on these kdump annotations as a long term maintainability, but having a FEAT to protect a well defined area of functionality seems more understandable long term.
Plus maybe an embedded user would disable FEAT_PRI for other reasons, somehow?
Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +4
-0@@ -4625,6 +4629,21 @@ static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)arm_smmu_setup_msis (smmu);/* Request interrupt lines */ + irq = smmu->gerr_irq; +if (irq) { + ret =devm_request_irq (smmu->dev, irq, arm_smmu_gerror_handler, +0 ,"arm-smmu-v3-gerror" , smmu);
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu) · line 4645
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +10
-0+if (ret <0 ) +dev_warn (smmu->dev,"failed to enable gerror irq\n" ); + }else { +dev_warn (smmu->dev, +"no gerr irq - errors will not be reported!\n" ); + } + +/* No EVTQ/PRIQ interrupts in kdump -- queues are disabled */ +if (is_kdump_kernel ()) +return ;
Then this would flow nicer, you just wrap evtq in a FEAT_EVTQ test and don't have to move things
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) · line 4696
Quoted 7 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +3
-0+> @@ -4682,19 +4691,30 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)/* * Cavium ThunderX2 implementation doesn't support unique irq * lines. Use a single irq line for all the SMMUv3 interrupts. +* +* In kdump, EVTQ/PRIQ are disabled, so no threaded handling. */
Why? I mean I get we might not need a threaded IRQ handler without PRI, but is there a reason kdump would not want to create one anyhow?
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) · line 4717
Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +6
-2-if (smmu->features &ARM_SMMU_FEAT_PRI ) - irqen_flags |=IRQ_CTRL_PRIQ_IRQEN ; +/* No EVTQ/PRIQ IRQ generation in kdump -- queues are disabled */ +if (!is_kdump_kernel ()) { + irqen_flags |=IRQ_CTRL_EVTQ_IRQEN ; +if (smmu->features &ARM_SMMU_FEAT_PRI ) + irqen_flags |=IRQ_CTRL_PRIQ_IRQEN ; + }
And here is also just a simple FEAT_EVTQ?
The overall thing looks broadly right to me
Signature · 1 line
Jason
Re: [PATCH v9 01/12] iommu/arm-smmu-v3: Do not enable EVTQ/PRIQ interrupts in kdump kernel
Quoted 14 lines · Jason Gunthorpe
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev) · line 2302
Quoted 9 lines
static
irqreturn_tarm_smmu_combined_irq_handler(int irq, void *dev){drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +5
-1-arm_smmu_gerror_handler (irq, dev); +irqreturn_t ret =arm_smmu_gerror_handler (irq, dev); + +/* In kdump, EVTQ/PRIQ are disabled and there is no thread to wake */ +if (is_kdump_kernel ()) +return ret;return IRQ_WAKE_THREAD ;To Robin's earlier point, how about some
ARM_SMMU_FEAT_EVTQ? We already haveARM_SMMU_FEAT_PRI. Unset these by kdump instead of sprinkling kdump things everywhere?
Quoted 3 lines
I'm not especially kean on these kdump annotations as a long term maintainability, but having a FEAT to protect a well defined area of functionality seems more understandable long term.
I've added a new patch adding ARM_SMMU_FEAT_EVTQ.
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) · line 4696
Quoted 7 lines
+> @@ -4682,19 +4691,30 @@ static int
arm_smmu_setup_irqs(struct arm_smmu_device *smmu)drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +2
-0/* * Cavium ThunderX2 implementation doesn't support unique irq * lines. Use a single irq line for all the SMMUv3 interrupts. +* +* In kdump, EVTQ/PRIQ are disabled, so no threaded handling. */ Why? I mean I get we might not need a threaded IRQ handler without PRI, but is there a reason kdump would not want to create one anyhow?
In kdump case, only GERROR is left. So, I thought it could match the non-combined case.
But it's probably okay to remain to be threaded. Also, since we gate interrupts with the FEAT_PRIQ and FEAT_EVTQ in the combined handler, it makes things here cleaner.
Thanks
Nicolin
PATCH v9 2/12 iommu/arm-smmu-v3: Skip EVTQ/PRIQ setup in kdump kernel
In reply to Nicolin ChenIn kdump cases, the crashed kernel's CDs and page tables can be corrupted, which could trigger event spamming. Also, we cannot serve page requests.
Skip the EVTQ/PRIQ setup entirely rather than enabling then disabling them.
Keep the queue allocations themselves, as their Q_MAX_SZ_SHIFT-capped sizes are negligible against a crashkernel reservation.
Also add some inline comments explaining that.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 44 +++++++++++++-------- 1 file changed, 28 insertions(+), 16 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +28 -16
@@ -4821,21 +4821,36 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)arm_smmu_cmdq_issue_cmd_with_sync ( smmu,arm_smmu_make_cmd_op (CMDQ_OP_TLBI_NSNH_ALL )); -/* Event queue */ -writeq_relaxed (smmu->evtq.q.q_base, smmu->base +ARM_SMMU_EVTQ_BASE ); -writel_relaxed (smmu->evtq.q.llq.prod, smmu->page1 +ARM_SMMU_EVTQ_PROD ); -writel_relaxed (smmu->evtq.q.llq.cons, smmu->page1 +ARM_SMMU_EVTQ_CONS ); - - enables |=CR0_EVTQEN ; - ret =arm_smmu_write_reg_sync (smmu, enables,ARM_SMMU_CR0 , -ARM_SMMU_CR0ACK ); -if (ret) { -dev_err (smmu->dev,"failed to enable event queue\n" ); -return ret; +/* +* Event queue +* +* Do not enable in a kdump case, as the crashed kernel's CDs and page +* tables may be corrupted, triggering event spamming. A disabled queue +* simply discards new events, without raising any global error. +*/ +if (!is_kdump_kernel ()) { +writeq_relaxed (smmu->evtq.q.q_base, + smmu->base +ARM_SMMU_EVTQ_BASE ); +writel_relaxed (smmu->evtq.q.llq.prod, + smmu->page1 +ARM_SMMU_EVTQ_PROD ); +writel_relaxed (smmu->evtq.q.llq.cons, + smmu->page1 +ARM_SMMU_EVTQ_CONS ); + + enables |=CR0_EVTQEN ; + ret =arm_smmu_write_reg_sync (smmu, enables,ARM_SMMU_CR0 , +ARM_SMMU_CR0ACK ); +if (ret) { +dev_err (smmu->dev,"failed to enable event queue\n" ); +return ret; + } } -/* PRI queue */ -if (smmu->features &ARM_SMMU_FEAT_PRI ) { +/* +* PRI queue +* +* Do not enable in a kdump case, as we cannot serve page requests. +*/ +if (!is_kdump_kernel () && (smmu->features &ARM_SMMU_FEAT_PRI )) {writeq_relaxed (smmu->priq.q.q_base, smmu->base +ARM_SMMU_PRIQ_BASE ); writel_relaxed(smmu->priq.q.llq.prod,
@@ -4868,9 +4883,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,
Signature · 1 line
2.43.0
Re: [PATCH v9 02/12] iommu/arm-smmu-v3: Skip EVTQ/PRIQ setup in kdump kernel
Quoted 6 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +0
-2@@ -4821,21 +4821,36 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)arm_smmu_cmdq_issue_cmd_with_sync ( smmu,arm_smmu_make_cmd_op (CMDQ_OP_TLBI_NSNH_ALL )); -/* Event queue */ -writeq_relaxed (smmu->evtq.q.q_base, smmu->base +ARM_SMMU_EVTQ_BASE );
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_device_reset(struct arm_smmu_device *smmu) · line 4831
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +8
-2-writel_relaxed (smmu->evtq.q.llq.prod, smmu->page1 +ARM_SMMU_EVTQ_PROD ); -writel_relaxed (smmu->evtq.q.llq.cons, smmu->page1 +ARM_SMMU_EVTQ_CONS ); +/* +* Event queue +* +* Do not enable in a kdump case, as the crashed kernel's CDs and page +* tables may be corrupted, triggering event spamming. A disabled queue +* simply discards new events, without raising any global error. +*/ +if (!is_kdump_kernel ()) {
same remark about a FEAT_EVTQ
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_device_reset(struct arm_smmu_device *smmu) · line 4854
Quoted 9 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +6
-2-/* PRI queue */ -if (smmu->features &ARM_SMMU_FEAT_PRI ) { +/* +* PRI queue +* +* Do not enable in a kdump case, as we cannot serve page requests. +*/ +if (!is_kdump_kernel () && (smmu->features &ARM_SMMU_FEAT_PRI )) { writeq_relaxed(smmu->priq.q.q_base,
I'm reading this again and wondering why not drop FEAT_PRI much earlier so we don't need this is_kdump_kernel() here?
The logic still looks OK to me
Signature · 1 line
Jason
Re: [PATCH v9 02/12] iommu/arm-smmu-v3: Skip EVTQ/PRIQ setup in kdump kernel
Quoted 5 lines · Jason Gunthorpe
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +2
-2@@ -4821,21 +4821,36 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu) -/* PRI queue */ -if (smmu->features &ARM_SMMU_FEAT_PRI ) { +/* +* PRI queue
Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_device_reset(struct arm_smmu_device *smmu) · line 4854
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +4
-0+* +* Do not enable in a kdump case, as we cannot serve page requests. +*/ +if (!is_kdump_kernel () && (smmu->features &ARM_SMMU_FEAT_PRI )) { writeq_relaxed(smmu->priq.q.q_base,I'm reading this again and wondering why not drop
FEAT_PRImuch earlier so we don't need thisis_kdump_kernel()here?
Yes. I added this:
@@ -5371,6 +5351,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);
Thanks
Nicolin
PATCH v9 3/12 iommu/arm-smmu-v3: Add strtab parse helpers to a new arm-smmu-v3-kexec.c
In reply to Nicolin ChenWhen 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 | 159 ++++++++++++++++++ 4 files changed, 175 insertions(+) create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c
drivers/iommu/arm/Kconfig +4 -0
@@ -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
@@ -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
@@ -1239,6 +1239,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 +159 -0
159 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v9 03/12] iommu/arm-smmu-v3: Add strtab parse helpers to a new arm-smmu-v3-kexec.c
Quoted 11 lines · 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.cholding the common read-only helpers:
-arm_smmu_kexec_parse_strtab_2lvl()
-arm_smmu_kexec_parse_strtab_linear()
-arm_smmu_kexec_check_strtab_l1_desc()[...]
Signature · 1 line
Jason
PATCH v9 4/12 iommu/arm-smmu-v3: Destroy vmid_map ida via devres
In reply to Nicolin ChenThe vmid_map ida is only destroyed in arm_smmu_device_remove(), which does not run when the probe fails. This is currently harmless, as the ida stays empty until the first S2 domain allocation, which can only happen after a successful probe.
An upcoming change will start reserving the crashed kernel's VMIDs in this ida, at the probe time of a kdump kernel. A probe failure would then leak the ida's internal allocations.
Register a devres action right after ida_init(), making devres the single owner that covers both the unbind and probe failure paths. Also, move the ida_init() call to the entry of arm_smmu_init_strtab(), as the reservation will run in the adoption path prior to the regular stream table setup.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +13 -7
@@ -4494,20 +4494,27 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)return 0 ; } +static void arm_smmu_deinit_strtab (void *data) +{ +struct arm_smmu_device *smmu = data; + +ida_destroy (&smmu->vmid_map); +} +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_deinit_strtab, smmu); +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 0 ; +return ret; }static int arm_smmu_init_structures (struct arm_smmu_device *smmu)
@@ -5548,7 +5555,6 @@ static void arm_smmu_device_remove(struct platform_device *pdev)iommu_device_sysfs_remove (&smmu->iommu);arm_smmu_device_disable (smmu);iopf_queue_free (smmu->evtq.iopf); -ida_destroy (&smmu->vmid_map); }static void arm_smmu_device_shutdown (struct platform_device *pdev)
Signature · 1 line
2.43.0
Re: [PATCH v9 04/12] iommu/arm-smmu-v3: Destroy vmid_map ida via devres
Quoted 10 lines · Nicolin Chen
The
vmid_mapida is only destroyed inarm_smmu_device_remove(), which does not run when the probe fails. This is currently harmless, as the ida stays empty until the first S2 domain allocation, which can only happen after a successful probe.An upcoming change will start reserving the crashed kernel's VMIDs in this ida, at the probe time of a kdump kernel. A probe failure would then leak the ida's internal allocations.
[...]
Signature · 1 line
Jason
PATCH v9 5/12 iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel
In reply to Nicolin ChenWhen 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.
--- 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
@@ -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
@@ -928,6 +928,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;
@@ -1250,6 +1251,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
@@ -1938,11 +1938,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) {
@@ -1954,8 +1966,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 ; }
@@ -4510,6 +4521,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
Re: [PATCH v9 05/12] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel
Quoted 5 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_SMMUENand setting the Global Bypass Attribute (GBPA) to ABORT.
Quoted 7 lines
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.[...]
Signature · 1 line
Jason
PATCH v9 6/12 iommu/arm-smmu-v3-kexec: Add a CD table parse helper
In reply to Nicolin ChenAn 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.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 ++ .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c | 42 +++++++++++++++++++ 2 files changed, 45 insertions(+)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +3 -0
@@ -1249,6 +1249,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 +42 -0
45 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v9 06/12] iommu/arm-smmu-v3-kexec: Add a CD table parse helper
Quoted 8 lines · 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()[...]
Signature · 1 line
Jason
PATCH v9 7/12 iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers
In reply to Nicolin ChenAn 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.
Since both kernels allocate ASIDs from 1, reserving any in-use ASID would often find it already taken in the xarray, either by another scan or by a live domain. A reservation made by another scan is permanent, so it is safe to rely on. A live domain will free its ASID for reuse at some point, and the scan must then fail and fall back. To tell these two cases apart, store the per-scan ID as an xarray value entry: a value entry identifies another scan's reservation and a pointer entry identifies a live domain. Note that the scan ID scopes a failing scan's rollback to exactly the entries that it inserted, so a rollback cannot erase another scan's reservations.
Also add an arm_smmu_kexec_resv_lock mutex serializing entire scans and any rollback of failing ones, so that a concurrently probing SMMU can only rely on the reservations that will persist.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 + .../iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c | 260 ++++++++++++++++++ 2 files changed, 264 insertions(+)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +4 -0
@@ -1241,6 +1241,8 @@ tegra241_cmdqv_probe(struct arm_smmu_device *smmu)#endif /* CONFIG_TEGRA241_CMDQV */ #ifdef CONFIG_ARM_SMMU_V3_KEXEC +extern struct mutex arm_smmu_kexec_resv_lock; +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,
@@ -1252,6 +1254,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 +260 -0
263 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v9 07/12] iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c +10
-0+static int arm_smmu_kexec_resv_asid (struct arm_smmu_device *smmu,u32 asid) +{ +int ret; + +/* A valid CD never has ASID 0; both kernels share the same HW limit */ +if (!asid || asid >=1UL << smmu->asid_bits) +return -EINVAL ; + +guard (mutex)(&arm_smmu_asid_lock); +
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c int arm_smmu_kexec_check_ste_cdtab(struct arm_smmu_device *smmu, u64 ste0, · line 235
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c +10
-0+/* The value entry marks the ASID as in-use and identifies its scan */ + ret =xa_insert (&arm_smmu_asid_xa, asid, +xa_mk_value (arm_smmu_kexec_scan_id),GFP_KERNEL ); +/* +* An -EBUSY against a value entry safely shares a permanent reservation +* made by another scan. A pointer entry means a live domain that will +* free its ASID for reuse eventually: keep -EBUSY to fail the scan. +*/ +if (ret == -EBUSY &&xa_is_value (xa_load (&arm_smmu_asid_xa, asid))) + ret =0 ;
Huh. Somewhere I had a patch to make ASID per smmu instance, just like vmid. I think you might actually need it here? Is it possible that one instance could probe, the core code could start changing domains and end up with an unintended ASID conflict on a later instance? Then things go sideways?
If you have that you can also drop the arm_smmu_kexec_resv_lock?
Quoted 3 lines
+/**
+ *arm_smmu_kexec_unresv_ids()- Roll back a failing reservation scan
+ * @smmu: SMMU device of this kernel that failed its reservation scan
And wouldn't need this either.
Actually I think this is overkill regardless, just leave the xarray with ASIDs reserved. Who cares?
The general logic looks otherwise OK to me
Jason
Signature · 1 line
Jason
Re: [PATCH v9 07/12] iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers
Quoted 17 lines · Jason Gunthorpe
Quoted 17 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c +17
-0+static int arm_smmu_kexec_resv_asid (struct arm_smmu_device *smmu,u32 asid) +{ +int ret; + +/* A valid CD never has ASID 0; both kernels share the same HW limit */ +if (!asid || asid >=1UL << smmu->asid_bits) +return -EINVAL ; + +guard (mutex)(&arm_smmu_asid_lock); + +/* The value entry marks the ASID as in-use and identifies its scan */ + ret =xa_insert (&arm_smmu_asid_xa, asid, +xa_mk_value (arm_smmu_kexec_scan_id),GFP_KERNEL ); +/* +* An -EBUSY against a value entry safely shares a permanent reservation +* made by another scan. A pointer entry means a live domain that will +* free its ASID for reuse eventually: keep -EBUSY to fail the scan.
Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c int arm_smmu_kexec_check_ste_cdtab(struct arm_smmu_device *smmu, u64 ste0, · line 235
Quoted 3 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kexec.c +3
-0+*/ +if (ret == -EBUSY &&xa_is_value (xa_load (&arm_smmu_asid_xa, asid))) + ret =0 ;Huh. Somewhere I had a patch to make ASID per smmu instance, just like vmid. I think you might actually need it here? Is it possible that one instance could probe, the core code could start changing domains and end up with an unintended ASID conflict on a later instance? Then things go sideways?
Yes. I've noticed a problem: SMMU-A allocates ASIDx from the global space for a default domain, later SMMU-B wants to reserve ASIDx.
So my local v10 adds a new patch making ASID per smmu. If you would like your version, I can swap that.
Quoted 2 lines
If you have that you can also drop the
arm_smmu_kexec_resv_lock?
Yes. The entire thing ended up cleaner actually.
Thanks
Nicolin
Re: [PATCH v9 07/12] iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers
In reply to Jason GunthorpeQuoted 8 lines · Jason Gunthorpe
Quoted 3 lines
+/**
+ *arm_smmu_kexec_unresv_ids()- Roll back a failing reservation scan
+ * @smmu: SMMU device of this kernel that failed its reservation scanAnd wouldn't need this either.
Actually I think this is overkill regardless, just leave the xarray with ASIDs reserved. Who cares?
ASID might be 8-bit that would have limited ASID slots. And it's a quite small step, so I think it might be worth keeping it.
Thanks
Nicolin
Re: [PATCH v9 07/12] iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers
Quoted 5 lines · Nicolin Chen
Quoted 4 lines · Jason Gunthorpe
Quoted 3 lines
+/**
+ *arm_smmu_kexec_unresv_ids()- Roll back a failing reservation scan
+ * @smmu: SMMU device of this kernel that failed its reservation scan
Quoted 7 lines
Quoted 4 lines
And wouldn't need this either.
Actually I think this is overkill regardless, just leave the xarray with ASIDs reserved. Who cares?
ASID might be 8-bit that would have limited ASID slots. And it's a quite small step, so I think it might be worth keeping it.
Once you have it per smmu just clear the entire xarray on some failure
Jason
Re: [PATCH v9 07/12] iommu/arm-smmu-v3-kexec: Add ASID/VMID reservation helpers
Quoted 8 lines · Jason Gunthorpe
Quoted 7 lines · Nicolin Chen
Quoted 6 lines · Jason Gunthorpe
Quoted 3 lines
+/**
+ *arm_smmu_kexec_unresv_ids()- Roll back a failing reservation scan
+ * @smmu: SMMU device of this kernel that failed its reservation scanAnd wouldn't need this either.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Actually I think this is overkill regardless, just leave the xarray with ASIDs reserved. Who cares?
ASID might be 8-bit that would have limited ASID slots. And it's a quite small step, so I think it might be worth keeping it.
Once you have it per smmu just clear the entire xarray on some failure
Yes, there are only two destroy() in the function now.
Nicolin
PATCH v9 8/12 iommu/arm-smmu-v3-kdump: Reserve crashed kernel's ASIDs and VMIDs
In reply to Nicolin ChenThe 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(), under the arm_smmu_kexec_resv_lock so that a failing scan can atomically roll back all of its reservations via arm_smmu_kexec_unresv_ids().
Nested STE's guest-owned CD table is in IPA space and left alone: its ASIDs only pair with the nonzero S2VMID being reserved, so they cannot alias this kernel's VMID-0 or EL2 stage-1 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.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c | 15 ++++++++++++++- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + 2 files changed, 15 insertions(+), 1 deletion(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kdump.c +14 -1
@@ -205,16 +205,29 @@ int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu) goto err; } +mutex_lock (&arm_smmu_kexec_resv_lock); + 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; } +mutex_unlock (&arm_smmu_kexec_resv_lock);return 0 ; +err_unresv: +/* The full reset will flush the entire TLB, so release everything */ +arm_smmu_kexec_unresv_ids (smmu); +mutex_unlock (&arm_smmu_kexec_resv_lock); err:dev_warn (smmu->dev,"falling back to full reset\n" );/*
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +1 -0
@@ -4516,6 +4516,7 @@ static int arm_smmu_init_strtab(struct arm_smmu_device *smmu) {int ret; +/* Init first, as a kdump adoption reserves in-use VMIDs in the ida */ ida_init (&smmu->vmid_map); ret =devm_add_action_or_reset (smmu->dev, arm_smmu_deinit_strtab, smmu);if (ret)
Signature · 1 line
2.43.0
Re: [PATCH v9 08/12] iommu/arm-smmu-v3-kdump: Reserve crashed kernel's ASIDs and VMIDs
Quoted 3 lines
Nested STE's guest-owned CD table is in IPA space and left alone: its ASIDs only pair with the nonzero S2VMID being reserved, so they cannot alias this kernel's VMID-0 or EL2 stage-1 domains.
This explanation is a bit off.. Nested ASIDs have a unique ASID space under their VMID. SO long as the VMID is reserved there is no reason to inspect the (VMID,ASID) list.
The only ASIDs we need to load are ones in the namespace the new kernel will use itself, which you explained here:
Quoted 4 lines
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.
That is a nice observation and it would have been really cool if this kernel could just use a unique VMID and not have to worry about ASID. Oh wel.
Seems fine otherwise
Signature · 1 line
Jason
Re: [PATCH v9 08/12] iommu/arm-smmu-v3-kdump: Reserve crashed kernel's ASIDs and VMIDs
Quoted 7 lines · Jason Gunthorpe
Quoted 3 lines
Nested STE's guest-owned CD table is in IPA space and left alone: its ASIDs only pair with the nonzero S2VMID being reserved, so they cannot alias this kernel's VMID-0 or EL2 stage-1 domains.
This explanation is a bit off.. Nested ASIDs have a unique ASID space under their VMID. SO long as the VMID is reserved there is no reason to inspect the (VMID,ASID) list.
Updated:
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.
Thanks
Nicolin
PATCH v9 9/12 iommu/arm-smmu-v3-kdump: Implement is_attach_deferred()
In reply to Nicolin ChenThough 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 | 22 ++++++++++++++++ .../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, 51 insertions(+), 15 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +22 -0
@@ -1208,6 +1208,21 @@ void arm_smmu_attach_commit(struct arm_smmu_attach_state *state);void arm_smmu_install_ste_for_dev (struct arm_smmu_master *master,const struct arm_smmu_ste *target); +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,
@@ -1263,6 +1278,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) {
@@ -1276,6 +1292,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
@@ -240,3 +240,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
@@ -2898,21 +2898,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) {
@@ -4318,6 +4303,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,
@@ -4326,6 +4320,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
Re: [PATCH v9 09/12] iommu/arm-smmu-v3-kdump: Implement is_attach_deferred()
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid, · line 1300
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +6
-0@@ -1276,6 +1292,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; +}
?
Looks OK otherwise
Signature · 1 line
Jason
Re: [PATCH v9 09/12] iommu/arm-smmu-v3-kdump: Implement is_attach_deferred()
Quoted 5 lines · Jason Gunthorpe
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1
-0@@ -1276,6 +1292,12 @@ arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid, {return -EOPNOTSUPP ; } +
Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid, · line 1300
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +5
-0+static inline bool +arm_smmu_kdump_is_attach_deferred (struct arm_smmu_master *master) +{ +return false; +}?
This is for !CONFIG_CRASH_DUMP case. I think we need it.
Thanks
Nicolin
Re: [PATCH v9 09/12] iommu/arm-smmu-v3-kdump: Implement is_attach_deferred()
Quoted 15 lines · Nicolin Chen
Quoted 5 lines · Jason Gunthorpe
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1
-0@@ -1276,6 +1292,12 @@ arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid, {return -EOPNOTSUPP ; } +Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid, · line 1300
Quoted 5 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +5
-0+static inline bool +arm_smmu_kdump_is_attach_deferred (struct arm_smmu_master *master) +{ +return false; +}?
Quoted 1 line
This is for !CONFIG_CRASH_DUMP case. I think we need it.
Hmm, I am testing b4 review.. I had written
#define arm_smmu_kdump_is_attach_deferred NULL
?
And I now realize b4 review drops lines starting with a #
Jason
Re: [PATCH v9 09/12] iommu/arm-smmu-v3-kdump: Implement is_attach_deferred()
Quoted 16 lines · Jason Gunthorpe
Quoted 15 lines · Nicolin Chen
Quoted 13 lines · Jason Gunthorpe
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu, u32 sid, · line 1300
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +6
-0@@ -1276,6 +1292,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; +}?
Quoted 5 lines
Quoted 1 line
This is for !CONFIG_CRASH_DUMP case. I think we need it.
Hmm, I am testing b4 review.. I had written
#define
arm_smmu_kdump_is_attach_deferredNULL
Hmm, I'm adding two-level is_attach_deferred functions:
arm_smmu_is_attached_deferred()
{
if (OPT_KDUMP_ADOPT)
arm_smmu_kdump_is_attach_deferred();
// internally doesn't check OPT_KDUMP_ADOPT
}[...]
.is_attach_deferred = arm_smmu_is_attached_deferred,
So, arm_smmu_kdump_is_attach_deferred cannot be NULL.
Nicolin
Re: [PATCH v9 09/12] iommu/arm-smmu-v3-kdump: Implement is_attach_deferred()
In reply to Jason GunthorpeQuoted 10 lines · Jason Gunthorpe
Quoted 2 lines
This is for !CONFIG_CRASH_DUMP case. I think we need it.
Hmm, I am testing b4 review.. I had written
#define
arm_smmu_kdump_is_attach_deferredNULL?
And I now realize b4 review drops lines starting with a #
I don't think that's intended -- I think the goal was to only drop leading lines starting with # until the first non-# line is reached (i.e. the header). I'll mark it as a bug.
-K
PATCH v9 10/12 iommu/arm-smmu-v3: Retain CR0_SMMUEN during kdump device reset
In reply to Nicolin ChenWhen 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.
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
@@ -4781,10 +4781,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 );
@@ -4814,12 +4832,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) {
@@ -4885,7 +4932,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
PATCH v9 11/12 iommu/arm-smmu-v3: Skip RMR bypass for kdump adoption
In reply to Nicolin ChenRMR 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
@@ -5421,6 +5421,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);
@@ -5437,10 +5445,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
Re: [PATCH v9 11/12] iommu/arm-smmu-v3: Skip RMR bypass for kdump adoption
Quoted 6 lines · 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-ownedtraffic, including vSMMU guest MSI cases built around RMR-described SIDs.
Quoted 7 lines
ARM_SMMU_OPT_KDUMP_ADOPTis 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.[...]
Signature · 1 line
Jason
PATCH v9 12/12 iommu/arm-smmu-v3: Detect ARM_SMMU_OPT_KDUMP_ADOPT in probe()
In reply to Nicolin Chenarm_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
@@ -1279,6 +1279,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) {
@@ -1298,6 +1299,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
@@ -259,3 +259,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
@@ -5257,6 +5257,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
Re: [PATCH v9 12/12] iommu/arm-smmu-v3: Detect ARM_SMMU_OPT_KDUMP_ADOPT in probe()
Quoted 8 lines · Nicolin Chen
arm_smmu_device_hw_probe()runs beforearm_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.
[...]
Signature · 1 line
Jason