[PATCH v3 0/8] tpm: Enable usage of TPM TIS with interrupts

[PATCH v3 0/8] tpm: Enable usage of TPM TIS with interrupts

[Stefan Berger]

This series of patches enables the usage of the TPM TIS with interrupts.
We use the unused IRQ 13, which is also accepted by Windows.

    Stefan

v2->v3:
 - Extended series to disable IRQ for TIS on sysbus

v1->v2:
 - Added updated DSDT

Stefan Berger (8):
  tpm_tis: Allow lowering of IRQ also when locality is not active
  tpm: Extend TPMIfClass with get_irqnum() function
  tests: Temporarily ignore DSDT table differences
  tpm: Split TPM_TIS_IRQ into TPM_TIS_ISA_IRQ and TPM_TIS_SYSBUS_IRQ
  acpi: Enable TPM IRQ
  tests: Add updated DSDT
  tpm: Guard irq related ops in case interrupts are disabled
  tpm: Disable interrupt support for TIS on sysbus

 hw/i386/acpi-build.c         |  11 +++++------
 hw/tpm/tpm_tis_common.c      |  12 +++++++++---
 hw/tpm/tpm_tis_isa.c         |  17 ++++++++++++++---
 hw/tpm/tpm_tis_sysbus.c      |  12 +++++++++++-
 include/hw/acpi/tpm.h        |   3 ++-
 include/sysemu/tpm.h         |  12 ++++++++++++
 tests/data/acpi/q35/DSDT.tis | Bin 8357 -> 8360 bytes
 7 files changed, 53 insertions(+), 14 deletions(-)

-- 
2.24.1

[PATCH v3 1/8] tpm_tis: Allow lowering of IRQ also when locality is not active

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

This patch fixes a bug that occurs when using interrupts. It
allows to lower the IRQ also when a locality is not active.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 hw/tpm/tpm_tis_common.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 1af4bce139..0f42696f1f 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -601,10 +601,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
         /* hard wired -- ignore */
         break;
     case TPM_TIS_REG_INT_STATUS:
-        if (s->active_locty != locty) {
-            break;
-        }
-
         /* clearing of interrupt flags */
         if (((val & TPM_TIS_INTERRUPTS_SUPPORTED)) &&
             (s->loc[locty].ints & TPM_TIS_INTERRUPTS_SUPPORTED)) {
-- 
2.24.1

[PATCH v3 2/8] tpm: Extend TPMIfClass with get_irqnum() function

[Stefan Berger]

From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>

Implement get_irqnum() as part of the TPMIfClass to get the assigned IRQ
number or TPM_IRQ_DISABLED (-1) in case IRQs cannot be used.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis_isa.c    |  9 +++++++++
 hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
 include/sysemu/tpm.h    | 12 ++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 30ba37079d..ed6d422f05 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
     return tpm_tis_get_tpm_version(s);
 }
 
+static int8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
+{
+    TPMStateISA *isadev = TPM_TIS_ISA(ti);
+    TPMState *s = &isadev->state;
+
+    return s->irq_num;
+}
+
 static void tpm_tis_isa_reset(DeviceState *dev)
 {
     TPMStateISA *isadev = TPM_TIS_ISA(dev);
@@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void *data)
     dc->reset = tpm_tis_isa_reset;
     tc->request_completed = tpm_tis_isa_request_completed;
     tc->get_version = tpm_tis_isa_get_tpm_version;
+    tc->get_irqnum = tpm_tis_isa_get_irqnum;
 }
 
 static const TypeInfo tpm_tis_isa_info = {
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index eced1fc843..86b3988be5 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
     return tpm_tis_get_tpm_version(s);
 }
 
+static int8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
+{
+    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
+    TPMState *s = &sbdev->state;
+
+    return s->irq_num;
+}
+
 static void tpm_tis_sysbus_reset(DeviceState *dev)
 {
     TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
@@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
     dc->reset = tpm_tis_sysbus_reset;
     tc->request_completed = tpm_tis_sysbus_request_completed;
     tc->get_version = tpm_tis_sysbus_get_tpm_version;
+    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
 }
 
 static const TypeInfo tpm_tis_sysbus_info = {
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 03fb25941c..a81fb4e18e 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -25,6 +25,8 @@ typedef enum TPMVersion {
     TPM_VERSION_2_0 = 2,
 } TPMVersion;
 
+#define TPM_IRQ_DISABLED  -1
+
 #define TYPE_TPM_IF "tpm-if"
 #define TPM_IF_CLASS(klass)                                 \
     OBJECT_CLASS_CHECK(TPMIfClass, (klass), TYPE_TPM_IF)
@@ -41,6 +43,7 @@ typedef struct TPMIfClass {
     enum TpmModel model;
     void (*request_completed)(TPMIf *obj, int ret);
     enum TPMVersion (*get_version)(TPMIf *obj);
+    int8_t (*get_irqnum)(TPMIf *obj);
 } TPMIfClass;
 
 #define TYPE_TPM_TIS_ISA            "tpm-tis"
@@ -74,4 +77,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
     return TPM_IF_GET_CLASS(ti)->get_version(ti);
 }
 
+static inline int8_t tpm_get_irqnum(TPMIf *ti)
+{
+    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
+        return TPM_IRQ_DISABLED;
+    }
+
+    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
+}
+
 #endif /* QEMU_TPM_H */
-- 
2.24.1

[PATCH v3 3/8] tests: Temporarily ignore DSDT table differences

[Stefan Berger]

Ignore DSDT table differences before enabling IRQ support for TPM.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..bb4ce8967b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/DSDT.tis",
-- 
2.24.1

[PATCH v3 4/8] tpm: Split TPM_TIS_IRQ into TPM_TIS_ISA_IRQ and TPM_TIS_SYSBUS_IRQ

[Stefan Berger]

Before the enablement of interrupts on PCs, split the TPM_TIS_IRQ
into TPM_TIS_ISA_IRQ for PCs and TPM_TIS_SYSBUS_IRQ for ARM.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis_isa.c    | 2 +-
 hw/tpm/tpm_tis_sysbus.c | 3 ++-
 include/hw/acpi/tpm.h   | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index ed6d422f05..27222a9a49 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -97,7 +97,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
 }
 
 static Property tpm_tis_isa_properties[] = {
-    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
+    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_ISA_IRQ),
     DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
     DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index 86b3988be5..bf4583c3f6 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -97,7 +97,8 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
 }
 
 static Property tpm_tis_sysbus_properties[] = {
-    DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ),
+    DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num,
+                       TPM_TIS_SYSBUS_IRQ),
     DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
     DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 1a2a57a21f..d5caee9771 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -24,7 +24,8 @@
 #define TPM_TIS_ADDR_BASE           0xFED40000
 #define TPM_TIS_ADDR_SIZE           0x5000
 
-#define TPM_TIS_IRQ                 5
+#define TPM_TIS_ISA_IRQ             5
+#define TPM_TIS_SYSBUS_IRQ          5
 
 #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
 #define TPM_TIS_LOCALITY_SHIFT      12
-- 
2.24.1

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

[Stefan Berger]

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) {
+                    aml_append(crs, aml_irq_no_flags(irq));
+                }
                 aml_append(dev, aml_name_decl("_CRS", crs));
 
                 tpm_build_ppi_acpi(tpm, dev);
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index d5caee9771..d356f2e06e 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -24,7 +24,7 @@
 #define TPM_TIS_ADDR_BASE           0xFED40000
 #define TPM_TIS_ADDR_SIZE           0x5000
 
-#define TPM_TIS_ISA_IRQ             5
+#define TPM_TIS_ISA_IRQ             13    /* only one possible */
 #define TPM_TIS_SYSBUS_IRQ          5
 
 #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
-- 
2.24.1

[PATCH v3 6/8] tests: Add updated DSDT

[Stefan Berger]

Add the updated DSDT following the interrupt enablement.

@@ -5,13 +5,13 @@
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/data/acpi/q35/DSDT.tis, Mon Jun 15 09:57:05 2020
+ * Disassembly of /tmp/aml-Y77YL0, Mon Jun 15 09:57:05 2020
  *
  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x000020A5 (8357)
+ *     Length           0x000020A8 (8360)
  *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
- *     Checksum         0xAD
+ *     Checksum         0x77
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPCDSDT"
  *     OEM Revision     0x00000001 (1)
@@ -3162,6 +3162,8 @@
                         0xFED40000,         // Address Base
                         0x00005000,         // Address Length
                         )
+                    IRQNoFlags ()
+                        {13}
                 })
                 OperationRegion (TPP2, SystemMemory, 0xFED45100, 0x5A)
                 Field (TPP2, AnyAcc, NoLock, Preserve)
**

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 tests/data/acpi/q35/DSDT.tis                | Bin 8357 -> 8360 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   1 -
 2 files changed, 1 deletion(-)

diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index 56b6fb0c3298517d080e38fea05a748b9f1dba54..3f9db960aa05d399fa7f8449e6db688788211832 100644
GIT binary patch
delta 64
zcmZ4LxWbXkCD<iog#rTuWBEp|KeC)oS~2m#PVoX>llkS`nVeK7N60A%iEs(FaWXJ6
UFkJb^5Wv8o#GtUbT~3Y(068!Z;Q#;t

delta 61
zcmZ4CxYUu$CD<iosR9E7<Jyf}e`GoRHDls~o#F-DC-cj>Gx@7bj*wH}7v$n=<78lD
RV7T&+A%KBlbC;YP695=#58(g+

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index bb4ce8967b..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.tis",
-- 
2.24.1

[PATCH v3 7/8] tpm: Guard irq related ops in case interrupts are disabled

[Stefan Berger]

Check for irq_num having the value of TPM_IRQ_DISABLED before calling any
IRQ related functions or allowing the user to try to enable interrupts.
Explicitly allow the value of TPM_IRQ_DISABLED in irq_num.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis_common.c | 12 +++++++++++-
 hw/tpm/tpm_tis_isa.c    |  6 ++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 0f42696f1f..13b233309e 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -359,7 +359,11 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
         val = s->loc[locty].inte;
         break;
     case TPM_TIS_REG_INT_VECTOR:
-        val = s->irq_num;
+        if (s->irq_num != TPM_IRQ_DISABLED) {
+            val = s->irq_num;
+        } else {
+            val = 0;
+        }
         break;
     case TPM_TIS_REG_INT_STATUS:
         val = s->loc[locty].ints;
@@ -591,6 +595,9 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
         if (s->active_locty != locty) {
             break;
         }
+        if (s->irq_num == TPM_IRQ_DISABLED) {
+            val &= ~TPM_TIS_INT_ENABLED;
+        }
 
         s->loc[locty].inte &= mask;
         s->loc[locty].inte |= (val & (TPM_TIS_INT_ENABLED |
@@ -601,6 +608,9 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
         /* hard wired -- ignore */
         break;
     case TPM_TIS_REG_INT_STATUS:
+        if (s->irq_num == TPM_IRQ_DISABLED) {
+            break;
+        }
         /* clearing of interrupt flags */
         if (((val & TPM_TIS_INTERRUPTS_SUPPORTED)) &&
             (s->loc[locty].ints & TPM_TIS_INTERRUPTS_SUPPORTED)) {
diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 27222a9a49..d72f733ead 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -127,13 +127,15 @@ static void tpm_tis_isa_realizefn(DeviceState *dev, Error **errp)
         error_setg(errp, "'tpmdev' property is required");
         return;
     }
-    if (s->irq_num > 15) {
+    if (s->irq_num > 15 && s->irq_num != TPM_IRQ_DISABLED) {
         error_setg(errp, "IRQ %d is outside valid range of 0 to 15",
                    s->irq_num);
         return;
     }
 
-    isa_init_irq(ISA_DEVICE(dev), &s->irq, s->irq_num);
+    if (s->irq_num != TPM_IRQ_DISABLED) {
+        isa_init_irq(ISA_DEVICE(dev), &s->irq, s->irq_num);
+    }
 
     memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
                                 TPM_TIS_ADDR_BASE, &s->mmio);
-- 
2.24.1

[PATCH v3 8/8] tpm: Disable interrupt support for TIS on sysbus

[Stefan Berger]

Disable interrupt support for the TIS on sysbus.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 include/hw/acpi/tpm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index d356f2e06e..21f81690a5 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -25,7 +25,7 @@
 #define TPM_TIS_ADDR_SIZE           0x5000
 
 #define TPM_TIS_ISA_IRQ             13    /* only one possible */
-#define TPM_TIS_SYSBUS_IRQ          5
+#define TPM_TIS_SYSBUS_IRQ          TPM_IRQ_DISABLED
 
 #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
 #define TPM_TIS_LOCALITY_SHIFT      12
-- 
2.24.1

Re: [PATCH v3 0/8] tpm: Enable usage of TPM TIS with interrupts

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200616205721.1191408-1-stefanb@linux.vnet.ibm.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      qga/commands.o
  CC      qga/guest-agent-command-state.o
  CC      qga/main.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  CC      qga/commands-posix.o
  CC      qga/channel-posix.o
  CC      qga/qapi-generated/qga-qapi-types.o
---
  GEN     docs/interop/qemu-ga-ref.html
  GEN     docs/interop/qemu-ga-ref.txt
  GEN     docs/interop/qemu-ga-ref.7
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-keymap
  LINK    ivshmem-client
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    ivshmem-server
  LINK    qemu-nbd
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-storage-daemon
  AS      pc-bios/optionrom/multiboot.o
  AS      pc-bios/optionrom/linuxboot.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-img
  CC      pc-bios/optionrom/linuxboot_dma.o
  AS      pc-bios/optionrom/kvmvapic.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  AS      pc-bios/optionrom/pvh.o
  CC      pc-bios/optionrom/pvh_main.o
  LINK    qemu-io
  LINK    qemu-edid
  BUILD   pc-bios/optionrom/multiboot.img
  LINK    fsdev/virtfs-proxy-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    scsi/qemu-pr-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  BUILD   pc-bios/optionrom/linuxboot.img
  BUILD   pc-bios/optionrom/linuxboot_dma.img
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  BUILD   pc-bios/optionrom/kvmvapic.img
  BUILD   pc-bios/optionrom/pvh.img
  BUILD   pc-bios/optionrom/multiboot.raw
---
  BUILD   pc-bios/optionrom/kvmvapic.raw
  BUILD   pc-bios/optionrom/pvh.raw
  SIGN    pc-bios/optionrom/multiboot.bin
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  SIGN    pc-bios/optionrom/linuxboot.bin
  SIGN    pc-bios/optionrom/linuxboot_dma.bin
  LINK    virtiofsd
  SIGN    pc-bios/optionrom/kvmvapic.bin
  SIGN    pc-bios/optionrom/pvh.bin
  LINK    vhost-user-input
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-ga
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/config-devices.h
---
  CC      x86_64-softmmu/hw/intc/ioapic.o
  CC      x86_64-softmmu/hw/isa/lpc_ich9.o
  CC      x86_64-softmmu/hw/net/virtio-net.o
/tmp/qemu-test/src/fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
/tmp/qemu-test/src/fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
/tmp/qemu-test/src/fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
                    ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                    !
/tmp/qemu-test/src/fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
            zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                     !
/tmp/qemu-test/src/fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
8 errors generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: fpu/softfloat.o] Error 1
make[1]: *** Waiting for unfinished jobs....
/tmp/qemu-test/src/migration/ram.c:919:45: error: implicit conversion from 'unsigned long' to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion]
            xbzrle_counters.encoding_rate = UINT64_MAX;
                                          ~ ^~~~~~~~~~
/usr/include/stdint.h:130:23: note: expanded from macro 'UINT64_MAX'
---
18446744073709551615UL
^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: migration/ram.o] Error 1
make: *** [Makefile:527: x86_64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 669, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=3e2a057199c44ed9abf9403bb7f6b884', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-wdflktbg/src/docker-src.2020-06-16-18.54.02.22725:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=3e2a057199c44ed9abf9403bb7f6b884
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-wdflktbg/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m30.084s
user    0m8.642s


The full log is available at
http://patchew.org/logs/20200616205721.1191408-1-stefanb@linux.vnet.ibm.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 1/8] tpm_tis: Allow lowering of IRQ also when locality is not active

[Marc-André Lureau]

On Wed, Jun 17, 2020 at 12:57 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> This patch fixes a bug that occurs when using interrupts. It
> allows to lower the IRQ also when a locality is not active.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  hw/tpm/tpm_tis_common.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
> index 1af4bce139..0f42696f1f 100644
> --- a/hw/tpm/tpm_tis_common.c
> +++ b/hw/tpm/tpm_tis_common.c
> @@ -601,10 +601,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
>          /* hard wired -- ignore */
>          break;
>      case TPM_TIS_REG_INT_STATUS:
> -        if (s->active_locty != locty) {
> -            break;
> -        }
> -
>          /* clearing of interrupt flags */
>          if (((val & TPM_TIS_INTERRUPTS_SUPPORTED)) &&
>              (s->loc[locty].ints & TPM_TIS_INTERRUPTS_SUPPORTED)) {
> --
> 2.24.1
>

Re: [PATCH v3 2/8] tpm: Extend TPMIfClass with get_irqnum() function

[Marc-André Lureau]

On Wed, Jun 17, 2020 at 12:57 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>
>
> Implement get_irqnum() as part of the TPMIfClass to get the assigned IRQ
> number or TPM_IRQ_DISABLED (-1) in case IRQs cannot be used.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  hw/tpm/tpm_tis_isa.c    |  9 +++++++++
>  hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
>  include/sysemu/tpm.h    | 12 ++++++++++++
>  3 files changed, 30 insertions(+)
>
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index 30ba37079d..ed6d422f05 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
>      return tpm_tis_get_tpm_version(s);
>  }
>
> +static int8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
> +{
> +    TPMStateISA *isadev = TPM_TIS_ISA(ti);
> +    TPMState *s = &isadev->state;
> +
> +    return s->irq_num;
> +}
> +
>  static void tpm_tis_isa_reset(DeviceState *dev)
>  {
>      TPMStateISA *isadev = TPM_TIS_ISA(dev);
> @@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void *data)
>      dc->reset = tpm_tis_isa_reset;
>      tc->request_completed = tpm_tis_isa_request_completed;
>      tc->get_version = tpm_tis_isa_get_tpm_version;
> +    tc->get_irqnum = tpm_tis_isa_get_irqnum;
>  }
>
>  static const TypeInfo tpm_tis_isa_info = {
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index eced1fc843..86b3988be5 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
>      return tpm_tis_get_tpm_version(s);
>  }
>
> +static int8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
> +{
> +    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
> +    TPMState *s = &sbdev->state;
> +
> +    return s->irq_num;
> +}
> +
>  static void tpm_tis_sysbus_reset(DeviceState *dev)
>  {
>      TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
> @@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>      dc->reset = tpm_tis_sysbus_reset;
>      tc->request_completed = tpm_tis_sysbus_request_completed;
>      tc->get_version = tpm_tis_sysbus_get_tpm_version;
> +    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
>  }
>
>  static const TypeInfo tpm_tis_sysbus_info = {
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 03fb25941c..a81fb4e18e 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -25,6 +25,8 @@ typedef enum TPMVersion {
>      TPM_VERSION_2_0 = 2,
>  } TPMVersion;
>
> +#define TPM_IRQ_DISABLED  -1
> +
>  #define TYPE_TPM_IF "tpm-if"
>  #define TPM_IF_CLASS(klass)                                 \
>      OBJECT_CLASS_CHECK(TPMIfClass, (klass), TYPE_TPM_IF)
> @@ -41,6 +43,7 @@ typedef struct TPMIfClass {
>      enum TpmModel model;
>      void (*request_completed)(TPMIf *obj, int ret);
>      enum TPMVersion (*get_version)(TPMIf *obj);
> +    int8_t (*get_irqnum)(TPMIf *obj);
>  } TPMIfClass;
>
>  #define TYPE_TPM_TIS_ISA            "tpm-tis"
> @@ -74,4 +77,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>      return TPM_IF_GET_CLASS(ti)->get_version(ti);
>  }
>
> +static inline int8_t tpm_get_irqnum(TPMIf *ti)
> +{
> +    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
> +        return TPM_IRQ_DISABLED;
> +    }
> +
> +    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
> +}
> +
>  #endif /* QEMU_TPM_H */
> --
> 2.24.1
>

Re: [PATCH v3 3/8] tests: Temporarily ignore DSDT table differences

[Marc-André Lureau]

On Wed, Jun 17, 2020 at 12:57 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> Ignore DSDT table differences before enabling IRQ support for TPM.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  tests/qtest/bios-tables-test-allowed-diff.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..bb4ce8967b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,2 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT.tis",
> --
> 2.24.1
>

Re: [PATCH v3 4/8] tpm: Split TPM_TIS_IRQ into TPM_TIS_ISA_IRQ and TPM_TIS_SYSBUS_IRQ

[Marc-André Lureau]

On Wed, Jun 17, 2020 at 12:58 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> Before the enablement of interrupts on PCs, split the TPM_TIS_IRQ
> into TPM_TIS_ISA_IRQ for PCs and TPM_TIS_SYSBUS_IRQ for ARM.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  hw/tpm/tpm_tis_isa.c    | 2 +-
>  hw/tpm/tpm_tis_sysbus.c | 3 ++-
>  include/hw/acpi/tpm.h   | 3 ++-
>  3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index ed6d422f05..27222a9a49 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -97,7 +97,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
>  }
>
>  static Property tpm_tis_isa_properties[] = {
> -    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
> +    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_ISA_IRQ),
>      DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
>      DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index 86b3988be5..bf4583c3f6 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -97,7 +97,8 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
>  }
>
>  static Property tpm_tis_sysbus_properties[] = {
> -    DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ),
> +    DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num,
> +                       TPM_TIS_SYSBUS_IRQ),
>      DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
>      DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index 1a2a57a21f..d5caee9771 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -24,7 +24,8 @@
>  #define TPM_TIS_ADDR_BASE           0xFED40000
>  #define TPM_TIS_ADDR_SIZE           0x5000
>
> -#define TPM_TIS_IRQ                 5
> +#define TPM_TIS_ISA_IRQ             5
> +#define TPM_TIS_SYSBUS_IRQ          5
>
>  #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
>  #define TPM_TIS_LOCALITY_SHIFT      12
> --
> 2.24.1
>

Re: [PATCH v3 8/8] tpm: Disable interrupt support for TIS on sysbus

[Marc-André Lureau]

On Wed, Jun 17, 2020 at 12:58 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> Disable interrupt support for the TIS on sysbus.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  include/hw/acpi/tpm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index d356f2e06e..21f81690a5 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -25,7 +25,7 @@
>  #define TPM_TIS_ADDR_SIZE           0x5000
>
>  #define TPM_TIS_ISA_IRQ             13    /* only one possible */
> -#define TPM_TIS_SYSBUS_IRQ          5
> +#define TPM_TIS_SYSBUS_IRQ          TPM_IRQ_DISABLED
>
>  #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
>  #define TPM_TIS_LOCALITY_SHIFT      12
> --
> 2.24.1
>

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

[Marc-André Lureau]

On Wed, Jun 17, 2020 at 12:57 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> 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>

Reviewed-by: Marc-André Lureau <marcandre.lureau@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) {
> +                    aml_append(crs, aml_irq_no_flags(irq));
> +                }
>                  aml_append(dev, aml_name_decl("_CRS", crs));
>
>                  tpm_build_ppi_acpi(tpm, dev);
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index d5caee9771..d356f2e06e 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -24,7 +24,7 @@
>  #define TPM_TIS_ADDR_BASE           0xFED40000
>  #define TPM_TIS_ADDR_SIZE           0x5000
>
> -#define TPM_TIS_ISA_IRQ             5
> +#define TPM_TIS_ISA_IRQ             13    /* only one possible */
>  #define TPM_TIS_SYSBUS_IRQ          5
>
>  #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
> --
> 2.24.1
>

Re: [PATCH v3 6/8] tests: Add updated DSDT

[Marc-André Lureau]

On Wed, Jun 17, 2020 at 12:57 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> Add the updated DSDT following the interrupt enablement.
>
> @@ -5,13 +5,13 @@
>   *
>   * Disassembling to symbolic ASL+ operators
>   *
> - * Disassembly of tests/data/acpi/q35/DSDT.tis, Mon Jun 15 09:57:05 2020
> + * Disassembly of /tmp/aml-Y77YL0, Mon Jun 15 09:57:05 2020
>   *
>   * Original Table Header:
>   *     Signature        "DSDT"
> - *     Length           0x000020A5 (8357)
> + *     Length           0x000020A8 (8360)
>   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math support
> - *     Checksum         0xAD
> + *     Checksum         0x77
>   *     OEM ID           "BOCHS "
>   *     OEM Table ID     "BXPCDSDT"
>   *     OEM Revision     0x00000001 (1)
> @@ -3162,6 +3162,8 @@
>                          0xFED40000,         // Address Base
>                          0x00005000,         // Address Length
>                          )
> +                    IRQNoFlags ()
> +                        {13}
>                  })
>                  OperationRegion (TPP2, SystemMemory, 0xFED45100, 0x5A)
>                  Field (TPP2, AnyAcc, NoLock, Preserve)
> **
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> CC: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  tests/data/acpi/q35/DSDT.tis                | Bin 8357 -> 8360 bytes
>  tests/qtest/bios-tables-test-allowed-diff.h |   1 -
>  2 files changed, 1 deletion(-)
>
> diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
> index 56b6fb0c3298517d080e38fea05a748b9f1dba54..3f9db960aa05d399fa7f8449e6db688788211832 100644
> GIT binary patch
> delta 64
> zcmZ4LxWbXkCD<iog#rTuWBEp|KeC)oS~2m#PVoX>llkS`nVeK7N60A%iEs(FaWXJ6
> UFkJb^5Wv8o#GtUbT~3Y(068!Z;Q#;t
>
> delta 61
> zcmZ4CxYUu$CD<iosR9E7<Jyf}e`GoRHDls~o#F-DC-cj>Gx@7bj*wH}7v$n=<78lD
> RV7T&+A%KBlbC;YP695=#58(g+
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index bb4ce8967b..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,2 +1 @@
>  /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/q35/DSDT.tis",
> --
> 2.24.1
>

Re: [PATCH v3 4/8] tpm: Split TPM_TIS_IRQ into TPM_TIS_ISA_IRQ and TPM_TIS_SYSBUS_IRQ

[Auger Eric]

Hi Stefan,

On 6/16/20 10:57 PM, Stefan Berger wrote:
> Before the enablement of interrupts on PCs, split the TPM_TIS_IRQ
> into TPM_TIS_ISA_IRQ for PCs and TPM_TIS_SYSBUS_IRQ for ARM.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  hw/tpm/tpm_tis_isa.c    | 2 +-
>  hw/tpm/tpm_tis_sysbus.c | 3 ++-
>  include/hw/acpi/tpm.h   | 3 ++-
>  3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index ed6d422f05..27222a9a49 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -97,7 +97,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
>  }
>  
>  static Property tpm_tis_isa_properties[] = {
> -    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
> +    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_ISA_IRQ),
irq_num is a uint32 prop while the default value we are going to assign
is -1. I guess we will end up with irq_num = 255?

Thanks

Eric
>      DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
>      DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index 86b3988be5..bf4583c3f6 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -97,7 +97,8 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
>  }
>  
>  static Property tpm_tis_sysbus_properties[] = {
> -    DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ),
> +    DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num,
> +                       TPM_TIS_SYSBUS_IRQ),
>      DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
>      DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index 1a2a57a21f..d5caee9771 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -24,7 +24,8 @@
>  #define TPM_TIS_ADDR_BASE           0xFED40000
>  #define TPM_TIS_ADDR_SIZE           0x5000
>  
> -#define TPM_TIS_IRQ                 5
> +#define TPM_TIS_ISA_IRQ             5
> +#define TPM_TIS_SYSBUS_IRQ          5
>  
>  #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
>  #define TPM_TIS_LOCALITY_SHIFT      12
> 

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

[Auger Eric]

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?

Thanks

Eric
> +                    aml_append(crs, aml_irq_no_flags(irq));
> +                }
>                  aml_append(dev, aml_name_decl("_CRS", crs));
>  
>                  tpm_build_ppi_acpi(tpm, dev);
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index d5caee9771..d356f2e06e 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -24,7 +24,7 @@
>  #define TPM_TIS_ADDR_BASE           0xFED40000
>  #define TPM_TIS_ADDR_SIZE           0x5000
>  
> -#define TPM_TIS_ISA_IRQ             5
> +#define TPM_TIS_ISA_IRQ             13    /* only one possible */
>  #define TPM_TIS_SYSBUS_IRQ          5
>  
>  #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
> 

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

Re: [PATCH v3 4/8] tpm: Split TPM_TIS_IRQ into TPM_TIS_ISA_IRQ and TPM_TIS_SYSBUS_IRQ

[Stefan Berger]

On 6/17/20 4:12 AM, Auger Eric wrote:
> Hi Stefan,
>
> On 6/16/20 10:57 PM, Stefan Berger wrote:
>> Before the enablement of interrupts on PCs, split the TPM_TIS_IRQ
>> into TPM_TIS_ISA_IRQ for PCs and TPM_TIS_SYSBUS_IRQ for ARM.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   hw/tpm/tpm_tis_isa.c    | 2 +-
>>   hw/tpm/tpm_tis_sysbus.c | 3 ++-
>>   include/hw/acpi/tpm.h   | 3 ++-
>>   3 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
>> index ed6d422f05..27222a9a49 100644
>> --- a/hw/tpm/tpm_tis_isa.c
>> +++ b/hw/tpm/tpm_tis_isa.c
>> @@ -97,7 +97,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
>>   }
>>   
>>   static Property tpm_tis_isa_properties[] = {
>> -    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
>> +    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_ISA_IRQ),
> irq_num is a uint32 prop while the default value we are going to assign
> is -1. I guess we will end up with irq_num = 255?

We can make it ~0, if that's better. Otherwise it does seem to become 
0xff ff ff ff with the current -1.


    Stefan

Re: [PATCH v3 4/8] tpm: Split TPM_TIS_IRQ into TPM_TIS_ISA_IRQ and TPM_TIS_SYSBUS_IRQ

[Auger Eric]

Hi Stefan,

On 6/17/20 2:06 PM, Stefan Berger wrote:
> On 6/17/20 4:12 AM, Auger Eric wrote:
>> Hi Stefan,
>>
>> On 6/16/20 10:57 PM, Stefan Berger wrote:
>>> Before the enablement of interrupts on PCs, split the TPM_TIS_IRQ
>>> into TPM_TIS_ISA_IRQ for PCs and TPM_TIS_SYSBUS_IRQ for ARM.
>>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>> ---
>>>   hw/tpm/tpm_tis_isa.c    | 2 +-
>>>   hw/tpm/tpm_tis_sysbus.c | 3 ++-
>>>   include/hw/acpi/tpm.h   | 3 ++-
>>>   3 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
>>> index ed6d422f05..27222a9a49 100644
>>> --- a/hw/tpm/tpm_tis_isa.c
>>> +++ b/hw/tpm/tpm_tis_isa.c
>>> @@ -97,7 +97,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
>>>   }
>>>     static Property tpm_tis_isa_properties[] = {
>>> -    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
>>> +    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num,
>>> TPM_TIS_ISA_IRQ),
>> irq_num is a uint32 prop while the default value we are going to assign
>> is -1. I guess we will end up with irq_num = 255?
> 
> We can make it ~0, if that's better. Otherwise it does seem to become
> 0xff ff ff ff with the current -1.
yes sorry that's what I meant.

Eric
> 
> 
>    Stefan
> 

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

[Michael S. Tsirkin]

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.
I'm guessing sb16 has it since it is useful for running extremely old OSes which might
have weird quirks for a specific hardware.

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

[Stefan Berger]

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.
>