Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   =?iso-8859-1?Q?Christian_L=F6hle?= <CLoehle@hyperstone.com>
To:     Avri Altman <Avri.Altman@wdc.com>,
        "adrian.hunter@intel.com" <adrian.hunter@intel.com>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>
Subject: RE: [PATCH] mmc: block: workaround long ioctl busy timeout
Thread-Topic: [PATCH] mmc: block: workaround long ioctl busy timeout
Thread-Index: Adkpq3tb+gXfT/KXQXawlvtJeLUDVwADfyhQAAB3+5A=
Date:   Mon, 16 Jan 2023 15:03:43 +0000
Message-ID: <5cb91ce2557f41398e23ac25e74b5dc0@hyperstone.com>
References: <68590206e8b044a2a71457cbbeda0794@hyperstone.com>
 <DM6PR04MB65759FBA12E7A48DD904E0D6FCC19@DM6PR04MB6575.namprd04.prod.outlook.com>
In-Reply-To: <DM6PR04MB65759FBA12E7A48DD904E0D6FCC19@DM6PR04MB6575.namprd04.prod.outlook.com>
Accept-Language: en-US, de-DE
Content-Language: en-US
Content-Type: text/plain;
        charset="iso-8859-1"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Precedence: bulk


>>Subject: RE: [PATCH] mmc: block: workaround long ioctl busy timeout
>>         cmd.opcode = idata->ic.opcode;
>>         cmd.arg = idata->ic.arg;
>>         cmd.flags = idata->ic.flags;
>> +       /* R1B flag might be removed here to work around hw, so save it */
>> +       poll_prog = (idata->rpmb || (cmd.flags & MMC_RSP_R1B) ==
>> MMC_RSP_R1B);
>> +       busy_timeout = idata->ic.cmd_timeout_ms ? :
>> +               MMC_BLK_TIMEOUT_MS;
> Isn't commit 23e09be254f9 already introduced the very same thing?
> Meaning mmc_poll_for_busy() is already called with the appropriate timeout?

mmc_poll_for_busy() is, but the problem is already at
mmc_wait_for_req(card->host, &mrq);
Drivers like SDHCI will setup their hardware timer for (in SDHCI) the data inhibit bit.
Drivers dont check if the set timeout is above their capabilities, that's why sanitize also
removes the busy flag.
So without this patch and issuing a secure erase that takes reasonably long, it will fail like so:

[  464.749702] Polling 19507500ms / 19507500ms for busy: CMD38 : 80000000
[  545.761530] mmc2: Timeout waiting for hardware interrupt.
[  545.762623] mmc2: sdhci: ============ SDHCI REGISTER DUMP ===========
[  545.763199] mmc2: sdhci: Sys addr:  0x00000000 | Version:  0x00001002
[  545.763776] mmc2: sdhci: Blk size:  0x00007200 | Blk cnt:  0x00000000
[  545.764353] mmc2: sdhci: Argument:  0x80000000 | Trn mode: 0x00000013
[  545.764928] mmc2: sdhci: Present:   0x1fef0006 | Host ctl: 0x00000035
[  545.765504] mmc2: sdhci: Power:     0x0000000b | Blk gap:  0x00000080
[  545.766080] mmc2: sdhci: Wake-up:   0x00000000 | Clock:    0x00000207
[  545.766656] mmc2: sdhci: Timeout:   0x0000000e | Int stat: 0x00000000
[  545.767231] mmc2: sdhci: Int enab:  0x02ff000b | Sig enab: 0x02ff000b
[  545.767807] mmc2: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
[  545.768382] mmc2: sdhci: Caps:      0x44edc880 | Caps_1:   0x800020f7
[  545.768959] mmc2: sdhci: Cmd:       0x0000261b | Max curr: 0x00000000
[  545.769534] mmc2: sdhci: Resp[0]:   0x00000800 | Resp[1]:  0xfff6dbff
[  545.770110] mmc2: sdhci: Resp[2]:   0x329f5903 | Resp[3]:  0x00900f00
[  545.770686] mmc2: sdhci: Host ctl2: 0x00000000
[  545.771089] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr: 0x0b4b1208
[  545.771665] mmc2: sdhci: ============================================
[  545.773325] sdhci-arasan fe330000.mmc: __mmc_blk_ioctl_cmd: CMD38 cmd error -110

(First print added by me, shows cmd_timeout_ms set by mmc-utils)
Erroring out already at
if (cmd.error) {
	dev_err(mmc_dev(card->host), "%s: cmd error %d\n",
					__func__, cmd.error);
	return cmd.error;
}
i.e. timeout set by user space is  being limited by the max hardware timeout.

Regards,
Christian

>
> Thanks,
> Avri

Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782