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é <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.

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 1000000000 /* 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
>