Re: [PATCH v2 02/13] tests/acpi: virt: allow acpi table changes for a new table: HEST

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Thu, 30 Jan 2025 15:38:30 +0100
Igor Mammedov <imammedo@redhat.com> escreveu:

> On Thu, 30 Jan 2025 14:03:24 +0100
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> 
> > Em Wed, 29 Jan 2025 16:03:28 +0100
> > Igor Mammedov <imammedo@redhat.com> escreveu:
> >   
> > > On Wed, 29 Jan 2025 09:04:08 +0100
> > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> > >     
> > > > The DSDT table will also be affected by such change.
> > > > 
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>      
> > > 
> > > move it right before the patch that would actually make changes to tables (10/13)    
> > 
> > Table changes happens on two patches:
> > 
> > - patch 03/13: acpi/ghes: add a firmware file with HEST address  
> 
> this one shouldn't affect bios tables test as it only checks ACPI and SMBIOS tables,
> and hest addr file is not either.

Heh, true.

> Do you really see test failing on this patch?

No. I just misunderstood the instructions, as it was not clear to me there 
that I shouldn't be adding there the HEST table.

> > 	HEST table was added here
> > 
> > - patch 10/13: arm/virt: Wire up a GED error device for ACPI / GHES 
> > 
> > 	DSDT changes happen here.
> > 
> > If the idea is to avoid make check to fail between those two patches,
> > we need either to split them on 4 patches (one before/one after each
> > change) or do like I did on this series: whitelist before patch 3,
> > update after patch 10.  
> 
> It would be better to group patches that should change ACPI tables
> close together so that a pair of whitelist/update could cover it.
> However it depends on how many changes are there, i.e. acpi diff
> should be digestible for a reader. So there is no hard border here,
> just use common sense.
> 
> However when the whitelist is covers all series where only few patches
> actually result in tables change, that miss-leads the reader since
> whitelist patch basically tells 'watch out for changes since this moment'
> and 'update' patch declares no more changes should happen.
> The same applies to bisection, where closer the gap between
> whitelist/update the better if the test case is the trigger.
> No need to be fanatical and do it around each patch,
> just make it observable (i.e. some small range of commits). 

Got it. Yeah, there was just one patch affecting DSDT table: the one
adding an AML representation for the GED notification device.

I fixed it for the next (hopefully the final) version.

Thanks,
Mauro