* [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s"
@ 2023-08-14 16:40 Jarkko Sakkinen
2023-08-14 20:00 ` Randy Dunlap
2023-08-14 23:13 ` Jerry Snitselaar
0 siblings, 2 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2023-08-14 16:40 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan Corbet, Peter Huewe, Jason Gunthorpe,
Richard Cochran, Paul E. McKenney, Catalin Marinas, Randy Dunlap,
Dave Hansen, Steven Rostedt (Google), Daniel Sneddon, linux-doc,
linux-kernel, netdev
Since for MMIO driver using FIFO registers, also known as tpm_tis, the
default (and tbh recommended) behaviour is now the polling mode, the
"tristate" workaround is no longer for benefit.
If someone wants to explicitly enable IRQs for a TPM chip that should be
without question allowed. It could very well be a piece hardware in the
existing deny list because of e.g. firmware update or something similar.
While at it, document the module parameter, as this was not done in 2006
when it first appeared in the mainline.
Link: https://lore.kernel.org/linux-integrity/20201015214430.17937-1-jsnitsel@redhat.com/
Link: https://lore.kernel.org/all/1145393776.4829.19.camel@localhost.localdomain/
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
.../admin-guide/kernel-parameters.txt | 7 ++
drivers/char/tpm/tpm_tis.c | 93 +------------------
2 files changed, 9 insertions(+), 91 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 722b6eca2e93..6354aa779178 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6340,6 +6340,13 @@
This will guarantee that all the other pcrs
are saved.
+ tpm_tis.interrupts= [HW,TPM]
+ Enable interrupts for the MMIO based physical layer
+ for the FIFO interface. By default it is set to false
+ (0). For more information about TPM hardware interfaces
+ defined by Trusted Computing Group (TCG) look up to
+ https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
+
tp_printk [FTRACE]
Have the tracepoints sent to printk as well as the
tracing ring buffer. This is useful for early boot up
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 7fa3d91042b2..077fdb73740c 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -27,7 +27,6 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/kernel.h>
-#include <linux/dmi.h>
#include "tpm.h"
#include "tpm_tis_core.h"
@@ -89,8 +88,8 @@ static inline void tpm_tis_iowrite32(u32 b, void __iomem *iobase, u32 addr)
tpm_tis_flush(iobase);
}
-static int interrupts;
-module_param(interrupts, int, 0444);
+static bool interrupts;
+module_param(interrupts, bool, 0444);
MODULE_PARM_DESC(interrupts, "Enable interrupts");
static bool itpm;
@@ -103,92 +102,6 @@ module_param(force, bool, 0444);
MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry");
#endif
-static int tpm_tis_disable_irq(const struct dmi_system_id *d)
-{
- if (interrupts == -1) {
- pr_notice("tpm_tis: %s detected: disabling interrupts.\n", d->ident);
- interrupts = 0;
- }
-
- return 0;
-}
-
-static const struct dmi_system_id tpm_tis_dmi_table[] = {
- {
- .callback = tpm_tis_disable_irq,
- .ident = "Framework Laptop (12th Gen Intel Core)",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (12th Gen Intel Core)"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "Framework Laptop (13th Gen Intel Core)",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (13th Gen Intel Core)"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "ThinkPad T490s",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "ThinkStation P360 Tiny",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkStation P360 Tiny"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "ThinkPad L490",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad L490"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "ThinkPad L590",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad L590"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "ThinkStation P620",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkStation P620"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "TUXEDO InfinityBook S 15/17 Gen7",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
- DMI_MATCH(DMI_PRODUCT_NAME, "TUXEDO InfinityBook S 15/17 Gen7"),
- },
- },
- {
- .callback = tpm_tis_disable_irq,
- .ident = "UPX-TGL",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
- DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01"),
- },
- },
- {}
-};
-
#if defined(CONFIG_PNP) && defined(CONFIG_ACPI)
static int has_hid(struct acpi_device *dev, const char *hid)
{
@@ -312,8 +225,6 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info)
int irq = -1;
int rc;
- dmi_check_system(tpm_tis_dmi_table);
-
rc = check_acpi_tpm2(dev);
if (rc)
return rc;
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s"
2023-08-14 16:40 [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s" Jarkko Sakkinen
@ 2023-08-14 20:00 ` Randy Dunlap
2023-08-16 20:25 ` Jarkko Sakkinen
2023-08-14 23:13 ` Jerry Snitselaar
1 sibling, 1 reply; 5+ messages in thread
From: Randy Dunlap @ 2023-08-14 20:00 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: Jonathan Corbet, Peter Huewe, Jason Gunthorpe, Richard Cochran,
Paul E. McKenney, Catalin Marinas, Dave Hansen,
Steven Rostedt (Google), Daniel Sneddon, linux-doc, linux-kernel,
netdev
Hi Jarkko,
On 8/14/23 09:40, Jarkko Sakkinen wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 722b6eca2e93..6354aa779178 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6340,6 +6340,13 @@
> This will guarantee that all the other pcrs
> are saved.
>
> + tpm_tis.interrupts= [HW,TPM]
> + Enable interrupts for the MMIO based physical layer
> + for the FIFO interface. By default it is set to false
> + (0). For more information about TPM hardware interfaces
> + defined by Trusted Computing Group (TCG) look up to
s/look up to/see/ would be much better IMO.
> + https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
> +
--
~Randy
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s"
2023-08-14 16:40 [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s" Jarkko Sakkinen
2023-08-14 20:00 ` Randy Dunlap
@ 2023-08-14 23:13 ` Jerry Snitselaar
2023-08-16 20:16 ` Jarkko Sakkinen
1 sibling, 1 reply; 5+ messages in thread
From: Jerry Snitselaar @ 2023-08-14 23:13 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Jonathan Corbet, Peter Huewe, Jason Gunthorpe,
Richard Cochran, Paul E. McKenney, Catalin Marinas, Randy Dunlap,
Dave Hansen, Steven Rostedt (Google), Daniel Sneddon, linux-doc,
linux-kernel, netdev
On Mon, Aug 14, 2023 at 07:40:53PM +0300, Jarkko Sakkinen wrote:
> Since for MMIO driver using FIFO registers, also known as tpm_tis, the
> default (and tbh recommended) behaviour is now the polling mode, the
> "tristate" workaround is no longer for benefit.
>
> If someone wants to explicitly enable IRQs for a TPM chip that should be
> without question allowed. It could very well be a piece hardware in the
> existing deny list because of e.g. firmware update or something similar.
>
> While at it, document the module parameter, as this was not done in 2006
> when it first appeared in the mainline.
>
> Link: https://lore.kernel.org/linux-integrity/20201015214430.17937-1-jsnitsel@redhat.com/
> Link: https://lore.kernel.org/all/1145393776.4829.19.camel@localhost.localdomain/
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
I was just typing an email to say that it looks like 6aaf663ee04a ("tpm_tis: Opt-in interrupts") will require
updating tpm_tis_disable_irq(), but you are already dealing with it. :)
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
> .../admin-guide/kernel-parameters.txt | 7 ++
> drivers/char/tpm/tpm_tis.c | 93 +------------------
> 2 files changed, 9 insertions(+), 91 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 722b6eca2e93..6354aa779178 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6340,6 +6340,13 @@
> This will guarantee that all the other pcrs
> are saved.
>
> + tpm_tis.interrupts= [HW,TPM]
> + Enable interrupts for the MMIO based physical layer
> + for the FIFO interface. By default it is set to false
> + (0). For more information about TPM hardware interfaces
> + defined by Trusted Computing Group (TCG) look up to
> + https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
> +
> tp_printk [FTRACE]
> Have the tracepoints sent to printk as well as the
> tracing ring buffer. This is useful for early boot up
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 7fa3d91042b2..077fdb73740c 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -27,7 +27,6 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/kernel.h>
> -#include <linux/dmi.h>
> #include "tpm.h"
> #include "tpm_tis_core.h"
>
> @@ -89,8 +88,8 @@ static inline void tpm_tis_iowrite32(u32 b, void __iomem *iobase, u32 addr)
> tpm_tis_flush(iobase);
> }
>
> -static int interrupts;
> -module_param(interrupts, int, 0444);
> +static bool interrupts;
> +module_param(interrupts, bool, 0444);
> MODULE_PARM_DESC(interrupts, "Enable interrupts");
>
> static bool itpm;
> @@ -103,92 +102,6 @@ module_param(force, bool, 0444);
> MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry");
> #endif
>
> -static int tpm_tis_disable_irq(const struct dmi_system_id *d)
> -{
> - if (interrupts == -1) {
> - pr_notice("tpm_tis: %s detected: disabling interrupts.\n", d->ident);
> - interrupts = 0;
> - }
> -
> - return 0;
> -}
> -
> -static const struct dmi_system_id tpm_tis_dmi_table[] = {
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "Framework Laptop (12th Gen Intel Core)",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (12th Gen Intel Core)"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "Framework Laptop (13th Gen Intel Core)",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (13th Gen Intel Core)"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "ThinkPad T490s",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "ThinkStation P360 Tiny",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkStation P360 Tiny"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "ThinkPad L490",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad L490"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "ThinkPad L590",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad L590"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "ThinkStation P620",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkStation P620"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "TUXEDO InfinityBook S 15/17 Gen7",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "TUXEDO InfinityBook S 15/17 Gen7"),
> - },
> - },
> - {
> - .callback = tpm_tis_disable_irq,
> - .ident = "UPX-TGL",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01"),
> - },
> - },
> - {}
> -};
> -
> #if defined(CONFIG_PNP) && defined(CONFIG_ACPI)
> static int has_hid(struct acpi_device *dev, const char *hid)
> {
> @@ -312,8 +225,6 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info)
> int irq = -1;
> int rc;
>
> - dmi_check_system(tpm_tis_dmi_table);
> -
> rc = check_acpi_tpm2(dev);
> if (rc)
> return rc;
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s"
2023-08-14 23:13 ` Jerry Snitselaar
@ 2023-08-16 20:16 ` Jarkko Sakkinen
0 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2023-08-16 20:16 UTC (permalink / raw)
To: Jerry Snitselaar
Cc: linux-integrity, Jonathan Corbet, Peter Huewe, Jason Gunthorpe,
Richard Cochran, Paul E. McKenney, Catalin Marinas, Randy Dunlap,
Dave Hansen, Steven Rostedt (Google), Daniel Sneddon, linux-doc,
linux-kernel, netdev
On Tue Aug 15, 2023 at 2:13 AM EEST, Jerry Snitselaar wrote:
> On Mon, Aug 14, 2023 at 07:40:53PM +0300, Jarkko Sakkinen wrote:
> > Since for MMIO driver using FIFO registers, also known as tpm_tis, the
> > default (and tbh recommended) behaviour is now the polling mode, the
> > "tristate" workaround is no longer for benefit.
> >
> > If someone wants to explicitly enable IRQs for a TPM chip that should be
> > without question allowed. It could very well be a piece hardware in the
> > existing deny list because of e.g. firmware update or something similar.
> >
> > While at it, document the module parameter, as this was not done in 2006
> > when it first appeared in the mainline.
> >
> > Link: https://lore.kernel.org/linux-integrity/20201015214430.17937-1-jsnitsel@redhat.com/
> > Link: https://lore.kernel.org/all/1145393776.4829.19.camel@localhost.localdomain/
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> I was just typing an email to say that it looks like 6aaf663ee04a ("tpm_tis: Opt-in interrupts") will require
> updating tpm_tis_disable_irq(), but you are already dealing with it. :)
>
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Thanks!
BR, Jarkko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s"
2023-08-14 20:00 ` Randy Dunlap
@ 2023-08-16 20:25 ` Jarkko Sakkinen
0 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2023-08-16 20:25 UTC (permalink / raw)
To: Randy Dunlap, linux-integrity
Cc: Jonathan Corbet, Peter Huewe, Jason Gunthorpe, Richard Cochran,
Paul E. McKenney, Catalin Marinas, Dave Hansen,
Steven Rostedt (Google), Daniel Sneddon, linux-doc, linux-kernel,
netdev
On Mon Aug 14, 2023 at 11:00 PM EEST, Randy Dunlap wrote:
> Hi Jarkko,
>
> On 8/14/23 09:40, Jarkko Sakkinen wrote:
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 722b6eca2e93..6354aa779178 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -6340,6 +6340,13 @@
> > This will guarantee that all the other pcrs
> > are saved.
> >
> > + tpm_tis.interrupts= [HW,TPM]
> > + Enable interrupts for the MMIO based physical layer
> > + for the FIFO interface. By default it is set to false
> > + (0). For more information about TPM hardware interfaces
> > + defined by Trusted Computing Group (TCG) look up to
>
> s/look up to/see/ would be much better IMO.
Agreed, thanks.
BR, Jarkko
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-16 20:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-14 16:40 [PATCH] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s" Jarkko Sakkinen
2023-08-14 20:00 ` Randy Dunlap
2023-08-16 20:25 ` Jarkko Sakkinen
2023-08-14 23:13 ` Jerry Snitselaar
2023-08-16 20:16 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).