diff mbox series

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

Message ID 1390b46682f2bac3587239d03a0ba22d18a9a044.1738137123.git.mchehab+huawei@kernel.org (mailing list archive)
State New
Headers show
Series [v2,01/13] acpi/ghes: Prepare to support multiple sources on ghes | expand

Commit Message

Mauro Carvalho Chehab Jan. 29, 2025, 8:04 a.m. UTC 
The DSDT table will also be affected by such change.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 tests/data/acpi/aarch64/virt/HEST           | 0
 tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
 2 files changed, 2 insertions(+)
 create mode 100644 tests/data/acpi/aarch64/virt/HEST

Comments

Igor Mammedov Jan. 29, 2025, 3:03 p.m. UTC | #1 
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)


> ---
>  tests/data/acpi/aarch64/virt/HEST           | 0
>  tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
>  2 files changed, 2 insertions(+)
>  create mode 100644 tests/data/acpi/aarch64/virt/HEST
> 
> diff --git a/tests/data/acpi/aarch64/virt/HEST b/tests/data/acpi/aarch64/virt/HEST
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8bf4..46298e38e7b8 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,3 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/aarch64/virt/HEST",
> +"tests/data/acpi/aarch64/virt/DSDT",

the list in not complete so 'make check-qtest' still fails
[12/13] has complete list of changed files
Mauro Carvalho Chehab Jan. 30, 2025, 1:03 p.m. UTC | #2 
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

	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.

Regards,
Mauro

> 
> 
> > ---
> >  tests/data/acpi/aarch64/virt/HEST           | 0
> >  tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
> >  2 files changed, 2 insertions(+)
> >  create mode 100644 tests/data/acpi/aarch64/virt/HEST
> > 
> > diff --git a/tests/data/acpi/aarch64/virt/HEST b/tests/data/acpi/aarch64/virt/HEST
> > new file mode 100644
> > index 000000000000..e69de29bb2d1
> > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> > index dfb8523c8bf4..46298e38e7b8 100644
> > --- a/tests/qtest/bios-tables-test-allowed-diff.h
> > +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> > @@ -1 +1,3 @@
> >  /* List of comma-separated changed AML files to ignore */
> > +"tests/data/acpi/aarch64/virt/HEST",
> > +"tests/data/acpi/aarch64/virt/DSDT",  
> 
> the list in not complete so 'make check-qtest' still fails
> [12/13] has complete list of changed files
> 



Thanks,
Mauro
Igor Mammedov Jan. 30, 2025, 2:38 p.m. UTC | #3 
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.

Do you really see test failing on this patch?


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

  
> 
> Regards,
> Mauro
> 
> > 
> >   
> > > ---
> > >  tests/data/acpi/aarch64/virt/HEST           | 0
> > >  tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
> > >  2 files changed, 2 insertions(+)
> > >  create mode 100644 tests/data/acpi/aarch64/virt/HEST
> > > 
> > > diff --git a/tests/data/acpi/aarch64/virt/HEST b/tests/data/acpi/aarch64/virt/HEST
> > > new file mode 100644
> > > index 000000000000..e69de29bb2d1
> > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> > > index dfb8523c8bf4..46298e38e7b8 100644
> > > --- a/tests/qtest/bios-tables-test-allowed-diff.h
> > > +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> > > @@ -1 +1,3 @@
> > >  /* List of comma-separated changed AML files to ignore */
> > > +"tests/data/acpi/aarch64/virt/HEST",
> > > +"tests/data/acpi/aarch64/virt/DSDT",    
> > 
> > the list in not complete so 'make check-qtest' still fails
> > [12/13] has complete list of changed files
> >   
> 
> 
> 
> Thanks,
> Mauro
>
diff mbox series

Patch

diff --git a/tests/data/acpi/aarch64/virt/HEST b/tests/data/acpi/aarch64/virt/HEST
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf4..46298e38e7b8 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@ 
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/aarch64/virt/HEST",
+"tests/data/acpi/aarch64/virt/DSDT",