Re: [PATCH v5 05/14] tpm_crb: move ACPI table building to device interface

[Stefan Berger <stefanb@linux.ibm.com>]

On 11/14/23 11:37, Stefan Berger wrote:
> 
> 
> On 11/13/23 21:09, Joelle van Dyne wrote:
>> This logic is similar to TPM TIS ISA device. Since TPM CRB can only
>> support TPM 2.0 backends, we check for this in realize.
>>
>> Signed-off-by: Joelle van Dyne <j@getutm.app>
>> ---
>>   hw/tpm/tpm_crb.h        |  2 ++
>>   hw/i386/acpi-build.c    | 16 +---------------
>>   hw/tpm/tpm_crb.c        | 16 ++++++++++++++++
>>   hw/tpm/tpm_crb_common.c | 19 +++++++++++++++++++
>>   4 files changed, 38 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/tpm/tpm_crb.h b/hw/tpm/tpm_crb.h
>> index 36863e1664..e6a86e3fd1 100644
>> --- a/hw/tpm/tpm_crb.h
>> +++ b/hw/tpm/tpm_crb.h
>> @@ -73,5 +73,7 @@ void tpm_crb_init_memory(Object *obj, TPMCRBState 
>> *s, Error **errp);
>>   void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void 
>> *saved_cmdmem);
>>   void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs,
>>                         const void *saved_cmdmem);
>> +void tpm_crb_build_aml(TPMIf *ti, Aml *scope, uint32_t baseaddr, 
>> uint32_t size,
>> +                       bool build_ppi);
>>
>>   #endif /* TPM_TPM_CRB_H */
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 80db183b78..7491cee2af 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -1792,21 +1792,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>
>>   #ifdef CONFIG_TPM
>>       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);
>> -
>> -        aml_append(sb_scope, dev);
>> +        call_dev_aml_func(DEVICE(tpm), scope);
> 
> For an x86_64 VM we have to call it directly otherwise the ACPI table 
> won't be there.
> 
>          tpm_crb_build_aml(tpm, sb_scope, TPM_CRB_ADDR_BASE, 
> TPM_CRB_ADDR_SIZE, true);
> 
> 
I looks like a good place for the moved code would be in hw/acpi/tpm.c