Re: [PATCH v2 0/9] RISC-V: ACPI: Namespace updates

[Igor Mammedov]

On Fri, 12 Jul 2024 13:51:04 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Fri, Jul 12, 2024 at 02:43:19PM +0200, Igor Mammedov wrote:
> > On Mon,  8 Jul 2024 17:17:32 +0530
> > Sunil V L <sunilvl@ventanamicro.com> wrote:
> >   
> > > This series adds few updates to RISC-V ACPI namespace for virt platform.
> > > Additionally, it has patches to enable ACPI table testing for RISC-V.
> > > 
> > > 1) PCI Link devices need to be created outside the scope of the PCI root
> > > complex to ensure correct probe ordering by the OS. This matches the
> > > example given in ACPI spec as well.
> > > 
> > > 2) Add PLIC and APLIC as platform devices as well to ensure probing
> > > order as per BRS spec [1] requirement.
> > > 
> > > 3) BRS spec requires RISC-V to use new ACPI ID for the generic UART. So,
> > > update the HID of the UART.
> > > 
> > > 4) Enabled ACPI tables tests for RISC-V which were originally part of
> > > [2] but couldn't get merged due to updates required in the expected AML
> > > files. I think combining those patches with this series makes it easier
> > > to merge since expected AML files are updated.
> > > 
> > > [1] - https://github.com/riscv-non-isa/riscv-brs
> > > [2] - https://lists.gnu.org/archive/html/qemu-devel/2024-06/msg04734.html 
> > >  
> > 
> > btw: CI is not happy about series, see:
> >  https://gitlab.com/imammedo/qemu/-/pipelines/1371119552
> > also 'cross-i686-tci' job routinely timeouts on bios-tables-test
> > but we still keep adding more tests to it.
> > We should either bump timeout to account for slowness or
> > disable bios-tables-test for that job.  
> 
> Asumming the test is functionally correct, and not hanging, then bumping
> the timeout is the right answer. You can do this in the meson.build
> file

I think test is fine, since once in a while it passes (I guess it depends on 
runner host/load)

Overal job timeout is 1h, but that's not what fails.
What I see is, the test aborts after 10min timeout.
it's likely we hit boot_sector_test()/acpi_find_rsdp_address_uefi() timeout.
That's what we should try to bump.

PS:
I've just started the job with 5min bump, lets see if it is enough.

> We should never disable tests only in CI, because non-CI users
> are just as likely to hit timeouts.
> 
> 
> With regards,
> Daniel