Re: kern/55512: sdmmc locking broken

[mlelstv%serpens.de@localhost (Michael van Elst)]

The following reply was made to PR kern/55512; it has been noted by GNATS.

From: mlelstv%serpens.de@localhost (Michael van Elst)
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/55512: sdmmc locking broken
Date: Wed, 22 Jul 2020 12:29:29 -0000 (UTC)

 martin%NetBSD.org@localhost writes:
 
 >For broken locking just check the source:
 
 >sdmmcvar.h:#define      SDMMC_LOCK(sc)
 >sdmmcvar.h:#define      SDMMC_UNLOCK(sc)
 
 >and notice that *some* calls of SDMMC_LOCK/SDMMC_UNLOCK have additional
 >locks around them:
 
 You see that SDMMC_LOCK/UNLOCK are just dummies.
 
 
 >Blindly removing all SDMMC_LOCK/SDMMC_UNLOCK and replacing them with sc_mtx
 >locks unfortunately does not work (causes locking against myself panics
 >during interrupt establish) - but probably is a first step forward and then
 >quite easy to cleanup.
 
 Or just remove the macros as the macros were ignored when locking was added
 later. The code also deviates from the OpenBSD original and from the current
 OpenBSD code.
 
 
 Your original issue:
 
 [   5.2286102] sunximmc1: WARNING: driver submitted a command while the controller was busy
 [   5.2386127] sdmmc1: extended I/O error 16, r=40992 p=0xc3ef6e94 l=4 read
 [   5.3486139] bwfm0: CHIPACTIVE
 [   5.3486139] bwfm0: wl0: May 16 2018 23:42:49 version 5.90.244 FWID 01-0
 
 The sunxi sdhc driver fails concurrent command requests which is fine for
 memory cards. But SDIO allows for some concurrent commands and the check
 is bogus and doesn't exist in other sdhc drivers.
 
 The same message with a kernel from last December and some debugging
 extra:
 
 sunximmc1: WARNING: driver submitted command 0035 while the controller was busy with 0034
 0x9a23de2c: netbsd:sdmmc_mmc_command+0x44
 0x9a23ded4: 909f2000
 sdmmc1: extended I/O error 16, r=40992 p=0x9a23ded4 l=4 read
 
 An extended I/O command (multiple data bytes) was issued while a direct I/O
 command (single data bytes or functions) command was busy.
 
 That's probably not allowed (the concurrent commands are for e.g.
 aborting a running extended I/O command).
 
 r=40992 (0xa020) should be the bwfm INTSTATUS register, the interrupt
 is probably triggered by the chipactive condition. Reading the status
 failed, but since it isn't cleared either, the interrupt condition
 persists and the interrupt is still handled later.
 
 The busy direct I/O command might be the driver polling the chip
 for enabled clocks in bwfm_sdio_buscore_prepare.
 
 The end of the attach phase is the only place where concurrent I/O
 can happen, and you see that it does (with little effect).
 
 To avoid this, the end of the attach phase needs either to be handled
 by sdmmc tasks (I was looking at the chipactive interrupt) or the tasks
 that handle interrupts must be deferred until the attachment code
 that accesses the chip is finished.
 
 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv%serpens.de@localhost
                                 "A potential Snark may lurk in every tree."