Re: [Qemu-devel] [PATCH] acpi: Fix TPM ACPI description to make TPM usable on Windows

[Stefan Berger]

On 03/30/2016 09:33 AM, Igor Mammedov wrote:
> On Mon, 21 Mar 2016 10:21:11 -0400
> Stefan Berger <address@hidden> wrote:
>
> > This patch addresses BZ 1281413.
> >
> > Fix the APCI description to make it work on Windows again. The ACPI
> > description was broken in commit 9e47226.
> above commit just added missing ASL description for TMP device
> and you also posted exactly similar patch back then.

Sorry, I referenced the wrong commit. Here's the bad one:

commit 72d97b3a543a9c2c820bd463ba24751ae4247ac3

> Current commit message is pretty useless, Pls describe in commit
> message what/how is broken and how/why patch fixes it.

I am not sure what exactly broke it. All I know is that the scope was 
changed and the device name were change (ISA.TPM versus TPM). This was 
the working ACPI description from QEMU v2.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}
            })


> > Signed-off-by: Stefan Berger <address@hidden>
> > ---
> >   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 325d8ce..c6e90b6 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2334,22 +2334,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);
> it's ISA device so it should stay in ISA scope,
> I'll post and alternative fix for TPM_TIS_ADDR_BASE as follow up
> to this series.

Alternative means it doesn't need this patch? Or it needs it and is a 
follow up on top of it?

> >               }
> >           }
> > +
> > +        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));
> silent change,
> why IRQ descriptor is commented out, it seems the device
> uses/initializes it?
> I'd split IRQ change in a separate patch that explains why it
> shouldn't be in TPM._CRS.

We can leave it there if it works. The bug report is related to Windows, 
which I don't have running in a VM. So the safest was to roll back to 
2.3.1 equivalent, which had the IRQ disabled.

    Stefan

> > +            aml_append(dev, aml_name_decl("_CRS", crs));
> > +            aml_append(sb_scope, dev);
> > +        }
> >           aml_append(dsdt, sb_scope);
> >       }
> >