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

[Stefan Berger]

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