| Message ID | 20211109140152.3310657-3-stefanb@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: Add missing ACPI device identification objects | expand |
On Tue, Nov 09, 2021 at 09:01:51AM -0500, Stefan Berger wrote: > Add missing device identification objects _STR and _UID. They will appear > as files 'description' and 'uid' under Linux sysfs. > > 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> > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/708 > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Do you want this in 6.2? > --- > hw/arm/virt-acpi-build.c | 1 + > hw/i386/acpi-build.c | 8 ++++++++ > 2 files changed, 9 insertions(+) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 674f902652..09456424aa 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -228,6 +228,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 a3ad6abd33..5bd2160a89 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1808,11 +1808,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"))); When we support more versions, won't this make us do annoying tricks to say so in the string? Why not just "TPM device" to future-proof it? > } 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))); > The ACPI spec mentions also matching on _CID. > aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > crs = aml_resource_template(); > @@ -1840,6 +1844,8 @@ 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)); > @@ -1847,6 +1853,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > 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); > > aml_append(sb_scope, dev); > -- > 2.31.1
On 11/9/21 09:20, Michael S. Tsirkin wrote: > On Tue, Nov 09, 2021 at 09:01:51AM -0500, Stefan Berger wrote: >> Add missing device identification objects _STR and _UID. They will appear >> as files 'description' and 'uid' under Linux sysfs. >> >> 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> >> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/708 >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > Do you want this in 6.2? Yes. > >> --- >> hw/arm/virt-acpi-build.c | 1 + >> hw/i386/acpi-build.c | 8 ++++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >> index 674f902652..09456424aa 100644 >> --- a/hw/arm/virt-acpi-build.c >> +++ b/hw/arm/virt-acpi-build.c >> @@ -228,6 +228,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 a3ad6abd33..5bd2160a89 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -1808,11 +1808,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"))); > > When we support more versions, won't this make us > do annoying tricks to say so in the string? > Why not just "TPM device" to future-proof it? I am not sure what other version there will be and I haven't seen any other descriptions than the one reported here: https://gitlab.com/qemu-project/qemu/-/issues/708 That's why I took TPM 2.0 device. My TPM 1.2 machine doesn't report it for a TPM 1.2. > >> haven } 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))); >> > The ACPI spec mentions also matching on _CID. "6.1.2 _CID (Compatible ID) This optional object is used to supply OSPM with a device?s Plug and Play-Compatible Device ID. Use _CID objects when a device has no other defined hardware standard method to report its compatible IDs" 6.1.12 _UID (Unique ID) This object provides OSPM with a logical device ID that does not change across reboots. This object is optional, but is required when the device has no other way to report a persistent unique device ID. The _UID must be unique across all devices with either a common _HID or _CID. Is _CID a must-have for TPM now? We have _HID. > >> aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); >> crs = aml_resource_template(); >> @@ -1840,6 +1844,8 @@ 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)); >> @@ -1847,6 +1853,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> >> 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); >> >> aml_append(sb_scope, dev); >> -- >> 2.31.1
On Tue, Nov 09, 2021 at 09:26:46AM -0500, Stefan Berger wrote: > > On 11/9/21 09:20, Michael S. Tsirkin wrote: > > On Tue, Nov 09, 2021 at 09:01:51AM -0500, Stefan Berger wrote: > > > Add missing device identification objects _STR and _UID. They will appear > > > as files 'description' and 'uid' under Linux sysfs. > > > > > > 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> > > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/708 > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > Do you want this in 6.2? > > Yes. > > > > > > > --- > > > hw/arm/virt-acpi-build.c | 1 + > > > hw/i386/acpi-build.c | 8 ++++++++ > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > > index 674f902652..09456424aa 100644 > > > --- a/hw/arm/virt-acpi-build.c > > > +++ b/hw/arm/virt-acpi-build.c > > > @@ -228,6 +228,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 a3ad6abd33..5bd2160a89 100644 > > > --- a/hw/i386/acpi-build.c > > > +++ b/hw/i386/acpi-build.c > > > @@ -1808,11 +1808,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"))); > > > > When we support more versions, won't this make us > > do annoying tricks to say so in the string? > > Why not just "TPM device" to future-proof it? > > I am not sure what other version there will be and I haven't seen any other > descriptions than the one reported here: > > https://gitlab.com/qemu-project/qemu/-/issues/708 > > That's why I took TPM 2.0 device. My TPM 1.2 machine doesn't report it for a > TPM 1.2. > > ok > > > > > haven } 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))); > > > > > The ACPI spec mentions also matching on _CID. > "6.1.2 _CID (Compatible ID) > This optional object is used to supply OSPM with a device?s Plug and > Play-Compatible Device ID. Use _CID > > objects when a device has no other defined hardware standard method to > report its compatible IDs" > > > 6.1.12 _UID (Unique ID) > This object provides OSPM with a logical device ID that does not change > across reboots. This object is > optional, but is required when the device has no other way to report a > persistent unique device ID. The > _UID must be unique across all devices with either a common _HID or _CID. > > > Is _CID a must-have for TPM now? We have _HID. the spec says UID is unique for all devices with a given CID or HID. Does this mean just HID is used if no CID? Or that all devices without CID must have unique UIDs? I think the former but just making sure. > > > > aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > > > crs = aml_resource_template(); > > > @@ -1840,6 +1844,8 @@ 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)); > > > @@ -1847,6 +1853,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > 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); > > > aml_append(sb_scope, dev); > > > -- > > > 2.31.1
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 674f902652..09456424aa 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -228,6 +228,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 a3ad6abd33..5bd2160a89 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1808,11 +1808,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(); @@ -1840,6 +1844,8 @@ 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)); @@ -1847,6 +1853,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 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); aml_append(sb_scope, dev);
Add missing device identification objects _STR and _UID. They will appear as files 'description' and 'uid' under Linux sysfs. 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> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/708 Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- hw/arm/virt-acpi-build.c | 1 + hw/i386/acpi-build.c | 8 ++++++++ 2 files changed, 9 insertions(+)