Re: [PATCH 14/33] tests: acpi: update expected blobs

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 02 Mar 2023 09:55:24 -0300
Fabiano Rosas <farosas@suse.de> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Wed, 1 Mar 2023 19:27:33 -0500
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >  
> >> On Fri, Feb 24, 2023 at 04:37:53PM +0100, Igor Mammedov wrote:  
> >> > only following context change:
> >> >  -  Local1 = Zero
> >> >     If ((Arg0 != ToUUID ("e5c937d0-3553-4d7a-9117-ea4d19c3434d") /* Device Labeling Interface */))
> >> >     {
> >> >         Return (Local0)
> >> >  ...
> >> >         Return (Local0)
> >> >     }
> >> > 
> >> >  +  Local1 = Zero
> >> >     Local2 = AIDX (DerefOf (Arg4 [Zero]), DerefOf (Arg4 [One]
> >> > 
> >> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>    
> >> 
> >> Nope:
> >> 
> >> diff -ru -N -IDisassembly -IChecksum '-I*     Length           ' old/asl/tests/data/acpi/q35/DSDT.viot.dsl new/asl/tests/data/acpi/q35/DSDT.viot.dsl
> >> :--- old/asl/tests/data/acpi/q35/DSDT.viot.dsl  2023-03-01 19:22:57.636454958 -0500
> >> :+++ new/asl/tests/data/acpi/q35/DSDT.viot.dsl  2023-03-01 19:22:58.451460462 -0500
> >> :@@ -148,7 +148,6 @@
> >>                      {
> >>                           0x00                                             // .
> >>                      }
> >> -                Local1 = Zero
> >>                  If ((Arg0 != ToUUID ("e5c937d0-3553-4d7a-9117-ea4d19c3434d") /* Device Labeling Interface */))
> >>                  {
> >>                      Return (Local0)
> >> :@@ -159,12 +158,14 @@
> >>                      Return (Local0)
> >>                  }
> >>  
> >> +                Local1 = Zero
> >>                  Local2 = AIDX (DerefOf (Arg4 [Zero]), DerefOf (Arg4 [One]
> >>                      ))
> >>                  If (!((Local2 == Zero) | (Local2 == 0xFFFFFFFF)))
> >>                  {
> >>                      Local1 |= One
> >>                      Local1 |= (One << 0x07)
> >> +                    Local1 |= (One << 0x05)
> >>                  }
> >>  
> >>                  Local0 [Zero] = Local1
> >> 
> >> 
> >> and the funny thing is, the second change is in the expected
> >> file but not in the code so this patch causes the test to fail.  
> >
> > above diff is remnants of some old version of the patch before
> > patches were reshuffled.
> >
> > for some reason q35/viot test gets skipped => no table rebuild => stale DSDT.viot table
> >
> > reverting recently merged commit c471eb4f40, I can get q35/viot test back:
> >
> > commit c471eb4f40445908c1be7bb11a37ac676a0edae7
> > Author: Fabiano Rosas <farosas@suse.de>
> > Date:   Wed Feb 8 16:46:57 2023 -0300
> >
> >     tests/qtest: Check for devices in bios-tables-test
> >
> > so checks aren't working as expected on my host for some reason
> >  
> 
> Was this a legitimate skip or do we have a bug in the way we check for
> devices? I.e. did you have CONFIG_VIRTIO_IOMMU in the build?

config should be there since with your patch reverted viot test runs and passes just fine
so I'd assume it is a bug somewhere in check code

> If we avoid the checks during rebuild and a device is not present in the
> build anyway, then that QEMU invocation would crash.
I'd take crash anytime vs watching over for successful SKIP,
so we wouldn't end up with stale tables when they should be actually updated
like it happened in this case.