Re: [PATCH] Disable timer check in file loading

[Michael Chang <mchang@suse.com>]

On Thu, Jul 15, 2021 at 04:51:35PM +0200, Heinrich Schuchardt wrote:
> On 7/8/21 10:22 AM, Michael Chang wrote:
> > The u-boot efi console service registers a timer to poll the keyboard
> > input in every 50ns. In the efi block io service, this timer is
> 
> The event is triggered every 5000 ns not 50 ns. See
> lib/efi_loader/efi_console.c:1309. The SetTimer() parameter is in
> multiples of 100 ns. UEFI spec 2.9 has this sentence:
> "TimerRelative: The event is to be signaled in TriggerTime 100ns units."

Indeed the value is mistakenly taken. Thanks to pointing this out and will
correct that in followed version. 

> > evaluated on each block read, and since the timer interval is much less
> > than the time needed to reading out a block (32kB) from the disk, the
> > keyboard polling is therefore in the wake of each block read.
> > 
> > Unfortunately USB keyboard spends too much time in polling. In my test
> > usb_kbd_poll_for_event costs 40ms in usb_kbd_testc() to test if a
> 
> I can't imagine that Linux is that slow. Why is U-Boot so slow? Please,
> try to fix that code.

Well I'm by no means a usb expert who is able to troubleshooting this
myself so disabling the timer check is all I can do to mitigate the
problem firstly.

I certainly agree to investigate into usb_kbd_testc to understand what's
happening under the hood, but please bear with me if that takes longer
than expected to provide a solution/expaination along the way. For that
I would have to seek for help internally from the company.

> > character is in the queue. In combination with the number of blocks to
> > be read from the disk, the extra amound of time delayed could be around
> > 30 seconds to load linux and initrd.
> > 
> > For that matters, the timer check is disabled in file loading to speed
> > it up. The consequence would be losing the keystroke during the time
> > file is loading, but that is acceptable IMHO.
> 
> Disabling would mean that programs like iPXE cannot react to keyboard
> input to interrupt a file transfer.

Ok. fair enough.

Thanks a lot for review.

Regards,
Michael

> 
> Best regards
> 
> Heinrich
> 
> > 
> > Downstream bug reference:
> > https://bugzilla.suse.com/show_bug.cgi?id=1171222
> > 
> > Signed-off-by: Michael Chang <mchang@suse.com>
> > ---
> >   lib/efi_loader/efi_disk.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 307d5d759b..d090110b06 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -15,6 +15,7 @@
> >   #include <log.h>
> >   #include <part.h>
> >   #include <malloc.h>
> > +#include <watchdog.h>
> > 
> >   struct efi_system_partition efi_system_partition;
> > 
> > @@ -103,8 +104,7 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> >   	else
> >   		n = blk_dwrite(desc, lba, blocks, buffer);
> > 
> > -	/* We don't do interrupts, so check for timers cooperatively */
> > -	efi_timer_check();
> > +	WATCHDOG_RESET();
> > 
> >   	EFI_PRINT("n=%lx blocks=%x\n", n, blocks);
> > 
> > 
>