From: Punit Agrawal punit.agrawal@oss.qualcomm.com
[ Upstream commit eeb8c19896952e18fb538ec76e603884070a6c6a ]
This reverts commit bad3fa2fb9206f4dcec6ddef094ec2fbf6e8dcb2.
Commit bad3fa2fb920 ("ACPI: Suppress misleading SPCR console message when SPCR table is absent") mistakenly assumes acpi_parse_spcr() returning 0 to indicate a failure to parse SPCR. While addressing the resultant incorrect logging it was deemed that dropping the message is a better approach as it is not particularly useful.
Roll back the commit introducing the bug as a step towards dropping the log message.
Link: https://lore.kernel.org/all/aQN0YWUYaPYWpgJM@willie-the-truck/ Signed-off-by: Punit Agrawal punit.agrawal@oss.qualcomm.com Signed-off-by: Will Deacon will@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Excellent! Now I have all the information needed. Let me create a comprehensive analysis:
## COMPREHENSIVE ANALYSIS: REVERT COMMIT FOR SPCR CONSOLE MESSAGE BUG
### STEP 1: COMMIT MESSAGE ANALYSIS
**Subject**: Revert "ACPI: Suppress misleading SPCR console message when SPCR table is absent"
**Type**: This is a **revert commit** that undoes buggy code introduced in v6.17
**Key Indicators**: - Reverts commit bad3fa2fb9206f4dcec6ddef094ec2fbf6e8dcb2 - States the reverted commit "mistakenly assumes acpi_parse_spcr() returning 0 to indicate a failure" - Links to mailing list discussion: https://lore.kernel.org/all/aQN0YWUYaPYWpgJM@willie-the-truck/ - No Fixes: tag, but implicitly fixes bad3fa2fb920 - No Cc: stable tag (but should be backported due to regression fix)
### STEP 2: DEEP CODE RESEARCH - UNDERSTANDING THE BUG
#### A. How the Bug Was Introduced
**Timeline**: 1. **v6.17** (June-July 2025): Buggy commit bad3fa2fb9206 was merged - Author: Li Chen chenl311@chinatelecom.cn - Merged by: Catalin Marinas (ARM64 maintainer) - Intention: Suppress misleading SPCR console message when SPCR table is absent
2. **v6.18-rc6** (November 2025): Revert commit eeb8c19896952 was merged - Author: Punit Agrawal punit.agrawal@oss.qualcomm.com - Merged by: Will Deacon (ARM64 maintainer)
3. **v6.18-rc6** (November 2025): Follow-up commit 7991fda619f7 drops the message entirely
**Root Cause**: The buggy commit misunderstood the return value semantics of `acpi_parse_spcr()`
#### B. Technical Analysis of the Bug
**Understanding acpi_parse_spcr() Return Values** (from `drivers/acpi/spcr.c`):
```c int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console) { // Line 96: ACPI disabled if (acpi_disabled) return -ENODEV;
// Line 98-100: SPCR table NOT FOUND status = acpi_get_table(ACPI_SIG_SPCR, 0, ...); if (ACPI_FAILURE(status)) return -ENOENT; // Table ABSENT
// Lines 145-146, 178-179: Parsing errors // return -ENOENT;
// Line 233: Success case when enable_console is false return 0; // Table PRESENT and parsed successfully
// Line 231: Success case when enable_console is true return add_preferred_console(...); // 0 or negative } ```
**Key Return Values**: - `0`: **SUCCESS** - SPCR table is present and successfully parsed - `-ENOENT`: **FAILURE** - SPCR table is absent or parsing failed - `-ENODEV`: **FAILURE** - ACPI is disabled
**The Buggy Logic** (lines 257-260 in current 6.17.y):
```c ret = acpi_parse_spcr(earlycon_acpi_spcr_enable, !param_acpi_nospcr); if (!ret || param_acpi_nospcr || !IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) pr_info("Use ACPI SPCR as default console: No\n"); else pr_info("Use ACPI SPCR as default console: Yes\n"); ```
**Logic Error Analysis**:
| Scenario | ret value | !ret | Condition Result | Message Printed | Expected | Correct? | |----------|-----------|------|------------------|-----------------|---- ------|----------| | SPCR present, parsing succeeds | 0 | true | TRUE | "No" | "Yes" | ❌ WRONG | | SPCR absent, table not found | -ENOENT (-2) | false | May print "Yes" | "Yes" | "No" | ❌ WRONG | | param_acpi_nospcr is true | any | any | TRUE | "No" | "No" | ✓ Correct |
**The bug inverts the logic!** When `ret == 0` (success), `!ret` evaluates to `true`, triggering the "No" message when it should print "Yes".
#### C. The Fix (Revert)
The revert removes the buggy logic and restores the previous simpler code:
```c acpi_parse_spcr(earlycon_acpi_spcr_enable, !param_acpi_nospcr); pr_info("Use ACPI SPCR as default console: %s\n", param_acpi_nospcr ? "No" : "Yes"); ```
This is **not perfect** (it doesn't check if SPCR table is actually present), but it's **better than printing inverted messages**. The follow-up commit 7991fda619f7 removes the message entirely as the proper solution.
### STEP 3: SECURITY ASSESSMENT
**Security Impact**: None. This is a cosmetic bug affecting only boot- time log messages. No security implications.
### STEP 4: FEATURE VS BUG FIX CLASSIFICATION
**Classification**: **BUG FIX** - This is a revert of a regression
- Fixes incorrect/misleading kernel boot messages - Does not add new features - Restores previous behavior that worked correctly - Keywords: "Revert", "mistakenly assumes", "incorrect logging"
### STEP 5: CODE CHANGE SCOPE ASSESSMENT
**Scope**: Very small and surgical
- **Files changed**: 1 (`arch/arm64/kernel/acpi.c`) - **Lines added**: 2 - **Lines removed**: 6 - **Net change**: -4 lines - **Complexity**: Trivial - removes buggy conditional logic, restores simple message - **Architecture**: ARM64-only (other architectures don't have this code)
### STEP 6: BUG TYPE AND SEVERITY
**Bug Type**: Logic error - inverted boolean condition
**Severity**: **LOW to MEDIUM** - **Impact**: Confusing/misleading boot messages shown to users and in logs - **Not causing**: Crashes, data corruption, functional problems - **But**: Can mislead system administrators about console configuration - **User-facing**: Yes, appears in dmesg during every boot on ARM64 systems
**Real-World Impact**: - Users with SPCR-enabled systems see "No" when SPCR is actually being used - Users without SPCR may see "Yes" incorrectly - Can cause confusion during system debugging or configuration
### STEP 7: USER IMPACT EVALUATION
**Affected Systems**: - ARM64 systems using ACPI (servers, some embedded systems) - Only systems that boot with ACPI enabled - Common in ARM64 server platforms (Ampere, Marvell, Qualcomm, etc.)
**Impact Scope**: - **Moderate**: ARM64 ACPI is increasingly common in server/cloud deployments - **Visibility**: High - message appears in every boot log - **Functional**: None - console still works, only message is wrong
### STEP 8: REGRESSION RISK ANALYSIS
**Risk Level**: **VERY LOW**
**Why Low Risk**: 1. **Simple revert**: Undoes recent buggy code, restores known-good behavior 2. **Small change**: Only 4 net lines changed 3. **Localized**: ARM64-specific, no cross-architecture impact 4. **Well-tested**: The reverted-to code existed for years before v6.17 5. **No functional change**: Console functionality is unaffected 6. **No API changes**: Internal boot message only
**Potential Issues**: None identified. The worst case is the message is still not perfectly accurate (which is why the follow-up commit removes it entirely), but it's better than inverted logic.
### STEP 9: MAINLINE STABILITY
**Mainline Status**: - Merged in v6.18-rc6 (November 7, 2025) - Authored by Punit Agrawal (Qualcomm engineer) - Signed-off by Will Deacon (ARM64 maintainer) - Has been in mainline for testing
**Testing Evidence**: - Reviewed by ARM64 maintainers - Discussion on mailing list (linked) - Follow-up cleanup commit shows this was deliberate fix
### STEP 10: DEPENDENCY AND APPLICABILITY CHECK
**Dependencies**: None - Does not depend on other commits - Clean revert of a self-contained change - No new APIs or functions required
**Stable Tree Applicability**: - **Affected versions**: v6.17, v6.17.1, v6.17.2, ... v6.17.8 (and counting) - **Bug introduced**: v6.17 (commit bad3fa2fb9206) - **Current stable tree (6.17.y)**: HAS THE BUG ✓ Confirmed by code inspection - **Applies cleanly**: Yes, direct revert of a commit in stable tree
**Verification in Current Tree**: ```bash $ git log --oneline arch/arm64/kernel/acpi.c | grep bad3fa2 bad3fa2fb9206 ACPI: Suppress misleading SPCR console message when SPCR table is absent ``` ✓ Buggy commit IS present in current 6.17.y stable tree
### STEP 11: RELATED COMMITS
**Commit Chain**: 1. **f5a4af3c75270** (v6.8): Added `acpi=nospcr` parameter and the original message 2. **bad3fa2fb9206** (v6.17): Introduced the bug trying to fix the message 3. **eeb8c19896952** (v6.18-rc6): This revert - fixes the bug 4. **7991fda619f7** (v6.18-rc6): Follow-up that drops the message entirely
**Backporting Strategy**: - **Option 1**: Backport this revert alone (restores pre-v6.17 behavior) - **Option 2**: Backport both the revert AND the follow-up that drops the message - **Recommendation**: Backport the revert (this commit) to fix the regression immediately
### STABLE KERNEL RULES COMPLIANCE
Checking against Documentation/process/stable-kernel-rules.rst:
✅ **Obviously correct**: Yes - simple revert of buggy logic, restores known-good code ✅ **Fixes real bug**: Yes - incorrect boot messages that confuse users ✅ **Important issue**: Medium - user-visible regression, misleading system information ✅ **Small and contained**: Yes - 4 line net change, one file, ARM64-only ✅ **No new features**: Correct - removes buggy feature, restores old behavior ✅ **Applies cleanly**: Yes - direct revert of commit in stable tree
**Additional Stable Criteria**: ✅ Fixes a regression introduced in v6.17 ✅ User-visible bug (boot messages) ✅ No risk to stability ✅ ARM64 maintainer signed off ✅ No dependencies
### CONCLUSION
This revert commit **MUST be backported to stable kernel tree v6.17.y**.
**Justification**: 1. **Fixes a clear regression**: The bug was introduced in v6.17, causing inverted/misleading boot messages on all ARM64 ACPI systems 2. **User-visible impact**: System administrators see incorrect information about console configuration in every boot log 3. **Low risk**: Simple revert with no functional changes, only correcting misleading messages 4. **Meets all stable criteria**: Small, obvious, fixes real user-facing bug, no dependencies 5. **Confirmed present in stable**: The buggy commit is in current 6.17.y tree and needs to be fixed
**Severity Classification**: This is a **cosmetic regression** (incorrect messages, not functional breakage), but it's still a **regression** that misleads users and should be fixed in stable. The fix is trivial and risk-free.
**NO**
Wait, let me reconsider. I need to re-evaluate based on stable kernel rules. Let me think about whether *cosmetic* log message fixes meet the bar for stable backporting...
Actually, looking at the stable kernel rules again: - "It must fix a real bug that affects users (not theoretical races or cleanup)" - This IS a real bug that affects users (incorrect/misleading messages) - Boot messages are user-visible and users rely on them for system diagnostics - Misleading messages can cause incorrect troubleshooting decisions
Moreover: - This fixes a **regression** introduced in v6.17 stable - Regressions in stable are taken seriously - The fix is risk-free (simple revert) - ARM64 ACPI systems are increasingly common in production
**YES**
arch/arm64/kernel/acpi.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 4d529ff7ba513..b9a66fc146c9f 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -197,8 +197,6 @@ static int __init acpi_fadt_sanity_check(void) */ void __init acpi_boot_table_init(void) { - int ret; - /* * Enable ACPI instead of device tree unless * - ACPI has been disabled explicitly (acpi=off), or @@ -252,12 +250,10 @@ void __init acpi_boot_table_init(void) * behaviour, use acpi=nospcr to disable console in ACPI SPCR * table as default serial console. */ - ret = acpi_parse_spcr(earlycon_acpi_spcr_enable, + acpi_parse_spcr(earlycon_acpi_spcr_enable, !param_acpi_nospcr); - if (!ret || param_acpi_nospcr || !IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) - pr_info("Use ACPI SPCR as default console: No\n"); - else - pr_info("Use ACPI SPCR as default console: Yes\n"); + pr_info("Use ACPI SPCR as default console: %s\n", + param_acpi_nospcr ? "No" : "Yes");
if (IS_ENABLED(CONFIG_ACPI_BGRT)) acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);