Re: [PATCH v5 2/3] acpi: tpm: Add missing device identification objects

[Igor Mammedov]

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