Re: [PATCH v3 4/4] arm/acpi: Add the TPM2.0 device under the DSDT

[Auger Eric <eric.auger@redhat.com>]

Hi Igor,

On 6/5/20 4:45 PM, Igor Mammedov wrote:
> On Mon,  1 Jun 2020 11:57:37 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
> 
>> In case it is dynamically instantiated, add the TPM 2.0 device object
>> under the DSDT table in the ACPI namespace. Its HID is MSFT0101
>> while its current resource settings (CRS) property is initialized
>> with the guest physical address and MMIO size of the device.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>>
>> ---
>>
>> v2 -> v3:
>> - use SYS_BUS_DEVICE() instead of
>>   (SysBusDevice *)object_dynamic_cast(OBJECT())
>>
>> v1 -> v2:
>> - use memory_region_size
>> - fix mingw compilation issue by casting to uint32_t
>> - added Stefan's R-b
>> ---
>>  hw/arm/virt-acpi-build.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 6d152ab481..05a3028500 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -46,6 +46,7 @@
>>  #include "hw/pci/pci.h"
>>  #include "hw/arm/virt.h"
>>  #include "hw/mem/nvdimm.h"
>> +#include "hw/platform-bus.h"
>>  #include "sysemu/numa.h"
>>  #include "sysemu/reset.h"
>>  #include "sysemu/tpm.h"
>> @@ -364,6 +365,36 @@ static void acpi_dsdt_add_power_button(Aml *scope)
>>      aml_append(scope, dev);
>>  }
>>  
>> +static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
>> +{
>> +    hwaddr pbus_base = vms->memmap[VIRT_PLATFORM_BUS].base;
>> +    PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
>> +    MemoryRegion *sbdev_mr;
>> +    SysBusDevice *sbdev;
>> +    hwaddr tpm_base;
>> +
>> +    sbdev = SYS_BUS_DEVICE(tpm_find());
>> +
>> +    tpm_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
>> +    assert(tpm_base != -1);
>> +
>> +    tpm_base += pbus_base;
>> +
>> +    sbdev_mr = sysbus_mmio_get_region(sbdev, 0);
>> +
>> +    Aml *dev = aml_device("TPM0");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> +    Aml *crs = aml_resource_template();
>> +    aml_append(crs,
>> +               aml_memory32_fixed(tpm_base,
>> +                                  (uint32_t)memory_region_size(sbdev_mr),
>> +                                  AML_READ_WRITE));
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +    aml_append(scope, dev);
>> +}
>> +
>>  static void
>>  build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>  {
>> @@ -758,6 +789,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>      }
>>  
>>      acpi_dsdt_add_power_button(scope);
>> +    acpi_dsdt_add_tpm(scope, vms);
> shouldn't be this guarded by check if TPM device is present?
Yes I should. the check was in v2 and was dropped in v3.

thanks

Eric
> 
> perhaps pass found here tpm to acpi_dsdt_add_tpm() as an argument
> 
>>  
>>      aml_append(dsdt, scope);
>>  
>