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

[Philippe Mathieu-Daudé]

Hi Havard,

On 8/20/20 10:30 PM, Havard Skinnemoen wrote:
> On Thu, Aug 20, 2020 at 10:46 AM Philippe Mathieu-Daudé  
> wrote:
>>
>> On 8/20/20 6:24 PM, Havard Skinnemoen wrote:
>>> On Wed, Aug 19, 2020 at 10:29 PM Philippe Mathieu-Daudé  
>>> 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
>  wrote:
>>
>> On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  
>> 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]= 0x,
> -[NPCM7XX_CLK_CLKSEL]= 0x004a,
> +[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.
>>
>> OK, this reminds me few more details about the problem I had with the
>> raspi3 when adding the ClockPowerResetManager block.
>> Found the branch. A bit bitter/sad it was more than 1 year ago.
>>
>> So if ARM_FEATURE_GENERIC_TIMER is available, Linux polls the CNTFRQ_EL0
>> register. At that time this register were using a fixed frequency:
>>
>> #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
>>
>> Xilinx' fork does it this way:
>> https://github.com/Xilinx/qemu/commit/9e939b54e2d
>>
>> Now I see Andrew Jeffery fixed that in 96eec6b2b38
>> ("target/arm: Prepare generic timer for per-platform CNTFRQ")
>> adding a 'cntfrq' property, which he then sets in the Aspeed
>> machine in commit 058d095532d ("ast2600: Configure CNTFRQ at 1125MHz").
>>
>> Maybe your SoC is simply missing this property?
> 
> Hmm, it doesn't look like Cortex-A9 has this property...
> 
> Unexpected error in object_property_find() at
> /usr/local/google/home/hskinnemoen/qemu/upstream/qom/object.c:1254:
> qemu-system-arm: Property '.cntfrq' not found
> 
> However, I did notice there are a lot of constraints on input
> frequencies to the CPU and various peripherals, and many of these are
> not met by the power-on reset values.

Oh OK.

> 
> For example, the UART needs a 24 MHz input clock, but there's no way
> to generate this from the default 25 MHz reference clock. However,
> PLL2 is set up to run at 960 MHz by default (as soon as it's locked),
> with a fixed /2 divider. The UART uses a /20 divider by default, so it
> gets a 25 MHz / 20 = 1.25 MHz clock with power-on defaults. However,
> switching the source to PLL2 results in 960 MHz / 2 / 20 = 24 MHz.

Yes, thanks to Damien's work we can now use the Clock API to 

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

[Havard Skinnemoen]

On Thu, Aug 20, 2020 at 10:46 AM Philippe Mathieu-Daudé  wrote:
>
> On 8/20/20 6:24 PM, Havard Skinnemoen wrote:
> > On Wed, Aug 19, 2020 at 10:29 PM Philippe Mathieu-Daudé  
> > 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
> >>>  wrote:
> 
>  On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  
>  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]= 0x,
> >>> -[NPCM7XX_CLK_CLKSEL]= 0x004a,
> >>> +[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.
>
> OK, this reminds me few more details about the problem I had with the
> raspi3 when adding the ClockPowerResetManager block.
> Found the branch. A bit bitter/sad it was more than 1 year ago.
>
> So if ARM_FEATURE_GENERIC_TIMER is available, Linux polls the CNTFRQ_EL0
> register. At that time this register were using a fixed frequency:
>
> #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
>
> Xilinx' fork does it this way:
> https://github.com/Xilinx/qemu/commit/9e939b54e2d
>
> Now I see Andrew Jeffery fixed that in 96eec6b2b38
> ("target/arm: Prepare generic timer for per-platform CNTFRQ")
> adding a 'cntfrq' property, which he then sets in the Aspeed
> machine in commit 058d095532d ("ast2600: Configure CNTFRQ at 1125MHz").
>
> Maybe your SoC is simply missing this property?

Hmm, it doesn't look like Cortex-A9 has this property...

Unexpected error in object_property_find() at
/usr/local/google/home/hskinnemoen/qemu/upstream/qom/object.c:1254:
qemu-system-arm: Property '.cntfrq' not found

However, I did notice there are a lot of constraints on input
frequencies to the CPU and various peripherals, and many of these are
not met by the power-on reset values.

For example, the UART needs a 24 MHz input clock, but there's no way
to generate this from the default 25 MHz reference clock. However,
PLL2 is set up to run at 960 MHz by default (as soon as it's locked),
with a fixed /2 divider. The UART uses a /20 divider by default, so it
gets a 25 MHz / 20 = 1.25 MHz clock with power-on defaults. However,
switching the source to PLL2 results in 960 MHz / 2 / 20 = 24 MHz.

Similarly, the CPU is supposed to run at 800 MHz, but it runs at 25
MHz with power-on defaults. PLL1 runs at 800 MHz by default, with a
fixed /2 divider. The boot loader doubles the feedback 

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

[Philippe Mathieu-Daudé]

On 8/20/20 6:24 PM, Havard Skinnemoen wrote:
> On Wed, Aug 19, 2020 at 10:29 PM Philippe Mathieu-Daudé  
> 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
>>>  wrote:

 On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  
 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]= 0x,
>>> -[NPCM7XX_CLK_CLKSEL]= 0x004a,
>>> +[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.

OK, this reminds me few more details about the problem I had with the
raspi3 when adding the ClockPowerResetManager block.
Found the branch. A bit bitter/sad it was more than 1 year ago.

So if ARM_FEATURE_GENERIC_TIMER is available, Linux polls the CNTFRQ_EL0
register. At that time this register were using a fixed frequency:

#define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */

Xilinx' fork does it this way:
https://github.com/Xilinx/qemu/commit/9e939b54e2d

Now I see Andrew Jeffery fixed that in 96eec6b2b38
("target/arm: Prepare generic timer for per-platform CNTFRQ")
adding a 'cntfrq' property, which he then sets in the Aspeed
machine in commit 058d095532d ("ast2600: Configure CNTFRQ at 1125MHz").

Maybe your SoC is simply missing this property?

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

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é  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
> >  wrote:
> >>
> >> On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  
> >> 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]= 0x,
> > -[NPCM7XX_CLK_CLKSEL]= 0x004a,
> > +[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

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

[Philippe Mathieu-Daudé]

On 8/20/20 7:29 AM, Philippe Mathieu-Daudé 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
>>  wrote:
>>>
>>> On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  
>>> 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.

Hmm IIRC other boards are affected (raspi and orange-pi).

Maybe it is time to define some static inlined boolean function
in "qemu/compiler.h", maybe qemu_build_optimized()? Or not inlined
function but simply expand to true/false:

 /**
  * qemu_build_not_reached()
  *
  * The compiler, during optimization, is expected to prove that a call
  * to this function cannot be reached and remove it.  If the compiler
  * supports QEMU_ERROR, this will be reported at compile time; o therwise
  * this will be reported at link time due to the missing symbol.
  */
 #if defined(__OPTIMIZE__) && !defined(__NO_INLINE__)
 extern void QEMU_NORETURN QEMU_ERROR("code path is reachable")
 qemu_build_not_reached(void);
 #else
 #define qemu_build_not_reached()  g_assert_not_reached()
 #endif
+
+#if defined(__OPTIMIZE__)
+#define qemu_build_optimized() true
+#else
+#define qemu_build_optimized() false
+#endif

>>
>> 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]= 0x,
>> -[NPCM7XX_CLK_CLKSEL]= 0x004a,
>> +[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
> +}
> 
>  static void npcm7xx_clk_enter_reset(Object *obj, ResetType type)
>  {
>  NPCM7xxCLKState *s = NPCM7XX_CLK(obj);
> 
>  QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
> 
>  switch (type) {
>  case RESET_TYPE_COLD:
>  memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
> +npcm7xx_clk_cold_reset_fixup(s);
>  s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  return;
>  }
>  ...
> 
> Regards,
> 
> Phil.
> 

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

[Philippe Mathieu-Daudé]

+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
>  wrote:
>>
>> On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  
>> 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]= 0x,
> -[NPCM7XX_CLK_CLKSEL]= 0x004a,
> +[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
+}

 static void npcm7xx_clk_enter_reset(Object *obj, ResetType type)
 {
 NPCM7xxCLKState *s = NPCM7XX_CLK(obj);

 QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));

 switch (type) {
 case RESET_TYPE_COLD:
 memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
+npcm7xx_clk_cold_reset_fixup(s);
 s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 return;
 }
 ...

Regards,

Phil.

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

[Havard Skinnemoen]

On Tue, Aug 11, 2020 at 8:26 PM Havard Skinnemoen
 wrote:
>
> On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  
> 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]= 0x,
-[NPCM7XX_CLK_CLKSEL]= 0x004a,
+[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?

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

[Havard Skinnemoen]

On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé  wrote:
>
> Hi Havard,
>
> On 8/11/20 2:46 AM, Havard Skinnemoen wrote:
> > This adds two acceptance tests for the quanta-gsj machine.
> >
> > One test downloads a lightly patched openbmc flash image from github and
> > verifies that it boots all the way to the login prompt.
> >
> > The other test downloads a kernel, initrd and dtb built from the same
> > openbmc source and verifies that the kernel detects all CPUs and boots
> > to the point where it can't find the root filesystem (because we have no
> > flash image in this case).
> >
> > Reviewed-by: Philippe Mathieu-Daudé 
> > Signed-off-by: Havard Skinnemoen 
> > ---
> >  tests/acceptance/boot_linux_console.py | 65 ++
> >  1 file changed, 65 insertions(+)
> >
> > diff --git a/tests/acceptance/boot_linux_console.py 
> > b/tests/acceptance/boot_linux_console.py
> > index 73cc69c499..8592f33a41 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -569,6 +569,71 @@ class BootLinuxConsole(LinuxKernelTest):
> >  'sda')
> >  # cubieboard's reboot is not functioning; omit reboot test.
> >
> > +def test_arm_quanta_gsj(self):
> > +"""
> > +:avocado: tags=arch:arm
> > +:avocado: tags=machine:quanta-gsj
> > +"""
> > +# 25 MiB compressed, 32 MiB uncompressed.
> > +image_url = (
> > +'https://github.com/hskinnemoen/openbmc/releases/download/'
> > +
> > '20200711-gsj-qemu-0/obmc-phosphor-image-gsj.static.mtd.gz')
> > +image_hash = '14895e634923345cb5c8776037ff7876df96f6b1'
> > +image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
> > +image_name = 'obmc.mtd'
> > +image_path = os.path.join(self.workdir, image_name)
> > +archive.gzip_uncompress(image_path_gz, image_path)
> > +
> > +self.vm.set_console()
> > +drive_args = 'file=' + image_path + ',if=mtd,bus=0,unit=0'
> > +self.vm.add_args('-drive', drive_args)
> > +self.vm.launch()
> > +
> > +self.wait_for_console_pattern('> BootBlock by Nuvoton')
> > +self.wait_for_console_pattern('>Device: Poleg BMC NPCM730')
> > +self.wait_for_console_pattern('>Skip DDR init.')
> > +self.wait_for_console_pattern('U-Boot ')
> > +self.wait_for_console_pattern('Booting Linux on physical CPU 0x0')
>
> Tests timeout using QEMU configured with '--enable-debug
> --extra-cflags=-ggdb':
>
> console: Booting Linux on physical CPU 0x0
> console: CPU1: thread -1, cpu 1, socket 0, mpidr 8001
>
> console: stmmaceth f0802000.eth: DMA HW capability register supported
> console: stmmaceth f0802000.eth: Normal descriptors
> console: stmmaceth f0802000.eth: Ring mode enabled
> console: stmmaceth f0802000.eth: device MAC address 92:f6:8f:80:9f:bb
> INTERRUPTED: Test interrupted by SIGTERM
> Runner error occurred: Timeout reached
> (91.05 s)
>
> My guess is unoptimized build makes guest hashing checks over
> the bitbanged SPI flash emulation takes too long.

Right, flash access is super slow when optimization is turned off.

> Trying with timeout=240s:
>
> console: [* ] (1 of 2) A start job is running for…dplug all Devices
> (44s / no limit)
> console: [**] (2 of 2) A start job is running for…Save Random Seed
> (44s / 10min 13s)
> console: [***   ] (2 of 2) A start job is running for…Save Random Seed
> (45s / 10min 13s)
> console: [ ***  ] (2 of 2) A start job is running for…Save Random Seed
> (45s / 10min 13s)
> console: [  *** ] (1 of 2) A start job is running for…dplug all Devices
> (46s / no limit)
> console: [   ***] (1 of 2) A start job is running for…dplug all Devices
> (46s / no limit)
> console: [**] (1 of 2) A start job is running for…dplug all Devices
> (47s / no limit)
> console: [ *] (2 of 2) A start job is running for…Save Random Seed
> (47s / 10min 13s)
> console: [**] (2 of 2) A start job is running for…Save Random Seed
> (48s / 10min 13s)
> console: [   ***] (2 of 2) A start job is running for…Save Random Seed
> (48s / 10min 13s)
> console: [  *** ] (1 of 2) A start job is running for…dplug all Devices
> (49s / no limit)
> console: [ ***  ] (1 of 2) A start job is running for…dplug all Devices
> (51s / no limit)
> console: [***   ] (1 of 2) A start job is running for…dplug all Devices
> (53s / no limit)
> console: [**] (2 of 2) A start job is running for…Save Random Seed
> (53s / 10min 13s)
> console: [* ] (2 of 2) A start job is running for…Save Random Seed
> (54s / 10min 13s)
> console: [**] (2 of 2) A start job is running for…Save Random Seed
> (55s / 10min 13s)
> console: [***   ] (1 of 2) A start job is running for…dplug all Devices
> (56s / no limit)
> console: [ ***  ] (1 of 2) A start job is running for…dplug all Devices
> (57s / no limit)
> console: [  *** ] (1 of 2) A start job is running 

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

[Philippe Mathieu-Daudé]

Hi Havard,

On 8/11/20 2:46 AM, Havard Skinnemoen wrote:
> This adds two acceptance tests for the quanta-gsj machine.
> 
> One test downloads a lightly patched openbmc flash image from github and
> verifies that it boots all the way to the login prompt.
> 
> The other test downloads a kernel, initrd and dtb built from the same
> openbmc source and verifies that the kernel detects all CPUs and boots
> to the point where it can't find the root filesystem (because we have no
> flash image in this case).
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Havard Skinnemoen 
> ---
>  tests/acceptance/boot_linux_console.py | 65 ++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> index 73cc69c499..8592f33a41 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -569,6 +569,71 @@ class BootLinuxConsole(LinuxKernelTest):
>  'sda')
>  # cubieboard's reboot is not functioning; omit reboot test.
>  
> +def test_arm_quanta_gsj(self):
> +"""
> +:avocado: tags=arch:arm
> +:avocado: tags=machine:quanta-gsj
> +"""
> +# 25 MiB compressed, 32 MiB uncompressed.
> +image_url = (
> +'https://github.com/hskinnemoen/openbmc/releases/download/'
> +'20200711-gsj-qemu-0/obmc-phosphor-image-gsj.static.mtd.gz')
> +image_hash = '14895e634923345cb5c8776037ff7876df96f6b1'
> +image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
> +image_name = 'obmc.mtd'
> +image_path = os.path.join(self.workdir, image_name)
> +archive.gzip_uncompress(image_path_gz, image_path)
> +
> +self.vm.set_console()
> +drive_args = 'file=' + image_path + ',if=mtd,bus=0,unit=0'
> +self.vm.add_args('-drive', drive_args)
> +self.vm.launch()
> +
> +self.wait_for_console_pattern('> BootBlock by Nuvoton')
> +self.wait_for_console_pattern('>Device: Poleg BMC NPCM730')
> +self.wait_for_console_pattern('>Skip DDR init.')
> +self.wait_for_console_pattern('U-Boot ')
> +self.wait_for_console_pattern('Booting Linux on physical CPU 0x0')

Tests timeout using QEMU configured with '--enable-debug
--extra-cflags=-ggdb':

console: Booting Linux on physical CPU 0x0
console: CPU1: thread -1, cpu 1, socket 0, mpidr 8001

console: stmmaceth f0802000.eth: DMA HW capability register supported
console: stmmaceth f0802000.eth: Normal descriptors
console: stmmaceth f0802000.eth: Ring mode enabled
console: stmmaceth f0802000.eth: device MAC address 92:f6:8f:80:9f:bb
INTERRUPTED: Test interrupted by SIGTERM
Runner error occurred: Timeout reached
(91.05 s)

My guess is unoptimized build makes guest hashing checks over
the bitbanged SPI flash emulation takes too long.

Trying with timeout=240s:

console: [* ] (1 of 2) A start job is running for…dplug all Devices
(44s / no limit)
console: [**] (2 of 2) A start job is running for…Save Random Seed
(44s / 10min 13s)
console: [***   ] (2 of 2) A start job is running for…Save Random Seed
(45s / 10min 13s)
console: [ ***  ] (2 of 2) A start job is running for…Save Random Seed
(45s / 10min 13s)
console: [  *** ] (1 of 2) A start job is running for…dplug all Devices
(46s / no limit)
console: [   ***] (1 of 2) A start job is running for…dplug all Devices
(46s / no limit)
console: [**] (1 of 2) A start job is running for…dplug all Devices
(47s / no limit)
console: [ *] (2 of 2) A start job is running for…Save Random Seed
(47s / 10min 13s)
console: [**] (2 of 2) A start job is running for…Save Random Seed
(48s / 10min 13s)
console: [   ***] (2 of 2) A start job is running for…Save Random Seed
(48s / 10min 13s)
console: [  *** ] (1 of 2) A start job is running for…dplug all Devices
(49s / no limit)
console: [ ***  ] (1 of 2) A start job is running for…dplug all Devices
(51s / no limit)
console: [***   ] (1 of 2) A start job is running for…dplug all Devices
(53s / no limit)
console: [**] (2 of 2) A start job is running for…Save Random Seed
(53s / 10min 13s)
console: [* ] (2 of 2) A start job is running for…Save Random Seed
(54s / 10min 13s)
console: [**] (2 of 2) A start job is running for…Save Random Seed
(55s / 10min 13s)
console: [***   ] (1 of 2) A start job is running for…dplug all Devices
(56s / no limit)
console: [ ***  ] (1 of 2) A start job is running for…dplug all Devices
(57s / no limit)
console: [  *** ] (1 of 2) A start job is running for…dplug all Devices
(57s / no limit)

Maybe you can disable some expensive services for the test purpose?
See examples of cmdline 'systemd.mask=' in test_arm_orangepi_bionic().

Compiled with -O2:

console: [  *** ] A start job is running for Load/Save Random Seed (1s /
9min 45s)
console: [ ***  ] A start job is running for Load/Save Random Seed 

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

[Havard Skinnemoen]

This adds two acceptance tests for the quanta-gsj machine.

One test downloads a lightly patched openbmc flash image from github and
verifies that it boots all the way to the login prompt.

The other test downloads a kernel, initrd and dtb built from the same
openbmc source and verifies that the kernel detects all CPUs and boots
to the point where it can't find the root filesystem (because we have no
flash image in this case).

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Havard Skinnemoen 
---
 tests/acceptance/boot_linux_console.py | 65 ++
 1 file changed, 65 insertions(+)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 73cc69c499..8592f33a41 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -569,6 +569,71 @@ class BootLinuxConsole(LinuxKernelTest):
 'sda')
 # cubieboard's reboot is not functioning; omit reboot test.
 
+def test_arm_quanta_gsj(self):
+"""
+:avocado: tags=arch:arm
+:avocado: tags=machine:quanta-gsj
+"""
+# 25 MiB compressed, 32 MiB uncompressed.
+image_url = (
+'https://github.com/hskinnemoen/openbmc/releases/download/'
+'20200711-gsj-qemu-0/obmc-phosphor-image-gsj.static.mtd.gz')
+image_hash = '14895e634923345cb5c8776037ff7876df96f6b1'
+image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
+image_name = 'obmc.mtd'
+image_path = os.path.join(self.workdir, image_name)
+archive.gzip_uncompress(image_path_gz, image_path)
+
+self.vm.set_console()
+drive_args = 'file=' + image_path + ',if=mtd,bus=0,unit=0'
+self.vm.add_args('-drive', drive_args)
+self.vm.launch()
+
+self.wait_for_console_pattern('> BootBlock by Nuvoton')
+self.wait_for_console_pattern('>Device: Poleg BMC NPCM730')
+self.wait_for_console_pattern('>Skip DDR init.')
+self.wait_for_console_pattern('U-Boot ')
+self.wait_for_console_pattern('Booting Linux on physical CPU 0x0')
+self.wait_for_console_pattern('CPU1: thread -1, cpu 1, socket 0')
+self.wait_for_console_pattern('OpenBMC Project Reference Distro')
+self.wait_for_console_pattern('gsj login:')
+
+def test_arm_quanta_gsj_initrd(self):
+"""
+:avocado: tags=arch:arm
+:avocado: tags=machine:quanta-gsj
+"""
+initrd_url = (
+'https://github.com/hskinnemoen/openbmc/releases/download/'
+'20200711-gsj-qemu-0/obmc-phosphor-initramfs-gsj.cpio.xz')
+initrd_hash = '98fefe5d7e56727b1eb17d5c00311b1b5c945300'
+initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
+kernel_url = (
+'https://github.com/hskinnemoen/openbmc/releases/download/'
+'20200711-gsj-qemu-0/uImage-gsj.bin')
+kernel_hash = 'fa67b2f141d56d39b3c54305c0e8a899c99eb2c7'
+kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+dtb_url = (
+'https://github.com/hskinnemoen/openbmc/releases/download/'
+'20200711-gsj-qemu-0/nuvoton-npcm730-gsj.dtb')
+dtb_hash = '18315f7006d7b688d8312d5c727eecd819aa36a4'
+dtb_path = self.fetch_asset(dtb_url, asset_hash=dtb_hash)
+
+self.vm.set_console()
+kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
+   'console=ttyS0,115200n8 '
+   'earlycon=uart8250,mmio32,0xf0001000')
+self.vm.add_args('-kernel', kernel_path,
+ '-initrd', initrd_path,
+ '-dtb', dtb_path,
+ '-append', kernel_command_line)
+self.vm.launch()
+
+self.wait_for_console_pattern('Booting Linux on physical CPU 0x0')
+self.wait_for_console_pattern('CPU1: thread -1, cpu 1, socket 0')
+self.wait_for_console_pattern(
+'Give root password for system maintenance')
+
 def test_arm_orangepi(self):
 """
 :avocado: tags=arch:arm
-- 
2.28.0.236.gb10cc79966-goog