All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2]  acpi: tpm: Fix TPM ACPI description
@ 2016-04-04  1:37 Stefan Berger
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413) Stefan Berger
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Stefan Berger @ 2016-04-04  1:37 UTC (permalink / raw)
  To: stefanb, qemu-devel, mst; +Cc: imammedo, Stefan Berger, crobinso

This series of patches fixes some problems with the TPM's ACPI
description.

Stefan Berger (2):
  acpi: tpm: Fix TPM ACPI description (BZ 1281413)
  acpi: tpm: Get the interrupt the device model is using

 hw/i386/acpi-build.c | 29 ++++++++++++++---------------
 hw/tpm/tpm_tis.c     |  5 ++++-
 include/sysemu/tpm.h |  6 +++---
 3 files changed, 21 insertions(+), 19 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413)
  2016-04-04  1:37 [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Stefan Berger
@ 2016-04-04  1:37 ` Stefan Berger
  2016-04-04  9:04   ` Michael S. Tsirkin
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using Stefan Berger
  2016-04-04  9:04 ` [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Michael S. Tsirkin
  2 siblings, 1 reply; 17+ messages in thread
From: Stefan Berger @ 2016-04-04  1:37 UTC (permalink / raw)
  To: stefanb, qemu-devel, mst; +Cc: imammedo, Stefan Berger, crobinso

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(-)

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using
  2016-04-04  1:37 [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Stefan Berger
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413) Stefan Berger
@ 2016-04-04  1:37 ` Stefan Berger
  2016-04-04  9:01   ` Michael S. Tsirkin
  2016-04-04 12:08   ` Igor Mammedov
  2016-04-04  9:04 ` [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Michael S. Tsirkin
  2 siblings, 2 replies; 17+ messages in thread
From: Stefan Berger @ 2016-04-04  1:37 UTC (permalink / raw)
  To: stefanb, qemu-devel, mst; +Cc: imammedo, Stefan Berger, crobinso

Get the interrupt the TPM device model is using. Do not activate
the interrupt in the ACPI description yet since the current
default value clashes with other devices.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/i386/acpi-build.c | 5 +++--
 hw/tpm/tpm_tis.c     | 5 ++++-
 include/sysemu/tpm.h | 6 +++---
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e11c721..ae872a1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -105,6 +105,7 @@ typedef struct AcpiMiscInfo {
     bool is_piix4;
     bool has_hpet;
     TPMVersion tpm_version;
+    uint32_t tpm_irq_num;
     const unsigned char *dsdt_code;
     unsigned dsdt_size;
     uint16_t pvpanic_port;
@@ -203,7 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
     }
 
     info->has_hpet = hpet_find();
-    info->tpm_version = tpm_get_version();
+    info->tpm_version = tpm_get_parameters(&info->tpm_irq_num);
     info->pvpanic_port = pvpanic_port();
     info->applesmc_io_base = applesmc_port();
 }
@@ -2345,7 +2346,7 @@ build_dsdt(GArray *table_data, GArray *linker,
             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(crs, aml_irq_no_flags(misc->tpm_irq_num));
             aml_append(dev, aml_name_decl("_CRS", crs));
             aml_append(sb_scope, dev);
         }
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 381e726..9f5b2f7 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -967,9 +967,12 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
 /*
  * Get the TPMVersion of the backend device being used
  */
-TPMVersion tpm_tis_get_tpm_version(Object *obj)
+TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num)
 {
     TPMState *s = TPM(obj);
+    TPMTISEmuState *tis = &s->s.tis;
+
+    *irq_num = tis->irq_num;
 
     return tpm_backend_get_tpm_version(s->be_driver);
 }
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index c8afa17..5be45d9 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -26,17 +26,17 @@ typedef enum  TPMVersion {
     TPM_VERSION_2_0 = 2,
 } TPMVersion;
 
-TPMVersion tpm_tis_get_tpm_version(Object *obj);
+TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num);
 
 #define TYPE_TPM_TIS                "tpm-tis"
 
-static inline TPMVersion tpm_get_version(void)
+static inline TPMVersion tpm_get_parameters(uint32_t *irq_num)
 {
 #ifdef CONFIG_TPM
     Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
 
     if (obj) {
-        return tpm_tis_get_tpm_version(obj);
+        return tpm_tis_get_tpm_parameters(obj, irq_num);
     }
 #endif
     return TPM_VERSION_UNSPEC;
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using Stefan Berger
@ 2016-04-04  9:01   ` Michael S. Tsirkin
  2016-04-04 10:10     ` Stefan Berger
  2016-04-04 12:08   ` Igor Mammedov
  1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2016-04-04  9:01 UTC (permalink / raw)
  To: Stefan Berger; +Cc: imammedo, crobinso, qemu-devel, stefanb

On Sun, Apr 03, 2016 at 09:37:56PM -0400, Stefan Berger wrote:
> Get the interrupt the TPM device model is using. Do not activate
> the interrupt in the ACPI description yet since the current
> default value clashes with other devices.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

Is this doing anything at all then?
I'd rather keep cleanups until post-2.6.

> ---
>  hw/i386/acpi-build.c | 5 +++--
>  hw/tpm/tpm_tis.c     | 5 ++++-
>  include/sysemu/tpm.h | 6 +++---
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e11c721..ae872a1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -105,6 +105,7 @@ typedef struct AcpiMiscInfo {
>      bool is_piix4;
>      bool has_hpet;
>      TPMVersion tpm_version;
> +    uint32_t tpm_irq_num;
>      const unsigned char *dsdt_code;
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
> @@ -203,7 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>      }
>  
>      info->has_hpet = hpet_find();
> -    info->tpm_version = tpm_get_version();
> +    info->tpm_version = tpm_get_parameters(&info->tpm_irq_num);
>      info->pvpanic_port = pvpanic_port();
>      info->applesmc_io_base = applesmc_port();
>  }
> @@ -2345,7 +2346,7 @@ build_dsdt(GArray *table_data, GArray *linker,
>              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(crs, aml_irq_no_flags(misc->tpm_irq_num));
>              aml_append(dev, aml_name_decl("_CRS", crs));
>              aml_append(sb_scope, dev);
>          }
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 381e726..9f5b2f7 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -967,9 +967,12 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
>  /*
>   * Get the TPMVersion of the backend device being used
>   */
> -TPMVersion tpm_tis_get_tpm_version(Object *obj)
> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num)
>  {
>      TPMState *s = TPM(obj);
> +    TPMTISEmuState *tis = &s->s.tis;
> +
> +    *irq_num = tis->irq_num;
>  
>      return tpm_backend_get_tpm_version(s->be_driver);
>  }
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index c8afa17..5be45d9 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -26,17 +26,17 @@ typedef enum  TPMVersion {
>      TPM_VERSION_2_0 = 2,
>  } TPMVersion;
>  
> -TPMVersion tpm_tis_get_tpm_version(Object *obj);
> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num);
>  
>  #define TYPE_TPM_TIS                "tpm-tis"
>  
> -static inline TPMVersion tpm_get_version(void)
> +static inline TPMVersion tpm_get_parameters(uint32_t *irq_num)
>  {
>  #ifdef CONFIG_TPM
>      Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
>  
>      if (obj) {
> -        return tpm_tis_get_tpm_version(obj);
> +        return tpm_tis_get_tpm_parameters(obj, irq_num);
>      }
>  #endif
>      return TPM_VERSION_UNSPEC;
> -- 
> 2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413)
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413) Stefan Berger
@ 2016-04-04  9:04   ` Michael S. Tsirkin
  2016-04-04 10:17     ` Stefan Berger
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2016-04-04  9:04 UTC (permalink / raw)
  To: Stefan Berger; +Cc: imammedo, crobinso, qemu-devel, stefanb

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2]  acpi: tpm: Fix TPM ACPI description
  2016-04-04  1:37 [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Stefan Berger
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413) Stefan Berger
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using Stefan Berger
@ 2016-04-04  9:04 ` Michael S. Tsirkin
  2016-04-04 10:31   ` Stefan Berger
  2 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2016-04-04  9:04 UTC (permalink / raw)
  To: Stefan Berger; +Cc: imammedo, crobinso, qemu-devel, stefanb

On Sun, Apr 03, 2016 at 09:37:54PM -0400, Stefan Berger wrote:
> This series of patches fixes some problems with the TPM's ACPI
> description.

Could you pls describe how does this interact with Igor's patch?


> Stefan Berger (2):
>   acpi: tpm: Fix TPM ACPI description (BZ 1281413)
>   acpi: tpm: Get the interrupt the device model is using
> 
>  hw/i386/acpi-build.c | 29 ++++++++++++++---------------
>  hw/tpm/tpm_tis.c     |  5 ++++-
>  include/sysemu/tpm.h |  6 +++---
>  3 files changed, 21 insertions(+), 19 deletions(-)
> 
> -- 
> 2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using
  2016-04-04  9:01   ` Michael S. Tsirkin
@ 2016-04-04 10:10     ` Stefan Berger
  2016-04-04 10:12       ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Berger @ 2016-04-04 10:10 UTC (permalink / raw)
  To: Michael S. Tsirkin, Stefan Berger; +Cc: imammedo, qemu-devel, crobinso

On 04/04/2016 05:01 AM, Michael S. Tsirkin wrote:
> On Sun, Apr 03, 2016 at 09:37:56PM -0400, Stefan Berger wrote:
>> Get the interrupt the TPM device model is using. Do not activate
>> the interrupt in the ACPI description yet since the current
>> default value clashes with other devices.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Is this doing anything at all then?

It's fixing the retrieval of data from the TPM model.

> I'd rather keep cleanups until post-2.6.
>
>> ---
>>   hw/i386/acpi-build.c | 5 +++--
>>   hw/tpm/tpm_tis.c     | 5 ++++-
>>   include/sysemu/tpm.h | 6 +++---
>>   3 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index e11c721..ae872a1 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -105,6 +105,7 @@ typedef struct AcpiMiscInfo {
>>       bool is_piix4;
>>       bool has_hpet;
>>       TPMVersion tpm_version;
>> +    uint32_t tpm_irq_num;
>>       const unsigned char *dsdt_code;
>>       unsigned dsdt_size;
>>       uint16_t pvpanic_port;
>> @@ -203,7 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>>       }
>>   
>>       info->has_hpet = hpet_find();
>> -    info->tpm_version = tpm_get_version();
>> +    info->tpm_version = tpm_get_parameters(&info->tpm_irq_num);
>>       info->pvpanic_port = pvpanic_port();
>>       info->applesmc_io_base = applesmc_port();
>>   }
>> @@ -2345,7 +2346,7 @@ build_dsdt(GArray *table_data, GArray *linker,
>>               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(crs, aml_irq_no_flags(misc->tpm_irq_num));
>>               aml_append(dev, aml_name_decl("_CRS", crs));
>>               aml_append(sb_scope, dev);
>>           }
>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>> index 381e726..9f5b2f7 100644
>> --- a/hw/tpm/tpm_tis.c
>> +++ b/hw/tpm/tpm_tis.c
>> @@ -967,9 +967,12 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
>>   /*
>>    * Get the TPMVersion of the backend device being used
>>    */
>> -TPMVersion tpm_tis_get_tpm_version(Object *obj)
>> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num)
>>   {
>>       TPMState *s = TPM(obj);
>> +    TPMTISEmuState *tis = &s->s.tis;
>> +
>> +    *irq_num = tis->irq_num;
>>   
>>       return tpm_backend_get_tpm_version(s->be_driver);
>>   }
>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
>> index c8afa17..5be45d9 100644
>> --- a/include/sysemu/tpm.h
>> +++ b/include/sysemu/tpm.h
>> @@ -26,17 +26,17 @@ typedef enum  TPMVersion {
>>       TPM_VERSION_2_0 = 2,
>>   } TPMVersion;
>>   
>> -TPMVersion tpm_tis_get_tpm_version(Object *obj);
>> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num);
>>   
>>   #define TYPE_TPM_TIS                "tpm-tis"
>>   
>> -static inline TPMVersion tpm_get_version(void)
>> +static inline TPMVersion tpm_get_parameters(uint32_t *irq_num)
>>   {
>>   #ifdef CONFIG_TPM
>>       Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
>>   
>>       if (obj) {
>> -        return tpm_tis_get_tpm_version(obj);
>> +        return tpm_tis_get_tpm_parameters(obj, irq_num);
>>       }
>>   #endif
>>       return TPM_VERSION_UNSPEC;
>> -- 
>> 2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using
  2016-04-04 10:10     ` Stefan Berger
@ 2016-04-04 10:12       ` Michael S. Tsirkin
  0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2016-04-04 10:12 UTC (permalink / raw)
  To: Stefan Berger; +Cc: imammedo, Stefan Berger, qemu-devel, crobinso

On Mon, Apr 04, 2016 at 06:10:38AM -0400, Stefan Berger wrote:
> On 04/04/2016 05:01 AM, Michael S. Tsirkin wrote:
> >On Sun, Apr 03, 2016 at 09:37:56PM -0400, Stefan Berger wrote:
> >>Get the interrupt the TPM device model is using. Do not activate
> >>the interrupt in the ACPI description yet since the current
> >>default value clashes with other devices.
> >>
> >>Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> >Is this doing anything at all then?
> 
> It's fixing the retrieval of data from the TPM model.

Right but you never tell guest it can use interrupts, right?
So why does it matter which interrupt we end up not using?

> >I'd rather keep cleanups until post-2.6.
> >
> >>---
> >>  hw/i386/acpi-build.c | 5 +++--
> >>  hw/tpm/tpm_tis.c     | 5 ++++-
> >>  include/sysemu/tpm.h | 6 +++---
> >>  3 files changed, 10 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >>index e11c721..ae872a1 100644
> >>--- a/hw/i386/acpi-build.c
> >>+++ b/hw/i386/acpi-build.c
> >>@@ -105,6 +105,7 @@ typedef struct AcpiMiscInfo {
> >>      bool is_piix4;
> >>      bool has_hpet;
> >>      TPMVersion tpm_version;
> >>+    uint32_t tpm_irq_num;
> >>      const unsigned char *dsdt_code;
> >>      unsigned dsdt_size;
> >>      uint16_t pvpanic_port;
> >>@@ -203,7 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> >>      }
> >>      info->has_hpet = hpet_find();
> >>-    info->tpm_version = tpm_get_version();
> >>+    info->tpm_version = tpm_get_parameters(&info->tpm_irq_num);
> >>      info->pvpanic_port = pvpanic_port();
> >>      info->applesmc_io_base = applesmc_port();
> >>  }
> >>@@ -2345,7 +2346,7 @@ build_dsdt(GArray *table_data, GArray *linker,
> >>              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(crs, aml_irq_no_flags(misc->tpm_irq_num));
> >>              aml_append(dev, aml_name_decl("_CRS", crs));
> >>              aml_append(sb_scope, dev);
> >>          }
> >>diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> >>index 381e726..9f5b2f7 100644
> >>--- a/hw/tpm/tpm_tis.c
> >>+++ b/hw/tpm/tpm_tis.c
> >>@@ -967,9 +967,12 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
> >>  /*
> >>   * Get the TPMVersion of the backend device being used
> >>   */
> >>-TPMVersion tpm_tis_get_tpm_version(Object *obj)
> >>+TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num)
> >>  {
> >>      TPMState *s = TPM(obj);
> >>+    TPMTISEmuState *tis = &s->s.tis;
> >>+
> >>+    *irq_num = tis->irq_num;
> >>      return tpm_backend_get_tpm_version(s->be_driver);
> >>  }
> >>diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> >>index c8afa17..5be45d9 100644
> >>--- a/include/sysemu/tpm.h
> >>+++ b/include/sysemu/tpm.h
> >>@@ -26,17 +26,17 @@ typedef enum  TPMVersion {
> >>      TPM_VERSION_2_0 = 2,
> >>  } TPMVersion;
> >>-TPMVersion tpm_tis_get_tpm_version(Object *obj);
> >>+TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num);
> >>  #define TYPE_TPM_TIS                "tpm-tis"
> >>-static inline TPMVersion tpm_get_version(void)
> >>+static inline TPMVersion tpm_get_parameters(uint32_t *irq_num)
> >>  {
> >>  #ifdef CONFIG_TPM
> >>      Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
> >>      if (obj) {
> >>-        return tpm_tis_get_tpm_version(obj);
> >>+        return tpm_tis_get_tpm_parameters(obj, irq_num);
> >>      }
> >>  #endif
> >>      return TPM_VERSION_UNSPEC;
> >>-- 
> >>2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413)
  2016-04-04  9:04   ` Michael S. Tsirkin
@ 2016-04-04 10:17     ` Stefan Berger
  2016-04-04 11:49       ` Igor Mammedov
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Berger @ 2016-04-04 10:17 UTC (permalink / raw)
  To: Michael S. Tsirkin, Stefan Berger; +Cc: imammedo, qemu-devel, crobinso

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description
  2016-04-04  9:04 ` [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Michael S. Tsirkin
@ 2016-04-04 10:31   ` Stefan Berger
  2016-04-04 12:11     ` Igor Mammedov
  2016-04-04 16:00     ` Michael S. Tsirkin
  0 siblings, 2 replies; 17+ messages in thread
From: Stefan Berger @ 2016-04-04 10:31 UTC (permalink / raw)
  To: Michael S. Tsirkin, Stefan Berger; +Cc: imammedo, qemu-devel, crobinso

On 04/04/2016 05:04 AM, Michael S. Tsirkin wrote:
> On Sun, Apr 03, 2016 at 09:37:54PM -0400, Stefan Berger wrote:
>> This series of patches fixes some problems with the TPM's ACPI
>> description.
> Could you pls describe how does this interact with Igor's patch?

Follow https://bugzilla.redhat.com/attachment.cgi?id=1137429 and 
attachment https://bugzilla.redhat.com/attachment.cgi?id=1137429 I don't 
think we need Igor's patch. I am posting what has been tested.

>
>
>> Stefan Berger (2):
>>    acpi: tpm: Fix TPM ACPI description (BZ 1281413)
>>    acpi: tpm: Get the interrupt the device model is using
>>
>>   hw/i386/acpi-build.c | 29 ++++++++++++++---------------
>>   hw/tpm/tpm_tis.c     |  5 ++++-
>>   include/sysemu/tpm.h |  6 +++---
>>   3 files changed, 21 insertions(+), 19 deletions(-)
>>
>> -- 
>> 2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413)
  2016-04-04 10:17     ` Stefan Berger
@ 2016-04-04 11:49       ` Igor Mammedov
  0 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2016-04-04 11:49 UTC (permalink / raw)
  To: Stefan Berger; +Cc: crobinso, Stefan Berger, qemu-devel, Michael S. Tsirkin

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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using
  2016-04-04  1:37 ` [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using Stefan Berger
  2016-04-04  9:01   ` Michael S. Tsirkin
@ 2016-04-04 12:08   ` Igor Mammedov
  1 sibling, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2016-04-04 12:08 UTC (permalink / raw)
  To: Stefan Berger; +Cc: crobinso, mst, qemu-devel, stefanb

On Sun,  3 Apr 2016 21:37:56 -0400
Stefan Berger <stefanb@us.ibm.com> wrote:

> Get the interrupt the TPM device model is using. Do not activate
> the interrupt in the ACPI description yet since the current
> default value clashes with other devices.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  hw/i386/acpi-build.c | 5 +++--
>  hw/tpm/tpm_tis.c     | 5 ++++-
>  include/sysemu/tpm.h | 6 +++---
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e11c721..ae872a1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -105,6 +105,7 @@ typedef struct AcpiMiscInfo {
>      bool is_piix4;
>      bool has_hpet;
>      TPMVersion tpm_version;
> +    uint32_t tpm_irq_num;
>      const unsigned char *dsdt_code;
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
> @@ -203,7 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>      }
>  
>      info->has_hpet = hpet_find();
> -    info->tpm_version = tpm_get_version();
> +    info->tpm_version = tpm_get_parameters(&info->tpm_irq_num);
it's a bit confusing to read, provided that IRQ is an object property
I would just do here:

  obj  = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
  if (obj)
    info->tpm_irq_num = object_property_get_int(obj, "irq", NULL);

may be do the same for version too and do away with tpm_get_*()
wrapper that pokes into object internals. (result is 1 less global
symbol to link and no ifdef magic in tpm.h)

>      info->pvpanic_port = pvpanic_port();
>      info->applesmc_io_base = applesmc_port();
>  }
> @@ -2345,7 +2346,7 @@ build_dsdt(GArray *table_data, GArray *linker,
>              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(crs, aml_irq_no_flags(misc->tpm_irq_num));
>              aml_append(dev, aml_name_decl("_CRS", crs));
>              aml_append(sb_scope, dev);
>          }
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 381e726..9f5b2f7 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -967,9 +967,12 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
>  /*
>   * Get the TPMVersion of the backend device being used
>   */
> -TPMVersion tpm_tis_get_tpm_version(Object *obj)
> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num)
>  {
>      TPMState *s = TPM(obj);
> +    TPMTISEmuState *tis = &s->s.tis;
> +
> +    *irq_num = tis->irq_num;
>  
>      return tpm_backend_get_tpm_version(s->be_driver);
>  }
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index c8afa17..5be45d9 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -26,17 +26,17 @@ typedef enum  TPMVersion {
>      TPM_VERSION_2_0 = 2,
>  } TPMVersion;
>  
> -TPMVersion tpm_tis_get_tpm_version(Object *obj);
> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t
> *irq_num); 
>  #define TYPE_TPM_TIS                "tpm-tis"
>  
> -static inline TPMVersion tpm_get_version(void)
> +static inline TPMVersion tpm_get_parameters(uint32_t *irq_num)
>  {
>  #ifdef CONFIG_TPM
>      Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
>  
>      if (obj) {
> -        return tpm_tis_get_tpm_version(obj);
> +        return tpm_tis_get_tpm_parameters(obj, irq_num);
>      }
>  #endif
>      return TPM_VERSION_UNSPEC;

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description
  2016-04-04 10:31   ` Stefan Berger
@ 2016-04-04 12:11     ` Igor Mammedov
  2016-04-08  7:51       ` Michael S. Tsirkin
  2016-04-04 16:00     ` Michael S. Tsirkin
  1 sibling, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2016-04-04 12:11 UTC (permalink / raw)
  To: Stefan Berger; +Cc: crobinso, Stefan Berger, qemu-devel, Michael S. Tsirkin

On Mon, 4 Apr 2016 06:31:43 -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:54PM -0400, Stefan Berger wrote:
> >> This series of patches fixes some problems with the TPM's ACPI
> >> description.
> > Could you pls describe how does this interact with Igor's patch?
> 
> Follow https://bugzilla.redhat.com/attachment.cgi?id=1137429 and 
> attachment https://bugzilla.redhat.com/attachment.cgi?id=1137429 I
> don't think we need Igor's patch. I am posting what has been tested.
I've also tested my patch + commented out irq, it also 'works'.
i.e. Windows doesn't see irq conflict as well.

> 
> >
> >
> >> Stefan Berger (2):
> >>    acpi: tpm: Fix TPM ACPI description (BZ 1281413)
> >>    acpi: tpm: Get the interrupt the device model is using
> >>
> >>   hw/i386/acpi-build.c | 29 ++++++++++++++---------------
> >>   hw/tpm/tpm_tis.c     |  5 ++++-
> >>   include/sysemu/tpm.h |  6 +++---
> >>   3 files changed, 21 insertions(+), 19 deletions(-)
> >>
> >> -- 
> >> 2.5.5
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description
  2016-04-04 10:31   ` Stefan Berger
  2016-04-04 12:11     ` Igor Mammedov
@ 2016-04-04 16:00     ` Michael S. Tsirkin
  2016-04-06  0:13       ` Stefan Berger
  1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2016-04-04 16:00 UTC (permalink / raw)
  To: Stefan Berger; +Cc: imammedo, Stefan Berger, qemu-devel, crobinso

On Mon, Apr 04, 2016 at 06:31:43AM -0400, Stefan Berger wrote:
> On 04/04/2016 05:04 AM, Michael S. Tsirkin wrote:
> >On Sun, Apr 03, 2016 at 09:37:54PM -0400, Stefan Berger wrote:
> >>This series of patches fixes some problems with the TPM's ACPI
> >>description.
> >Could you pls describe how does this interact with Igor's patch?
> 
> Follow https://bugzilla.redhat.com/attachment.cgi?id=1137429 and attachment
> https://bugzilla.redhat.com/attachment.cgi?id=1137429 I don't think we need
> Igor's patch. I am posting what has been tested.

I might just merge the change 1/2 under the justification "we don't know
what's going on, let's revert to old behaviour for now even if it seems
broken". But let's be clear this is what is going on here,
the commit log at least does not explain what the issues
are and how they are fixed.


> >
> >
> >>Stefan Berger (2):
> >>   acpi: tpm: Fix TPM ACPI description (BZ 1281413)
> >>   acpi: tpm: Get the interrupt the device model is using
> >>
> >>  hw/i386/acpi-build.c | 29 ++++++++++++++---------------
> >>  hw/tpm/tpm_tis.c     |  5 ++++-
> >>  include/sysemu/tpm.h |  6 +++---
> >>  3 files changed, 21 insertions(+), 19 deletions(-)
> >>
> >>-- 
> >>2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description
  2016-04-04 16:00     ` Michael S. Tsirkin
@ 2016-04-06  0:13       ` Stefan Berger
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Berger @ 2016-04-06  0:13 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: imammedo, Stefan Berger, qemu-devel, crobinso

On 04/04/2016 12:00 PM, Michael S. Tsirkin wrote:
> On Mon, Apr 04, 2016 at 06:31:43AM -0400, Stefan Berger wrote:
>> On 04/04/2016 05:04 AM, Michael S. Tsirkin wrote:
>>> On Sun, Apr 03, 2016 at 09:37:54PM -0400, Stefan Berger wrote:
>>>> This series of patches fixes some problems with the TPM's ACPI
>>>> description.
>>> Could you pls describe how does this interact with Igor's patch?
>> Follow https://bugzilla.redhat.com/attachment.cgi?id=1137429 and attachment
>> https://bugzilla.redhat.com/attachment.cgi?id=1137429 I don't think we need
>> Igor's patch. I am posting what has been tested.
> I might just merge the change 1/2 under the justification "we don't know
> what's going on, let's revert to old behaviour for now even if it seems
> broken". But let's be clear this is what is going on here,
> the commit log at least does not explain what the issues
> are and how they are fixed.


Thx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description
  2016-04-04 12:11     ` Igor Mammedov
@ 2016-04-08  7:51       ` Michael S. Tsirkin
  2016-04-08  8:13         ` Igor Mammedov
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2016-04-08  7:51 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: Stefan Berger, Stefan Berger, crobinso, qemu-devel

On Mon, Apr 04, 2016 at 02:11:05PM +0200, Igor Mammedov wrote:
> On Mon, 4 Apr 2016 06:31:43 -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:54PM -0400, Stefan Berger wrote:
> > >> This series of patches fixes some problems with the TPM's ACPI
> > >> description.
> > > Could you pls describe how does this interact with Igor's patch?
> > 
> > Follow https://bugzilla.redhat.com/attachment.cgi?id=1137429 and 
> > attachment https://bugzilla.redhat.com/attachment.cgi?id=1137429 I
> > don't think we need Igor's patch. I am posting what has been tested.
> I've also tested my patch + commented out irq, it also 'works'.
> i.e. Windows doesn't see irq conflict as well.

Igor, could you please post a complete patchset for testing?
It seems preferable to me.

> > 
> > >
> > >
> > >> Stefan Berger (2):
> > >>    acpi: tpm: Fix TPM ACPI description (BZ 1281413)
> > >>    acpi: tpm: Get the interrupt the device model is using
> > >>
> > >>   hw/i386/acpi-build.c | 29 ++++++++++++++---------------
> > >>   hw/tpm/tpm_tis.c     |  5 ++++-
> > >>   include/sysemu/tpm.h |  6 +++---
> > >>   3 files changed, 21 insertions(+), 19 deletions(-)
> > >>
> > >> -- 
> > >> 2.5.5
> > 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description
  2016-04-08  7:51       ` Michael S. Tsirkin
@ 2016-04-08  8:13         ` Igor Mammedov
  0 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2016-04-08  8:13 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Stefan Berger, Stefan Berger, crobinso, qemu-devel

On Fri, 8 Apr 2016 10:51:30 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Apr 04, 2016 at 02:11:05PM +0200, Igor Mammedov wrote:
> > On Mon, 4 Apr 2016 06:31:43 -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:54PM -0400, Stefan Berger wrote:  
> > > >> This series of patches fixes some problems with the TPM's ACPI
> > > >> description.  
> > > > Could you pls describe how does this interact with Igor's patch?  
> > > 
> > > Follow https://bugzilla.redhat.com/attachment.cgi?id=1137429 and 
> > > attachment https://bugzilla.redhat.com/attachment.cgi?id=1137429 I
> > > don't think we need Igor's patch. I am posting what has been tested.  
> > I've also tested my patch + commented out irq, it also 'works'.
> > i.e. Windows doesn't see irq conflict as well.  
> 
> Igor, could you please post a complete patchset for testing?
> It seems preferable to me.
Ok, I'll post a2 patches, 1st adding range to PCI0._CRS and
2nd commenting out IRQ entry with a hope that Stefan will fix
IRQ handling in 2.7 dev cycle.

> 
> > >   
> > > >
> > > >  
> > > >> Stefan Berger (2):
> > > >>    acpi: tpm: Fix TPM ACPI description (BZ 1281413)
> > > >>    acpi: tpm: Get the interrupt the device model is using
> > > >>
> > > >>   hw/i386/acpi-build.c | 29 ++++++++++++++---------------
> > > >>   hw/tpm/tpm_tis.c     |  5 ++++-
> > > >>   include/sysemu/tpm.h |  6 +++---
> > > >>   3 files changed, 21 insertions(+), 19 deletions(-)
> > > >>
> > > >> -- 
> > > >> 2.5.5  
> > >   

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-04-08  8:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04  1:37 [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Stefan Berger
2016-04-04  1:37 ` [Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413) Stefan Berger
2016-04-04  9:04   ` Michael S. Tsirkin
2016-04-04 10:17     ` Stefan Berger
2016-04-04 11:49       ` Igor Mammedov
2016-04-04  1:37 ` [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using Stefan Berger
2016-04-04  9:01   ` Michael S. Tsirkin
2016-04-04 10:10     ` Stefan Berger
2016-04-04 10:12       ` Michael S. Tsirkin
2016-04-04 12:08   ` Igor Mammedov
2016-04-04  9:04 ` [Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description Michael S. Tsirkin
2016-04-04 10:31   ` Stefan Berger
2016-04-04 12:11     ` Igor Mammedov
2016-04-08  7:51       ` Michael S. Tsirkin
2016-04-08  8:13         ` Igor Mammedov
2016-04-04 16:00     ` Michael S. Tsirkin
2016-04-06  0:13       ` Stefan Berger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.