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
>> <hskinnem...@google.com> wrote:
>>>
>>> On Tue, Aug 11, 2020 at 1:48 AM Philippe Mathieu-Daudé <f4...@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.
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] = 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
> +}
>
> 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.
>
