| Message ID | 1459733876-22908-2-git-send-email-stefanb@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
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? > --- > 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
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
On Mon, 4 Apr 2016 06:17:38 -0400 Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > 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. Since TPM_TIS_ADDR_BASE is outside of PCI window of PCI0, we need to add it explicitly to PCI0._CRS. Moving out of scope is plainly wrong and works just by accident. Granted that Michael is fine with hiding IRQ and provided that you'll fix IRQ handling later, I won't object commenting IRQ entry for 2.6. > > > > > > > > >> --- > >> 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 >
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); }
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> --- hw/i386/acpi-build.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)