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

[Igor Mammedov]

On Tue, 16 Jul 2024 16:28:07 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Tue, 16 Jul 2024 17:56:11 +0530
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > On Mon, Jul 15, 2024 at 02:43:52PM +0200, Igor Mammedov wrote:  
> > > On Sun, 14 Jul 2024 03:46:36 -0400
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >     
> > > > On Fri, Jul 12, 2024 at 03:50:10PM +0200, Igor Mammedov wrote:    
> > > > > 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.   
> > > > >    
> > > > 
> > > > Because we should wait for 5min CPU time, not wall time.
> > > > Why don't we do that?
> > > > Something like getrusage should work I think.
> > > >     
> > > 
> > > It turned out to be a meson timeout that's set individually per test file.
> > > I'll send a patch later on.
> > >     
> > Hi Igor,
> > 
> > I am unable to get msys2-64bit test in CI to pass. I tried including
> > your change in meson as well but no luck. I can't guess how enabling
> > bios-tables-test for RISC-V is affecting this particular test. Does this
> > pass for you? 
> > 
> > https://gitlab.com/vlsunil/qemu/-/jobs/7343701148  
> 
> it doesn't pass for me either,
> but bios-tables-test is not among those that timed out,
> so I'd ignore failure in this case

as in your case it was sparc target tests that timed out:
https://gitlab.com/imammedo/qemu/-/jobs/7352989984

CCIng sparc folks as well

> 
> > 
> > Thanks!
> > Sunil
> >   
>