[PATCH] Support booting USB drives with a write protect switch enabled

[Mike Banon]

At least some USB drives with a write protect switch (e.g. Netac U335)
could report "MEDIUM NOT PRESENT" for a while if a write protection is
enabled. Instead of stopping the initialization attempts immediately,
stop only after getting this report for 3 times, to ensure the
successful initialization of such a "broken hardware".

Signed-off-by: Mike Banon

diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c
index 1f15081..6b6fea9 100644
--- a/src/hw/blockcmd.c
+++ b/src/hw/blockcmd.c
@@ -144,8 +144,9 @@ scsi_is_ready(struct disk_op_s *op)
     dprintf(6, "scsi_is_ready (drive=%p)\n", op->drive_fl);

/* Retry TEST UNIT READY for 5 seconds unless MEDIUM NOT PRESENT is
-     * reported by the device.  If the device reports "IN PROGRESS",
+     * reported by the device 3 times.  If the device reports "IN PROGRESS",
      * 30 seconds is added. */
+    int tries = 3;
     int in_progress = 0;
     u32 end = timer_calc(5000);
     for (;;) {
@@ -167,8 +168,11 @@ scsi_is_ready(struct disk_op_s *op)

// Sense succeeded.
         if (sense.asc == 0x3a) { /* MEDIUM NOT PRESENT */
-            dprintf(1, "Device reports MEDIUM NOT PRESENT\n");
-            return -1;
+            tries--;
+            dprintf(1, "Device reports MEDIUM NOT PRESENT - %d tries left\n",
+                tries);
+            if (!tries)
+                return -1;
         }

if (sense.asc == 0x04 && sense.ascq == 0x01 && !in_progress) {

[Kevin O'Connor]

On Thu, Dec 03, 2020 at 07:06:59AM +0300, Mike Banon wrote:
> At least some USB drives with a write protect switch (e.g. Netac U335)
> could report "MEDIUM NOT PRESENT" for a while if a write protection is
> enabled. Instead of stopping the initialization attempts immediately,
> stop only after getting this report for 3 times, to ensure the
> successful initialization of such a "broken hardware".

Thanks.  In general it seems fine to me.  I do have a couple of questions.

How long does this add to the total boot time?  Some users are
particularly sensitive to total boot time, and the "MEDIUM NOT
PRESENT" check is specifically there to reduce total boot time.  If we
retry that check it may be a regression for some users.

Does the troublesome device just require 3 retries, or is it time
based (that is, does it just require additional time to detect there
is something present, or is there something special about the third
request)?

-Kevin
> Signed-off-by: Mike Banon
>
> diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c
> index 1f15081..6b6fea9 100644
> --- a/src/hw/blockcmd.c
> +++ b/src/hw/blockcmd.c
> @@ -144,8 +144,9 @@ scsi_is_ready(struct disk_op_s *op)
>      dprintf(6, "scsi_is_ready (drive=%p)\n", op->drive_fl);
>  /* Retry TEST UNIT READY for 5 seconds unless MEDIUM NOT PRESENT is
> * reported by the device.  If the device reports "IN PROGRESS",
> * reported by the device 3 times.  If the device reports "IN PROGRESS",
> * 30 seconds is added. */
> int tries = 3;
> int in_progress = 0;
> u32 end = timer_calc(5000);
> for (;;) {
>
> @@ -167,8 +168,11 @@ scsi_is_ready(struct disk_op_s *op)
>      // Sense succeeded.
>      if (sense.asc == 0x3a) { /* MEDIUM NOT PRESENT */
>        dprintf(1, "Device reports MEDIUM NOT PRESENT\n");
>        return -1;
>        tries--;
>        dprintf(1, "Device reports MEDIUM NOT PRESENT - %d tries left\n",
>            tries);
>        if (!tries)
>            return -1;
>    }
>
>    if (sense.asc == 0x04 && sense.ascq == 0x01 && !in_progress) {
>
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Mike Banon]

Each try takes about ~3300 ticks of a timer (got this info by some
inserts of u32 cur = timer_read(); dprintf(1, "tries %d=%d\n", tries,
cur);). A bit unsure how it translates to seconds (the CPU frequency
of my laptop fluctuates between 1.4GHz and 2.5GHz), so I did some
stopwatch tests and can't notice anything. It's an extra ~0.3 seconds
delay if I plug this USB drive at all, but no delay difference between
a normal and "write protect" mode (only in "write protect" it behaves
erroneously).

I've looked through a lot of logs and this USB in a "write protect"
mode always works on a 2nd try, even if I insert "msleep(5000);"
before the 1st try, so I guess it's a try based. As this code is only
for blockcmd, I'm not getting any extra delay if i.e. a DVD disc isn't
present in a DVD drive. And to be honest I haven't encountered any
cases where a 3rd try was needed, I put one more just in case and to
make my fix more universal.

On Wed, Dec 9, 2020 at 8:36 PM Kevin O'Connor  wrote:
> On Thu, Dec 03, 2020 at 07:06:59AM +0300, Mike Banon wrote:
> > At least some USB drives with a write protect switch (e.g. Netac U335)
> > could report "MEDIUM NOT PRESENT" for a while if a write protection is
> > enabled. Instead of stopping the initialization attempts immediately,
> > stop only after getting this report for 3 times, to ensure the
> > successful initialization of such a "broken hardware".
>
> Thanks.  In general it seems fine to me.  I do have a couple of questions.
>
> How long does this add to the total boot time?  Some users are
> particularly sensitive to total boot time, and the "MEDIUM NOT
> PRESENT" check is specifically there to reduce total boot time.  If we
> retry that check it may be a regression for some users.
>
> Does the troublesome device just require 3 retries, or is it time
> based (that is, does it just require additional time to detect there
> is something present, or is there something special about the third
> request)?
>
> -Kevin
> > Signed-off-by: Mike Banon
> >
> > diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c
> > index 1f15081..6b6fea9 100644
> > --- a/src/hw/blockcmd.c
> > +++ b/src/hw/blockcmd.c
> > @@ -144,8 +144,9 @@ scsi_is_ready(struct disk_op_s *op)
> >      dprintf(6, "scsi_is_ready (drive=%p)\n", op->drive_fl);
> >  /* Retry TEST UNIT READY for 5 seconds unless MEDIUM NOT PRESENT is
> > * reported by the device.  If the device reports "IN PROGRESS",
> > * reported by the device 3 times.  If the device reports "IN PROGRESS",
> > * 30 seconds is added. */
> > int tries = 3;
> > int in_progress = 0;
> > u32 end = timer_calc(5000);
> > for (;;) {
> >
> > @@ -167,8 +168,11 @@ scsi_is_ready(struct disk_op_s *op)
> >      // Sense succeeded.
> >      if (sense.asc == 0x3a) { /* MEDIUM NOT PRESENT */
> >        dprintf(1, "Device reports MEDIUM NOT PRESENT\n");
> >        return -1;
> >        tries--;
> >        dprintf(1, "Device reports MEDIUM NOT PRESENT - %d tries left\n",
> >            tries);
> >        if (!tries)
> >            return -1;
> >    }
> >
> >    if (sense.asc == 0x04 && sense.ascq == 0x01 && !in_progress) {
> >
> > SeaBIOS mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]

[Kevin O'Connor]

On Sun, Dec 13, 2020 at 03:30:45PM +0300, Mike Banon wrote:
> Each try takes about ~3300 ticks of a timer (got this info by some
> inserts of u32 cur = timer_read(); dprintf(1, "tries %d=%d\n", tries,
> cur);). A bit unsure how it translates to seconds (the CPU frequency
> of my laptop fluctuates between 1.4GHz and 2.5GHz), so I did some
> stopwatch tests and can't notice anything. It's an extra ~0.3 seconds
> delay if I plug this USB drive at all, but no delay difference between
> a normal and "write protect" mode (only in "write protect" it behaves
> erroneously).
>
> I've looked through a lot of logs and this USB in a "write protect"
> mode always works on a 2nd try, even if I insert "msleep(5000);"
> before the 1st try, so I guess it's a try based. As this code is only
> for blockcmd, I'm not getting any extra delay if i.e. a DVD disc isn't
> present in a DVD drive. And to be honest I haven't encountered any
> cases where a 3rd try was needed, I put one more just in case and to
> make my fix more universal.

Thanks.  I committed this change.

-Kevin

[Mike Banon]

Thank you so much Kevin, really happy to hear about this ;) Wish you a nice day

On Mon, Dec 21, 2020 at 7:00 PM Kevin O'Connor  wrote:
> On Sun, Dec 13, 2020 at 03:30:45PM +0300, Mike Banon wrote:
> > Each try takes about ~3300 ticks of a timer (got this info by some
> > inserts of u32 cur = timer_read(); dprintf(1, "tries %d=%d\n", tries,
> > cur);). A bit unsure how it translates to seconds (the CPU frequency
> > of my laptop fluctuates between 1.4GHz and 2.5GHz), so I did some
> > stopwatch tests and can't notice anything. It's an extra ~0.3 seconds
> > delay if I plug this USB drive at all, but no delay difference between
> > a normal and "write protect" mode (only in "write protect" it behaves
> > erroneously).
> >
> > I've looked through a lot of logs and this USB in a "write protect"
> > mode always works on a 2nd try, even if I insert "msleep(5000);"
> > before the 1st try, so I guess it's a try based. As this code is only
> > for blockcmd, I'm not getting any extra delay if i.e. a DVD disc isn't
> > present in a DVD drive. And to be honest I haven't encountered any
> > cases where a 3rd try was needed, I put one more just in case and to
> > make my fix more universal.
>
> Thanks.  I committed this change.
>
> -Kevin