| Message ID | 20211223022310.575496-3-stefanb@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: Add missing ACPI device identification objects | expand |
On Wed, 22 Dec 2021, 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> Reviewed-by: Ani Sinha <ani@anisinha.ca> > Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com > --- > 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 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..2fb70847cb 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,6 +1848,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"))); Should we put a check here to make sure it is a 2.0 version like the code hunk above for TIS_ISA? I looked around and it seems CRB is only available for TPM 2.0 in which case it is probably ok but still making sure. https://qemu.readthedocs.io/en/latest/specs/tpm.html > crs = aml_resource_template(); > aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, > TPM_CRB_ADDR_SIZE, AML_READ_WRITE)); > @@ -1851,6 +1857,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 2021/12/23 10:23, 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> > Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com > --- > 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 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))); > For ARM part Reviewed-by: Shannon Zhao <shannon.zhaosl@gmail.com> > Aml *crs = aml_resource_template(); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 8383b83ee3..2fb70847cb 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,6 +1848,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)); > @@ -1851,6 +1857,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);
On Wed, 22 Dec 2021 21:23:09 -0500 Stefan Berger <stefanb@linux.ibm.com> wrote: > Add missing device identification objects _STR and _UID. They will appear why, does it break anything or it's just cosmetic? > 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> > Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com > --- > 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 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..2fb70847cb 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))); why it's 1, and not 0 as in virt-arm? > > aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > crs = aml_resource_template(); > @@ -1844,6 +1848,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)); > @@ -1851,6 +1857,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); > no necessary ^^^ empty line > + aml_append(dev, aml_name_decl("_UID", aml_int(1))); > + > tpm_build_ppi_acpi(tpm, dev); > > aml_append(sb_scope, dev);
On 1/4/22 04:55, Igor Mammedov wrote: > On Wed, 22 Dec 2021 21:23:09 -0500 > Stefan Berger <stefanb@linux.ibm.com> wrote: > >> Add missing device identification objects _STR and _UID. They will appear > why, does it break anything or it's just cosmetic? I don't know about whether any software needs these entries but it's driven by this: https://gitlab.com/qemu-project/qemu/-/issues/708 > >> 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> >> Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com >> --- >> 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 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..2fb70847cb 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))); > why it's 1, and not 0 as in virt-arm? Marc-Andre and I looked at machines with hardware TPMs and that's what we found there as well, a '1'. > >> >> aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); >> crs = aml_resource_template(); >> @@ -1844,6 +1848,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)); >> @@ -1851,6 +1857,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> >> aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); >> > no necessary ^^^ empty line fixed > >> + aml_append(dev, aml_name_decl("_UID", aml_int(1))); >> + >> tpm_build_ppi_acpi(tpm, dev); >> >> aml_append(sb_scope, dev);
On Tue, 4 Jan 2022, Stefan Berger wrote: > > On 1/4/22 04:55, Igor Mammedov wrote: > > On Wed, 22 Dec 2021 21:23:09 -0500 > > Stefan Berger <stefanb@linux.ibm.com> wrote: > > > > > Add missing device identification objects _STR and _UID. They will appear > > why, does it break anything or it's just cosmetic? > > I don't know about whether any software needs these entries but it's driven by > this: > > https://gitlab.com/qemu-project/qemu/-/issues/708 Ok so you might want to add Resolves: https://gitlab.com/qemu-project/qemu/-/issues/708 in the commit message. Please see: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message > > > > > > 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> > > > Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com > > > --- > > > 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 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..2fb70847cb 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))); > > why it's 1, and not 0 as in virt-arm? > > Marc-Andre and I looked at machines with hardware TPMs and that's what we > found there as well, a '1'. > > > > > > > aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > > > crs = aml_resource_template(); > > > @@ -1844,6 +1848,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)); > > > @@ -1851,6 +1857,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); > > > > > no necessary ^^^ empty line > fixed > > > > > + aml_append(dev, aml_name_decl("_UID", aml_int(1))); > > > + > > > tpm_build_ppi_acpi(tpm, dev); > > > aml_append(sb_scope, dev); >
On 1/4/22 10:15, Ani Sinha wrote: > > On Tue, 4 Jan 2022, Stefan Berger wrote: > >> On 1/4/22 04:55, Igor Mammedov wrote: >>> On Wed, 22 Dec 2021 21:23:09 -0500 >>> Stefan Berger <stefanb@linux.ibm.com> wrote: >>> >>>> Add missing device identification objects _STR and _UID. They will appear >>> why, does it break anything or it's just cosmetic? >> I don't know about whether any software needs these entries but it's driven by >> this: >> >> https://gitlab.com/qemu-project/qemu/-/issues/708 > Ok so you might want to add > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/708 > > in the commit message. Please see: > https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message Ooops, I will change this here to Resolves: Fixes: https://gitlab.com/qemu-project/qemu/-/issues/708 > >>>> 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> >>>> Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com >>>> --- >>>> 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 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..2fb70847cb 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))); >>> why it's 1, and not 0 as in virt-arm? >> Marc-Andre and I looked at machines with hardware TPMs and that's what we >> found there as well, a '1'. >> >> >>>> aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); >>>> crs = aml_resource_template(); >>>> @@ -1844,6 +1848,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)); >>>> @@ -1851,6 +1857,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >>>> aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); >>>> >>> no necessary ^^^ empty line >> fixed >>>> + aml_append(dev, aml_name_decl("_UID", aml_int(1))); >>>> + >>>> tpm_build_ppi_acpi(tpm, dev); >>>> aml_append(sb_scope, dev);
On Tue, 4 Jan 2022 09:48:32 -0500 Stefan Berger <stefanb@linux.ibm.com> wrote: > On 1/4/22 04:55, Igor Mammedov wrote: > > On Wed, 22 Dec 2021 21:23:09 -0500 > > Stefan Berger <stefanb@linux.ibm.com> wrote: > > > >> Add missing device identification objects _STR and _UID. They will appear > > why, does it break anything or it's just cosmetic? > > I don't know about whether any software needs these entries but it's > driven by this: > > https://gitlab.com/qemu-project/qemu/-/issues/708 > > > > > >> 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> > >> Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com > >> --- > >> 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 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..2fb70847cb 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))); > > why it's 1, and not 0 as in virt-arm? > > Marc-Andre and I looked at machines with hardware TPMs and that's what > we found there as well, a '1'. perhaps mention that in commit message > > > > > >> > >> aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > >> crs = aml_resource_template(); > >> @@ -1844,6 +1848,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)); > >> @@ -1851,6 +1857,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> > >> aml_append(dev, aml_name_decl("_STA", aml_int(0xf))); > >> > > no necessary ^^^ empty line > fixed > > > >> + aml_append(dev, aml_name_decl("_UID", aml_int(1))); > >> + > >> tpm_build_ppi_acpi(tpm, dev); > >> > >> aml_append(sb_scope, dev); >
On 1/4/22 11:34, Igor Mammedov wrote: > On Tue, 4 Jan 2022 09:48:32 -0500 > Stefan Berger <stefanb@linux.ibm.com> wrote: > >> On 1/4/22 04:55, Igor Mammedov wrote: >>> On Wed, 22 Dec 2021 21:23:09 -0500 >>> Stefan Berger <stefanb@linux.ibm.com> wrote: >>> >>>> Add missing device identification objects _STR and _UID. They will appear >>> why, does it break anything or it's just cosmetic? >> I don't know about whether any software needs these entries but it's >> driven by this: >> >> https://gitlab.com/qemu-project/qemu/-/issues/708 >> >> >>> >>>> 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> >>>> Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com >>>> --- >>>> 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 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..2fb70847cb 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))); >>> why it's 1, and not 0 as in virt-arm? >> Marc-Andre and I looked at machines with hardware TPMs and that's what >> we found there as well, a '1'. > perhaps mention that in commit message Added to v5 2/3.
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..2fb70847cb 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,6 +1848,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)); @@ -1851,6 +1857,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> Message-id: 20211110133559.3370990-3-stefanb@linux.ibm.com --- hw/arm/virt-acpi-build.c | 1 + hw/i386/acpi-build.c | 8 ++++++++ 2 files changed, 9 insertions(+)