1. Patchew
  2. QEMU
  3. [PATCH 0/7] tests: Add test cases for TPM 1.2 ACPI tables
  4. View patch

[PATCH 3/7] tests: Add suffix tpm2 or tpm12 to ACPI table files

Stefan Berger posted 7 patches 4 years, 7 months ago
Maintainers: Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Igor Mammedov <imammedo@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH 3/7] tests: Add suffix tpm2 or tpm12 to ACPI table files
Posted by Stefan Berger 4 years, 7 months ago 
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} | Bin
 tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} | Bin
 tests/qtest/bios-tables-test.c                  |   3 ++-
 3 files changed, 2 insertions(+), 1 deletion(-)
 rename tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} (100%)
 rename tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} (100%)

diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis.tpm2
similarity index 100%
rename from tests/data/acpi/q35/DSDT.tis
rename to tests/data/acpi/q35/DSDT.tis.tpm2
diff --git a/tests/data/acpi/q35/TPM2.tis b/tests/data/acpi/q35/TPM2.tis.tpm2
similarity index 100%
rename from tests/data/acpi/q35/TPM2.tis
rename to tests/data/acpi/q35/TPM2.tis.tpm2
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 93c9d306b5..4ccbe56158 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1101,7 +1101,8 @@ static void test_acpi_tcg_tpm(const char *machine, const char *tpm_if,
     TPMTestState test;
     test_data data;
     GThread *thread;
-    char *args, *variant = g_strdup_printf(".%s", tpm_if);
+    const char *suffix = tpm_version == TPM_VERSION_2_0 ? "tpm2" : "tpm12";
+    char *args, *variant = g_strdup_printf(".%s.%s", tpm_if, suffix);
 
     tpm_tis_base_addr = base;
 
-- 
2.31.1
Re: [PATCH 3/7] tests: Add suffix tpm2 or tpm12 to ACPI table files
Posted by Igor Mammedov 4 years, 7 months ago 
On Wed, 30 Jun 2021 11:37:19 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} | Bin
>  tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} | Bin
>  tests/qtest/bios-tables-test.c                  |   3 ++-
>  3 files changed, 2 insertions(+), 1 deletion(-)
>  rename tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} (100%)
>  rename tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} (100%)

it fails checkpatch which falsely detects it as adding new files

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found

looks like checkpatch needs to be fixed to handle testcase variant renaming.


> diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis.tpm2
> similarity index 100%
> rename from tests/data/acpi/q35/DSDT.tis
> rename to tests/data/acpi/q35/DSDT.tis.tpm2
> diff --git a/tests/data/acpi/q35/TPM2.tis b/tests/data/acpi/q35/TPM2.tis.tpm2
> similarity index 100%
> rename from tests/data/acpi/q35/TPM2.tis
> rename to tests/data/acpi/q35/TPM2.tis.tpm2
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 93c9d306b5..4ccbe56158 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1101,7 +1101,8 @@ static void test_acpi_tcg_tpm(const char *machine, const char *tpm_if,
>      TPMTestState test;
>      test_data data;
>      GThread *thread;
> -    char *args, *variant = g_strdup_printf(".%s", tpm_if);
> +    const char *suffix = tpm_version == TPM_VERSION_2_0 ? "tpm2" : "tpm12";
> +    char *args, *variant = g_strdup_printf(".%s.%s", tpm_if, suffix);
>  
>      tpm_tis_base_addr = base;
>
Re: [PATCH 3/7] tests: Add suffix tpm2 or tpm12 to ACPI table files
Posted by Stefan Berger 4 years, 7 months ago 
On 7/8/21 9:59 AM, Igor Mammedov wrote:
> On Wed, 30 Jun 2021 11:37:19 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} | Bin
>>   tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} | Bin
>>   tests/qtest/bios-tables-test.c                  |   3 ++-
>>   3 files changed, 2 insertions(+), 1 deletion(-)
>>   rename tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} (100%)
>>   rename tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} (100%)
> it fails checkpatch which falsely detects it as adding new files
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found
>
> looks like checkpatch needs to be fixed to handle testcase variant renaming.

Is this a show stopper for upstreaming it?
Re: [PATCH 3/7] tests: Add suffix tpm2 or tpm12 to ACPI table files
Posted by Igor Mammedov 4 years, 7 months ago 
On Thu, 8 Jul 2021 10:17:51 -0400
Stefan Berger <stefanb@linux.ibm.com> wrote:

> On 7/8/21 9:59 AM, Igor Mammedov wrote:
> > On Wed, 30 Jun 2021 11:37:19 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >  
> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> >> ---
> >>   tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} | Bin
> >>   tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} | Bin
> >>   tests/qtest/bios-tables-test.c                  |   3 ++-
> >>   3 files changed, 2 insertions(+), 1 deletion(-)
> >>   rename tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} (100%)
> >>   rename tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} (100%)  
> > it fails checkpatch which falsely detects it as adding new files
> >
> > ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found
> >
> > ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found
> >
> > looks like checkpatch needs to be fixed to handle testcase variant renaming.  
> 
> Is this a show stopper for upstreaming it?
patch itself looks fine to me but I'm not sure if it's possible CCing Michael/Peter.
Re: [PATCH 3/7] tests: Add suffix tpm2 or tpm12 to ACPI table files
Posted by Michael S. Tsirkin 4 years, 7 months ago 
On Thu, Jul 08, 2021 at 10:17:51AM -0400, Stefan Berger wrote:
> 
> On 7/8/21 9:59 AM, Igor Mammedov wrote:
> > On Wed, 30 Jun 2021 11:37:19 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> > 
> > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > > ---
> > >   tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} | Bin
> > >   tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} | Bin
> > >   tests/qtest/bios-tables-test.c                  |   3 ++-
> > >   3 files changed, 2 insertions(+), 1 deletion(-)
> > >   rename tests/data/acpi/q35/{DSDT.tis => DSDT.tis.tpm2} (100%)
> > >   rename tests/data/acpi/q35/{TPM2.tis => TPM2.tis.tpm2} (100%)
> > it fails checkpatch which falsely detects it as adding new files
> > 
> > ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found
> > 
> > ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/TPM2.tis.tpm2 and tests/qtest/bios-tables-test.c found
> > 
> > looks like checkpatch needs to be fixed to handle testcase variant renaming.
> 
> Is this a show stopper for upstreaming it?

It is. The message is wrong (we can teach checkpatch to say
do not add or rename) but the point is correct: don't touch
binary files and code in the same patch because such binary change
patches can't be cherry-picked rebased etc.

So pls follow the usual rules:
- allow diff
- change code
- change expected binary
- disallow diff

-- 
MST

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.