Re: [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413)

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

On 04/04/2016 05:04 AM, Michael S. Tsirkin wrote:
> On Sun, Apr 03, 2016 at 09:37:55PM -0400, Stefan Berger wrote:
>> This patch addresses BZ 1281413.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1281413
>>
>> Fix the APCI description to make it work on operating systems that are
>> more strict about the contents of the TPM's ACPI description than Linux
>> is. The ACPI description was broken in commit 9e472263.
>>
>> We roll back the ACPI description to where it was in QEMU 2.3.1 and
>> deactivate the interrupt, modify the scope to \_SB, and change the
>> name of the device back to 'TPM' from 'ISA.TPM'. Here's the ACPI
>> description from QEMU 2.3.1:
>>
>>      Scope(\_SB) {
>>          /* TPM with emulated TPM TIS interface */
>>          Device (TPM) {
>>              Name (_HID, EisaID ("PNP0C31"))
>>              Name (_CRS, ResourceTemplate ()
>>              {
>>                  Memory32Fixed (ReadWrite, TPM_TIS_ADDR_BASE, TPM_TIS_ADDR_SIZE)
>>                  // older Linux tpm_tis drivers do not work with IRQ
>>                  //IRQNoFlags () {TPM_TIS_IRQ}
>>              })
>>              Method (_STA, 0, NotSerialized) {
>>                  Return (0x0F)
>>              }
>>          }
>>      }
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Can we instead use Igor's patch
>      pc: acpi: tpm: add missing MMIO resource to PCI0._CRS
> and only drop the irq_no_flags value?

Igor's patch adds this here:

+
+    if (misc->tpm_version != TPM_VERSION_UNSPEC) {
+        aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
+                   TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
+    }


Now we that memory description there twice? I am not sure whether this 
is necessary, but you are the APCI experts.



>
>
>> ---
>>   hw/i386/acpi-build.c | 26 ++++++++++++--------------
>>   1 file changed, 12 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 35180ef..e11c721 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -2335,22 +2335,20 @@ build_dsdt(GArray *table_data, GArray *linker,
>>                   Aml *scope = aml_scope("PCI0");
>>                   /* Scan all PCI buses. Generate tables to support hotplug. */
>>                   build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
>> -
>> -                if (misc->tpm_version != TPM_VERSION_UNSPEC) {
>> -                    dev = aml_device("ISA.TPM");
>> -                    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
>> -                    aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
>> -                    crs = aml_resource_template();
>> -                    aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
>> -                               TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
>> -                    aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
>> -                    aml_append(dev, aml_name_decl("_CRS", crs));
>> -                    aml_append(scope, dev);
>> -                }
>> -
>> -                aml_append(sb_scope, scope);
>>               }
>>           }
>> +
>> +        if (misc->tpm_version != TPM_VERSION_UNSPEC) {
>> +            dev = aml_device("TPM");
>> +            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
>> +            aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
>> +            crs = aml_resource_template();
>> +            aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
>> +                       TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
>> +            //aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
>> +            aml_append(dev, aml_name_decl("_CRS", crs));
>> +            aml_append(sb_scope, dev);
>> +        }
>>           aml_append(dsdt, sb_scope);
>>       }
>>   
>> -- 
>> 2.5.5