Re: [PATCH v3 5/8] acpi: Enable TPM IRQ

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

On 6/17/20 4:22 AM, Auger Eric wrote:
> Hi Stefan,
>
> On 6/16/20 10:57 PM, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Move the TPM TIS IRQ to unused IRQ 13, which is the only one accepted by
>> Windows. Query for the TPM's irq number and enable the TPM IRQ unless
>> TPM_IRQ_DISABLED is returned.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>   hw/i386/acpi-build.c  | 11 +++++------
>>   include/hw/acpi/tpm.h |  2 +-
>>   2 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 900f786d08..bb9a7f8497 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -2021,6 +2021,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>               build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
>>   
>>               if (TPM_IS_TIS_ISA(tpm)) {
>> +                int8_t irq = tpm_get_irqnum(tpm);
>>                   if (misc->tpm_version == TPM_VERSION_2_0) {
>>                       dev = aml_device("TPM");
>>                       aml_append(dev, aml_name_decl("_HID",
>> @@ -2035,12 +2036,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>                   crs = aml_resource_template();
>>                   aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
>>                              TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
>> -                /*
>> -                    FIXME: TPM_TIS_IRQ=5 conflicts with PNP0C0F irqs,
>> -                    Rewrite to take IRQ from TPM device model and
>> -                    fix default IRQ value there to use some unused IRQ
>> -                 */
>> -                /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
>> +
>> +                if (irq != TPM_IRQ_DISABLED) {
> Out of curiosity what is the goal to expose the irq num as a property
> settable by the end-user if only 13 is known to work in all cases. At
> least shouldn't we warn the end-user in case he attempts to change the
> default value?

For Windows only IRQ 13 works (and I am not sure whether this has always 
been like this), Linux accepts several other ones. As for exposing it to 
the end-user, I may have taken this from soundblaster (sb16.c), which 
also exposes it. If someone plays around with the irq numbers I would 
say they must have some more Pc knowledge than  just trying random numbers.


    Stefan