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

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

On 6/18/20 4:12 PM, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2020 at 07:59:51AM -0400, Stefan Berger wrote:
>> 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
> So is this useful to anyone? If no I'd say drop it.


So we can remove command line options?


> I'm guessing sb16 has it since it is useful for running extremely old OSes which might
> have weird quirks for a specific hardware.
>