Re: [xen-unstable-smoke test] 169781: regressions - FAIL

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 27 Apr 2022, Julien Grall wrote:
> On 27/04/2022 17:38, osstest service owner wrote:
> > flight 169781 xen-unstable-smoke real [real]
> > flight 169785 xen-unstable-smoke real-retest [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/169781/
> > http://logs.test-lab.xenproject.org/osstest/logs/169785/
> > 
> > Regressions :-(
> > 
> > Tests which did not succeed and are blocking,
> > including tests which could not be run:
> >   test-arm64-arm64-xl-xsm       8 xen-boot                 fail REGR. vs.
> > 169773
> 
> Well, I was overly optimistic :(. This now breaks in the ITS code:
> 
> Apr 27 13:23:14.324831 (XEN) Xen call trace:
> Apr 27 13:23:14.324855 (XEN)    [<000000000022a678>]
> alloc_xenheap_pages+0x178/0x194 (PC)
> Apr 27 13:23:14.336856 (XEN)    [<000000000022a670>]
> alloc_xenheap_pages+0x170/0x194 (LR)
> Apr 27 13:23:14.336886 (XEN)    [<0000000000237770>] _xmalloc+0x144/0x294
> Apr 27 13:23:14.348773 (XEN)    [<00000000002378d4>] _xzalloc+0x14/0x30
> Apr 27 13:23:14.348808 (XEN)    [<000000000027b4e4>]
> gicv3_lpi_init_rdist+0x54/0x324
> Apr 27 13:23:14.348835 (XEN)    [<0000000000279898>]
> arch/arm/gic-v3.c#gicv3_cpu_init+0x128/0x46c
> Apr 27 13:23:14.360799 (XEN)    [<0000000000279bfc>]
> arch/arm/gic-v3.c#gicv3_secondary_cpu_init+0x20/0x50
> Apr 27 13:23:14.372796 (XEN)    [<0000000000277054>]
> gic_init_secondary_cpu+0x18/0x30
> Apr 27 13:23:14.372829 (XEN)    [<0000000000284518>]
> start_secondary+0x1a8/0x234
> Apr 27 13:23:14.372856 (XEN)    [<0000010722aa4200>] 0000010722aa4200
> Apr 27 13:23:14.384793 (XEN)
> Apr 27 13:23:14.384823 (XEN)
> Apr 27 13:23:14.384845 (XEN) ****************************************
> Apr 27 13:23:14.384869 (XEN) Panic on CPU 2:
> Apr 27 13:23:14.384891 (XEN) Assertion '!in_irq() && (local_irq_is_enabled()
> || num_online_cpus() <= 1)' failed at common/page_alloc.c:2212
> Apr 27 13:23:14.396805 (XEN) ****************************************
> 
> The GICv3 LPI code contains a few calls to xmalloc() that will be done while
> initializing the GIC CPU interface. I don't think we can delay the
> initialization of the LPI part past local_irq_enable(). So I think we will
> need to allocate the memory when preparing the CPU.
> 
> Any thoughts?


As a general principle I think the ASSERT is a good idea, and it should
make the code better and safer. I would not change the code to make the
ASSERT go away if not to improve the code.

In this case, gicv3_lpi_init_rdist and gicv3_lpi_allocate_pendtable
should be __init functions although they are not marked as __init at the
moment.

It seems to me that it is acceptable to allocate memory with interrupt
disabled during __init. I cannot see any drawbacks with it. I think we
should change the ASSERT to only trigger after __init: system_state ==
SYS_STATE_active.

What do you think?