* [PATCH v5 0/3] tpm: Add missing ACPI device identification objects
@ 2022-01-04 17:58 Stefan Berger
2022-01-04 17:58 ` [PATCH v5 1/3] tests: acpi: prepare for updated TPM related tables Stefan Berger
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Stefan Berger @ 2022-01-04 17:58 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, Stefan Berger
This series of patches adds missing ACPI device identification objects _STR
and _UID to TPM 1.2 and TPM 2 ACPI tables.
Stefan
v5:
- Changed Fixes: tag to Resolves:
- Mentioned in 2/3 that choice of uid '1' is based on inspection of hardware
TPMs' sysfs entries
v4:
- Rebased on current master: regenerated binaries in patch 3
v3:
- Dropped replacement of ACPI tables with empty files in 1/3.
- Reduced ignored files
Stefan Berger (3):
tests: acpi: prepare for updated TPM related tables
acpi: tpm: Add missing device identification objects
tests: acpi: Add updated TPM related tables
hw/arm/virt-acpi-build.c | 1 +
hw/i386/acpi-build.c | 7 +++++++
tests/data/acpi/q35/DSDT.tis.tpm12 | Bin 8894 -> 8900 bytes
tests/data/acpi/q35/DSDT.tis.tpm2 | Bin 8894 -> 8921 bytes
4 files changed, 8 insertions(+)
--
2.31.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 1/3] tests: acpi: prepare for updated TPM related tables
2022-01-04 17:58 [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Stefan Berger
@ 2022-01-04 17:58 ` Stefan Berger
2022-01-06 16:56 ` Igor Mammedov
2022-01-04 17:58 ` [PATCH v5 2/3] acpi: tpm: Add missing device identification objects Stefan Berger
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Stefan Berger @ 2022-01-04 17:58 UTC (permalink / raw)
To: qemu-devel
Cc: Ani Sinha, marcandre.lureau, Igor Mammedov, Michael S . Tsirkin,
Stefan Berger
Replace existing TPM related tables, that are about to change, with
empty files.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <ani@anisinha.ca>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Message-id: 20211223022310.575496-2-stefanb@linux.ibm.com
---
tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..5d80e408d4 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/q35/DSDT.tis.tpm12",
+"tests/data/acpi/q35/DSDT.tis.tpm2",
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-04 17:58 [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Stefan Berger
2022-01-04 17:58 ` [PATCH v5 1/3] tests: acpi: prepare for updated TPM related tables Stefan Berger
@ 2022-01-04 17:58 ` Stefan Berger
2022-01-06 8:36 ` Igor Mammedov
2022-01-04 17:58 ` [PATCH v5 3/3] tests: acpi: Add updated TPM related tables Stefan Berger
2022-01-04 18:09 ` [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Daniel P. Berrangé
3 siblings, 1 reply; 15+ messages in thread
From: Stefan Berger @ 2022-01-04 17:58 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S . Tsirkin, Shannon Zhao, Igor Mammedov, Ani Sinha,
marcandre.lureau, Stefan Berger
Add missing TPM device identification objects _STR and _UID. They will
appear as files 'description' and 'uid' under Linux sysfs.
Following inspection of sysfs entries for hardware TPMs we chose
uid '1'.
Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <ani@anisinha.ca>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/708
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Shannon Zhao <shannon.zhaosl@gmail.com>
Message-id: 20211223022310.575496-3-stefanb@linux.ibm.com
---
hw/arm/virt-acpi-build.c | 1 +
hw/i386/acpi-build.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index d0f4867fdf..f2514ce77c 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -229,6 +229,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
Aml *dev = aml_device("TPM0");
aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
+ aml_append(dev, aml_name_decl("_STR", aml_string("TPM 2.0 Device")));
aml_append(dev, aml_name_decl("_UID", aml_int(0)));
Aml *crs = aml_resource_template();
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 8383b83ee3..05740b7f15 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1812,11 +1812,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
dev = aml_device("TPM");
aml_append(dev, aml_name_decl("_HID",
aml_string("MSFT0101")));
+ aml_append(dev,
+ aml_name_decl("_STR",
+ aml_string("TPM 2.0 Device")));
} else {
dev = aml_device("ISA.TPM");
aml_append(dev, aml_name_decl("_HID",
aml_eisaid("PNP0C31")));
}
+ aml_append(dev, aml_name_decl("_UID", aml_int(1)));
aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
crs = aml_resource_template();
@@ -1844,12 +1848,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
if (TPM_IS_CRB(tpm)) {
dev = aml_device("TPM");
aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
+ aml_append(dev, aml_name_decl("_STR",
+ aml_string("TPM 2.0 Device")));
crs = aml_resource_template();
aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE,
TPM_CRB_ADDR_SIZE, AML_READ_WRITE));
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
+ aml_append(dev, aml_name_decl("_UID", aml_int(1)));
tpm_build_ppi_acpi(tpm, dev);
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 3/3] tests: acpi: Add updated TPM related tables
2022-01-04 17:58 [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Stefan Berger
2022-01-04 17:58 ` [PATCH v5 1/3] tests: acpi: prepare for updated TPM related tables Stefan Berger
2022-01-04 17:58 ` [PATCH v5 2/3] acpi: tpm: Add missing device identification objects Stefan Berger
@ 2022-01-04 17:58 ` Stefan Berger
2022-01-04 18:09 ` [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Daniel P. Berrangé
3 siblings, 0 replies; 15+ messages in thread
From: Stefan Berger @ 2022-01-04 17:58 UTC (permalink / raw)
To: qemu-devel
Cc: Ani Sinha, marcandre.lureau, Igor Mammedov, Michael S . Tsirkin,
Stefan Berger
The updated TPM related tables have the following additions:
Device (TPM)
{
Name (_HID, "MSFT0101" /* TPM 2.0 Security Device */) // _HID: Hardware ID
+ Name (_STR, "TPM 2.0 Device") // _STR: Description String
+ Name (_UID, One) // _UID: Unique ID
Name (_STA, 0x0F) // _STA: Status
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <ani@anisinha.ca>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Message-id: 20211223022310.575496-4-stefanb@linux.ibm.com
---
tests/data/acpi/q35/DSDT.tis.tpm12 | Bin 8894 -> 8900 bytes
tests/data/acpi/q35/DSDT.tis.tpm2 | Bin 8894 -> 8921 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 2 --
3 files changed, 2 deletions(-)
diff --git a/tests/data/acpi/q35/DSDT.tis.tpm12 b/tests/data/acpi/q35/DSDT.tis.tpm12
index 0ebdf6fbd77967f1ab5d5337b7b1fed314cfaca8..fb9dd1f0599afd6b555ea570ecd00a3bb227aa84 100644
GIT binary patch
delta 50
zcmdnzdc>8>CD<k8h!O(><KvB7q6(a@S~2m#PVoZ1lQk6FnOs#T7b=LdgnGI#Zf;Sq
GVgdkr91X<)
delta 45
zcmX@&y3du%CD<iopArKDqxwcJQ3Xza&6xOLr+5MP$r=joO#Uj93l&5+_b6B}0RSYz
B3@!iw
diff --git a/tests/data/acpi/q35/DSDT.tis.tpm2 b/tests/data/acpi/q35/DSDT.tis.tpm2
index dcbb7f0af377425db53130e8ba1c62c09c22e006..00d732e46f5d9d056e557bd026fa30f9db3b8c30 100644
GIT binary patch
delta 70
zcmdnzdefE5CD<k8rV;}KBgaNAQ3Wn9?U?vrr+5J;?a7)7ZcJWklM5BZ#e;Z50(=#W
a^b8bSQp+-vQyDnoLp@y>H@7HQF#!OXcoHoD
delta 46
zcmccVy3du%CD<iopArKD<D-pSq6%F8nlbUgPVoZnnv*pZ+?f1TCKoD*Z(gim#smOL
C=M6sq
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 5d80e408d4..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.tis.tpm12",
-"tests/data/acpi/q35/DSDT.tis.tpm2",
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v5 0/3] tpm: Add missing ACPI device identification objects
2022-01-04 17:58 [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Stefan Berger
` (2 preceding siblings ...)
2022-01-04 17:58 ` [PATCH v5 3/3] tests: acpi: Add updated TPM related tables Stefan Berger
@ 2022-01-04 18:09 ` Daniel P. Berrangé
2022-01-04 18:14 ` Stefan Berger
3 siblings, 1 reply; 15+ messages in thread
From: Daniel P. Berrangé @ 2022-01-04 18:09 UTC (permalink / raw)
To: Stefan Berger; +Cc: marcandre.lureau, qemu-devel
On Tue, Jan 04, 2022 at 12:58:03PM -0500, Stefan Berger wrote:
> This series of patches adds missing ACPI device identification objects _STR
> and _UID to TPM 1.2 and TPM 2 ACPI tables.
What was the practical impact on guests (if any) from these ACPI
objects being missing ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 0/3] tpm: Add missing ACPI device identification objects
2022-01-04 18:09 ` [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Daniel P. Berrangé
@ 2022-01-04 18:14 ` Stefan Berger
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Berger @ 2022-01-04 18:14 UTC (permalink / raw)
To: Daniel P. Berrangé, Thomas Huth; +Cc: marcandre.lureau, qemu-devel
On 1/4/22 13:09, Daniel P. Berrangé wrote:
> On Tue, Jan 04, 2022 at 12:58:03PM -0500, Stefan Berger wrote:
>> This series of patches adds missing ACPI device identification objects _STR
>> and _UID to TPM 1.2 and TPM 2 ACPI tables.
> What was the practical impact on guests (if any) from these ACPI
> objects being missing ?
As discussed in v4: I don't know if there's software relying on this or
just a user who found these to be missing:
https://gitlab.com/qemu-project/qemu/-/issues/708
I can only say that these entries were missing, so I added them. It
makes TPM ACPI more complete, for sure. Maybe someone else knows more if
there's something out there that actually needs this. I'd be curious as
well.
Stefan
>
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-04 17:58 ` [PATCH v5 2/3] acpi: tpm: Add missing device identification objects Stefan Berger
@ 2022-01-06 8:36 ` Igor Mammedov
2022-01-06 11:40 ` Michael S. Tsirkin
2022-01-06 13:53 ` Stefan Berger
0 siblings, 2 replies; 15+ messages in thread
From: Igor Mammedov @ 2022-01-06 8:36 UTC (permalink / raw)
To: Stefan Berger
Cc: Ani Sinha, marcandre.lureau, Michael S . Tsirkin, qemu-devel,
Shannon Zhao
On Tue, 4 Jan 2022 12:58:05 -0500
Stefan Berger <stefanb@linux.ibm.com> wrote:
> Add missing TPM device identification objects _STR and _UID. They will
> appear as files 'description' and 'uid' under Linux sysfs.
>
> Following inspection of sysfs entries for hardware TPMs we chose
> uid '1'.
My guess would be that buy default (in case of missing UID), OSPM
will start enumerate from 0. So I think 0 is more safer choice
when it comes to compatibility.
Can you smoke test TPM with Windows, and check if adding UID doesn't
break anything if VM actually uses TMP (though I'm not sure how to
check it on Windows, maybe install Windows 11 without this patch
and then see if it still boots pre-installed VM and nothing is broken
after this patch)?
> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <ani@anisinha.ca>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/708
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Ani Sinha <ani@anisinha.ca>
> Reviewed-by: Shannon Zhao <shannon.zhaosl@gmail.com>
> Message-id: 20211223022310.575496-3-stefanb@linux.ibm.com
> ---
> hw/arm/virt-acpi-build.c | 1 +
> hw/i386/acpi-build.c | 7 +++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index d0f4867fdf..f2514ce77c 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -229,6 +229,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
>
> Aml *dev = aml_device("TPM0");
> aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
> + aml_append(dev, aml_name_decl("_STR", aml_string("TPM 2.0 Device")));
> aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>
> Aml *crs = aml_resource_template();
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 8383b83ee3..05740b7f15 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1812,11 +1812,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> dev = aml_device("TPM");
> aml_append(dev, aml_name_decl("_HID",
> aml_string("MSFT0101")));
> + aml_append(dev,
> + aml_name_decl("_STR",
> + aml_string("TPM 2.0 Device")));
> } else {
> dev = aml_device("ISA.TPM");
> aml_append(dev, aml_name_decl("_HID",
> aml_eisaid("PNP0C31")));
> }
> + aml_append(dev, aml_name_decl("_UID", aml_int(1)));
>
> aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> crs = aml_resource_template();
> @@ -1844,12 +1848,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> if (TPM_IS_CRB(tpm)) {
> dev = aml_device("TPM");
> aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
> + aml_append(dev, aml_name_decl("_STR",
> + aml_string("TPM 2.0 Device")));
> crs = aml_resource_template();
> aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE,
> TPM_CRB_ADDR_SIZE, AML_READ_WRITE));
> aml_append(dev, aml_name_decl("_CRS", crs));
>
> aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> + aml_append(dev, aml_name_decl("_UID", aml_int(1)));
>
> tpm_build_ppi_acpi(tpm, dev);
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-06 8:36 ` Igor Mammedov
@ 2022-01-06 11:40 ` Michael S. Tsirkin
2022-01-06 13:53 ` Stefan Berger
1 sibling, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2022-01-06 11:40 UTC (permalink / raw)
To: Igor Mammedov
Cc: Ani Sinha, marcandre.lureau, Shannon Zhao, qemu-devel, Stefan Berger
On Thu, Jan 06, 2022 at 09:36:36AM +0100, Igor Mammedov wrote:
> On Tue, 4 Jan 2022 12:58:05 -0500
> Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> > Add missing TPM device identification objects _STR and _UID. They will
> > appear as files 'description' and 'uid' under Linux sysfs.
> >
> > Following inspection of sysfs entries for hardware TPMs we chose
> > uid '1'.
>
> My guess would be that buy default (in case of missing UID), OSPM
> will start enumerate from 0. So I think 0 is more safer choice
> when it comes to compatibility.
>
> Can you smoke test TPM with Windows, and check if adding UID doesn't
> break anything if VM actually uses TMP (though I'm not sure how to
> check it on Windows, maybe install Windows 11 without this patch
> and then see if it still boots pre-installed VM and nothing is broken
> after this patch)?
Given out experience with these things, I would add compat
machinery and avoid changing things for existing machine types.
Should be sufficient to address these concerns right Igor?
>
> > Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Igor Mammedov <imammedo@redhat.com>
> > Cc: Ani Sinha <ani@anisinha.ca>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/708
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > Reviewed-by: Ani Sinha <ani@anisinha.ca>
> > Reviewed-by: Shannon Zhao <shannon.zhaosl@gmail.com>
> > Message-id: 20211223022310.575496-3-stefanb@linux.ibm.com
> > ---
> > hw/arm/virt-acpi-build.c | 1 +
> > hw/i386/acpi-build.c | 7 +++++++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index d0f4867fdf..f2514ce77c 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -229,6 +229,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
> >
> > Aml *dev = aml_device("TPM0");
> > aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
> > + aml_append(dev, aml_name_decl("_STR", aml_string("TPM 2.0 Device")));
> > aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> >
> > Aml *crs = aml_resource_template();
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 8383b83ee3..05740b7f15 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1812,11 +1812,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > dev = aml_device("TPM");
> > aml_append(dev, aml_name_decl("_HID",
> > aml_string("MSFT0101")));
> > + aml_append(dev,
> > + aml_name_decl("_STR",
> > + aml_string("TPM 2.0 Device")));
> > } else {
> > dev = aml_device("ISA.TPM");
> > aml_append(dev, aml_name_decl("_HID",
> > aml_eisaid("PNP0C31")));
> > }
> > + aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> >
> > aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> > crs = aml_resource_template();
> > @@ -1844,12 +1848,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > if (TPM_IS_CRB(tpm)) {
> > dev = aml_device("TPM");
> > aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
> > + aml_append(dev, aml_name_decl("_STR",
> > + aml_string("TPM 2.0 Device")));
> > crs = aml_resource_template();
> > aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE,
> > TPM_CRB_ADDR_SIZE, AML_READ_WRITE));
> > aml_append(dev, aml_name_decl("_CRS", crs));
> >
> > aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> > + aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> >
> > tpm_build_ppi_acpi(tpm, dev);
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-06 8:36 ` Igor Mammedov
2022-01-06 11:40 ` Michael S. Tsirkin
@ 2022-01-06 13:53 ` Stefan Berger
2022-01-06 13:56 ` Michael S. Tsirkin
1 sibling, 1 reply; 15+ messages in thread
From: Stefan Berger @ 2022-01-06 13:53 UTC (permalink / raw)
To: Igor Mammedov
Cc: Ani Sinha, marcandre.lureau, Michael S . Tsirkin, qemu-devel,
Shannon Zhao
On 1/6/22 03:36, Igor Mammedov wrote:
> On Tue, 4 Jan 2022 12:58:05 -0500
> Stefan Berger <stefanb@linux.ibm.com> wrote:
>
>> Add missing TPM device identification objects _STR and _UID. They will
>> appear as files 'description' and 'uid' under Linux sysfs.
>>
>> Following inspection of sysfs entries for hardware TPMs we chose
>> uid '1'.
> My guess would be that buy default (in case of missing UID), OSPM
> will start enumerate from 0. So I think 0 is more safer choice
> when it comes to compatibility.
>
> Can you smoke test TPM with Windows, and check if adding UID doesn't
> break anything if VM actually uses TMP (though I'm not sure how to
> check it on Windows, maybe install Windows 11 without this patch
> and then see if it still boots pre-installed VM and nothing is broken
> after this patch)?
>
I smoke tested it with the posted patches applied to v6.2.0 and started
3 VMs with it:
- Linux shows uid = 1 and the description "TPM 2.0 Device" in sysfs
- Win 10 and Win 11 tpm.msc tool are both showing that the TPM is 'ready
for use'
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-06 13:53 ` Stefan Berger
@ 2022-01-06 13:56 ` Michael S. Tsirkin
2022-01-06 14:01 ` Stefan Berger
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2022-01-06 13:56 UTC (permalink / raw)
To: Stefan Berger
Cc: Ani Sinha, Igor Mammedov, Shannon Zhao, qemu-devel, marcandre.lureau
On Thu, Jan 06, 2022 at 08:53:00AM -0500, Stefan Berger wrote:
>
> On 1/6/22 03:36, Igor Mammedov wrote:
> > On Tue, 4 Jan 2022 12:58:05 -0500
> > Stefan Berger <stefanb@linux.ibm.com> wrote:
> >
> > > Add missing TPM device identification objects _STR and _UID. They will
> > > appear as files 'description' and 'uid' under Linux sysfs.
> > >
> > > Following inspection of sysfs entries for hardware TPMs we chose
> > > uid '1'.
> > My guess would be that buy default (in case of missing UID), OSPM
> > will start enumerate from 0. So I think 0 is more safer choice
> > when it comes to compatibility.
> >
> > Can you smoke test TPM with Windows, and check if adding UID doesn't
> > break anything if VM actually uses TMP (though I'm not sure how to
> > check it on Windows, maybe install Windows 11 without this patch
> > and then see if it still boots pre-installed VM and nothing is broken
> > after this patch)?
> >
> I smoke tested it with the posted patches applied to v6.2.0 and started 3
> VMs with it:
>
> - Linux shows uid = 1 and the description "TPM 2.0 Device" in sysfs
>
> - Win 10 and Win 11 tpm.msc tool are both showing that the TPM is 'ready for
> use'
>
> Stefan
>
Just to make sure, what Igor was concerned about is issues like
we had with e.g. network devices, when changing UID makes
windows think it's a new device and lose configuration
created on old qemu on boot with a new qemu.
Not sure what can be configured with a TPM device though ...
--
MST
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-06 13:56 ` Michael S. Tsirkin
@ 2022-01-06 14:01 ` Stefan Berger
2022-01-06 16:55 ` Igor Mammedov
2022-01-06 17:38 ` Ani Sinha
0 siblings, 2 replies; 15+ messages in thread
From: Stefan Berger @ 2022-01-06 14:01 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Ani Sinha, Igor Mammedov, Shannon Zhao, qemu-devel, marcandre.lureau
On 1/6/22 08:56, Michael S. Tsirkin wrote:
> On Thu, Jan 06, 2022 at 08:53:00AM -0500, Stefan Berger wrote:
>> On 1/6/22 03:36, Igor Mammedov wrote:
>>> On Tue, 4 Jan 2022 12:58:05 -0500
>>> Stefan Berger <stefanb@linux.ibm.com> wrote:
>>>
>>>> Add missing TPM device identification objects _STR and _UID. They will
>>>> appear as files 'description' and 'uid' under Linux sysfs.
>>>>
>>>> Following inspection of sysfs entries for hardware TPMs we chose
>>>> uid '1'.
>>> My guess would be that buy default (in case of missing UID), OSPM
>>> will start enumerate from 0. So I think 0 is more safer choice
>>> when it comes to compatibility.
>>>
>>> Can you smoke test TPM with Windows, and check if adding UID doesn't
>>> break anything if VM actually uses TMP (though I'm not sure how to
>>> check it on Windows, maybe install Windows 11 without this patch
>>> and then see if it still boots pre-installed VM and nothing is broken
>>> after this patch)?
>>>
>> I smoke tested it with the posted patches applied to v6.2.0 and started 3
>> VMs with it:
>>
>> - Linux shows uid = 1 and the description "TPM 2.0 Device" in sysfs
>>
>> - Win 10 and Win 11 tpm.msc tool are both showing that the TPM is 'ready for
>> use'
>>
>> Stefan
>>
> Just to make sure, what Igor was concerned about is issues like
> we had with e.g. network devices, when changing UID makes
> windows think it's a new device and lose configuration
> created on old qemu on boot with a new qemu.
> Not sure what can be configured with a TPM device though ...
The VMs were all created on an old qemu and booted into the patched
qemu. They hadn't seen the new ACPI entries before, for sure not when
they were installed.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-06 14:01 ` Stefan Berger
@ 2022-01-06 16:55 ` Igor Mammedov
2022-01-06 18:07 ` Stefan Berger
2022-01-06 17:38 ` Ani Sinha
1 sibling, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2022-01-06 16:55 UTC (permalink / raw)
To: Stefan Berger
Cc: Ani Sinha, marcandre.lureau, Shannon Zhao, qemu-devel,
Michael S. Tsirkin
On Thu, 6 Jan 2022 09:01:36 -0500
Stefan Berger <stefanb@linux.ibm.com> wrote:
> On 1/6/22 08:56, Michael S. Tsirkin wrote:
> > On Thu, Jan 06, 2022 at 08:53:00AM -0500, Stefan Berger wrote:
> >> On 1/6/22 03:36, Igor Mammedov wrote:
> >>> On Tue, 4 Jan 2022 12:58:05 -0500
> >>> Stefan Berger <stefanb@linux.ibm.com> wrote:
> >>>
> >>>> Add missing TPM device identification objects _STR and _UID. They will
> >>>> appear as files 'description' and 'uid' under Linux sysfs.
> >>>>
> >>>> Following inspection of sysfs entries for hardware TPMs we chose
> >>>> uid '1'.
> >>> My guess would be that buy default (in case of missing UID), OSPM
> >>> will start enumerate from 0. So I think 0 is more safer choice
> >>> when it comes to compatibility.
> >>>
> >>> Can you smoke test TPM with Windows, and check if adding UID doesn't
> >>> break anything if VM actually uses TMP (though I'm not sure how to
> >>> check it on Windows, maybe install Windows 11 without this patch
> >>> and then see if it still boots pre-installed VM and nothing is broken
> >>> after this patch)?
> >>>
> >> I smoke tested it with the posted patches applied to v6.2.0 and started 3
> >> VMs with it:
> >>
> >> - Linux shows uid = 1 and the description "TPM 2.0 Device" in sysfs
> >>
> >> - Win 10 and Win 11 tpm.msc tool are both showing that the TPM is 'ready for
> >> use'
> >>
> >> Stefan
> >>
> > Just to make sure, what Igor was concerned about is issues like
> > we had with e.g. network devices, when changing UID makes
> > windows think it's a new device and lose configuration
> > created on old qemu on boot with a new qemu.
> > Not sure what can be configured with a TPM device though ...
>
> The VMs were all created on an old qemu and booted into the patched
> qemu. They hadn't seen the new ACPI entries before, for sure not when
> they were installed.
In that case I would not bother with compat machinery
(my stance on APCI and compat knobs haven't changed and it
is avoid it if possible, sometimes that backfires but overall
keeps code simpler, otherwise it would be unreadable mess
(it's already complex enough))
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/3] tests: acpi: prepare for updated TPM related tables
2022-01-04 17:58 ` [PATCH v5 1/3] tests: acpi: prepare for updated TPM related tables Stefan Berger
@ 2022-01-06 16:56 ` Igor Mammedov
0 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2022-01-06 16:56 UTC (permalink / raw)
To: Stefan Berger
Cc: Ani Sinha, marcandre.lureau, qemu-devel, Michael S . Tsirkin
On Tue, 4 Jan 2022 12:58:04 -0500
Stefan Berger <stefanb@linux.ibm.com> wrote:
> Replace existing TPM related tables, that are about to change, with
> empty files.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <ani@anisinha.ca>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Acked-by: Ani Sinha <ani@anisinha.ca>
> Message-id: 20211223022310.575496-2-stefanb@linux.ibm.com
Acked-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..5d80e408d4 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/q35/DSDT.tis.tpm12",
> +"tests/data/acpi/q35/DSDT.tis.tpm2",
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-06 14:01 ` Stefan Berger
2022-01-06 16:55 ` Igor Mammedov
@ 2022-01-06 17:38 ` Ani Sinha
1 sibling, 0 replies; 15+ messages in thread
From: Ani Sinha @ 2022-01-06 17:38 UTC (permalink / raw)
To: Stefan Berger
Cc: Igor Mammedov, Shannon Zhao, qemu-devel, marcandre.lureau,
Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 610 bytes --]
On Thu, Jan 6, 2022 at 19:31 Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> >>> Can you smoke test TPM with Windows, and check if adding UID doesn't
> >>> break anything if VM actually uses TMP (though I'm not sure how to
> >>> check it on Windows, maybe install Windows 11 without this patch
> >>> and then see if it still boots pre-installed VM and nothing is broken
> >>> after this patch)?
>
>
> The VMs were all created on an old qemu and booted into the patched
> qemu.
Stupid question - should we also check the other way as well? Install on
patches qemu and try to boot on the old unpatched qemu?
[-- Attachment #2: Type: text/html, Size: 1074 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects
2022-01-06 16:55 ` Igor Mammedov
@ 2022-01-06 18:07 ` Stefan Berger
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Berger @ 2022-01-06 18:07 UTC (permalink / raw)
To: Igor Mammedov
Cc: Ani Sinha, marcandre.lureau, Shannon Zhao, qemu-devel,
Michael S. Tsirkin
On 1/6/22 11:55, Igor Mammedov wrote:
> On Thu, 6 Jan 2022 09:01:36 -0500
> Stefan Berger <stefanb@linux.ibm.com> wrote:
>
>> On 1/6/22 08:56, Michael S. Tsirkin wrote:
>>> On Thu, Jan 06, 2022 at 08:53:00AM -0500, Stefan Berger wrote:
>>>> On 1/6/22 03:36, Igor Mammedov wrote:
>>>>> On Tue, 4 Jan 2022 12:58:05 -0500
>>>>> Stefan Berger <stefanb@linux.ibm.com> wrote:
>>>>>
>>>>>> Add missing TPM device identification objects _STR and _UID. They will
>>>>>> appear as files 'description' and 'uid' under Linux sysfs.
>>>>>>
>>>>>> Following inspection of sysfs entries for hardware TPMs we chose
>>>>>> uid '1'.
>>>>> My guess would be that buy default (in case of missing UID), OSPM
>>>>> will start enumerate from 0. So I think 0 is more safer choice
>>>>> when it comes to compatibility.
>>>>>
>>>>> Can you smoke test TPM with Windows, and check if adding UID doesn't
>>>>> break anything if VM actually uses TMP (though I'm not sure how to
>>>>> check it on Windows, maybe install Windows 11 without this patch
>>>>> and then see if it still boots pre-installed VM and nothing is broken
>>>>> after this patch)?
>>>>>
>>>> I smoke tested it with the posted patches applied to v6.2.0 and started 3
>>>> VMs with it:
>>>>
>>>> - Linux shows uid = 1 and the description "TPM 2.0 Device" in sysfs
>>>>
>>>> - Win 10 and Win 11 tpm.msc tool are both showing that the TPM is 'ready for
>>>> use'
>>>>
>>>> Stefan
>>>>
>>> Just to make sure, what Igor was concerned about is issues like
>>> we had with e.g. network devices, when changing UID makes
>>> windows think it's a new device and lose configuration
>>> created on old qemu on boot with a new qemu.
>>> Not sure what can be configured with a TPM device though ...
>> The VMs were all created on an old qemu and booted into the patched
>> qemu. They hadn't seen the new ACPI entries before, for sure not when
>> they were installed.
> In that case I would not bother with compat machinery
>
> (my stance on APCI and compat knobs haven't changed and it
> is avoid it if possible, sometimes that backfires but overall
> keeps code simpler, otherwise it would be unreadable mess
> (it's already complex enough))
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
Another test I did was to remove the TPM 2, boot the win 10 and 11 VMs
ran tpm.msc (no TPM there), shut them down, added the TPM 2 and ran
tpm.msc again. Same result: TPM is 'ready for use'.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-01-06 18:08 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 17:58 [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Stefan Berger
2022-01-04 17:58 ` [PATCH v5 1/3] tests: acpi: prepare for updated TPM related tables Stefan Berger
2022-01-06 16:56 ` Igor Mammedov
2022-01-04 17:58 ` [PATCH v5 2/3] acpi: tpm: Add missing device identification objects Stefan Berger
2022-01-06 8:36 ` Igor Mammedov
2022-01-06 11:40 ` Michael S. Tsirkin
2022-01-06 13:53 ` Stefan Berger
2022-01-06 13:56 ` Michael S. Tsirkin
2022-01-06 14:01 ` Stefan Berger
2022-01-06 16:55 ` Igor Mammedov
2022-01-06 18:07 ` Stefan Berger
2022-01-06 17:38 ` Ani Sinha
2022-01-04 17:58 ` [PATCH v5 3/3] tests: acpi: Add updated TPM related tables Stefan Berger
2022-01-04 18:09 ` [PATCH v5 0/3] tpm: Add missing ACPI device identification objects Daniel P. Berrangé
2022-01-04 18:14 ` Stefan Berger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.