Re: [PATCH v7 13/13] tests/acceptance: console boot tests for quanta-gsj

[Havard Skinnemoen]

On Wed, Aug 19, 2020 at 10:29 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> +Eric / Richard for compiler optimizations.
>
> On 8/20/20 3:53 AM, Havard Skinnemoen wrote:
> > On Tue, Aug 11, 2020 at 8:26 PM Havard Skinnemoen
> > <hskinnemoen@google.com> wrote:
> >>
> >> On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé <f4bug@amsat.org> 
> >> wrote:
> >>> INTERRUPTED: Test interrupted by SIGTERM
> >>> Runner error occurred: Timeout reached
> >>> (240.45 s)
> >>>
> >>> Is that expected?
> >>
> >> I'm not sure why it only happens when running direct kernel boot with
> >> unoptimized qemu, but it seems a little happier if I enable a few more
> >> peripherals that I have queued up (sd, ehci, ohci and rng), though not
> >> enough.
> >>
> >> It still stalls for an awfully long time on "console: Run /init as
> >> init process" though. I'm not sure what it's doing there. With -O2 it
> >> only takes a couple of seconds to move on.
> >
> > So it turns out that the kernel gets _really_ sluggish when skipping
> > the clock initialization normally done by the boot loader.
> >
> > I changed the reset value of CLKSEL like this:
> >
> > diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
> > index 21ab4200d1..5e9849410f 100644
> > --- a/hw/misc/npcm7xx_clk.c
> > +++ b/hw/misc/npcm7xx_clk.c
> > @@ -67,7 +67,7 @@ enum NPCM7xxCLKRegisters {
> >   */
> >  static const uint32_t cold_reset_values[NPCM7XX_CLK_NR_REGS] = {
> >      [NPCM7XX_CLK_CLKEN1]        = 0xffffffff,
> > -    [NPCM7XX_CLK_CLKSEL]        = 0x004aaaaa,
> > +    [NPCM7XX_CLK_CLKSEL]        = 0x004aaba9,
> >      [NPCM7XX_CLK_CLKDIV1]       = 0x5413f855,
> >      [NPCM7XX_CLK_PLLCON0]       = 0x00222101 | PLLCON_LOKI,
> >      [NPCM7XX_CLK_PLLCON1]       = 0x00202101 | PLLCON_LOKI,
> >
> > which switches the CPU core and UART to run from PLL2 instead of
> > CLKREF (25 MHz).
> >
> > With this change, the test passes without optimization:
> >
> >  (02/19) 
> > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_quanta_gsj_initrd:
> > PASS (39.62 s)
> >
> > It doesn't look like this change hurts booting from the bootrom (IIUC
> > the nuvoton bootblock overwrites CLKSEL anyway), but it's not super
> > clean.
> >
> > Perhaps I should make it conditional on kernel_filename being set? Or
> > would it be better to provide a write_board_setup hook for this?
>
> QEMU prefers to avoid ifdef'ry at all cost. However I find this
> approach acceptable (anyway up to the maintainer):
>
> +static void npcm7xx_clk_cold_reset_fixup(NPCM7xxCLKState *s)
> +{
> +#ifndef __OPTIMIZE__
> +    /*
> +     * When built without optimization, ...
> +     * so run CPU core and UART from PLL2 instead of CLKREF.
> +     */
> +    s->regs[NPCM7XX_CLK_CLKSEL] |= 0x103,
> +#endif
> +}

I think this is actually a problem regardless of optimization level.
Turning optimization off amplifies the problem, but the problem is
still there with optimization on.

This does not affect booting a full flash image, as these fixups (and
more) are done by the boot loader in that case.

Havard