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

[Fabiano Rosas]

Igor Mammedov <imammedo@redhat.com> writes:

> 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
>

Probably easier to just revert the patch then. We have stopped building
bios-tables-test.c when CONFIG_PARALLEL is not present, so the build
--without-default-devices is already covered.

We would only need those checks if PCI_DEVICES=n or one of the
individual CONFIGs, which we have discussed previously that is not worth
the effort to keep track of.