Bug 263928 - sdhci_xenon puts SD card in read-only mode on 13.1
Summary: sdhci_xenon puts SD card in read-only mode on 13.1
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 13.1-RELEASE
Hardware: arm64 Any
: --- Affects Some People
Assignee: Security Team
URL: https://security.freebsd.org/advisori...
Keywords:
Depends on:
Blocks: 264030
  Show dependency treegraph
 
Reported: 2022-05-11 19:42 UTC by Mike Cui
Modified: 2023-02-13 15:57 UTC (History)
7 users (show)

See Also:
koobs: maintainer-feedback? (mw)
koobs: mfc-stable13?
koobs: mfc-stable12-


Attachments
patch (840 bytes, patch)
2022-05-17 02:28 UTC, Mike Cui
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Cui 2022-05-11 19:42:22 UTC
I was running 13.0-Release on espressobin booting off SD card. After upgrading to 13.1, the system no longer boots because the SD card is in read-only mode and the root filesystem cannot be mounted.

Log on 13.1:

sdhci_xenon0: <Armada Xenon SDHCI controller> mem 0xd0000-0xd02ff,0x1e808-0x1e80b irq 25 on simplebus1
mmc0: <MMC/SD bus> on sdhci_xenon0
mmcsd0: 32GB <SDHC GB1QT 3.0 SN 2C0B63BC MFG 04/2020 by 27 SM> (read-only) at mmc0 50.0MHz/4bit/65535-block

Log on 13.0:

sdhci_xenon0: <Armada Xenon SDHCI controller> mem 0xd0000-0xd02ff,0x1e808-0x1e80b irq 25 on simplebus1
mmc0: <MMC/SD bus> on sdhci_xenon0
mmcsd0: 32GB <SDHC GB1QT 3.0 SN 2C0B63BC MFG 04/2020 by 27 SM> at mmc0 50.0MHz/4bit/65535-block

Between 13.0-Release and 13.1-RC6: the following 6 changes were added:
1b19617e9662 sdhci_xenon: remove redundant code in property parsing
a2f06581802b sdhci_xenon: add AP807 compatible string
8bb448b66e9d sdhci_xenon: add UHS support
7518736826a2 sdhci_xenon: improve the VCCQ voltage switch sequence
68be9ab5bd53 sdhci_xenon: allow to properly disable the UHS signaling
707ab1f0643a sdhci_xenon: enable MMC FDT parsing

After reverting all 6 changes I can get 13.1-RC6 to boot again.
Comment 1 Daniel Engberg freebsd_committer freebsd_triage 2022-05-12 00:56:39 UTC
Mike Cui,
Since there are several variants around of the ESPRESSObin, do you happen to know which one you have?

Marcin,
Can you have a look at this?
Comment 2 Mike Cui 2022-05-12 01:03:55 UTC
(In reply to Daniel Engberg from comment #1)
I have the esspressobin v7.
Comment 3 Mike Cui 2022-05-12 03:27:56 UTC
I found the bug in this commit: 707ab1f0643a sdhci_xenon: enable MMC FDT parsing

Patch for the fix:

diff --git a/sys/dev/sdhci/sdhci_xenon.c b/sys/dev/sdhci/sdhci_xenon.c
index b6f7513245eb..7ed94907e478 100644
--- a/sys/dev/sdhci/sdhci_xenon.c
+++ b/sys/dev/sdhci/sdhci_xenon.c
@@ -183,7 +183,7 @@ sdhci_xenon_get_ro(device_t bus, device_t dev)
        struct sdhci_xenon_softc *sc = device_get_softc(bus);
 
        return (sdhci_generic_get_ro(bus, dev) ^
-           (sc->mmc_helper.props & MMC_PROP_WP_INVERTED));
+           !!(sc->mmc_helper.props & MMC_PROP_WP_INVERTED));
 }
 
 static bool
Comment 4 mw 2022-05-12 15:00:35 UTC
In HEAD and stable/13 this exact line was changed to bool structure field - so for sure it works (see https://cgit.freebsd.org/src/commit/sys/dev/sdhci?id=d78e464d23304084be17cb8db8981558f2829d6c) - I am certain it works fine with DT and ACPI.

Anyway, without this patch I do not recall issues with accessing the SD or MMC (I used MacchiatoBin and CN913x boards), however it was long ago and I'd have to check. That's a pity it's to late to react for and MFS to 13.1...
Comment 5 Mike Cui 2022-05-17 00:27:07 UTC
(In reply to mw from comment #4)
Can we get the fix into a patch release? Or does the fix have to wait for 13.2?
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2022-05-17 00:35:44 UTC
(In reply to Mike Cui from comment #3)

Could you please include this patch as an attachment Mike, thank you
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2022-05-17 00:37:34 UTC
(In reply to Mike Cui from comment #5)

For sufficiently high priority or impact issues, errata notices can be requested for resolution in subsequent pX updates within a release branch
Comment 8 Mike Cui 2022-05-17 02:28:06 UTC
Created attachment 233978 [details]
patch

Here is the one-line patch which fixes the issue on releng/13.1

However, stable/13 has the commit 693af80b74359d38417ae5da7a1f02c83ec8332a (cherry picked from d78e464d23304084be17cb8db8981558f2829d6c) which also fixes this issue.
Comment 9 mw 2022-05-17 14:51:07 UTC
(In reply to Mike Cui from comment #8)
For errata I'd go with custom one-liner patch. The MFC is much bigger change and the stable/13 already diverged in this area - I kept these changes out of 13.1 release on purpose, as I considered the MFS as too late (around 13.1-rc1). @koobs what would you advise?
Comment 10 Mike Cui 2023-01-04 04:50:34 UTC
(In reply to mw from comment #9)
Any chance we can get this into a patch release?
Comment 11 mw 2023-01-05 15:30:08 UTC
Hi Mike,

Apologies for such a delay. I reached out to re@ if taking a fix for this problem as an errata notice is still possible. I'll update how it goes.

Best regards,
Marcin
Comment 12 Xin LI freebsd_committer freebsd_triage 2023-01-16 17:55:44 UTC
Move to secteam@ queue as this is an EN candidate.
Comment 13 mw 2023-02-08 18:48:01 UTC
The patch was merged, as a part of the latest 13.1 errata
https://cgit.freebsd.org/src/commit/?id=4b31a7861af02ec5c1528e20d375a7e9813cdc80
Comment 14 Gordon Tetlow freebsd_committer freebsd_triage 2023-02-13 15:57:13 UTC
Published as FreeBSD-EN-23:02.sdhci.