tpm: tpm_tis: Disable interrupts for AEON UPX-i11

PATCH In review Peter Ujfalusi 3 participants 10 messages May 17, 2023 – May 24, 2023

Subsystems (2)

Keys-trusted TPM device driver

lore

Share & tools

lore

Peter Ujfalusi

author

2023-05-17 12:28 UTC #0 lore raw

PATCH 1/1 tpm: tpm_tis: Disable interrupts for AEON UPX-i11

The interrupts initially works on the device but they will stop arriving after about 200 interrupts.

On system reboot/shutdown this will cause a long wait (120000 jiffies).

The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")

Prior to this point the interrupts were not enabled on this machine.

Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")

Signed-off-by Peter Ujfalusi <[email protected]>

---
Hi,

This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices

I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.

Regards,
Peter

 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/char/tpm/tpm_tis.c +7 -0

index 7af389806643..aad682c2ab21 100644
@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
+		},
+	},
 	{}
 };

Signature · 1 line

2.40.1

Péter Ujfalusi

author

2023-05-17 13:15 UTC #1 lore raw

Quoted 15 lines · Peter Ujfalusi

The interrupts initially works on the device but they will stop arriving after about 200 interrupts.
On system reboot/shutdown this will cause a long wait (120000 jiffies).
The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Prior to this point the interrupts were not enabled on this machine.
Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Signed-off-by Peter Ujfalusi <[email protected]>
---
Hi,

Quoted 6 lines

This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices
I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.

In 6.3 the kernel prints this on boot:

# dmesg -w | grep tpm
tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1B, rev-id 22)
tpm tpm0: [Firmware Bug]: TPM interrupt not working, polling instead

It is interesting that with 6.4 most of the times the interrupts got enabled (without this patch) resulting stall during reboot/shutdown but there are few boots when the driver falls back to polling and thus the TPM driver works.

The command which 'locks' the system is TPM2_CC_SHUTDOWN, it is given TPM_UNDEFINED as duration index by tpm2_ordinal_duration_index().

Quoted 25 lines

Regards,
Peter

 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/char/tpm/tpm_tis.c +7 -0

index 7af389806643..aad682c2ab21 100644
@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
+		},
+	},
 	{}
 };

Signature · 1 line

Péter

Jarkko Sakkinen

maintainer Reviewed-by on v1 1/1

2023-05-18 18:38 UTC #2 lore raw

In reply to Peter Ujfalusi

Quoted 11 lines · Peter Ujfalusi

The interrupts initially works on the device but they will stop arriving after about 200 interrupts.
On system reboot/shutdown this will cause a long wait (120000 jiffies).
The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Prior to this point the interrupts were not enabled on this machine.
Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")

Complements -> Fixes

Quoted 37 lines

Signed-off-by Peter Ujfalusi <[email protected]>
---
Hi,
This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices
I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.
Regards,
Peter
 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_tis.c +7 -0
index 7af389806643..aad682c2ab21 100644
@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
+		},
+	},
 	{}
 };
 
Signature · 1 line
2.40.1

Reviewed-by Jarkko Sakkinen <[email protected]>

BR, Jarkko

Jarkko Sakkinen

maintainer

2023-05-18 18:47 UTC #3 lore raw

Quoted 50 lines · Jarkko Sakkinen

Quoted 11 lines · Peter Ujfalusi
The interrupts initially works on the device but they will stop arriving after about 200 interrupts.
On system reboot/shutdown this will cause a long wait (120000 jiffies).
The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Prior to this point the interrupts were not enabled on this machine.
Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Complements -> Fixes
Quoted 35 lines
Signed-off-by Peter Ujfalusi <[email protected]>
---
Hi,
This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices
I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.
Regards,
Peter
 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_tis.c +7 -0
index 7af389806643..aad682c2ab21 100644
@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
+		},
+	},
 	{}
 };
 

Quoted 4 lines

Quoted 2 lines
Signature · 1 line
2.40.1
Reviewed-by Jarkko Sakkinen <[email protected]>

I adjusted the commit message a bit:

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=d46f47575cf97d397fbe8a6150353f41d2917936

BR, Jarkko

Jerry Snitselaar

2023-05-18 18:50 UTC #4 lore raw

In reply to Peter Ujfalusi

Quoted 33 lines · Peter Ujfalusi

The interrupts initially works on the device but they will stop arriving after about 200 interrupts.
On system reboot/shutdown this will cause a long wait (120000 jiffies).
The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Prior to this point the interrupts were not enabled on this machine.
Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Signed-off-by Peter Ujfalusi <[email protected]>
---
Hi,
This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices
I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.
Regards,
Peter
 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_tis.c +0 -0
index 7af389806643..aad682c2ab21 100644

drivers/char/tpm/tpm_tis.c static const struct dmi_system_id tpm_tis_dmi_table[] = { · line 127

Quoted 6 lines

drivers/char/tpm/tpm_tis.c +3 -0

@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",

Is this the product version returned by dmidecode? If yes, then the entry could be made more specific by adding a DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable for this device instead of any system that matches the vendor AAEON.

Quoted 10 lines

drivers/char/tpm/tpm_tis.c +4 -0
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
+		},
+	},
 	{}
 };
 
Signature · 1 line
2.40.1

Jarkko Sakkinen

maintainer

2023-05-18 18:53 UTC #5 lore raw

Quoted 38 lines · Jerry Snitselaar

Quoted 37 lines · Peter Ujfalusi
The interrupts initially works on the device but they will stop arriving after about 200 interrupts.
On system reboot/shutdown this will cause a long wait (120000 jiffies).
The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Prior to this point the interrupts were not enabled on this machine.
Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Signed-off-by Peter Ujfalusi <[email protected]>
---
Hi,
This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices
I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.
Regards,
Peter
 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_tis.c +1 -0
index 7af389806643..aad682c2ab21 100644
@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{

Quoted 8 lines

drivers/char/tpm/tpm_tis.c static const struct dmi_system_id tpm_tis_dmi_table[] = { · line 127
Quoted 2 lines
drivers/char/tpm/tpm_tis.c +2 -0
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",
Is this the product version returned by dmidecode? If yes, then the entry could be made more specific by adding a DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable for this device instead of any system that matches the vendor AAEON.

I can squash this to the commit I pushed (it is not yet mirrored to linux-next), if I get the dmidecode info.

BR, Jarkko

Péter Ujfalusi

author

2023-05-18 20:23 UTC #6 lore raw

Quoted 39 lines · Jarkko Sakkinen

Quoted 38 lines · Jerry Snitselaar
Quoted 37 lines · Peter Ujfalusi
The interrupts initially works on the device but they will stop arriving after about 200 interrupts.
On system reboot/shutdown this will cause a long wait (120000 jiffies).
The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Prior to this point the interrupts were not enabled on this machine.
Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Signed-off-by Peter Ujfalusi <[email protected]>
---
Hi,
This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices
I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.
Regards,
Peter
 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_tis.c +1 -0
index 7af389806643..aad682c2ab21 100644
@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{

Quoted 8 lines

Quoted 8 lines
drivers/char/tpm/tpm_tis.c static const struct dmi_system_id tpm_tis_dmi_table[] = { · line 127
Quoted 2 lines
drivers/char/tpm/tpm_tis.c +2 -0
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",
Is this the product version returned by dmidecode? If yes, then the entry could be made more specific by adding a DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable for this device instead of any system that matches the vendor AAEON.

The version is used to differentiate the revisions of the UPX-i11 boards, and this issue present in all revisions.

Quoted 2 lines

I can squash this to the commit I pushed (it is not yet mirrored to linux-next), if I get the dmidecode info.

System Information
Manufacturer: AAEON
Product Name: UPX-TGL01
Version: V1.0
Serial Number: Default string
UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 Wake-up Type: Power Switch
SKU Number: Default string
Family: Default string

I have used this description as it it is used for SOF, probably DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01")
should be added?

Oh, yes, the product name match should be there, we have TigerLake specific matching, so SOF is looking for AAEON device with TGL.

Sorry for missing this.

Quoted 2 lines

BR, Jarkko

Signature · 1 line

Péter

Péter Ujfalusi

author

2023-05-22 07:39 UTC #7 lore raw

Quoted 17 lines · Péter Ujfalusi

The version is used to differentiate the revisions of the UPX-i11 boards, and this issue present in all revisions.
Quoted 2 lines
I can squash this to the commit I pushed (it is not yet mirrored to linux-next), if I get the dmidecode info.
System Information
Manufacturer: AAEON
Product Name: UPX-TGL01
Version: V1.0
Serial Number: Default string
UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 Wake-up Type: Power Switch
SKU Number: Default string
Family: Default string

Quoted 3 lines

I have used this description as it it is used for SOF, probably DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01")
should be added?

Jarkko: I have tested that adding the
DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01")
works.

I would also do a small update to commit message: "120000 jiffies" to "120000 msec".
On my setup 120000 msec ends up to be 120000 jiffies.

How do you prefer to handle this?
I can send a v2 on top of linux-next / mainline
I can send a fixup patch which can be squashed to the patch you have in your master branch atm
Or you add this line by yourself?

Either way is fine for me, whichever works best for you.

Signature · 1 line

Péter

Jarkko Sakkinen

maintainer

2023-05-24 02:50 UTC #8 lore raw

Quoted 31 lines · Péter Ujfalusi

Quoted 17 lines · Péter Ujfalusi
The version is used to differentiate the revisions of the UPX-i11 boards, and this issue present in all revisions.
Quoted 2 lines
I can squash this to the commit I pushed (it is not yet mirrored to linux-next), if I get the dmidecode info.
System Information
Manufacturer: AAEON
Product Name: UPX-TGL01
Version: V1.0
Serial Number: Default string
UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 Wake-up Type: Power Switch
SKU Number: Default string
Family: Default string
Quoted 3 lines
I have used this description as it it is used for SOF, probably DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01")
should be added?
Jarkko: I have tested that adding the
DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01")
works.
I would also do a small update to commit message: "120000 jiffies" to "120000 msec".
On my setup 120000 msec ends up to be 120000 jiffies.

Quoted 7 lines

How do you prefer to handle this?
I can send a v2 on top of linux-next / mainline
I can send a fixup patch which can be squashed to the patch you have in your master branch atm
Or you add this line by yourself?
Either way is fine for me, whichever works best for you.

If you want to send a follow-up please do, and I can pick it.

BR, Jarkko

Jarkko Sakkinen

maintainer

2023-05-24 02:25 UTC #9 lore raw

In reply to Péter Ujfalusi

Quoted 66 lines · Péter Ujfalusi

Quoted 39 lines · Jarkko Sakkinen
Quoted 38 lines · Jerry Snitselaar
Quoted 37 lines · Peter Ujfalusi
The interrupts initially works on the device but they will stop arriving after about 200 interrupts.
On system reboot/shutdown this will cause a long wait (120000 jiffies).
The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Prior to this point the interrupts were not enabled on this machine.
Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Signed-off-by Peter Ujfalusi <[email protected]>
---
Hi,
This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have
e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices
I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4.
Regards,
Peter
 drivers/char/tpm/tpm_tis.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/char/tpm/tpm_tis.c +1 -0
index 7af389806643..aad682c2ab21 100644
@@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
 		},
 	},
+	{
Quoted 8 lines
Quoted 8 lines
drivers/char/tpm/tpm_tis.c static const struct dmi_system_id tpm_tis_dmi_table[] = { · line 127
Quoted 2 lines
drivers/char/tpm/tpm_tis.c +2 -0
+		.callback = tpm_tis_disable_irq,
+		.ident = "UPX-TGL",
Is this the product version returned by dmidecode? If yes, then the entry could be made more specific by adding a DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable for this device instead of any system that matches the vendor AAEON.
The version is used to differentiate the revisions of the UPX-i11 boards, and this issue present in all revisions.
Quoted 2 lines
I can squash this to the commit I pushed (it is not yet mirrored to linux-next), if I get the dmidecode info.
System Information
Manufacturer: AAEON
Product Name: UPX-TGL01
Version: V1.0
Serial Number: Default string
UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 Wake-up Type: Power Switch
SKU Number: Default string
Family: Default string

Quoted 8 lines

I have used this description as it it is used for SOF, probably DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01")
should be added?
Oh, yes, the product name match should be there, we have TigerLake specific matching, so SOF is looking for AAEON device with TGL.
Sorry for missing this.

I sent a PR with the original patch because I know that you've tested it and it has worked for you.

BR, Jarkko