Re: [ 11/15] ahci: Add identifiers for ASM106x devices
On Thu Jan 24 13, Greg Kroah-Hartman wrote:
> 3.0-stable review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Alan Cox
>
> commit 7b4f6ecacb14f384adc1a5a67ad95eb082c02bd1 upstream.
>
> They don't always appear as AHCI class devices but instead as IDE class.
>
> Based on an initial patch by Hiroaki Nito
>
> Resolves-bug: https://bugzilla.kernel.org/show_bug.cgi?id=42804
> Signed-off-by: Alan Cox
> Signed-off-by: Jeff Garzik
> Signed-off-by: Abdallah Chatila
> Signed-off-by: Greg Kroah-Hartman
>
> ---
> drivers/ata/ahci.c |6 ++
> include/linux/pci_ids.h |2 ++
> 2 files changed, 8 insertions(+)
>
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -402,6 +402,12 @@ static const struct pci_device_id ahci_p
> /* Promise */
> { PCI_VDEVICE(PROMISE, 0x3f20), board_ahci }, /* PDC42819 */
>
> + /* Asmedia */
> + { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */
> + { PCI_VDEVICE(ASMEDIA, 0x0602), board_ahci }, /* ASM1060 */
> + { PCI_VDEVICE(ASMEDIA, 0x0611), board_ahci }, /* ASM1061 */
> + { PCI_VDEVICE(ASMEDIA, 0x0612), board_ahci }, /* ASM1062 */
> +
> /* Generic, PCI class code for AHCI */
> { PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> PCI_CLASS_STORAGE_SATA_AHCI, 0xff, board_ahci },
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2390,6 +2390,8 @@
>
> #define PCI_VENDOR_ID_AZWAVE 0x1a3b
>
> +#define PCI_VENDOR_ID_ASMEDIA0x1b21
> +
> #define PCI_VENDOR_ID_TEKRAM 0x1de1
> #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
There is a whitespace error in this patch:
Applying: ahci: Add identifiers for ASM106x devices
/root/linux/linux/.git/rebase-apply/patch:12: space before tab in indent.
/* Asmedia */
warning: 1 line adds whitespace errors.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
section mismatch for acpi_unmap_lsapic()
Commit 13ad20c1 "x86 cpu_hotplug: unmap cpu2node when the cpu is hotremoved" in linux-next added code to acpi_unmap_lsapic() that causes section mismatch warnings: WARNING: vmlinux.o(.text+0x694f2): Section mismatch in reference from the function acpi_unmap_lsapic() to the function .cpuinit.text:numa_clear_node() WARNING: vmlinux.o(.text+0x694eb): Section mismatch in reference from the function acpi_unmap_lsapic() to the variable .cpuinit.data:__apicid_to_node Does acpi_unmap_lsapic() need a wrapper like the one that was made for acpi_map_lsapic() or can it just be annotated __ref ? I guess my question is would be there be a reason that the wrapper was created for acpi_map_lsapic() instead of just annotating __ref besides allowing the code for _apic_map_lsapic() to be dropped when HOTPLUG_CPU wasn't configured? Jerry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] watchdog: watchdog-test: make term() static
In 3.5 warning during build 'no previous prototype for term'. Since it
is only used in watchdog-test.c make term() static.
Signed-off-by: Jerry Snitselaar
---
Documentation/watchdog/src/watchdog-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/watchdog/src/watchdog-test.c
b/Documentation/watchdog/src/watchdog-test.c
index 73ff5cc..3da8229 100644
--- a/Documentation/watchdog/src/watchdog-test.c
+++ b/Documentation/watchdog/src/watchdog-test.c
@@ -31,7 +31,7 @@ static void keep_alive(void)
* or "-e" to enable the card.
*/
-void term(int sig)
+static void term(int sig)
{
close(fd);
fprintf(stderr, "Stopping watchdog ticks...\n");
--
1.7.12.rc0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] watchdog: watchdog-test: make term() static
On Thu, Jul 26, 2012 at 8:52 PM, Jerry Snitselaar wrote:
> In 3.5 warning during build 'no previous prototype for term'. Since it
> is only used in watchdog-test.c make term() static.
>
> Signed-off-by: Jerry Snitselaar
> ---
> Documentation/watchdog/src/watchdog-test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/watchdog/src/watchdog-test.c
> b/Documentation/watchdog/src/watchdog-test.c
> index 73ff5cc..3da8229 100644
> --- a/Documentation/watchdog/src/watchdog-test.c
> +++ b/Documentation/watchdog/src/watchdog-test.c
> @@ -31,7 +31,7 @@ static void keep_alive(void)
> * or "-e" to enable the card.
> */
>
> -void term(int sig)
> +static void term(int sig)
> {
> close(fd);
> fprintf(stderr, "Stopping watchdog ticks...\n");
> --
> 1.7.12.rc0
>
Disregard this patch. I didn't see Randy's patch when I did a search
in my email.
Jerry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
suspicious RCU usage with unlazy_walk()
Running 3.11.0-07547-g44598f9 I hit the following last night. I have
not had it reproduce since then. Should there be some lock cleanup in
the error path prior to the dput() call like unlock_rcu_walk()?
[ 1705.083706] ===
[ 1705.083709] [ INFO: suspicious RCU usage. ]
[ 1705.083713] 3.11.0-07547-g44598f9 #1 Not tainted
[ 1705.083715] ---
[ 1705.083719] /home/snits/dev/linux/include/linux/rcupdate.h:471 Illegal
context switch in RCU read-side critical section!
[ 1705.083721]
other info that might help us debug this:
[ 1705.083726]
rcu_scheduler_active = 1, debug_locks = 1
[ 1705.083730] 3 locks held by systemd-journal/384:
[ 1705.083733] #0: (&vfsmount_lock){..}, at: []
path_init+0x196/0x5e0
[ 1705.083748] #1: (rcu_read_lock){.+.+..}, at: []
path_init+0x18a/0x5e0
[ 1705.083759] #2: (&(&fs->lock)->rlock){+.+...}, at: []
unlazy_walk+0x62/0x2a0
[ 1705.083769]
stack backtrace:
[ 1705.083775] CPU: 0 PID: 384 Comm: systemd-journal Not tainted
3.11.0-07547-g44598f9 #1
[ 1705.083778] Hardware name: Dell Inc. Studio XPS 8100/0G3HR7, BIOS A05
07/08/2010
[ 1705.083781] 0001 880424c07c00 817fd6ae
88042577c0e0
[ 1705.083791] 880424c07c30 810bd613
81c8a470
[ 1705.083799] 0205 8803f5a3f838 880424c07c58
81084f17
[ 1705.083878] Call Trace:
[ 1705.083895] [] dump_stack+0x54/0x74
[ 1705.083907] [] lockdep_rcu_suspicious+0x103/0x110
[ 1705.083918] [] __might_sleep+0x57/0x210
[ 1705.083926] [] dput+0x3d/0x290
[ 1705.083932] [] unlazy_walk+0x284/0x2a0
[ 1705.083938] [] lookup_fast+0x183/0x2a0
[ 1705.083945] [] link_path_walk+0x225/0x8f0
[ 1705.083952] [] path_openat+0x237/0x640
[ 1705.083958] [] do_filp_open+0x3a/0x90
[ 1705.083966] [] ? __alloc_fd+0x1df/0x200
[ 1705.083973] [] do_sys_open+0x16c/0x200
[ 1705.083979] [] ? syscall_trace_enter+0x1fd/0x2c0
[ 1705.083985] [] SyS_open+0x1e/0x20
[ 1705.083992] [] tracesys+0xdd/0xe2
[ 1705.083997] BUG: sleeping function called from invalid context at
/home/snits/dev/linux/fs/dcache.c:517
[ 1705.084000] in_atomic(): 1, irqs_disabled(): 0, pid: 384, name:
systemd-journal
[ 1705.084003] 3 locks held by systemd-journal/384:
[ 1705.084006] #0: (&vfsmount_lock){..}, at: []
path_init+0x196/0x5e0
[ 1705.084018] #1: (rcu_read_lock){.+.+..}, at: []
path_init+0x18a/0x5e0
[ 1705.084028] #2: (&(&fs->lock)->rlock){+.+...}, at: []
unlazy_walk+0x62/0x2a0
[ 1705.084041] CPU: 0 PID: 384 Comm: systemd-journal Not tainted
3.11.0-07547-g44598f9 #1
[ 1705.084044] Hardware name: Dell Inc. Studio XPS 8100/0G3HR7, BIOS A05
07/08/2010
[ 1705.084047] 81c8a470 880424c07c30 817fd6ae
[ 1705.084055] 880424c07c58 810850ba 8803f5a3f838
88042ec042e8
[ 1705.084063] 0001 880424c07c88 811c302d
880424c07e60
[ 1705.084071] Call Trace:
[ 1705.084078] [] dump_stack+0x54/0x74
[ 1705.084084] [] __might_sleep+0x1fa/0x210
[ 1705.084090] [] dput+0x3d/0x290
[ 1705.084096] [] unlazy_walk+0x284/0x2a0
[ 1705.084102] [] lookup_fast+0x183/0x2a0
[ 1705.084108] [] link_path_walk+0x225/0x8f0
[ 1705.084114] [] path_openat+0x237/0x640
[ 1705.084120] [] do_filp_open+0x3a/0x90
[ 1705.084126] [] ? __alloc_fd+0x1df/0x200
[ 1705.084133] [] do_sys_open+0x16c/0x200
[ 1705.084138] [] ? syscall_trace_enter+0x1fd/0x2c0
[ 1705.084144] [] SyS_open+0x1e/0x20
[ 1705.084149] [] tracesys+0xdd/0xe2
[ 1705.084154] BUG: scheduling while atomic: systemd-journal/384/0x1004
[ 1705.084157] 3 locks held by systemd-journal/384:
[ 1705.084159] #0: (&vfsmount_lock){..}, at: []
path_init+0x196/0x5e0
[ 1705.084170] #1: (rcu_read_lock){.+.+..}, at: []
path_init+0x18a/0x5e0
[ 1705.084181] #2: (&(&fs->lock)->rlock){+.+...}, at: []
unlazy_walk+0x62/0x2a0
[ 1705.084192] Modules linked in: fuse ebtable_nat ebtables ipt_MASQUERADE
iptable_nat nf_nat_ipv4 nf_nat xt_CHECKSUM iptable_mangle b\
ridge stp llc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4
nf_defrag_ipv4 xt_conntrack nf_conntrack ip6table_filter \
ip6_tables tun it87 hwmon_vid coretemp snd_hda_codec_hdmi uvcvideo
snd_hda_codec_realtek videobuf2_vmalloc videobuf2_memops snd_hda_in\
tel snd_hda_codec snd_hwdep videobuf2_core snd_seq snd_seq_device snd_pcm
videodev snd_page_alloc snd_timer snd broadcom tg3 ptp pcspk\
r lpc_ich mfd_core nfsd i7core_edac soundcore serio_raw pps_core i2c_i801
edac_core auth_rpcgss oid_registry nfs_acl lockd sunrpc binf\
mt_misc uinput autofs4 firewire_ohci firewire_core radeon drm_kms_helper ttm
usb_storage kvm_intel kvm
[ 1705.084318] CPU: 0 PID: 384 Comm: systemd-journal Not tainted
3.11.0-07547-g44598f9 #1
[ 1705.084322] Hardware name: Dell Inc. Studio XPS 8100/0G3HR7, BIOS A05
07/08/2010
[ 1705.084325] 88043fc13dc0 880424c07bb8 817fd6ae
88042577c0e0
[ 1705.084334] 880424c
[PATCH] acpi_i2c: set MODULE_LICENSE, MODULE_AUTHOR, and MODULE_DESCRIPTION
Without MODULE_LICENSE set, I get the following with modprobe:
acpi_i2c: module license 'unspecified' taints kernel.
acpi_i2c: Unknown symbol i2c_new_device (err 0)
acpi_i2c: Unknown symbol acpi_dev_get_resources (err 0)
acpi_i2c: Unknown symbol acpi_dev_resource_interrupt (err 0)
acpi_i2c: Unknown symbol acpi_dev_free_resource_list (err 0)
Signed-off-by: Jerry Snitselaar
---
drivers/acpi/acpi_i2c.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/acpi/acpi_i2c.c b/drivers/acpi/acpi_i2c.c
index 82045e3..2c5c65d 100644
--- a/drivers/acpi/acpi_i2c.c
+++ b/drivers/acpi/acpi_i2c.c
@@ -14,9 +14,14 @@
#include
#include
#include
+#include
ACPI_MODULE_NAME("i2c");
+MODULE_DESCRIPTION("ACPI I2C enumeration support");
+MODULE_AUTHOR("Mika Westerberg");
+MODULE_LICENSE("GPL");
+
static int acpi_i2c_add_resource(struct acpi_resource *ares, void *data)
{
struct i2c_board_info *info = data;
--
1.8.4.rc3.2.g2c2b664
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] acpi_i2c: set MODULE_LICENSE, MODULE_AUTHOR, and MODULE_DESCRIPTION
Without MODULE_LICENSE get, I get the following with modprobe:
acpi_i2c: module license 'unspecified' taints kernel.
acpi_i2c: Unknown symbol i2c_new_device (err 0)
acpi_i2c: Unknown symbol acpi_dev_get_resources (err 0)
acpi_i2c: Unknown symbol acpi_dev_resource_interrupt (err 0)
acpi_i2c: Unknown symbol acpi_dev_free_resource_list (err 0)
Signed-off-by: Jerry Snitselaar
---
drivers/acpi/acpi_i2c.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/acpi/acpi_i2c.c b/drivers/acpi/acpi_i2c.c
index 82045e3..2c5c65d 100644
--- a/drivers/acpi/acpi_i2c.c
+++ b/drivers/acpi/acpi_i2c.c
@@ -14,9 +14,14 @@
#include
#include
#include
+#include
ACPI_MODULE_NAME("i2c");
+MODULE_DESCRIPTION("ACPI I2C enumeration support");
+MODULE_AUTHOR("Mika Westerberg");
+MODULE_LICENSE("GPL");
+
static int acpi_i2c_add_resource(struct acpi_resource *ares, void *data)
{
struct i2c_board_info *info = data;
--
1.8.4.rc3.2.g2c2b664
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] acpi_i2c: set MODULE_LICENSE, MODULE_AUTHOR, and MODULE_DESCRIPTION
Sorry for the double send. This one was apparently blocked by the greylisting on vger.kernel.org and it was hours before I received notification of that. I forgot to bcc myself and had no record of the git send-email successfully completing. Just ignore this patch. Jerry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] acpi_i2c: set MODULE_LICENSE, MODULE_AUTHOR, and MODULE_DESCRIPTION
On Tue Aug 20 13, Rafael J. Wysocki wrote: > On Monday, August 19, 2013 09:16:14 AM Mika Westerberg wrote: > > On Fri, Aug 16, 2013 at 06:26:35PM -0700, Jerry Snitselaar wrote: > > > Without MODULE_LICENSE set, I get the following with modprobe: > > > > > > acpi_i2c: module license 'unspecified' taints kernel. > > > acpi_i2c: Unknown symbol i2c_new_device (err 0) > > > acpi_i2c: Unknown symbol acpi_dev_get_resources (err 0) > > > acpi_i2c: Unknown symbol acpi_dev_resource_interrupt (err 0) > > > acpi_i2c: Unknown symbol acpi_dev_free_resource_list (err 0) > > > > > > Signed-off-by: Jerry Snitselaar > > > > Looks good to me. > > > > Acked-by: Mika Westerbeg > > Well, OK, but do we need to be able to build that as a module? > > Maybe it should just be yes or no? > > Rafael > Perhaps have depends on I2C=y and be a bool instead of tristate? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] acpi_i2c: set MODULE_LICENSE, MODULE_AUTHOR, and MODULE_DESCRIPTION
On Tue Aug 20 13, Rafael J. Wysocki wrote: > On Monday, August 19, 2013 04:35:29 PM Jerry Snitselaar wrote: > > On Tue Aug 20 13, Rafael J. Wysocki wrote: > > > On Monday, August 19, 2013 09:16:14 AM Mika Westerberg wrote: > > > > On Fri, Aug 16, 2013 at 06:26:35PM -0700, Jerry Snitselaar wrote: > > > > > Without MODULE_LICENSE set, I get the following with modprobe: > > > > > > > > > > acpi_i2c: module license 'unspecified' taints kernel. > > > > > acpi_i2c: Unknown symbol i2c_new_device (err 0) > > > > > acpi_i2c: Unknown symbol acpi_dev_get_resources (err 0) > > > > > acpi_i2c: Unknown symbol acpi_dev_resource_interrupt (err 0) > > > > > acpi_i2c: Unknown symbol acpi_dev_free_resource_list (err 0) > > > > > > > > > > Signed-off-by: Jerry Snitselaar > > > > > > > > Looks good to me. > > > > > > > > Acked-by: Mika Westerbeg > > > > > > Well, OK, but do we need to be able to build that as a module? > > > > > > Maybe it should just be yes or no? > > > > > > Rafael > > > > > > > Perhaps have depends on I2C=y and be a bool instead of tristate? > > Yes, that's the idea. > Does this look okay Mika? [PATCH] acpi_i2c: do not build as loadable module Change from tristate to bool, and depend on I2C=y Signed-off-by: Jerry Snitselaar --- drivers/acpi/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 100bd72..183a309 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -181,8 +181,9 @@ config ACPI_DOCK drive bays such as the IBM Ultrabay and the Dell Module Bay. config ACPI_I2C - def_tristate I2C - depends on I2C + bool "I2C" + depends on I2C=y + default n help ACPI I2C enumeration support. -- 1.8.4.rc3.2.g2c2b664 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] acpi_i2c: set MODULE_LICENSE, MODULE_AUTHOR, and MODULE_DESCRIPTION
On Tue Aug 20 13, Mika Westerberg wrote: > On Mon, Aug 19, 2013 at 08:34:03PM -0700, Jerry Snitselaar wrote: > > On Tue Aug 20 13, Rafael J. Wysocki wrote: > > > On Monday, August 19, 2013 04:35:29 PM Jerry Snitselaar wrote: > > > > On Tue Aug 20 13, Rafael J. Wysocki wrote: > > > > > On Monday, August 19, 2013 09:16:14 AM Mika Westerberg wrote: > > > > > > On Fri, Aug 16, 2013 at 06:26:35PM -0700, Jerry Snitselaar wrote: > > > > > > > Without MODULE_LICENSE set, I get the following with modprobe: > > > > > > > > > > > > > > acpi_i2c: module license 'unspecified' taints kernel. > > > > > > > acpi_i2c: Unknown symbol i2c_new_device (err 0) > > > > > > > acpi_i2c: Unknown symbol acpi_dev_get_resources (err 0) > > > > > > > acpi_i2c: Unknown symbol acpi_dev_resource_interrupt (err 0) > > > > > > > acpi_i2c: Unknown symbol acpi_dev_free_resource_list (err 0) > > > > > > > > > > > > > > Signed-off-by: Jerry Snitselaar > > > > > > > > > > > > Looks good to me. > > > > > > > > > > > > Acked-by: Mika Westerbeg > > > > > > > > > > Well, OK, but do we need to be able to build that as a module? > > > > > > > > > > Maybe it should just be yes or no? > > > > > > > > > > Rafael > > > > > > > > > > > > > Perhaps have depends on I2C=y and be a bool instead of tristate? > > > > > > Yes, that's the idea. > > > > > Does this look okay Mika? > > > > [PATCH] acpi_i2c: do not build as loadable module > > > > Change from tristate to bool, and depend on I2C=y > > I'm not sure about this. Does the below mean that we can't build the ACPI > I2C enumeration at all if I2C core is compiled as module? Yes, that was what Rafael was suggesting. If the ability to compile as a module if I2C is a module is needed, then we need the 1st patch. Jerry > > > > > Signed-off-by: Jerry Snitselaar > > --- > > drivers/acpi/Kconfig | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > > index 100bd72..183a309 100644 > > --- a/drivers/acpi/Kconfig > > +++ b/drivers/acpi/Kconfig > > @@ -181,8 +181,9 @@ config ACPI_DOCK > > drive bays such as the IBM Ultrabay and the Dell Module Bay. > > > > config ACPI_I2C > > - def_tristate I2C > > - depends on I2C > > + bool "I2C" > > + depends on I2C=y > > + default n > > help > > ACPI I2C enumeration support. > > > > -- > > 1.8.4.rc3.2.g2c2b664 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fib_trie: potential out of bounds access in trie_show_stats()
With the <= max condition in the for loop, it will be always go 1
element further than needed. If the condition for the while loop is
never met, then max is MAX_STAT_DEPTH, and for loop will walk off the
end of nodesizes[].
Signed-off-by: Jerry Snitselaar
---
net/ipv4/fib_trie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 49616fe..108a1e9c 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2133,7 +2133,7 @@ static void trie_show_stats(struct seq_file *seq, struct
trie_stat *stat)
max--;
pointers = 0;
- for (i = 1; i <= max; i++)
+ for (i = 1; i < max; i++)
if (stat->nodesizes[i] != 0) {
seq_printf(seq, " %u: %u", i, stat->nodesizes[i]);
pointers += (1<nodesizes[i];
--
1.8.3.2.701.g8c4e4ec
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] btrfs: return EPERM in btrfs_rm_device()
Currently there are error paths in btrfs_rm_device() where EINVAL is
returned telling the user they passed an invalid argument even though
they passed a valid device. Change to return EPERM instead as the
operation is not permitted.
Signed-off-by: Jerry Snitselaar
---
fs/btrfs/volumes.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5cbb7f4..3e1586c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1392,14 +1392,14 @@ int btrfs_rm_device(struct btrfs_root *root, char
*device_path)
if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices <= 4) {
printk(KERN_ERR "btrfs: unable to go below four devices "
"on raid10\n");
- ret = -EINVAL;
+ ret = -EPERM;
goto out;
}
if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices <= 2) {
printk(KERN_ERR "btrfs: unable to go below two "
"devices on raid1\n");
- ret = -EINVAL;
+ ret = -EPERM;
goto out;
}
@@ -1449,14 +1449,14 @@ int btrfs_rm_device(struct btrfs_root *root, char
*device_path)
if (device->is_tgtdev_for_dev_replace) {
pr_err("btrfs: unable to remove the dev_replace target dev\n");
- ret = -EINVAL;
+ ret = -EPERM;
goto error_brelse;
}
if (device->writeable && root->fs_info->fs_devices->rw_devices == 1) {
printk(KERN_ERR "btrfs: unable to remove the only writeable "
"device\n");
- ret = -EINVAL;
+ ret = -EPERM;
goto error_brelse;
}
--
1.8.2.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ASoC: core: remove unused variable in soc_probe() in linux-next
With commit 28d528c8 "ASoC: core: Remove pointless error on card
registration failure", the variable ret is no longer used in
soc_probe() and generates an unused variable warning during a build.
Signed-off-by: Jerry Snitselaar
---
sound/soc/soc-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 2d98ffc..7adf115 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1816,7 +1816,6 @@ base_error:
static int soc_probe(struct platform_device *pdev)
{
struct snd_soc_card *card = platform_get_drvdata(pdev);
- int ret = 0;
/*
* no card, so machine driver should be registering card
--
1.7.12.rc1.17.g9a7365c
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: revert "x86: Fix S4 regression"
On Wed Jul 25 12, Takao Indoh wrote: > >Thanks for tracking this, Takao! > > > >I bet you are using x86_64 not x86 PAE? If so, could you try this patch > >https://patchwork.kernel.org/patch/1195751/ > >? I already reviewed it. > > Great, I applied it and now kdump works. Thanks! > > Thanks, > Takao Indoh > This patch from Jacob Shin solves the problem, and seems like it might be a better solution. [PATCH 2/5] x86: find_early_table_space based on memory ranges that are being mapped https://lkml.org/lkml/2012/8/9/540 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: revert "x86: Fix S4 regression"
On Sat Aug 11 12, Jerry Snitselaar wrote: > On Wed Jul 25 12, Takao Indoh wrote: > > >Thanks for tracking this, Takao! > > > > > >I bet you are using x86_64 not x86 PAE? If so, could you try this patch > > >https://patchwork.kernel.org/patch/1195751/ > > >? I already reviewed it. > > > > Great, I applied it and now kdump works. Thanks! > > > > Thanks, > > Takao Indoh > > > > This patch from Jacob Shin solves the problem, and seems like it might > be a better solution. > > [PATCH 2/5] x86: find_early_table_space based on memory ranges that > are being mapped > > https://lkml.org/lkml/2012/8/9/540 > Actually, apply that series of 5 patches. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] device_cgroup: don't grab mutex in rcu callback
commit 103a197 "security/device_cgroup: lock assert fails in
dev_exception_clean()" grabs devcgroup_mutex to fix assert failure,
but mutex can't be grabbed in rcu callback. Since there shouldn't be
any other references when css_free is called, mutex isn't needed for
list cleanup in devcgroup_css_free().
Signed-off-by: Jerry Snitselaar
Acked-by: Tejun Heo
---
security/device_cgroup.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index d794abc..1c69e38 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -159,6 +159,16 @@ static void dev_exception_rm(struct dev_cgroup *dev_cgroup,
}
}
+static void __dev_exception_clean(struct dev_cgroup *dev_cgroup)
+{
+ struct dev_exception_item *ex, *tmp;
+
+ list_for_each_entry_safe(ex, tmp, &dev_cgroup->exceptions, list) {
+ list_del_rcu(&ex->list);
+ kfree_rcu(ex, rcu);
+ }
+}
+
/**
* dev_exception_clean - frees all entries of the exception list
* @dev_cgroup: dev_cgroup with the exception list to be cleaned
@@ -167,14 +177,9 @@ static void dev_exception_rm(struct dev_cgroup *dev_cgroup,
*/
static void dev_exception_clean(struct dev_cgroup *dev_cgroup)
{
- struct dev_exception_item *ex, *tmp;
-
lockdep_assert_held(&devcgroup_mutex);
- list_for_each_entry_safe(ex, tmp, &dev_cgroup->exceptions, list) {
- list_del_rcu(&ex->list);
- kfree_rcu(ex, rcu);
- }
+ __dev_exception_clean(dev_cgroup);
}
/*
@@ -215,9 +220,7 @@ static void devcgroup_css_free(struct cgroup *cgroup)
struct dev_cgroup *dev_cgroup;
dev_cgroup = cgroup_to_devcgroup(cgroup);
- mutex_lock(&devcgroup_mutex);
- dev_exception_clean(dev_cgroup);
- mutex_unlock(&devcgroup_mutex);
+ __dev_exception_clean(dev_cgroup);
kfree(dev_cgroup);
}
--
1.8.1.1.293.gfe73786
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] device_cgroup: don't grab mutex in rcu callback
commit 103a197 "security/device_cgroup: lock assert fails in
dev_exception_clean()" grabs devcgroup_mutex to fix assert failure,
but mutex can't be grabbed in rcu callback. Since there shouldn't be
any other references when css_free is called, mutex isn't needed for
list cleanup in devcgroup_css_free().
Signed-off-by: Jerry Snitselaar
Acked-by: Tejun Heo
Acked-by: Aristeu Rozanski
---
security/device_cgroup.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index d794abc..1c69e38 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -159,6 +159,16 @@ static void dev_exception_rm(struct dev_cgroup *dev_cgroup,
}
}
+static void __dev_exception_clean(struct dev_cgroup *dev_cgroup)
+{
+ struct dev_exception_item *ex, *tmp;
+
+ list_for_each_entry_safe(ex, tmp, &dev_cgroup->exceptions, list) {
+ list_del_rcu(&ex->list);
+ kfree_rcu(ex, rcu);
+ }
+}
+
/**
* dev_exception_clean - frees all entries of the exception list
* @dev_cgroup: dev_cgroup with the exception list to be cleaned
@@ -167,14 +177,9 @@ static void dev_exception_rm(struct dev_cgroup *dev_cgroup,
*/
static void dev_exception_clean(struct dev_cgroup *dev_cgroup)
{
- struct dev_exception_item *ex, *tmp;
-
lockdep_assert_held(&devcgroup_mutex);
- list_for_each_entry_safe(ex, tmp, &dev_cgroup->exceptions, list) {
- list_del_rcu(&ex->list);
- kfree_rcu(ex, rcu);
- }
+ __dev_exception_clean(dev_cgroup);
}
/*
@@ -215,9 +220,7 @@ static void devcgroup_css_free(struct cgroup *cgroup)
struct dev_cgroup *dev_cgroup;
dev_cgroup = cgroup_to_devcgroup(cgroup);
- mutex_lock(&devcgroup_mutex);
- dev_exception_clean(dev_cgroup);
- mutex_unlock(&devcgroup_mutex);
+ __dev_exception_clean(dev_cgroup);
kfree(dev_cgroup);
}
--
1.8.1.1.293.gfe73786
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Fixing CVE-2017-15361
On Wed Oct 25 17, Jarkko Sakkinen wrote: On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote: On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen wrote: > I'm implementing a fix for CVE-2017-15361 that simply blacklists > vulnerable FW versions. I think this is the only responsible action from > my side that I can do. I'm not sure this is ideal - do Infineon have any Linux tooling for performing firmware updates, and if so will that continue working if the device is blacklisted? It's also a poor user experience to have systems using TPM-backed disk encryption keys suddenly rendered unbootable, and making it as easy as possible for people to do an upgrade and then re-seal secrets with new keys feels like the correct approach. I talked today with Alexander Steffen in the KS unconference and we concluded that this would be a terrible idea. Alexander stated the following things about FW updates (Alexander, please correct me if I state something incorrectly or if you have something to add): * FW update can be constructed either in a way that the keys in the NVRAM are not cleared or in a way that they are cleared. * FW update cannot be directly applied to the TPM but must come as part of the firmware update from the vendor. If that is the case, can the two of you get Intel to update the fw for the tpm in the nuc5i5myhe (slb9665) :) ? It has needed an update for a while, due to issues with context management. My understanding (quite likely I misunderstood) from a recent discussion with Peter was that it was possible to update the fw. I proposed the following as an alternative: * Print a message to the klog (which log level would be appropriate?). * Possibly sleep for few seconds. Is this a good idea? While writing this email yet another alternative popped into my mind: what if we allow only in-kernel use but disallow the use of /dev/tpm0? You could still use trusted keys. Here are all the ideas that I have and I am open for better alternatives. /Jarkko
Re: char/tpm: Improve a size determination in nine functions
On Wed Oct 18 17, SF Markus Elfring wrote: For 1/4 and 2/4: explain why the message can be omitted. Why did you not reply directly with this request for the update steps with the subject “Delete an error message for a failed memory allocation in tpm_…()”? https://patchwork.kernel.org/patch/10009405/ https://patchwork.kernel.org/patch/10009415/ I find that there can be difficulty to show an appropriate information source for the reasonable explanation of this change pattern. Shouldn't this information source for the explanation be the submitter? I'd hope they understand what it is they are submitting. Remove sentence about Coccinelle. I got the impression that there is a bit of value in such a kind of attribution. That's all. I assume that there might be also some communication challenges involved. 3/4: definitive NAK, too much noise compared to value. I tried to reduce deviations from the Linux coding style again. You do not like such an attempt for this software area so far. 4/4: this a good commit message. Why did you not reply directly with this feedback for the update step “[PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection”? https://patchwork.kernel.org/patch/10009429/ https://lkml.kernel.org/r/<09a2c3a1-1b10-507d-a866-258b570f6...@users.sourceforge.net> Requires a Tested-by before can be accepted, which I'm not able to give. I am curious on how this detail will evolve. Regards, Markus
Re: [PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf
On Wed May 23 18, Jarkko Sakkinen wrote: On Fri, May 18, 2018 at 03:30:32PM -0700, Jerry Snitselaar wrote: On Mon Mar 26 18, Jarkko Sakkinen wrote: > In order to make struct tpm_buf the first class object for constructing TPM > commands, migrate tpm2_shutdown() to use it. In addition, removed the klog > entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already > prints an error message. > > Signed-off-by: Jarkko Sakkinen Reviewed-by: Jerry Snitselaar Thanky you Jerry. I'll do one more round for this only to update kdoc. It does not involve code changes so are you OK if you I keep your reviewed-by despite this update? /Jarkko Sure
Re: [PATCH] tpm_tis: verify locality released before returning from release_locality
On Mon May 28 18, Laurent Bigonville wrote:
Hello,
Top posting, sorry.
I don't know if I did it well to include the "Tested-by" tag because I
don't see that the patch has landed in linus branch already.
And as far as I understand, this will not be in the upcoming 4.17
release as we are already late in the cycle?
Kind regards,
Laurent Bigonville
It should go into his branch during the merge window for 4.18.
Le 11/05/18 à 21:02, Laurent Bigonville a écrit :
Le 05/05/18 à 22:03, Jerry Snitselaar a écrit :
On Sat May 05 18, Jerry Snitselaar wrote:
For certain tpm chips releasing locality can take long enough that a
subsequent call to request_locality will see the locality as being
active when the access register is read in check_locality. So check
that the locality has been released before returning from
release_locality.
Cc: Jarkko Sakkinen
Cc: Peter Huewe
Cc: Jason Gunthorpe
Reported-by: Laurent Bigonville
Signed-off-by: Jerry Snitselaar
Tested-by: Laurent Bigonville
---
drivers/char/tpm/tpm_tis_core.c | 47
-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c
b/drivers/char/tpm/tpm_tis_core.c
index 5a1f47b43947..d547cd309dbd 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -143,13 +143,58 @@ static bool check_locality(struct tpm_chip
*chip, int l)
return false;
}
+static bool locality_inactive(struct tpm_chip *chip, int l)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ int rc;
+ u8 access;
+
+ rc = tpm_tis_read8(priv, TPM_ACCESS(l), &access);
+ if (rc < 0)
+ return false;
+
+ if ((access & (TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY))
+ == TPM_ACCESS_VALID)
+ return true;
+
+ return false;
+}
+
static int release_locality(struct tpm_chip *chip, int l)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ unsigned long stop, timeout;
+ long rc;
tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
- return 0;
+ stop = jiffies + chip->timeout_a;
+
+ if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+again:
+ timeout = stop - jiffies;
+ if ((long)timeout <= 0)
+ return -1;
+
+ rc = wait_event_interruptible_timeout(priv->int_queue,
+ (locality_inactive(chip, l)),
+ timeout);
+
+ if (rc > 0)
+ return 0;
+
+ if (rc == -ERESTARTSYS && freezing(current)) {
+ clear_thread_flag(TIF_SIGPENDING);
+ goto again;
+ }
+ } else {
+ do {
+ if (locality_inactive(chip, l))
+ return 0;
+ tpm_msleep(TPM_TIMEOUT);
+ } while (time_before(jiffies, stop));
+ }
+ return -1;
}
static int request_locality(struct tpm_chip *chip, int l)
--
2.15.0
Laurent,
Can you try this patch with your system since it is the one
that has exhibited the problem so far. I've tested on a
tpm2.0 and tpm1.2 system here.
Regards,
Jerry
Re: [PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf
On Mon Mar 26 18, Jarkko Sakkinen wrote:
In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
prints an error message.
Signed-off-by: Jarkko Sakkinen
Reviewed-by: Jerry Snitselaar
---
drivers/char/tpm/tpm2-cmd.c | 44
1 file changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 96c77c8e7f40..7665661d9230 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,10 +27,6 @@ enum tpm2_session_attributes {
TPM2_SA_CONTINUE_SESSION= BIT(0),
};
-struct tpm2_startup_in {
- __be16 startup_type;
-} __packed;
-
struct tpm2_get_tpm_pt_in {
__be32 cap_id;
__be32 property_id;
@@ -55,7 +51,6 @@ struct tpm2_get_random_out {
} __packed;
union tpm2_cmd_params {
- struct tpm2_startup_in startup_in;
struct tpm2_get_tpm_pt_in get_tpm_pt_in;
struct tpm2_get_tpm_pt_out get_tpm_pt_out;
struct tpm2_get_random_in getrandom_in;
@@ -412,11 +407,8 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32
handle,
int rc;
rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
- if (rc) {
- dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
-handle);
+ if (rc)
return;
- }
tpm_buf_append_u32(&buf, handle);
@@ -762,40 +754,28 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32
property_id, u32 *value,
}
EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
-#define TPM2_SHUTDOWN_IN_SIZE \
- (sizeof(struct tpm_input_header) + \
-sizeof(struct tpm2_startup_in))
-
-static const struct tpm_input_header tpm2_shutdown_header = {
- .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
- .length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
- .ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
-};
-
/**
* tpm2_shutdown() - send shutdown command to the TPM chip
*
+ * In places where shutdown command is sent there's no much we can do except
+ * print the error code on a system failure.
+ *
* @chip: TPM chip to use.
* @shutdown_type: shutdown type. The value is either
* TPM_SU_CLEAR or TPM_SU_STATE.
*/
void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
{
- struct tpm2_cmd cmd;
+ struct tpm_buf buf;
int rc;
- cmd.header.in = tpm2_shutdown_header;
- cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
-
- rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
- "stopping the TPM");
-
- /* In places where shutdown command is sent there's no much we can do
-* except print the error code on a system failure.
-*/
- if (rc < 0 && rc != -EPIPE)
- dev_warn(&chip->dev, "transmit returned %d while stopping the
TPM",
-rc);
+ rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+ if (rc)
+ return;
+ tpm_buf_append_u16(&buf, shutdown_type);
+ tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+"stopping the TPM");
+ tpm_buf_destroy(&buf);
}
/*
--
2.15.1
Re: [PATCH v4 2/4] tpm: migrate tpm2_probe() to use struct tpm_buf
On Mon Mar 26 18, Jarkko Sakkinen wrote:
In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_probe() to use it.
Signed-off-by: Jarkko Sakkinen
Acked-by: Jay Freyensee
Reviewed-by: Jerry Snitselaar
---
drivers/char/tpm/tpm2-cmd.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7665661d9230..7bffd0fd1dca 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -844,30 +844,35 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
/**
* tpm2_probe() - probe TPM 2.0
- * @chip: TPM chip to use
+ * @chip: a TPM chip to probe
*
- * Return: < 0 error and 0 on success.
+ * Return: 0 on success,
+ * -errno otherwise
*
- * Send idempotent TPM 2.0 command and see whether TPM 2.0 chip replied based
on
- * the reply tag.
+ * Send an idempotent TPM 2.0 command and see whether there is TPM2 chip in the
+ * other end based on the response tag. The flag TPM_CHIP_FLAG_TPM2 is set if
+ * this is the case.
*/
int tpm2_probe(struct tpm_chip *chip)
{
- struct tpm2_cmd cmd;
+ struct tpm_output_header *out;
+ struct tpm_buf buf;
int rc;
- cmd.header.in = tpm2_get_tpm_pt_header;
- cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
- cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
- cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
- rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
- if (rc < 0)
+ rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ if (rc)
return rc;
-
- if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
- chip->flags |= TPM_CHIP_FLAG_TPM2;
-
+ tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+ tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
+ tpm_buf_append_u32(&buf, 1);
+ rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+ /* We ignore TPM return codes on purpose. */
+ if (rc >= 0) {
+ out = (struct tpm_output_header *)buf.data;
+ if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
+ chip->flags |= TPM_CHIP_FLAG_TPM2;
+ }
+ tpm_buf_destroy(&buf);
return 0;
}
EXPORT_SYMBOL_GPL(tpm2_probe);
--
2.15.1
Re: [PATCH v4 3/4] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf
On Mon Mar 26 18, Jarkko Sakkinen wrote:
In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_get_tpm_pt() to use it.
Signed-off-by: Jarkko Sakkinen
Reviewed-by: Jerry Snitselaar
---
drivers/char/tpm/tpm2-cmd.c | 63 +
1 file changed, 23 insertions(+), 40 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7bffd0fd1dca..b3b52f9eb65f 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,20 +27,6 @@ enum tpm2_session_attributes {
TPM2_SA_CONTINUE_SESSION= BIT(0),
};
-struct tpm2_get_tpm_pt_in {
- __be32 cap_id;
- __be32 property_id;
- __be32 property_cnt;
-} __packed;
-
-struct tpm2_get_tpm_pt_out {
- u8 more_data;
- __be32 subcap_id;
- __be32 property_cnt;
- __be32 property_id;
- __be32 value;
-} __packed;
-
struct tpm2_get_random_in {
__be16 size;
} __packed;
@@ -51,8 +37,6 @@ struct tpm2_get_random_out {
} __packed;
union tpm2_cmd_params {
- struct tpm2_get_tpm_pt_in get_tpm_pt_in;
- struct tpm2_get_tpm_pt_out get_tpm_pt_out;
struct tpm2_get_random_in getrandom_in;
struct tpm2_get_random_out getrandom_out;
};
@@ -379,19 +363,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t
max)
return total ? total : -EIO;
}
-#define TPM2_GET_TPM_PT_IN_SIZE \
- (sizeof(struct tpm_input_header) + \
-sizeof(struct tpm2_get_tpm_pt_in))
-
-#define TPM2_GET_TPM_PT_OUT_BODY_SIZE \
-sizeof(struct tpm2_get_tpm_pt_out)
-
-static const struct tpm_input_header tpm2_get_tpm_pt_header = {
- .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
- .length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
- .ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
-};
-
/**
* tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
* @chip: TPM chip to use
@@ -725,6 +696,14 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
return rc;
}
+struct tpm2_get_cap_out {
+ u8 more_data;
+ __be32 subcap_id;
+ __be32 property_cnt;
+ __be32 property_id;
+ __be32 value;
+} __packed;
+
/**
* tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
* @chip: TPM chip to use.
@@ -737,19 +716,23 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
const char *desc)
{
- struct tpm2_cmd cmd;
+ struct tpm2_get_cap_out *out;
+ struct tpm_buf buf;
int rc;
- cmd.header.in = tpm2_get_tpm_pt_header;
- cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
- cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
- cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
- rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
- TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
- if (!rc)
- *value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
-
+ rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ if (rc)
+ return rc;
+ tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+ tpm_buf_append_u32(&buf, property_id);
+ tpm_buf_append_u32(&buf, 1);
+ rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+ if (!rc) {
+ out = (struct tpm2_get_cap_out *)
+ &buf.data[TPM_HEADER_SIZE];
+ *value = be32_to_cpu(out->value);
+ }
+ tpm_buf_destroy(&buf);
return rc;
}
EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
--
2.15.1
Re: [PATCH] docs: Extend trusted keys documentation for TPM 2.0
On Mon Nov 05 18, Jerry Snitselaar wrote: On Fri Oct 19 18, Stefan Berger wrote: Extend the documentation for trusted keys with documentation for how to set up a key for a TPM 2.0 so it can be used with a TPM 2.0 as well. Signed-off-by: Stefan Berger Reviewed-by: Mimi Zohar --- .../security/keys/trusted-encrypted.rst | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst index 3bb24e09a332..6ec6bb2ac497 100644 --- a/Documentation/security/keys/trusted-encrypted.rst +++ b/Documentation/security/keys/trusted-encrypted.rst @@ -18,10 +18,33 @@ integrity verifications match. A loaded Trusted Key can be updated with new when the kernel and initramfs are updated. The same key can have many saved blobs under different PCR values, so multiple boots are easily supported. +TPM 1.2 +--- + By default, trusted keys are sealed under the SRK, which has the default authorization value (20 zeros). This can be set at takeownership time with the trouser's utility: "tpm_takeownership -u -z". +TPM 2.0 +--- + +The user must first create a storage key and make it persistent, so the key is +available after reboot. This can be done using the following commands. + +With the IBM TSS 2 stack:: + + #> tsscreateprimary -hi o -st + Handle 8000 + #> tssevictcontrol -hi o -ho 8000 -hp 8101 + +Or with the Intel TSS 2 stack:: + + #> tpm2_createprimary --hierarchy o -G rsa2048 -o key.ctxt + [...] + handle: 0x80FF + #> tpm2_evictcontrol -c key.ctxt -p 0x8101 + persistentHandle: 0x8101 + Is that the correct option for tpm2_evictcontrol? What I'm seeing in the versions I have is -S or -persistent= for specifying the persistent handle. Other than that looks good to me. William, is the above correct? Usage:: keyctl add trusted name "new keylen [options]" ring @@ -30,7 +53,9 @@ Usage:: keyctl print keyid options: - keyhandle=ascii hex value of sealing key default 0x4000 (SRK) + keyhandle=ascii hex value of sealing key + TPM 1.2: default 0x4000 (SRK) + TPM 2.0: no default; must be passed every time keyauth= ascii hex auth for sealing key default 0x00...i (40 ascii zeros) blobauth= ascii hex auth for sealed data default 0x00... @@ -84,6 +109,10 @@ Examples of trusted and encrypted key usage: Create and save a trusted key named "kmk" of length 32 bytes:: +Note: When using a TPM 2.0 with a persistent key with handle 0x8101, +append 'keyhandle=0x8101' to statements between quotes, such as +"new 32 keyhandle=0x8101". + $ keyctl add trusted kmk "new 32" @u 440502848 -- 2.17.2
Re: [PATCH] docs: Extend trusted keys documentation for TPM 2.0
On Fri Oct 19 18, Stefan Berger wrote: Extend the documentation for trusted keys with documentation for how to set up a key for a TPM 2.0 so it can be used with a TPM 2.0 as well. Signed-off-by: Stefan Berger Reviewed-by: Mimi Zohar Acked-by: Jerry Snitselaar --- .../security/keys/trusted-encrypted.rst | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst index 3bb24e09a332..6ec6bb2ac497 100644 --- a/Documentation/security/keys/trusted-encrypted.rst +++ b/Documentation/security/keys/trusted-encrypted.rst @@ -18,10 +18,33 @@ integrity verifications match. A loaded Trusted Key can be updated with new when the kernel and initramfs are updated. The same key can have many saved blobs under different PCR values, so multiple boots are easily supported. +TPM 1.2 +--- + By default, trusted keys are sealed under the SRK, which has the default authorization value (20 zeros). This can be set at takeownership time with the trouser's utility: "tpm_takeownership -u -z". +TPM 2.0 +--- + +The user must first create a storage key and make it persistent, so the key is +available after reboot. This can be done using the following commands. + +With the IBM TSS 2 stack:: + + #> tsscreateprimary -hi o -st + Handle 8000 + #> tssevictcontrol -hi o -ho 8000 -hp 8101 + +Or with the Intel TSS 2 stack:: + + #> tpm2_createprimary --hierarchy o -G rsa2048 -o key.ctxt + [...] + handle: 0x80FF + #> tpm2_evictcontrol -c key.ctxt -p 0x8101 + persistentHandle: 0x8101 + Usage:: keyctl add trusted name "new keylen [options]" ring @@ -30,7 +53,9 @@ Usage:: keyctl print keyid options: - keyhandle=ascii hex value of sealing key default 0x4000 (SRK) + keyhandle=ascii hex value of sealing key + TPM 1.2: default 0x4000 (SRK) + TPM 2.0: no default; must be passed every time keyauth= ascii hex auth for sealing key default 0x00...i (40 ascii zeros) blobauth= ascii hex auth for sealed data default 0x00... @@ -84,6 +109,10 @@ Examples of trusted and encrypted key usage: Create and save a trusted key named "kmk" of length 32 bytes:: +Note: When using a TPM 2.0 with a persistent key with handle 0x8101, +append 'keyhandle=0x8101' to statements between quotes, such as +"new 32 keyhandle=0x8101". + $ keyctl add trusted kmk "new 32" @u 440502848 -- 2.17.2
Re: [PATCH] docs: Extend trusted keys documentation for TPM 2.0
On Fri Oct 19 18, Stefan Berger wrote: Extend the documentation for trusted keys with documentation for how to set up a key for a TPM 2.0 so it can be used with a TPM 2.0 as well. Signed-off-by: Stefan Berger Reviewed-by: Mimi Zohar --- .../security/keys/trusted-encrypted.rst | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst index 3bb24e09a332..6ec6bb2ac497 100644 --- a/Documentation/security/keys/trusted-encrypted.rst +++ b/Documentation/security/keys/trusted-encrypted.rst @@ -18,10 +18,33 @@ integrity verifications match. A loaded Trusted Key can be updated with new when the kernel and initramfs are updated. The same key can have many saved blobs under different PCR values, so multiple boots are easily supported. +TPM 1.2 +--- + By default, trusted keys are sealed under the SRK, which has the default authorization value (20 zeros). This can be set at takeownership time with the trouser's utility: "tpm_takeownership -u -z". +TPM 2.0 +--- + +The user must first create a storage key and make it persistent, so the key is +available after reboot. This can be done using the following commands. + +With the IBM TSS 2 stack:: + + #> tsscreateprimary -hi o -st + Handle 8000 + #> tssevictcontrol -hi o -ho 8000 -hp 8101 + +Or with the Intel TSS 2 stack:: + + #> tpm2_createprimary --hierarchy o -G rsa2048 -o key.ctxt + [...] + handle: 0x80FF + #> tpm2_evictcontrol -c key.ctxt -p 0x8101 + persistentHandle: 0x8101 + Is that the correct option for tpm2_evictcontrol? What I'm seeing in the versions I have is -S or -persistent= for specifying the persistent handle. Other than that looks good to me. Usage:: keyctl add trusted name "new keylen [options]" ring @@ -30,7 +53,9 @@ Usage:: keyctl print keyid options: - keyhandle=ascii hex value of sealing key default 0x4000 (SRK) + keyhandle=ascii hex value of sealing key + TPM 1.2: default 0x4000 (SRK) + TPM 2.0: no default; must be passed every time keyauth= ascii hex auth for sealing key default 0x00...i (40 ascii zeros) blobauth= ascii hex auth for sealed data default 0x00... @@ -84,6 +109,10 @@ Examples of trusted and encrypted key usage: Create and save a trusted key named "kmk" of length 32 bytes:: +Note: When using a TPM 2.0 with a persistent key with handle 0x8101, +append 'keyhandle=0x8101' to statements between quotes, such as +"new 32 keyhandle=0x8101". + $ keyctl add trusted kmk "new 32" @u 440502848 -- 2.17.2
[PATCH] tracing: Export tracing clock functions
Critical tracepoint hooks shoud never call anything that takes a lock,
so they are unable to call getrawmonotonic() or ktime_get().
Export the rest of the tracing clock functions so can be used in
tracepoint hooks.
Cc: Steven Rostedt
Cc: Ingo Molnar
Signed-off-by: Jerry Snitselaar
---
kernel/trace/trace_clock.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/trace/trace_clock.c b/kernel/trace/trace_clock.c
index 57b67b1..0f06532 100644
--- a/kernel/trace/trace_clock.c
+++ b/kernel/trace/trace_clock.c
@@ -56,6 +56,7 @@ u64 notrace trace_clock(void)
{
return local_clock();
}
+EXPORT_SYMBOL_GPL(trace_clock);
/*
* trace_jiffy_clock(): Simply use jiffies as a clock counter.
@@ -68,6 +69,7 @@ u64 notrace trace_clock_jiffies(void)
{
return jiffies_64_to_clock_t(jiffies_64 - INITIAL_JIFFIES);
}
+EXPORT_SYMBOL_GPL(trace_clock_jiffies);
/*
* trace_clock_global(): special globally coherent trace clock
@@ -123,6 +125,7 @@ u64 notrace trace_clock_global(void)
return now;
}
+EXPORT_SYMBOL_GPL(trace_clock_global);
static atomic64_t trace_counter;
--
2.4.0.rc3.3.g6eb1401
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] tracing: Export tracing clock functions
Critical tracepoint hooks should never call anything that takes a lock,
so they are unable to call getrawmonotonic() or ktime_get().
Export the rest of the tracing clock functions so can be used in
tracepoint hooks.
Background: We have a customer that adds their own module and registers
a tracepoint hook to sched_wakeup. They were using ktime_get() for a
time source, but it grabs a seq lock and caused a deadlock to occur.
v2: added background info and fixed typo
Cc: Steven Rostedt
Cc: Ingo Molnar
Signed-off-by: Jerry Snitselaar
---
kernel/trace/trace_clock.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/trace/trace_clock.c b/kernel/trace/trace_clock.c
index 57b67b1..0f06532 100644
--- a/kernel/trace/trace_clock.c
+++ b/kernel/trace/trace_clock.c
@@ -56,6 +56,7 @@ u64 notrace trace_clock(void)
{
return local_clock();
}
+EXPORT_SYMBOL_GPL(trace_clock);
/*
* trace_jiffy_clock(): Simply use jiffies as a clock counter.
@@ -68,6 +69,7 @@ u64 notrace trace_clock_jiffies(void)
{
return jiffies_64_to_clock_t(jiffies_64 - INITIAL_JIFFIES);
}
+EXPORT_SYMBOL_GPL(trace_clock_jiffies);
/*
* trace_clock_global(): special globally coherent trace clock
@@ -123,6 +125,7 @@ u64 notrace trace_clock_global(void)
return now;
}
+EXPORT_SYMBOL_GPL(trace_clock_global);
static atomic64_t trace_counter;
--
2.4.0.rc3.3.g6eb1401
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/33] iommu: Move iommu_group setup to IOMMU core code
On Fri May 29 20, Jerry Snitselaar wrote:
On Tue Apr 14 20, Joerg Roedel wrote:
Hi,
here is the second version of this patch-set. The first version with
some more introductory text can be found here:
https://lore.kernel.org/lkml/20200407183742.4344-1-j...@8bytes.org/
Changes v1->v2:
* Rebased to v5.7-rc1
* Re-wrote the arm-smmu changes as suggested by Robin Murphy
* Re-worked the Exynos patches to hopefully not break the
driver anymore
* Fixed a missing mutex_unlock() reported by Marek Szyprowski,
thanks for that.
There is also a git-branch available with these patches applied:
https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
Please review.
Thanks,
Joerg
Joerg Roedel (32):
iommu: Move default domain allocation to separate function
iommu/amd: Implement iommu_ops->def_domain_type call-back
iommu/vt-d: Wire up iommu_ops->def_domain_type
iommu/amd: Remove dma_mask check from check_device()
iommu/amd: Return -ENODEV in add_device when device is not handled by
IOMMU
iommu: Add probe_device() and remove_device() call-backs
iommu: Move default domain allocation to iommu_probe_device()
iommu: Keep a list of allocated groups in __iommu_probe_device()
iommu: Move new probe_device path to separate function
iommu: Split off default domain allocation from group assignment
iommu: Move iommu_group_create_direct_mappings() out of
iommu_group_add_device()
iommu: Export bus_iommu_probe() and make is safe for re-probing
iommu/amd: Remove dev_data->passthrough
iommu/amd: Convert to probe/release_device() call-backs
iommu/vt-d: Convert to probe/release_device() call-backs
iommu/arm-smmu: Convert to probe/release_device() call-backs
iommu/pamu: Convert to probe/release_device() call-backs
iommu/s390: Convert to probe/release_device() call-backs
iommu/virtio: Convert to probe/release_device() call-backs
iommu/msm: Convert to probe/release_device() call-backs
iommu/mediatek: Convert to probe/release_device() call-backs
iommu/mediatek-v1 Convert to probe/release_device() call-backs
iommu/qcom: Convert to probe/release_device() call-backs
iommu/rockchip: Convert to probe/release_device() call-backs
iommu/tegra: Convert to probe/release_device() call-backs
iommu/renesas: Convert to probe/release_device() call-backs
iommu/omap: Remove orphan_dev tracking
iommu/omap: Convert to probe/release_device() call-backs
iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
iommu/exynos: Convert to probe/release_device() call-backs
iommu: Remove add_device()/remove_device() code-paths
iommu: Unexport iommu_group_get_for_dev()
Sai Praneeth Prakhya (1):
iommu: Add def_domain_type() callback in iommu_ops
drivers/iommu/amd_iommu.c | 97
drivers/iommu/amd_iommu_types.h | 1 -
drivers/iommu/arm-smmu-v3.c | 38 +--
drivers/iommu/arm-smmu.c| 39 ++--
drivers/iommu/exynos-iommu.c| 24 +-
drivers/iommu/fsl_pamu_domain.c | 22 +-
drivers/iommu/intel-iommu.c | 68 +-
drivers/iommu/iommu.c | 393 +---
drivers/iommu/ipmmu-vmsa.c | 60 ++---
drivers/iommu/msm_iommu.c | 34 +--
drivers/iommu/mtk_iommu.c | 24 +-
drivers/iommu/mtk_iommu_v1.c| 50 ++--
drivers/iommu/omap-iommu.c | 99 ++--
drivers/iommu/qcom_iommu.c | 24 +-
drivers/iommu/rockchip-iommu.c | 26 +--
drivers/iommu/s390-iommu.c | 22 +-
drivers/iommu/tegra-gart.c | 24 +-
drivers/iommu/tegra-smmu.c | 31 +--
drivers/iommu/virtio-iommu.c| 41 +---
include/linux/iommu.h | 21 +-
20 files changed, 533 insertions(+), 605 deletions(-)
--
2.17.1
___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi Joerg,
With this patchset, I have an epyc system where if I boot with
iommu=nopt and force a dump I will see some io page faults for a nic
on the system. The vmcore is harvested and the system reboots. I
haven't reproduced it on other systems yet, but without the patchset I
don't see the io page faults during the kdump.
Regards,
Jerry
I just hit an issue on a separate intel based system (kdump iommu=nopt),
where it panics in during intel_iommu_attach_device, in is_aux_domain,
due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
get set to a valid address until the domain_add_dev_info call.
Is it as simple as the following?
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 29d3940847d3..f1bbeed46a4c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct iommu_domain
*domain)
{
struct device_domain_info *info = dev->archdata.iommu;
- return info && info->auxd_enabled &&
- domain->type ==
Re: [PATCH v2 00/33] iommu: Move iommu_group setup to IOMMU core code
On Mon Jun 01 20, Jerry Snitselaar wrote:
On Fri May 29 20, Jerry Snitselaar wrote:
On Tue Apr 14 20, Joerg Roedel wrote:
Hi,
here is the second version of this patch-set. The first version with
some more introductory text can be found here:
https://lore.kernel.org/lkml/20200407183742.4344-1-j...@8bytes.org/
Changes v1->v2:
* Rebased to v5.7-rc1
* Re-wrote the arm-smmu changes as suggested by Robin Murphy
* Re-worked the Exynos patches to hopefully not break the
driver anymore
* Fixed a missing mutex_unlock() reported by Marek Szyprowski,
thanks for that.
There is also a git-branch available with these patches applied:
https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-probe-device-v2
Please review.
Thanks,
Joerg
Joerg Roedel (32):
iommu: Move default domain allocation to separate function
iommu/amd: Implement iommu_ops->def_domain_type call-back
iommu/vt-d: Wire up iommu_ops->def_domain_type
iommu/amd: Remove dma_mask check from check_device()
iommu/amd: Return -ENODEV in add_device when device is not handled by
IOMMU
iommu: Add probe_device() and remove_device() call-backs
iommu: Move default domain allocation to iommu_probe_device()
iommu: Keep a list of allocated groups in __iommu_probe_device()
iommu: Move new probe_device path to separate function
iommu: Split off default domain allocation from group assignment
iommu: Move iommu_group_create_direct_mappings() out of
iommu_group_add_device()
iommu: Export bus_iommu_probe() and make is safe for re-probing
iommu/amd: Remove dev_data->passthrough
iommu/amd: Convert to probe/release_device() call-backs
iommu/vt-d: Convert to probe/release_device() call-backs
iommu/arm-smmu: Convert to probe/release_device() call-backs
iommu/pamu: Convert to probe/release_device() call-backs
iommu/s390: Convert to probe/release_device() call-backs
iommu/virtio: Convert to probe/release_device() call-backs
iommu/msm: Convert to probe/release_device() call-backs
iommu/mediatek: Convert to probe/release_device() call-backs
iommu/mediatek-v1 Convert to probe/release_device() call-backs
iommu/qcom: Convert to probe/release_device() call-backs
iommu/rockchip: Convert to probe/release_device() call-backs
iommu/tegra: Convert to probe/release_device() call-backs
iommu/renesas: Convert to probe/release_device() call-backs
iommu/omap: Remove orphan_dev tracking
iommu/omap: Convert to probe/release_device() call-backs
iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
iommu/exynos: Convert to probe/release_device() call-backs
iommu: Remove add_device()/remove_device() code-paths
iommu: Unexport iommu_group_get_for_dev()
Sai Praneeth Prakhya (1):
iommu: Add def_domain_type() callback in iommu_ops
drivers/iommu/amd_iommu.c | 97
drivers/iommu/amd_iommu_types.h | 1 -
drivers/iommu/arm-smmu-v3.c | 38 +--
drivers/iommu/arm-smmu.c| 39 ++--
drivers/iommu/exynos-iommu.c| 24 +-
drivers/iommu/fsl_pamu_domain.c | 22 +-
drivers/iommu/intel-iommu.c | 68 +-
drivers/iommu/iommu.c | 393 +---
drivers/iommu/ipmmu-vmsa.c | 60 ++---
drivers/iommu/msm_iommu.c | 34 +--
drivers/iommu/mtk_iommu.c | 24 +-
drivers/iommu/mtk_iommu_v1.c| 50 ++--
drivers/iommu/omap-iommu.c | 99 ++--
drivers/iommu/qcom_iommu.c | 24 +-
drivers/iommu/rockchip-iommu.c | 26 +--
drivers/iommu/s390-iommu.c | 22 +-
drivers/iommu/tegra-gart.c | 24 +-
drivers/iommu/tegra-smmu.c | 31 +--
drivers/iommu/virtio-iommu.c| 41 +---
include/linux/iommu.h | 21 +-
20 files changed, 533 insertions(+), 605 deletions(-)
--
2.17.1
___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi Joerg,
With this patchset, I have an epyc system where if I boot with
iommu=nopt and force a dump I will see some io page faults for a nic
on the system. The vmcore is harvested and the system reboots. I
haven't reproduced it on other systems yet, but without the patchset I
don't see the io page faults during the kdump.
Regards,
Jerry
I just hit an issue on a separate intel based system (kdump iommu=nopt),
where it panics in during intel_iommu_attach_device, in is_aux_domain,
due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
get set to a valid address until the domain_add_dev_info call.
Is it as simple as the following?
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 29d3940847d3..f1bbeed46a4c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct iommu_domain
*domain)
{
struct device_domain_info *info = dev->archdata.iommu;
- return info && info->auxd_enable
Re: [PATCH v2 00/33] iommu: Move iommu_group setup to IOMMU core code
On Tue Jun 02 20, Lu Baolu wrote:
Hi Jerry,
On 6/1/20 6:42 PM, Jerry Snitselaar wrote:
Hi Joerg,
With this patchset, I have an epyc system where if I boot with
iommu=nopt and force a dump I will see some io page faults for a nic
on the system. The vmcore is harvested and the system reboots. I
haven't reproduced it on other systems yet, but without the patchset I
don't see the io page faults during the kdump.
Regards,
Jerry
I just hit an issue on a separate intel based system (kdump iommu=nopt),
where it panics in during intel_iommu_attach_device, in is_aux_domain,
due to device_domain_info being DEFER_DEVICE_DOMAIN_INFO. That doesn't
get set to a valid address until the domain_add_dev_info call.
Is it as simple as the following?
I guess you won't hit this issue if you use iommu/next branch of Joerg's
tree. We've changed to use a generic helper to retrieve the valid per
device iommu data or NULL (if there's no).
Best regards,
baolu
Yeah, that will solve the panic.
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 29d3940847d3..f1bbeed46a4c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5053,8 +5053,8 @@ is_aux_domain(struct device *dev, struct
iommu_domain *domain)
{
struct device_domain_info *info = dev->archdata.iommu;
- return info && info->auxd_enabled &&
- domain->type == IOMMU_DOMAIN_UNMANAGED;
+ return info && info != DEFER_DEVICE_DOMAIN_INFO &&
+ info->auxd_enabled && domain->type ==
IOMMU_DOMAIN_UNMANAGED;
}
static void auxiliary_link_device(struct dmar_domain *domain,
Regards,
Jerry
___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Implement deferred domain attachment
On Mon May 18 20, Joerg Roedel wrote: On Fri, May 15, 2020 at 08:23:13PM +0100, Robin Murphy wrote: But that's not what this is; this is (supposed to be) the exact same "don't actually perform the attach yet" logic as before, just restricting it to default domains in the one place that it actually needs to be, so as not to fundamentally bugger up iommu_attach_device() in a way that prevents it from working as expected at the correct point later. You are right, that is better. I tested it and it seems to work. Updated diff attached, with a minor cleanup included. Mind sending it as a proper patch I can send upstream? Thanks, Joerg I should have this tested this afternoon.
Re: [PATCH] iommu: Implement deferred domain attachment
On Mon May 18 20, Joerg Roedel wrote:
On Fri, May 15, 2020 at 08:23:13PM +0100, Robin Murphy wrote:
But that's not what this is; this is (supposed to be) the exact same "don't
actually perform the attach yet" logic as before, just restricting it to
default domains in the one place that it actually needs to be, so as not to
fundamentally bugger up iommu_attach_device() in a way that prevents it from
working as expected at the correct point later.
You are right, that is better. I tested it and it seems to work. Updated
diff attached, with a minor cleanup included. Mind sending it as a
proper patch I can send upstream?
Thanks,
Joerg
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 7b375421afba..a9d02bc3ab5b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -693,6 +693,15 @@ static int iommu_group_create_direct_mappings(struct
iommu_group *group,
return ret;
}
+static bool iommu_is_attach_deferred(struct iommu_domain *domain,
+struct device *dev)
+{
+ if (domain->ops->is_attach_deferred)
+ return domain->ops->is_attach_deferred(domain, dev);
+
+ return false;
+}
+
/**
* iommu_group_add_device - add a device to an iommu group
* @group: the group into which to add the device (reference should be held)
@@ -705,6 +714,7 @@ int iommu_group_add_device(struct iommu_group *group,
struct device *dev)
{
int ret, i = 0;
struct group_device *device;
+ struct iommu_domain *domain;
device = kzalloc(sizeof(*device), GFP_KERNEL);
if (!device)
@@ -747,7 +757,8 @@ int iommu_group_add_device(struct iommu_group *group,
struct device *dev)
mutex_lock(&group->mutex);
list_add_tail(&device->list, &group->devices);
- if (group->domain)
+ domain = group->domain;
+ if (domain && !iommu_is_attach_deferred(domain, dev))
ret = __iommu_attach_device(group->domain, dev);
mutex_unlock(&group->mutex);
if (ret)
@@ -1653,9 +1664,6 @@ static int __iommu_attach_device(struct iommu_domain
*domain,
struct device *dev)
{
int ret;
- if ((domain->ops->is_attach_deferred != NULL) &&
- domain->ops->is_attach_deferred(domain, dev))
- return 0;
if (unlikely(domain->ops->attach_dev == NULL))
return -ENODEV;
@@ -1727,8 +1735,7 @@ EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
static void __iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
{
- if ((domain->ops->is_attach_deferred != NULL) &&
- domain->ops->is_attach_deferred(domain, dev))
+ if (iommu_is_attach_deferred(domain, dev))
return;
if (unlikely(domain->ops->detach_dev == NULL))
___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
This worked for me as well.
Re: [PATCH] iommu/amd: Add sanity check for interrupt remapping table length macros
Suravee Suthikulpanit @ 2020-12-10 09:24 MST:
> Currently, macros related to the interrupt remapping table length are
> defined separately. This has resulted in an oversight in which one of
> the macros were missed when changing the length. To prevent this,
> redefine the macros to add built-in sanity check.
>
> Also, rename macros to use the name of the DTE[IntTabLen] field as
> specified in the AMD IOMMU specification. There is no functional change.
>
> Suggested-by: Linus Torvalds
> Reviewed-by: Tom Lendacky
> Signed-off-by: Suravee Suthikulpanit
> Cc: Will Deacon
> Cc: Jerry Snitselaar
> Cc: Joerg Roedel
> ---
> drivers/iommu/amd/amd_iommu_types.h | 19 ++-
> drivers/iommu/amd/init.c| 6 +++---
> drivers/iommu/amd/iommu.c | 2 +-
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h
> b/drivers/iommu/amd/amd_iommu_types.h
> index 494b42a31b7a..899ce62df3f0 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -255,11 +255,19 @@
> /* Bit value definition for dte irq remapping fields*/
> #define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
> #define DTE_IRQ_REMAP_INTCTL_MASK(0x3ULL << 60)
> -#define DTE_IRQ_TABLE_LEN_MASK (0xfULL << 1)
> #define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
> -#define DTE_IRQ_TABLE_LEN (9ULL << 1)
> #define DTE_IRQ_REMAP_ENABLE1ULL
>
> +/*
> + * AMD IOMMU hardware only support 512 IRTEs despite
> + * the architectural limitation of 2048 entries.
> + */
> +#define DTE_INTTAB_ALIGNMENT128
> +#define DTE_INTTABLEN_VALUE 9ULL
> +#define DTE_INTTABLEN (DTE_INTTABLEN_VALUE << 1)
> +#define DTE_INTTABLEN_MASK (0xfULL << 1)
> +#define MAX_IRQS_PER_TABLE (1 << DTE_INTTABLEN_VALUE)
> +
> #define PAGE_MODE_NONE0x00
> #define PAGE_MODE_1_LEVEL 0x01
> #define PAGE_MODE_2_LEVEL 0x02
> @@ -409,13 +417,6 @@ extern bool amd_iommu_np_cache;
> /* Only true if all IOMMUs support device IOTLBs */
> extern bool amd_iommu_iotlb_sup;
>
> -/*
> - * AMD IOMMU hardware only support 512 IRTEs despite
> - * the architectural limitation of 2048 entries.
> - */
> -#define MAX_IRQS_PER_TABLE 512
> -#define IRQ_TABLE_ALIGNMENT 128
> -
> struct irq_remap_table {
> raw_spinlock_t lock;
> unsigned min_index;
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 23a790f8f550..6bec8913d064 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -989,10 +989,10 @@ static bool copy_device_table(void)
>
> irq_v = old_devtb[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
> int_ctl = old_devtb[devid].data[2] & DTE_IRQ_REMAP_INTCTL_MASK;
> - int_tab_len = old_devtb[devid].data[2] & DTE_IRQ_TABLE_LEN_MASK;
> + int_tab_len = old_devtb[devid].data[2] & DTE_INTTABLEN_MASK;
> if (irq_v && (int_ctl || int_tab_len)) {
> if ((int_ctl != DTE_IRQ_REMAP_INTCTL) ||
> - (int_tab_len != DTE_IRQ_TABLE_LEN)) {
> + (int_tab_len != DTE_INTTABLEN)) {
> pr_err("Wrong old irq remapping flag: %#x\n",
> devid);
> return false;
> }
> @@ -2674,7 +2674,7 @@ static int __init early_amd_iommu_init(void)
> remap_cache_sz = MAX_IRQS_PER_TABLE * (sizeof(u64) * 2);
> amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
> remap_cache_sz,
> - IRQ_TABLE_ALIGNMENT,
> + DTE_INTTAB_ALIGNMENT,
> 0, NULL);
> if (!amd_iommu_irq_cache)
> goto out;
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index b9cf59443843..f7abf16d1e3a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3191,7 +3191,7 @@ static void set_dte_irq_entry(u16 devid, struct
> irq_remap_table *table)
> dte &= ~DTE_IRQ_PHYS_ADDR_MASK;
> dte |= iommu_virt_to_phys(table->table);
> dte |= DTE_IRQ_REMAP_INTCTL;
> - dte |= DTE_IRQ_TABLE_LEN;
> + dte |= DTE_INTTABLEN;
> dte |= DTE_IRQ_REMAP_ENABLE;
>
> amd_iommu_dev_table[devid].data[2] = dte;
Reviewed-by: Jerry Snitselaar
Re: [PATCH] iommu/amd: Set DTE[IntTabLen] to represent 512 IRTEs
Suravee Suthikulpanit @ 2020-12-07 02:19 MST:
> According to the AMD IOMMU spec, the commit 73db2fc595f3
> ("iommu/amd: Increase interrupt remapping table limit to 512 entries")
> also requires the interrupt table length (IntTabLen) to be set to 9
> (power of 2) in the device table mapping entry (DTE).
>
> Fixes: 73db2fc595f3 ("iommu/amd: Increase interrupt remapping table limit to
> 512 entries")
> Reported-by: Jerry Snitselaar
> Signed-off-by: Suravee Suthikulpanit
> ---
> drivers/iommu/amd/amd_iommu_types.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h
> b/drivers/iommu/amd/amd_iommu_types.h
> index 89647700bab2..494b42a31b7a 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -257,7 +257,7 @@
> #define DTE_IRQ_REMAP_INTCTL_MASK(0x3ULL << 60)
> #define DTE_IRQ_TABLE_LEN_MASK (0xfULL << 1)
> #define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
> -#define DTE_IRQ_TABLE_LEN (8ULL << 1)
> +#define DTE_IRQ_TABLE_LEN (9ULL << 1)
> #define DTE_IRQ_REMAP_ENABLE1ULL
>
> #define PAGE_MODE_NONE0x00
Reviewed-by: Jerry Snitselaar
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
Matthew Garrett @ 2020-10-15 15:39 MST:
> On Thu, Oct 15, 2020 at 2:44 PM Jerry Snitselaar wrote:
>>
>> There is a misconfiguration in the bios of the gpio pin used for the
>> interrupt in the T490s. When interrupts are enabled in the tpm_tis
>> driver code this results in an interrupt storm. This was initially
>> reported when we attempted to enable the interrupt code in the tpm_tis
>> driver, which previously wasn't setting a flag to enable it. Due to
>> the reports of the interrupt storm that code was reverted and we went back
>> to polling instead of using interrupts. Now that we know the T490s problem
>> is a firmware issue, add code to check if the system is a T490s and
>> disable interrupts if that is the case. This will allow us to enable
>> interrupts for everyone else. If the user has a fixed bios they can
>> force the enabling of interrupts with tpm_tis.interrupts=1 on the
>> kernel command line.
>
> I think an implication of this is that systems haven't been
> well-tested with interrupts enabled. In general when we've found a
> firmware issue in one place it ends up happening elsewhere as well, so
> it wouldn't surprise me if there are other machines that will also be
> unhappy with interrupts enabled. Would it be possible to automatically
> detect this case (eg, if we get more than a certain number of
> interrupts in a certain timeframe immediately after enabling the
> interrupt) and automatically fall back to polling in that case? It
> would also mean that users with fixed firmware wouldn't need to pass a
> parameter.
I believe Matthew is correct here. I found another system today
with completely different vendor for both the system and the tpm chip.
In addition another Lenovo model, the L490, has the issue.
This initial attempt at a solution like Matthew suggested works on
the system I found today, but I imagine it is all sorts of wrong.
In the 2 systems where I've seen it, there are about 10 interrupts
in around 1.5 seconds, and then the irq code shuts down the interrupt
because they aren't being handled.
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 49ae09ac604f..478e9d02a3fa 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -27,6 +27,11 @@
#include "tpm.h"
#include "tpm_tis_core.h"
+static unsigned int time_start = 0;
+static bool storm_check = true;
+static bool storm_killed = false;
+static u32 irqs_fired = 0;
+
static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value);
static void tpm_tis_enable_interrupt(struct tpm_chip *chip, u8 mask)
@@ -464,25 +469,31 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const
u8 *buf, size_t len)
return rc;
}
-static void disable_interrupts(struct tpm_chip *chip)
+static void __disable_interrupts(struct tpm_chip *chip)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
u32 intmask;
int rc;
- if (priv->irq == 0)
- return;
-
rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
if (rc < 0)
intmask = 0;
intmask &= ~TPM_GLOBAL_INT_ENABLE;
rc = tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
+ chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+}
+
+static void disable_interrupts(struct tpm_chip *chip)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ if (priv->irq == 0)
+ return;
+
+ __disable_interrupts(chip);
devm_free_irq(chip->dev.parent, priv->irq, chip);
priv->irq = 0;
- chip->flags &= ~TPM_CHIP_FLAG_IRQ;
}
/*
@@ -528,6 +539,12 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf,
size_t len)
int rc, irq;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ if (unlikely(storm_killed)) {
+ devm_free_irq(chip->dev.parent, priv->irq, chip);
+ priv->irq = 0;
+ storm_killed = false;
+ }
+
if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested)
return tpm_tis_send_main(chip, buf, len);
@@ -748,6 +765,21 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
u32 interrupt;
int i, rc;
+ if (storm_check) {
+ irqs_fired++;
+
+ if (!time_start) {
+ time_start = jiffies_to_msecs(jiffies);
+ } else if ((irqs_fired > 1000) && (jiffies_to_msecs(jiffies) -
jiffies < 500)) {
+ __disable_interrupts(chip);
+ storm_check = false;
+ storm_killed = true;
+ return IRQ_HANDLED;
+ } else if ((
Re: [GIT PULL] IOMMU fix for 5.10 (-final)
Will Deacon @ 2020-12-09 11:50 MST: > On Wed, Dec 09, 2020 at 10:07:46AM -0800, Linus Torvalds wrote: >> On Wed, Dec 9, 2020 at 6:12 AM Will Deacon wrote: >> > >> > Please pull this one-liner AMD IOMMU fix for 5.10. It's actually a fix >> > for a fix, where the size of the interrupt remapping table was increased >> > but a related constant for the size of the interrupt table was forgotten. >> >> Pulled. > > Thanks. > >> However, why didn't this then add some sanity checking for the two >> different #defines to be in sync? >> >> IOW, something like >> >>#define AMD_IOMMU_IRQ_TABLE_SHIFT 9 >> >>#define MAX_IRQS_PER_TABLE (1 << AMD_IOMMU_IRQ_TABLE_SHIFT) >>#define DTE_IRQ_TABLE_LEN ((u64)AMD_IOMMU_IRQ_TABLE_SHIFT << 1) Since the field in the device table entry format expects it to be n where there are 2^n entries in the table I guess it should be: #define DTE_IRQ_TABLE_LEN 9 #define MAX_IRQS_PER_TABLE (1 << DTE_IRQ_TABLE_LEN) >> >> or whatever. Hmm? > > This looks like a worthwhile change to me, but I don't have any hardware > so I've been very reluctant to make even "obvious" driver changes here. > > Suravee -- please can you post a patch implementing the above? > >> That way this won't happen again, but perhaps equally importantly the >> linkage will be more clear, and there won't be those random constants. >> >> Naming above is probably garbage - I assume there's some actual >> architectural name for that irq table length field in the DTE? > > The one in the spec is even better: "IntTabLen". > > Will > ___ > iommu mailing list > io...@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] IOMMU fix for 5.10 (-final)
On Wed, Dec 9, 2020 at 12:12 PM Jerry Snitselaar wrote: > > > Will Deacon @ 2020-12-09 11:50 MST: > > > On Wed, Dec 09, 2020 at 10:07:46AM -0800, Linus Torvalds wrote: > >> On Wed, Dec 9, 2020 at 6:12 AM Will Deacon wrote: > >> > > >> > Please pull this one-liner AMD IOMMU fix for 5.10. It's actually a fix > >> > for a fix, where the size of the interrupt remapping table was increased > >> > but a related constant for the size of the interrupt table was forgotten. > >> > >> Pulled. > > > > Thanks. > > > >> However, why didn't this then add some sanity checking for the two > >> different #defines to be in sync? > >> > >> IOW, something like > >> > >>#define AMD_IOMMU_IRQ_TABLE_SHIFT 9 > >> > >>#define MAX_IRQS_PER_TABLE (1 << AMD_IOMMU_IRQ_TABLE_SHIFT) > >>#define DTE_IRQ_TABLE_LEN ((u64)AMD_IOMMU_IRQ_TABLE_SHIFT << 1) > > Since the field in the device table entry format expects it to be n > where there are 2^n entries in the table I guess it should be: > > #define DTE_IRQ_TABLE_LEN 9 > #define MAX_IRQS_PER_TABLE (1 << DTE_IRQ_TABLE_LEN) > No, ignore that. I'm being stupid. > >> > >> or whatever. Hmm? > > > > This looks like a worthwhile change to me, but I don't have any hardware > > so I've been very reluctant to make even "obvious" driver changes here. > > > > Suravee -- please can you post a patch implementing the above? > > > >> That way this won't happen again, but perhaps equally importantly the > >> linkage will be more clear, and there won't be those random constants. > >> > >> Naming above is probably garbage - I assume there's some actual > >> architectural name for that irq table length field in the DTE? > > > > The one in the spec is even better: "IntTabLen". > > > > Will > > ___ > > iommu mailing list > > io...@lists.linux-foundation.org > > https://lists.linuxfoundation.org/mailman/listinfo/iommu >
Re: [GIT PULL] IOMMU fix for 5.10 (-final)
On Wed, Dec 9, 2020 at 12:18 PM Linus Torvalds wrote: > > On Wed, Dec 9, 2020 at 11:12 AM Jerry Snitselaar wrote: > > > > Since the field in the device table entry format expects it to be n > > where there are 2^n entries in the table I guess it should be: > > > > #define DTE_IRQ_TABLE_LEN 9 > > #define MAX_IRQS_PER_TABLE (1 << DTE_IRQ_TABLE_LEN) > > No, that "DTE_IRQ_TABLE_LEN" is not the size shift - it's the size > shift value in that DTE field, which is shifted up by 1. > > That's why the current code does that > >#define DTE_IRQ_TABLE_LEN (9ULL << 1) > > there.. > > Which was why I suggested that new #define that is the *actual* shift > value, and then the DTE thing and the MAX_IRQS_PER_TABLE values would > depend on that. > >Linus > Yes, when I read it my head was translating it as setting them both to 512 and then I forgot that it gets shifted over 1. Which considering I was the once who noticed the original problem of it still being 8 was a nice brain fart. This should be fixed like you suggest.
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
Hans de Goede @ 2020-11-19 07:42 MST: > Hi, > > On 11/19/20 7:36 AM, Jerry Snitselaar wrote: >> >> Matthew Garrett @ 2020-10-15 15:39 MST: >> >>> On Thu, Oct 15, 2020 at 2:44 PM Jerry Snitselaar >>> wrote: >>>> >>>> There is a misconfiguration in the bios of the gpio pin used for the >>>> interrupt in the T490s. When interrupts are enabled in the tpm_tis >>>> driver code this results in an interrupt storm. This was initially >>>> reported when we attempted to enable the interrupt code in the tpm_tis >>>> driver, which previously wasn't setting a flag to enable it. Due to >>>> the reports of the interrupt storm that code was reverted and we went back >>>> to polling instead of using interrupts. Now that we know the T490s problem >>>> is a firmware issue, add code to check if the system is a T490s and >>>> disable interrupts if that is the case. This will allow us to enable >>>> interrupts for everyone else. If the user has a fixed bios they can >>>> force the enabling of interrupts with tpm_tis.interrupts=1 on the >>>> kernel command line. >>> >>> I think an implication of this is that systems haven't been >>> well-tested with interrupts enabled. In general when we've found a >>> firmware issue in one place it ends up happening elsewhere as well, so >>> it wouldn't surprise me if there are other machines that will also be >>> unhappy with interrupts enabled. Would it be possible to automatically >>> detect this case (eg, if we get more than a certain number of >>> interrupts in a certain timeframe immediately after enabling the >>> interrupt) and automatically fall back to polling in that case? It >>> would also mean that users with fixed firmware wouldn't need to pass a >>> parameter. >> >> I believe Matthew is correct here. I found another system today >> with completely different vendor for both the system and the tpm chip. >> In addition another Lenovo model, the L490, has the issue. >> >> This initial attempt at a solution like Matthew suggested works on >> the system I found today, but I imagine it is all sorts of wrong. >> In the 2 systems where I've seen it, there are about 10 interrupts >> in around 1.5 seconds, and then the irq code shuts down the interrupt >> because they aren't being handled. > > Is that with your patch? The IRQ should be silenced as soon as > devm_free_irq(chip->dev.parent, priv->irq, chip); is called. > No that is just with James' patchset that enables interrupts for tpm_tis. It looks like the irq is firing, but the tpm's int_status register is clear, so the handler immediately returns IRQ_NONE. After that happens 10 times the core irq code shuts down the irq, but it isn't released so I could still see the stats in /proc/interrupts. With my attempt below on top of that patchset it releases the irq. I had to stick the check prior to it checking the int_status register because it is cleared and the handler returns, and I couldn't do the devm_free_irq directly in tis_int_handler, so I tried sticking it in tpm_tis_send where the other odd irq testing code was already located. I'm not sure if there is another place that would work better for calling the devm_free_irq. > Depending on if we can get your storm-detection to work or not, > we might also choose to just never try to use the IRQ (at least on > x86 systems). AFAIK the TPM is never used for high-throughput stuff > so the polling overhead should not be a big deal (and I'm getting the feeling > that Windows always polls). > I was wondering about Windows as well. In addition to the Lenovo systems which the T490s had Nuvoton tpm, the system I found yesterday was a development system we have from a partner with an Infineon tpm. Dan Williams has seen it internally at Intel as well on some system. > Regards, > > Hans > > > >> diff --git a/drivers/char/tpm/tpm_tis_core.c >> b/drivers/char/tpm/tpm_tis_core.c >> index 49ae09ac604f..478e9d02a3fa 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -27,6 +27,11 @@ >> #include "tpm.h" >> #include "tpm_tis_core.h" >> >> +static unsigned int time_start = 0; >> +static bool storm_check = true; >> +static bool storm_killed = false; >> +static u32 irqs_fired = 0; >> + >> static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value); >> >> static void tpm_tis_enable_interrupt(struct tpm_chip *chip, u8 mask
Re: Question about domain_init (v5.3-v5.7)
Lu Baolu @ 2020-11-26 19:12 MST: > Hi Jerry, > > On 11/27/20 5:35 AM, Jerry Snitselaar wrote: >> Lu Baolu @ 2020-11-26 04:01 MST: >> >>> Hi Jerry, >>> >>> On 2020/11/26 4:27, Jerry Snitselaar wrote: >>>> Is there a reason we check the requested guest address width against >>>> the >>>> iommu's mgaw, instead of the agaw that we already know for the iommu? >>>> I've run into a case with a new system where the mgaw reported is 57, >>>> but if they set PAE to 46 instead of 52 in the bios, then sagaw reports >>>> the highest supported agaw is 48 and the domain_init code fails here. In >>> >>> Isn't this a platform bug? If it's too late to fix it in the BIOS, you >>> maybe have to add a platform specific quirk to set mgaw to the highest >>> supported agaw? >>> >>> Best regards, >>> baolu >> Is there somewhere you can point me to that discusses how they >> should be >> setting the mgaw? I misunderstood when I previously asked you about >> whether the mgaw could be a value that was greater than any of sagaw. >> If it is a bios issue, then they should fix it there. > > MGAW indicates the max gpa width supported by 2nd translation. The VT-d > spec requires that this value must be at least equal to the host > physical addressibility. According to this, BIOS is good, right? > > For this failure case, domain_init() just wants to find a suitable agaw > for the private domain. I think it makes sense to check against > iommu->agaw instead of cap_mgaw. > > Best regards, > baolu > >From this bit in the spec about MGAW: Guest addressability for a given DMA request is limited to the minimum of the value reported through this field and the adjusted guest address width of the corresponding page-table structure. (Adjusted guest address widths supported by hardware are reported through the SAGAW field). That does suggest it should be adjusted down to the sagaw value in this case, yes? Just want to make sure I'm understanding it correctly. >> >>> >>>> other places like prepare_domain_attach_device, the dmar domain agaw >>>> gets adjusted down to the iommu agaw. The agaw of the iommu gets >>>> determined based off what is reported for sagaw. I'm wondering if it >>>> can't instead do: >>>> --- >>>>drivers/iommu/intel-iommu.c | 4 ++-- >>>>1 file changed, 2 insertions(+), 2 deletions(-) >>>> diff --git a/drivers/iommu/intel-iommu.c >>>> b/drivers/iommu/intel-iommu.c >>>> index 6ca5c92ef2e5..a8e41ec36d9e 100644 >>>> --- a/drivers/iommu/intel-iommu.c >>>> +++ b/drivers/iommu/intel-iommu.c >>>> @@ -1862,8 +1862,8 @@ static int domain_init(struct dmar_domain *domain, >>>> struct intel_iommu *iommu, >>>>domain_reserve_special_ranges(domain); >>>>/* calculate AGAW */ >>>> - if (guest_width > cap_mgaw(iommu->cap)) >>>> - guest_width = cap_mgaw(iommu->cap); >>>> + if (guest_width > agaw_to_width(iommu->agaw)) >>>> + guest_width = agaw_to_width(iommu->agaw); >>>>domain->gaw = guest_width; >>>>adjust_width = guestwidth_to_adjustwidth(guest_width); >>>>agaw = width_to_agaw(adjust_width); >>>> -- >>>> 2.27.0 >>>> >>>> Thoughts? With the former code the ehci device for the ilo fails when >>>> trying to get a private domain. >>>> Thanks, >>>> Jerry >>>> >>
Re: Question about domain_init (v5.3-v5.7)
Jerry Snitselaar @ 2020-11-30 10:50 MST: > Lu Baolu @ 2020-11-26 19:12 MST: > >> Hi Jerry, >> >> On 11/27/20 5:35 AM, Jerry Snitselaar wrote: >>> Lu Baolu @ 2020-11-26 04:01 MST: >>> >>>> Hi Jerry, >>>> >>>> On 2020/11/26 4:27, Jerry Snitselaar wrote: >>>>> Is there a reason we check the requested guest address width against >>>>> the >>>>> iommu's mgaw, instead of the agaw that we already know for the iommu? >>>>> I've run into a case with a new system where the mgaw reported is 57, >>>>> but if they set PAE to 46 instead of 52 in the bios, then sagaw reports >>>>> the highest supported agaw is 48 and the domain_init code fails here. In >>>> >>>> Isn't this a platform bug? If it's too late to fix it in the BIOS, you >>>> maybe have to add a platform specific quirk to set mgaw to the highest >>>> supported agaw? >>>> >>>> Best regards, >>>> baolu >>> Is there somewhere you can point me to that discusses how they >>> should be >>> setting the mgaw? I misunderstood when I previously asked you about >>> whether the mgaw could be a value that was greater than any of sagaw. >>> If it is a bios issue, then they should fix it there. >> >> MGAW indicates the max gpa width supported by 2nd translation. The VT-d >> spec requires that this value must be at least equal to the host >> physical addressibility. According to this, BIOS is good, right? >> >> For this failure case, domain_init() just wants to find a suitable agaw >> for the private domain. I think it makes sense to check against >> iommu->agaw instead of cap_mgaw. >> >> Best regards, >> baolu >> > > From this bit in the spec about MGAW: > > Guest addressability for a given DMA request is limited to the > minimum of the value reported through this field and the adjusted > guest address width of the corresponding page-table structure. > (Adjusted guest address widths supported by hardware are reported > through the SAGAW field). > > That does suggest it should be adjusted down to the sagaw value in this case, > yes? > Just want to make sure I'm understanding it correctly. Or I guess that is really talking about if you had an mgaw lower than the the sagaw, the dma request would be limited to that lower mgaw value. > >>> >>>> >>>>> other places like prepare_domain_attach_device, the dmar domain agaw >>>>> gets adjusted down to the iommu agaw. The agaw of the iommu gets >>>>> determined based off what is reported for sagaw. I'm wondering if it >>>>> can't instead do: >>>>> --- >>>>>drivers/iommu/intel-iommu.c | 4 ++-- >>>>>1 file changed, 2 insertions(+), 2 deletions(-) >>>>> diff --git a/drivers/iommu/intel-iommu.c >>>>> b/drivers/iommu/intel-iommu.c >>>>> index 6ca5c92ef2e5..a8e41ec36d9e 100644 >>>>> --- a/drivers/iommu/intel-iommu.c >>>>> +++ b/drivers/iommu/intel-iommu.c >>>>> @@ -1862,8 +1862,8 @@ static int domain_init(struct dmar_domain *domain, >>>>> struct intel_iommu *iommu, >>>>> domain_reserve_special_ranges(domain); >>>>> /* calculate AGAW */ >>>>> - if (guest_width > cap_mgaw(iommu->cap)) >>>>> - guest_width = cap_mgaw(iommu->cap); >>>>> + if (guest_width > agaw_to_width(iommu->agaw)) >>>>> + guest_width = agaw_to_width(iommu->agaw); >>>>> domain->gaw = guest_width; >>>>> adjust_width = guestwidth_to_adjustwidth(guest_width); >>>>> agaw = width_to_agaw(adjust_width); >>>>> -- >>>>> 2.27.0 >>>>> >>>>> Thoughts? With the former code the ehci device for the ilo fails when >>>>> trying to get a private domain. >>>>> Thanks, >>>>> Jerry >>>>> >>>
[PATCH] tpm_tis: Disable interrupts if interrupt storm detected
When enabling the interrupt code for the tpm_tis driver we have
noticed some systems have a bios issue causing an interrupt storm to
occur. The issue isn't limited to a single tpm or system vendor
so keeping a denylist of systems with the issue isn't optimal. Instead
try to detect the problem occurring, disable interrupts, and revert to
polling when it happens.
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Cc: Peter Huewe
Cc: James Bottomley
Cc: Matthew Garrett
Cc: Hans de Goede
Signed-off-by: Jerry Snitselaar
---
drivers/char/tpm/tpm_tis_core.c | 34 +
drivers/char/tpm/tpm_tis_core.h | 2 ++
2 files changed, 36 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 92c51c6cfd1b..19115a628f25 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -24,6 +24,8 @@
#include
#include
#include
+#include
+#include
#include "tpm.h"
#include "tpm_tis_core.h"
@@ -711,13 +713,30 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip,
u8 status)
}
}
+static struct workqueue_struct *tpm_tis_wq;
+static DEFINE_MUTEX(tpm_tis_wq_lock);
+
static irqreturn_t tis_int_handler(int dummy, void *dev_id)
{
struct tpm_chip *chip = dev_id;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ static bool check_storm = true;
+ static unsigned int check_start;
u32 interrupt;
int i, rc;
+ if (unlikely(check_storm)) {
+ if (!check_start) {
+ check_start = jiffies_to_msecs(jiffies);
+ } else if ((kstat_irqs(priv->irq) > 1000) &&
+ (jiffies_to_msecs(jiffies) - check_start < 500)) {
+ check_storm = false;
+ queue_work(tpm_tis_wq, &priv->storm_work);
+ } else if (jiffies_to_msecs(jiffies) - check_start >= 500) {
+ check_storm = false;
+ }
+ }
+
rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
if (rc < 0)
return IRQ_NONE;
@@ -943,6 +962,14 @@ static const struct tpm_class_ops tpm_tis = {
.clk_enable = tpm_tis_clkrun_enable,
};
+static void tpm_tis_storm_work(struct work_struct *work)
+{
+ struct tpm_tis_data *priv = container_of(work, struct tpm_tis_data,
storm_work);
+
+ disable_interrupts(priv->chip);
+ dev_warn(&priv->chip->dev, "Interrupt storm detected, using
polling.\n");
+}
+
int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
const struct tpm_tis_phy_ops *phy_ops,
acpi_handle acpi_dev_handle)
@@ -959,6 +986,13 @@ int tpm_tis_core_init(struct device *dev, struct
tpm_tis_data *priv, int irq,
if (IS_ERR(chip))
return PTR_ERR(chip);
+ priv->chip = chip;
+ tpm_tis_wq = alloc_workqueue("tpm_tis_wq", WQ_MEM_RECLAIM, 0);
+ if (!tpm_tis_wq)
+ return -ENOMEM;
+
+ INIT_WORK(&priv->storm_work, tpm_tis_storm_work);
+
#ifdef CONFIG_ACPI
chip->acpi_dev_handle = acpi_dev_handle;
#endif
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 9b2d32a59f67..973297ee2e16 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -95,6 +95,8 @@ struct tpm_tis_data {
u16 clkrun_enabled;
wait_queue_head_t int_queue;
wait_queue_head_t read_queue;
+ struct work_struct storm_work;
+ struct tpm_chip *chip;
const struct tpm_tis_phy_ops *phy_ops;
unsigned short rng_quality;
};
--
2.27.0
[PATCH v2] tpm_tis: Disable interrupts if interrupt storm detected
When enabling the interrupt code for the tpm_tis driver we have
noticed some systems have a bios issue causing an interrupt storm to
occur. The issue isn't limited to a single tpm or system manufacturer
so keeping a denylist of systems with the issue isn't optimal. Instead
try to detect the problem occurring, disable interrupts, and revert to
polling when it happens.
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Cc: Peter Huewe
Cc: James Bottomley
Cc: Matthew Garrett
Cc: Hans de Goede
Signed-off-by: Jerry Snitselaar
---
v2: drop tpm_tis specific workqueue and use just system_wq
drivers/char/tpm/tpm_tis_core.c | 27 +++
drivers/char/tpm/tpm_tis_core.h | 2 ++
2 files changed, 29 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 23b60583928b..72cc8a5a152c 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -24,6 +24,8 @@
#include
#include
#include
+#include
+#include
#include "tpm.h"
#include "tpm_tis_core.h"
@@ -745,9 +747,23 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
{
struct tpm_chip *chip = dev_id;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ static bool check_storm = true;
+ static unsigned int check_start;
u32 interrupt;
int i, rc;
+ if (unlikely(check_storm)) {
+ if (!check_start) {
+ check_start = jiffies_to_msecs(jiffies);
+ } else if ((kstat_irqs(priv->irq) > 1000) &&
+ (jiffies_to_msecs(jiffies) - check_start < 500)) {
+ check_storm = false;
+ schedule_work(&priv->storm_work);
+ } else if (jiffies_to_msecs(jiffies) - check_start >= 500) {
+ check_storm = false;
+ }
+ }
+
rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
if (rc < 0)
return IRQ_NONE;
@@ -987,6 +1003,14 @@ static const struct tpm_class_ops tpm_tis = {
.clk_enable = tpm_tis_clkrun_enable,
};
+static void tpm_tis_storm_work(struct work_struct *work)
+{
+ struct tpm_tis_data *priv = container_of(work, struct tpm_tis_data,
storm_work);
+
+ disable_interrupts(priv->chip);
+ dev_warn(&priv->chip->dev, "Interrupt storm detected, using
polling.\n");
+}
+
int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
const struct tpm_tis_phy_ops *phy_ops,
acpi_handle acpi_dev_handle)
@@ -1003,6 +1027,9 @@ int tpm_tis_core_init(struct device *dev, struct
tpm_tis_data *priv, int irq,
if (IS_ERR(chip))
return PTR_ERR(chip);
+ priv->chip = chip;
+ INIT_WORK(&priv->storm_work, tpm_tis_storm_work);
+
#ifdef CONFIG_ACPI
chip->acpi_dev_handle = acpi_dev_handle;
#endif
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index edeb5dc61c95..5630f294dc0c 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -95,6 +95,8 @@ struct tpm_tis_data {
u16 clkrun_enabled;
wait_queue_head_t int_queue;
wait_queue_head_t read_queue;
+ struct work_struct storm_work;
+ struct tpm_chip *chip;
const struct tpm_tis_phy_ops *phy_ops;
unsigned short rng_quality;
};
--
2.27.0
Re: [PATCH v2] tpm_tis: Disable interrupts if interrupt storm detected
Jerry Snitselaar @ 2020-11-30 19:58 MST:
> When enabling the interrupt code for the tpm_tis driver we have
> noticed some systems have a bios issue causing an interrupt storm to
> occur. The issue isn't limited to a single tpm or system manufacturer
> so keeping a denylist of systems with the issue isn't optimal. Instead
> try to detect the problem occurring, disable interrupts, and revert to
> polling when it happens.
>
> Cc: Jarkko Sakkinen
> Cc: Jason Gunthorpe
> Cc: Peter Huewe
> Cc: James Bottomley
> Cc: Matthew Garrett
> Cc: Hans de Goede
> Signed-off-by: Jerry Snitselaar
> ---
> v2: drop tpm_tis specific workqueue and use just system_wq
>
> drivers/char/tpm/tpm_tis_core.c | 27 +++
> drivers/char/tpm/tpm_tis_core.h | 2 ++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 23b60583928b..72cc8a5a152c 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -24,6 +24,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include "tpm.h"
> #include "tpm_tis_core.h"
>
> @@ -745,9 +747,23 @@ static irqreturn_t tis_int_handler(int dummy, void
> *dev_id)
> {
> struct tpm_chip *chip = dev_id;
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> + static bool check_storm = true;
> + static unsigned int check_start;
> u32 interrupt;
> int i, rc;
>
> + if (unlikely(check_storm)) {
> + if (!check_start) {
> + check_start = jiffies_to_msecs(jiffies);
> + } else if ((kstat_irqs(priv->irq) > 1000) &&
> +(jiffies_to_msecs(jiffies) - check_start < 500)) {
> + check_storm = false;
> + schedule_work(&priv->storm_work);
> + } else if (jiffies_to_msecs(jiffies) - check_start >= 500) {
> + check_storm = false;
> + }
> + }
> +
> rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
> if (rc < 0)
> return IRQ_NONE;
> @@ -987,6 +1003,14 @@ static const struct tpm_class_ops tpm_tis = {
> .clk_enable = tpm_tis_clkrun_enable,
> };
>
> +static void tpm_tis_storm_work(struct work_struct *work)
> +{
> + struct tpm_tis_data *priv = container_of(work, struct tpm_tis_data,
> storm_work);
> +
> + disable_interrupts(priv->chip);
> + dev_warn(&priv->chip->dev, "Interrupt storm detected, using
> polling.\n");
> +}
> +
> int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> const struct tpm_tis_phy_ops *phy_ops,
> acpi_handle acpi_dev_handle)
> @@ -1003,6 +1027,9 @@ int tpm_tis_core_init(struct device *dev, struct
> tpm_tis_data *priv, int irq,
> if (IS_ERR(chip))
> return PTR_ERR(chip);
>
> + priv->chip = chip;
> + INIT_WORK(&priv->storm_work, tpm_tis_storm_work);
> +
> #ifdef CONFIG_ACPI
> chip->acpi_dev_handle = acpi_dev_handle;
> #endif
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index edeb5dc61c95..5630f294dc0c 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -95,6 +95,8 @@ struct tpm_tis_data {
> u16 clkrun_enabled;
> wait_queue_head_t int_queue;
> wait_queue_head_t read_queue;
> + struct work_struct storm_work;
> + struct tpm_chip *chip;
> const struct tpm_tis_phy_ops *phy_ops;
> unsigned short rng_quality;
> };
I've tested this with the Intel platform that has an Infineon chip that
I found the other week. It works, but isn't the complete fix. With this
on top of James' patchset I sometimes see the message "Lost Interrupt
waiting for TPM stat", so I guess there needs to be a check in
wait_for_tpm_stat and request_locality to see if interrupts were
disabled when the wait_event_interruptible_timeout call times out.
Re: [PATCH v2] tpm_tis: Disable interrupts if interrupt storm detected
James Bottomley @ 2020-12-03 14:05 MST: > On Thu, 2020-12-03 at 13:14 -0700, Jerry Snitselaar wrote: >> Jerry Snitselaar @ 2020-12-02 23:11 MST: > [...] >> > The interrupt storm detection code works on the T490s. I'm not sure >> > what is going on with the L490. I will see if I can get access to >> > one. >> > >> > Jerry >> >> Lenovo verified that the L490 hangs. > > Just to confirm, that's this system: > > https://www.lenovo.com/us/en/laptops/thinkpad/thinkpad-l/ThinkPad-L490/p/22TP2TBL490 > > We could ask if lenovo will give us one, but if not we could pull a > Jens. [the backstory is that when Jens was doing queueing in the block > layer, there were lots of SATA devices that didn't work quite right but > you couldn't tell unless you actually tried them out. Getting > manufacturers to send samples is rather arduous, so he took to ordering > them online, testing them out, and then returning them for a full > refund within the allowed window] > > It looks like Lenovo has a nice christmas returns policy: > > https://www.lenovo.com/us/en/shopping-faq/#returns > > James Yes, that is the one. I'm seeing if we have any located somewhere, or if Lenovo will loan me one. I think for the time being the patch that disabled interrupts for the T490s could be changed to it for the L490 instead. I'll post a v3 of my current patchset. It would probably make sense for it to go in with your patches when they land.
[PATCH v3 2/4] drm/i915/pmu: Use kstat_irqs to get interrupt count
Now that kstat_irqs is exported, get rid of count_interrupts in
i915_pmu.c
Cc: Thomas Gleixner
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: David Airlie
Cc: Daniel Vetter
Cc: intel-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Cc: Peter Huewe
Cc: James Bottomley
Cc: Matthew Garrett
Cc: Hans de Goede
Signed-off-by: Jerry Snitselaar
---
drivers/gpu/drm/i915/i915_pmu.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 69c0fa20eba1..a3e63f03da8c 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -423,22 +423,6 @@ static enum hrtimer_restart i915_sample(struct hrtimer
*hrtimer)
return HRTIMER_RESTART;
}
-static u64 count_interrupts(struct drm_i915_private *i915)
-{
- /* open-coded kstat_irqs() */
- struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
- u64 sum = 0;
- int cpu;
-
- if (!desc || !desc->kstat_irqs)
- return 0;
-
- for_each_possible_cpu(cpu)
- sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
-
- return sum;
-}
-
static void i915_pmu_event_destroy(struct perf_event *event)
{
struct drm_i915_private *i915 =
@@ -581,7 +565,7 @@ static u64 __i915_pmu_event_read(struct perf_event *event)
USEC_PER_SEC /* to MHz */);
break;
case I915_PMU_INTERRUPTS:
- val = count_interrupts(i915);
+ val = kstat_irqs(i915->drm.pdev->irq);
break;
case I915_PMU_RC6_RESIDENCY:
val = get_rc6(&i915->gt);
--
2.27.0
[PATCH v3 0/4] tpm_tis: Detect interrupt storms
This patchset is an attempt to try and catch tpm_tis devices that have interrupt storm issues, disable the interrupt, and use polling. In 2016 the tpm_tis interrupt code was accidently disabled, and polling was just being used. When we initially tried to enable interrupts again there were some reports of systems being hit with interrupt storms. It turned out that the ThinkPad T490s had misconfigured a gpio pin being used for the interrupt. The problem is more widespread though, with interrupt storms also being seen on other platforms and different TPM vendors. With the L490 the system hangs at tpm_tis initialization even with the detection code, so change the earlier detection code that used dmi to look for the T490s to instead look for the L490 and disable interrupts. Since kstat_irqs needs to be exported to allow building of tpm_tis as a module, I've included a patch to change the i915_pmu code to use kstat_irqs where before it was using its own version. If this isn't desired it can be dropped. I've been testing this on top of James' proposed patchset which re-enables interrupts for tpm_tis. With the patchsets applied it detects the problem on the T490s and on the Ice Lake development system where I found the issue. I have Lenovo verifying that the dmi detection code will now detect the L490 and avoid the hang it experiences. I'm also working on getting access to an L490 to see if I can figure out what the underlying issue is. Changes from v2: - Export kstat_irqs to allow building tpm_tis as a module. - Change i915_pmu.c to use kstat_irqs instead of it's own version count_interrupts. - Change include from linux/kernel_stat.h to linux/irq.h. - Change dmi checking code to now look for L490 instead of T490s. Changes from v1: - drop tpm_tis specific workqueue and use just system_w. Jerry Snitselaar (4): irq: export kstat_irqs drm/i915/pmu: Use kstat_irqs to get interrupt count tpm_tis: Disable interrupts if interrupt storm detected tpm_tis: Disable Interrupts on the ThinkPad L490 Cc: Thomas Gleixner Cc: Jani Nikula Cc: Rodrigo Vivi Cc: David Airlie Cc: Daniel Vetter Cc: intel-...@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Cc: Jarkko Sakkinen Cc: Jason Gunthorpe Cc: Peter Huewe Cc: James Bottomley Cc: Matthew Garrett Cc: Hans de Goede Cc: linux-integr...@vger.kernel.org drivers/char/tpm/tpm_tis.c | 4 ++-- drivers/char/tpm/tpm_tis_core.c | 27 +++ drivers/char/tpm/tpm_tis_core.h | 2 ++ drivers/gpu/drm/i915/i915_pmu.c | 18 +- include/linux/irqdesc.h | 1 + kernel/irq/irqdesc.c| 1 + 6 files changed, 34 insertions(+), 19 deletions(-) -- 2.27.0
[PATCH v3 3/4] tpm_tis: Disable interrupts if interrupt storm detected
When enabling the interrupt code for the tpm_tis driver we have
noticed some systems have a bios issue causing an interrupt storm to
occur. The issue isn't limited to a single tpm or system manufacturer
so keeping a denylist of systems with the issue isn't optimal. Instead
try to detect the problem occurring, disable interrupts, and revert to
polling when it happens.
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Cc: Peter Huewe
Cc: James Bottomley
Cc: Matthew Garrett
Cc: Hans de Goede
Signed-off-by: Jerry Snitselaar
---
v3: - Change include from linux/kernel_stat.h to linux/irq.h
v2: - drop tpm_tis specific workqueue and use just system_w
drivers/char/tpm/tpm_tis_core.c | 27 +++
drivers/char/tpm/tpm_tis_core.h | 2 ++
2 files changed, 29 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 92c51c6cfd1b..d817ff5664d1 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -24,6 +24,8 @@
#include
#include
#include
+#include
+#include
#include "tpm.h"
#include "tpm_tis_core.h"
@@ -715,9 +717,23 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
{
struct tpm_chip *chip = dev_id;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ static bool check_storm = true;
+ static unsigned int check_start;
u32 interrupt;
int i, rc;
+ if (unlikely(check_storm)) {
+ if (!check_start) {
+ check_start = jiffies_to_msecs(jiffies);
+ } else if ((kstat_irqs(priv->irq) > 1000) &&
+ (jiffies_to_msecs(jiffies) - check_start < 500)) {
+ check_storm = false;
+ schedule_work(&priv->storm_work);
+ } else if (jiffies_to_msecs(jiffies) - check_start >= 500) {
+ check_storm = false;
+ }
+ }
+
rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
if (rc < 0)
return IRQ_NONE;
@@ -943,6 +959,14 @@ static const struct tpm_class_ops tpm_tis = {
.clk_enable = tpm_tis_clkrun_enable,
};
+static void tpm_tis_storm_work(struct work_struct *work)
+{
+ struct tpm_tis_data *priv = container_of(work, struct tpm_tis_data,
storm_work);
+
+ disable_interrupts(priv->chip);
+ dev_warn(&priv->chip->dev, "Interrupt storm detected, using
polling.\n");
+}
+
int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
const struct tpm_tis_phy_ops *phy_ops,
acpi_handle acpi_dev_handle)
@@ -959,6 +983,9 @@ int tpm_tis_core_init(struct device *dev, struct
tpm_tis_data *priv, int irq,
if (IS_ERR(chip))
return PTR_ERR(chip);
+ priv->chip = chip;
+ INIT_WORK(&priv->storm_work, tpm_tis_storm_work);
+
#ifdef CONFIG_ACPI
chip->acpi_dev_handle = acpi_dev_handle;
#endif
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 9b2d32a59f67..973297ee2e16 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -95,6 +95,8 @@ struct tpm_tis_data {
u16 clkrun_enabled;
wait_queue_head_t int_queue;
wait_queue_head_t read_queue;
+ struct work_struct storm_work;
+ struct tpm_chip *chip;
const struct tpm_tis_phy_ops *phy_ops;
unsigned short rng_quality;
};
--
2.27.0
[PATCH v3 1/4] irq: export kstat_irqs
To try and detect potential interrupt storms that have been occurring with tpm_tis devices it was suggested to use kstat_irqs() to get the number of interrupts. Since tpm_tis can be built as a module it needs kstat_irqs exported. Reported-by: kernel test robot Cc: Thomas Gleixner Cc: Jarkko Sakkinen Cc: Jason Gunthorpe Cc: Peter Huewe Cc: James Bottomley Cc: Matthew Garrett Cc: Hans de Goede Signed-off-by: Jerry Snitselaar --- include/linux/irqdesc.h | 1 + kernel/irq/irqdesc.c| 1 + 2 files changed, 2 insertions(+) diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h index 5745491303e0..fff88c1f1ac6 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -153,6 +153,7 @@ static inline void generic_handle_irq_desc(struct irq_desc *desc) } int generic_handle_irq(unsigned int irq); +unsigned int kstat_irqs(unsigned int irq); #ifdef CONFIG_HANDLE_DOMAIN_IRQ /* diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 1a7723604399..12398ef1796b 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -1000,6 +1000,7 @@ unsigned int kstat_irqs(unsigned int irq) sum += *per_cpu_ptr(desc->kstat_irqs, cpu); return sum; } +EXPORT_SYMBOL_GPL(kstat_irqs); /** * kstat_irqs_usr - Get the statistics for an interrupt -- 2.27.0
[PATCH v3 4/4] tpm_tis: Disable Interrupts on the ThinkPad L490
The interrupt storm detection code detects the issue on the ThinkPad
T490s, but the L490 still hangs at initialization. So swap out the
T490s for the L490 in the dmi check.
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Cc: Peter Huewe
Cc: James Bottomley
Cc: Matthew Garrett
Cc: Hans de Goede
Signed-off-by: Jerry Snitselaar
---
drivers/char/tpm/tpm_tis.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 4ed6e660273a..7322e0986a83 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -77,10 +77,10 @@ static int tpm_tis_disable_irq(const struct dmi_system_id
*d)
static const struct dmi_system_id tpm_tis_dmi_table[] = {
{
.callback = tpm_tis_disable_irq,
- .ident = "ThinkPad T490s",
+ .ident = "ThinkPad L490",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad L490"),
},
},
{}
--
2.27.0
Re: [PATCH v3 1/4] irq: export kstat_irqs
Thomas Gleixner @ 2020-12-06 10:54 MST: > Jerry, > > On Fri, Dec 04 2020 at 18:43, Jerry Snitselaar wrote: > > The proper prefix is 'genirq:' git log kernel/irq/irqdesc.c would have > told you. > >> To try and detect potential interrupt storms that >> have been occurring with tpm_tis devices it was suggested >> to use kstat_irqs() to get the number of interrupts. >> Since tpm_tis can be built as a module it needs kstat_irqs >> exported. > > I'm not really enthused about exporting this without making it at least > safe. Using it from an interrupt handler is obviously safe vs. concurrent > removal, but the next driver writer who thinks this is cool is going to > get it wrong for sure. > > Though I still have to figure out what the advantage of invoking a > function which needs to do a radix tree lookup over a device local > counter is just to keep track of this. > > I'll reply on the TPM part of this as well. > > Thanks, > > tglx I can rework it to use a device local counter.
Re: [PATCH v3 2/4] drm/i915/pmu: Use kstat_irqs to get interrupt count
Thomas Gleixner @ 2020-12-06 09:38 MST:
> On Fri, Dec 04 2020 at 18:43, Jerry Snitselaar wrote:
>
>> Now that kstat_irqs is exported, get rid of count_interrupts in
>> i915_pmu.c
>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -423,22 +423,6 @@ static enum hrtimer_restart i915_sample(struct hrtimer
>> *hrtimer)
>> return HRTIMER_RESTART;
>> }
>>
>> -static u64 count_interrupts(struct drm_i915_private *i915)
>> -{
>> -/* open-coded kstat_irqs() */
>> -struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
>> -u64 sum = 0;
>> -int cpu;
>> -
>> -if (!desc || !desc->kstat_irqs)
>> -return 0;
>> -
>> -for_each_possible_cpu(cpu)
>> -sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
>> -
>> -return sum;
>> -}
>
> May I ask why this has been merged in the first place?
>
> Nothing in a driver has ever to fiddle with the internals of an irq
> descriptor. We have functions for properly accessing them. Just because
> C allows to fiddle with everything is not a justification. If the
> required function is not exported then adding the export with a proper
> explanation is not asked too much.
>
> Also this lacks protection or at least a comment why this can be called
> safely and is not subject to a concurrent removal of the irq descriptor.
> The same problem exists when calling kstat_irqs(). It's even documented
> at the top of the function.
>
> Thanks,
>
> tglx
I don't know the history behind this bit. I stumbled across it in cscope
when looking for places using kstat_irqs.
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
Jarkko Sakkinen @ 2020-11-23 20:26 MST:
> On Wed, Nov 18, 2020 at 11:36:20PM -0700, Jerry Snitselaar wrote:
>>
>> Matthew Garrett @ 2020-10-15 15:39 MST:
>>
>> > On Thu, Oct 15, 2020 at 2:44 PM Jerry Snitselaar
>> > wrote:
>> >>
>> >> There is a misconfiguration in the bios of the gpio pin used for the
>> >> interrupt in the T490s. When interrupts are enabled in the tpm_tis
>> >> driver code this results in an interrupt storm. This was initially
>> >> reported when we attempted to enable the interrupt code in the tpm_tis
>> >> driver, which previously wasn't setting a flag to enable it. Due to
>> >> the reports of the interrupt storm that code was reverted and we went back
>> >> to polling instead of using interrupts. Now that we know the T490s problem
>> >> is a firmware issue, add code to check if the system is a T490s and
>> >> disable interrupts if that is the case. This will allow us to enable
>> >> interrupts for everyone else. If the user has a fixed bios they can
>> >> force the enabling of interrupts with tpm_tis.interrupts=1 on the
>> >> kernel command line.
>> >
>> > I think an implication of this is that systems haven't been
>> > well-tested with interrupts enabled. In general when we've found a
>> > firmware issue in one place it ends up happening elsewhere as well, so
>> > it wouldn't surprise me if there are other machines that will also be
>> > unhappy with interrupts enabled. Would it be possible to automatically
>> > detect this case (eg, if we get more than a certain number of
>> > interrupts in a certain timeframe immediately after enabling the
>> > interrupt) and automatically fall back to polling in that case? It
>> > would also mean that users with fixed firmware wouldn't need to pass a
>> > parameter.
>>
>> I believe Matthew is correct here. I found another system today
>> with completely different vendor for both the system and the tpm chip.
>> In addition another Lenovo model, the L490, has the issue.
>>
>> This initial attempt at a solution like Matthew suggested works on
>> the system I found today, but I imagine it is all sorts of wrong.
>> In the 2 systems where I've seen it, there are about 10 interrupts
>> in around 1.5 seconds, and then the irq code shuts down the interrupt
>> because they aren't being handled.
>>
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c
>> b/drivers/char/tpm/tpm_tis_core.c
>> index 49ae09ac604f..478e9d02a3fa 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -27,6 +27,11 @@
>> #include "tpm.h"
>> #include "tpm_tis_core.h"
>>
>> +static unsigned int time_start = 0;
>> +static bool storm_check = true;
>> +static bool storm_killed = false;
>> +static u32 irqs_fired = 0;
>
> Maybe kstat_irqs() would be a better idea than ad hoc stats.
>
Thanks, yes that would be better.
>> +
>> static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value);
>>
>> static void tpm_tis_enable_interrupt(struct tpm_chip *chip, u8 mask)
>> @@ -464,25 +469,31 @@ static int tpm_tis_send_data(struct tpm_chip *chip,
>> const u8 *buf, size_t len)
>> return rc;
>> }
>>
>> -static void disable_interrupts(struct tpm_chip *chip)
>> +static void __disable_interrupts(struct tpm_chip *chip)
>> {
>> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>> u32 intmask;
>> int rc;
>>
>> - if (priv->irq == 0)
>> - return;
>> -
>> rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
>> if (rc < 0)
>> intmask = 0;
>>
>> intmask &= ~TPM_GLOBAL_INT_ENABLE;
>> rc = tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
>> + chip->flags &= ~TPM_CHIP_FLAG_IRQ;
>> +}
>> +
>> +static void disable_interrupts(struct tpm_chip *chip)
>> +{
>> + struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>>
>> + if (priv->irq == 0)
>> + return;
>> +
>> + __disable_interrupts(chip);
>> devm_free_irq(chip->dev.parent, priv->irq, chip);
>> priv->irq = 0;
>> - chip->flags &= ~TPM_CHIP_FLAG_IRQ;
>> }
>>
>> /*
>> @@ -528,6 +539,12
Question about domain_init (v5.3-v5.7)
Is there a reason we check the requested guest address width against the iommu's mgaw, instead of the agaw that we already know for the iommu? I've run into a case with a new system where the mgaw reported is 57, but if they set PAE to 46 instead of 52 in the bios, then sagaw reports the highest supported agaw is 48 and the domain_init code fails here. In other places like prepare_domain_attach_device, the dmar domain agaw gets adjusted down to the iommu agaw. The agaw of the iommu gets determined based off what is reported for sagaw. I'm wondering if it can't instead do: --- drivers/iommu/intel-iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 6ca5c92ef2e5..a8e41ec36d9e 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1862,8 +1862,8 @@ static int domain_init(struct dmar_domain *domain, struct intel_iommu *iommu, domain_reserve_special_ranges(domain); /* calculate AGAW */ - if (guest_width > cap_mgaw(iommu->cap)) - guest_width = cap_mgaw(iommu->cap); + if (guest_width > agaw_to_width(iommu->agaw)) + guest_width = agaw_to_width(iommu->agaw); domain->gaw = guest_width; adjust_width = guestwidth_to_adjustwidth(guest_width); agaw = width_to_agaw(adjust_width); -- 2.27.0 Thoughts? With the former code the ehci device for the ilo fails when trying to get a private domain. Thanks, Jerry
Re: [PATCH v2] tpm_tis: Disable interrupts if interrupt storm detected
Jerry Snitselaar @ 2020-11-30 20:26 MST:
> Jerry Snitselaar @ 2020-11-30 19:58 MST:
>
>> When enabling the interrupt code for the tpm_tis driver we have
>> noticed some systems have a bios issue causing an interrupt storm to
>> occur. The issue isn't limited to a single tpm or system manufacturer
>> so keeping a denylist of systems with the issue isn't optimal. Instead
>> try to detect the problem occurring, disable interrupts, and revert to
>> polling when it happens.
>>
>> Cc: Jarkko Sakkinen
>> Cc: Jason Gunthorpe
>> Cc: Peter Huewe
>> Cc: James Bottomley
>> Cc: Matthew Garrett
>> Cc: Hans de Goede
>> Signed-off-by: Jerry Snitselaar
>> ---
>> v2: drop tpm_tis specific workqueue and use just system_wq
>>
>> drivers/char/tpm/tpm_tis_core.c | 27 +++
>> drivers/char/tpm/tpm_tis_core.h | 2 ++
>> 2 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c
>> b/drivers/char/tpm/tpm_tis_core.c
>> index 23b60583928b..72cc8a5a152c 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -24,6 +24,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> #include "tpm.h"
>> #include "tpm_tis_core.h"
>>
>> @@ -745,9 +747,23 @@ static irqreturn_t tis_int_handler(int dummy, void
>> *dev_id)
>> {
>> struct tpm_chip *chip = dev_id;
>> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>> +static bool check_storm = true;
>> +static unsigned int check_start;
>> u32 interrupt;
>> int i, rc;
>>
>> +if (unlikely(check_storm)) {
>> +if (!check_start) {
>> +check_start = jiffies_to_msecs(jiffies);
>> +} else if ((kstat_irqs(priv->irq) > 1000) &&
>> + (jiffies_to_msecs(jiffies) - check_start < 500)) {
>> +check_storm = false;
>> +schedule_work(&priv->storm_work);
>> +} else if (jiffies_to_msecs(jiffies) - check_start >= 500) {
>> +check_storm = false;
>> +}
>> +}
>> +
>> rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
>> if (rc < 0)
>> return IRQ_NONE;
>> @@ -987,6 +1003,14 @@ static const struct tpm_class_ops tpm_tis = {
>> .clk_enable = tpm_tis_clkrun_enable,
>> };
>>
>> +static void tpm_tis_storm_work(struct work_struct *work)
>> +{
>> +struct tpm_tis_data *priv = container_of(work, struct tpm_tis_data,
>> storm_work);
>> +
>> +disable_interrupts(priv->chip);
>> +dev_warn(&priv->chip->dev, "Interrupt storm detected, using
>> polling.\n");
>> +}
>> +
>> int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int
>> irq,
>>const struct tpm_tis_phy_ops *phy_ops,
>>acpi_handle acpi_dev_handle)
>> @@ -1003,6 +1027,9 @@ int tpm_tis_core_init(struct device *dev, struct
>> tpm_tis_data *priv, int irq,
>> if (IS_ERR(chip))
>> return PTR_ERR(chip);
>>
>> +priv->chip = chip;
>> +INIT_WORK(&priv->storm_work, tpm_tis_storm_work);
>> +
>> #ifdef CONFIG_ACPI
>> chip->acpi_dev_handle = acpi_dev_handle;
>> #endif
>> diff --git a/drivers/char/tpm/tpm_tis_core.h
>> b/drivers/char/tpm/tpm_tis_core.h
>> index edeb5dc61c95..5630f294dc0c 100644
>> --- a/drivers/char/tpm/tpm_tis_core.h
>> +++ b/drivers/char/tpm/tpm_tis_core.h
>> @@ -95,6 +95,8 @@ struct tpm_tis_data {
>> u16 clkrun_enabled;
>> wait_queue_head_t int_queue;
>> wait_queue_head_t read_queue;
>> +struct work_struct storm_work;
>> +struct tpm_chip *chip;
>> const struct tpm_tis_phy_ops *phy_ops;
>> unsigned short rng_quality;
>> };
>
> I've tested this with the Intel platform that has an Infineon chip that
> I found the other week. It works, but isn't the complete fix. With this
> on top of James' patchset I sometimes see the message "Lost Interrupt
> waiting for TPM stat", so I guess there needs to be a check in
> wait_for_tpm_stat and request_locality to see if interrupts were
> disabled when the wait_event_interruptible_timeout call times out.
As kernel test robot pointed out. kstat_irqs isn't visible when tpm_tis
builds as a module. It looks like it is only called by kstat_irq_usrs,
and that is only by the fs/proc code. I have a patch to export it, but
the i915 driver open codes their own version instead of using it. Is
there any reason not to export it?
Re: [PATCH] iommu/amd: Increase interrupt remapping table limit to 512 entries
Suravee Suthikulpanit @ 2020-10-14 19:50 MST:
> Certain device drivers allocate IO queues on a per-cpu basis.
> On AMD EPYC platform, which can support up-to 256 cpu threads,
> this can exceed the current MAX_IRQ_PER_TABLE limit of 256,
> and result in the error message:
>
> AMD-Vi: Failed to allocate IRTE
>
> This has been observed with certain NVME devices.
>
> AMD IOMMU hardware can actually support upto 512 interrupt
> remapping table entries. Therefore, update the driver to
> match the hardware limit.
>
> Please note that this also increases the size of interrupt remapping
> table to 8KB per device when using the 128-bit IRTE format.
>
> Signed-off-by: Suravee Suthikulpanit
> ---
> drivers/iommu/amd/amd_iommu_types.h | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h
> b/drivers/iommu/amd/amd_iommu_types.h
> index 30a5d412255a..427484c45589 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -406,7 +406,11 @@ extern bool amd_iommu_np_cache;
> /* Only true if all IOMMUs support device IOTLBs */
> extern bool amd_iommu_iotlb_sup;
>
> -#define MAX_IRQS_PER_TABLE 256
> +/*
> + * AMD IOMMU hardware only support 512 IRTEs despite
> + * the architectural limitation of 2048 entries.
> + */
> +#define MAX_IRQS_PER_TABLE 512
> #define IRQ_TABLE_ALIGNMENT 128
>
> struct irq_remap_table {
With this change should DTE_IRQ_TABLE_LEN be changed to 9? IIUC the spec
correctly leaving it at 8 is saying the table is 256 entries long.
Regards,
Jerry
Re: [PATCH v2] tpm_tis: Disable interrupts if interrupt storm detected
Jarkko Sakkinen @ 2020-12-02 09:49 MST:
> On Tue, Dec 01, 2020 at 12:59:23PM -0700, Jerry Snitselaar wrote:
>>
>> Jerry Snitselaar @ 2020-11-30 20:26 MST:
>>
>> > Jerry Snitselaar @ 2020-11-30 19:58 MST:
>> >
>> >> When enabling the interrupt code for the tpm_tis driver we have
>> >> noticed some systems have a bios issue causing an interrupt storm to
>> >> occur. The issue isn't limited to a single tpm or system manufacturer
>> >> so keeping a denylist of systems with the issue isn't optimal. Instead
>> >> try to detect the problem occurring, disable interrupts, and revert to
>> >> polling when it happens.
>> >>
>> >> Cc: Jarkko Sakkinen
>> >> Cc: Jason Gunthorpe
>> >> Cc: Peter Huewe
>> >> Cc: James Bottomley
>> >> Cc: Matthew Garrett
>> >> Cc: Hans de Goede
>> >> Signed-off-by: Jerry Snitselaar
>> >> ---
>> >> v2: drop tpm_tis specific workqueue and use just system_wq
>> >>
>> >> drivers/char/tpm/tpm_tis_core.c | 27 +++
>> >> drivers/char/tpm/tpm_tis_core.h | 2 ++
>> >> 2 files changed, 29 insertions(+)
>> >>
>> >> diff --git a/drivers/char/tpm/tpm_tis_core.c
>> >> b/drivers/char/tpm/tpm_tis_core.c
>> >> index 23b60583928b..72cc8a5a152c 100644
>> >> --- a/drivers/char/tpm/tpm_tis_core.c
>> >> +++ b/drivers/char/tpm/tpm_tis_core.c
>> >> @@ -24,6 +24,8 @@
>> >> #include
>> >> #include
>> >> #include
>> >> +#include
>> >> +#include
>> >> #include "tpm.h"
>> >> #include "tpm_tis_core.h"
>> >>
>> >> @@ -745,9 +747,23 @@ static irqreturn_t tis_int_handler(int dummy, void
>> >> *dev_id)
>> >> {
>> >> struct tpm_chip *chip = dev_id;
>> >> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>> >> + static bool check_storm = true;
>> >> + static unsigned int check_start;
>> >> u32 interrupt;
>> >> int i, rc;
>> >>
>> >> + if (unlikely(check_storm)) {
>> >> + if (!check_start) {
>> >> + check_start = jiffies_to_msecs(jiffies);
>> >> + } else if ((kstat_irqs(priv->irq) > 1000) &&
>> >> +(jiffies_to_msecs(jiffies) - check_start < 500)) {
>> >> + check_storm = false;
>> >> + schedule_work(&priv->storm_work);
>> >> + } else if (jiffies_to_msecs(jiffies) - check_start >= 500) {
>> >> + check_storm = false;
>> >> + }
>> >> + }
>> >> +
>> >> rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
>> >> if (rc < 0)
>> >> return IRQ_NONE;
>> >> @@ -987,6 +1003,14 @@ static const struct tpm_class_ops tpm_tis = {
>> >> .clk_enable = tpm_tis_clkrun_enable,
>> >> };
>> >>
>> >> +static void tpm_tis_storm_work(struct work_struct *work)
>> >> +{
>> >> + struct tpm_tis_data *priv = container_of(work, struct tpm_tis_data,
>> >> storm_work);
>> >> +
>> >> + disable_interrupts(priv->chip);
>> >> + dev_warn(&priv->chip->dev, "Interrupt storm detected, using
>> >> polling.\n");
>> >> +}
>> >> +
>> >> int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int
>> >> irq,
>> >> const struct tpm_tis_phy_ops *phy_ops,
>> >> acpi_handle acpi_dev_handle)
>> >> @@ -1003,6 +1027,9 @@ int tpm_tis_core_init(struct device *dev, struct
>> >> tpm_tis_data *priv, int irq,
>> >> if (IS_ERR(chip))
>> >> return PTR_ERR(chip);
>> >>
>> >> + priv->chip = chip;
>> >> + INIT_WORK(&priv->storm_work, tpm_tis_storm_work);
>> >> +
>> >> #ifdef CONFIG_ACPI
>> >> chip->acpi_dev_handle = acpi_dev_handle;
>> >> #endif
>> >> diff --git a/drivers/char/tpm/tpm_tis_core.h
>> >> b/drivers/char/tpm/tpm_tis_core.h
>> >> index edeb5dc61c95..5630f294dc0c 100644
>> >> --- a/drivers/char/tpm/tpm_tis_core.h
>> >> +++ b/drivers/char/tpm/tp
Re: [PATCH v2] tpm_tis: Disable interrupts if interrupt storm detected
Jerry Snitselaar @ 2020-12-02 17:02 MST:
> Jarkko Sakkinen @ 2020-12-02 09:49 MST:
>
>> On Tue, Dec 01, 2020 at 12:59:23PM -0700, Jerry Snitselaar wrote:
>>>
>>> Jerry Snitselaar @ 2020-11-30 20:26 MST:
>>>
>>> > Jerry Snitselaar @ 2020-11-30 19:58 MST:
>>> >
>>> >> When enabling the interrupt code for the tpm_tis driver we have
>>> >> noticed some systems have a bios issue causing an interrupt storm to
>>> >> occur. The issue isn't limited to a single tpm or system manufacturer
>>> >> so keeping a denylist of systems with the issue isn't optimal. Instead
>>> >> try to detect the problem occurring, disable interrupts, and revert to
>>> >> polling when it happens.
>>> >>
>>> >> Cc: Jarkko Sakkinen
>>> >> Cc: Jason Gunthorpe
>>> >> Cc: Peter Huewe
>>> >> Cc: James Bottomley
>>> >> Cc: Matthew Garrett
>>> >> Cc: Hans de Goede
>>> >> Signed-off-by: Jerry Snitselaar
>>> >> ---
>>> >> v2: drop tpm_tis specific workqueue and use just system_wq
>>> >>
>>> >> drivers/char/tpm/tpm_tis_core.c | 27 +++
>>> >> drivers/char/tpm/tpm_tis_core.h | 2 ++
>>> >> 2 files changed, 29 insertions(+)
>>> >>
>>> >> diff --git a/drivers/char/tpm/tpm_tis_core.c
>>> >> b/drivers/char/tpm/tpm_tis_core.c
>>> >> index 23b60583928b..72cc8a5a152c 100644
>>> >> --- a/drivers/char/tpm/tpm_tis_core.c
>>> >> +++ b/drivers/char/tpm/tpm_tis_core.c
>>> >> @@ -24,6 +24,8 @@
>>> >> #include
>>> >> #include
>>> >> #include
>>> >> +#include
>>> >> +#include
>>> >> #include "tpm.h"
>>> >> #include "tpm_tis_core.h"
>>> >>
>>> >> @@ -745,9 +747,23 @@ static irqreturn_t tis_int_handler(int dummy, void
>>> >> *dev_id)
>>> >> {
>>> >> struct tpm_chip *chip = dev_id;
>>> >> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>>> >> +static bool check_storm = true;
>>> >> +static unsigned int check_start;
>>> >> u32 interrupt;
>>> >> int i, rc;
>>> >>
>>> >> +if (unlikely(check_storm)) {
>>> >> +if (!check_start) {
>>> >> +check_start = jiffies_to_msecs(jiffies);
>>> >> +} else if ((kstat_irqs(priv->irq) > 1000) &&
>>> >> + (jiffies_to_msecs(jiffies) - check_start <
>>> >> 500)) {
>>> >> +check_storm = false;
>>> >> +schedule_work(&priv->storm_work);
>>> >> +} else if (jiffies_to_msecs(jiffies) - check_start >=
>>> >> 500) {
>>> >> +check_storm = false;
>>> >> +}
>>> >> +}
>>> >> +
>>> >> rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality),
>>> >> &interrupt);
>>> >> if (rc < 0)
>>> >> return IRQ_NONE;
>>> >> @@ -987,6 +1003,14 @@ static const struct tpm_class_ops tpm_tis = {
>>> >> .clk_enable = tpm_tis_clkrun_enable,
>>> >> };
>>> >>
>>> >> +static void tpm_tis_storm_work(struct work_struct *work)
>>> >> +{
>>> >> +struct tpm_tis_data *priv = container_of(work, struct
>>> >> tpm_tis_data, storm_work);
>>> >> +
>>> >> +disable_interrupts(priv->chip);
>>> >> +dev_warn(&priv->chip->dev, "Interrupt storm detected, using
>>> >> polling.\n");
>>> >> +}
>>> >> +
>>> >> int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv,
>>> >> int irq,
>>> >>const struct tpm_tis_phy_ops *phy_ops,
>>> >>acpi_handle acpi_dev_handle)
>>> >> @@ -1003,6 +1027,9 @@ int tpm_tis_core_init(struct device *dev, struct
>>> >> tpm_tis_data *priv, int irq
Re: [PATCH v2] tpm_tis: Disable interrupts if interrupt storm detected
Jerry Snitselaar @ 2020-12-02 23:11 MST:
> Jerry Snitselaar @ 2020-12-02 17:02 MST:
>
>> Jarkko Sakkinen @ 2020-12-02 09:49 MST:
>>
>>> On Tue, Dec 01, 2020 at 12:59:23PM -0700, Jerry Snitselaar wrote:
>>>>
>>>> Jerry Snitselaar @ 2020-11-30 20:26 MST:
>>>>
>>>> > Jerry Snitselaar @ 2020-11-30 19:58 MST:
>>>> >
>>>> >> When enabling the interrupt code for the tpm_tis driver we have
>>>> >> noticed some systems have a bios issue causing an interrupt storm to
>>>> >> occur. The issue isn't limited to a single tpm or system manufacturer
>>>> >> so keeping a denylist of systems with the issue isn't optimal. Instead
>>>> >> try to detect the problem occurring, disable interrupts, and revert to
>>>> >> polling when it happens.
>>>> >>
>>>> >> Cc: Jarkko Sakkinen
>>>> >> Cc: Jason Gunthorpe
>>>> >> Cc: Peter Huewe
>>>> >> Cc: James Bottomley
>>>> >> Cc: Matthew Garrett
>>>> >> Cc: Hans de Goede
>>>> >> Signed-off-by: Jerry Snitselaar
>>>> >> ---
>>>> >> v2: drop tpm_tis specific workqueue and use just system_wq
>>>> >>
>>>> >> drivers/char/tpm/tpm_tis_core.c | 27 +++
>>>> >> drivers/char/tpm/tpm_tis_core.h | 2 ++
>>>> >> 2 files changed, 29 insertions(+)
>>>> >>
>>>> >> diff --git a/drivers/char/tpm/tpm_tis_core.c
>>>> >> b/drivers/char/tpm/tpm_tis_core.c
>>>> >> index 23b60583928b..72cc8a5a152c 100644
>>>> >> --- a/drivers/char/tpm/tpm_tis_core.c
>>>> >> +++ b/drivers/char/tpm/tpm_tis_core.c
>>>> >> @@ -24,6 +24,8 @@
>>>> >> #include
>>>> >> #include
>>>> >> #include
>>>> >> +#include
>>>> >> +#include
>>>> >> #include "tpm.h"
>>>> >> #include "tpm_tis_core.h"
>>>> >>
>>>> >> @@ -745,9 +747,23 @@ static irqreturn_t tis_int_handler(int dummy, void
>>>> >> *dev_id)
>>>> >> {
>>>> >> struct tpm_chip *chip = dev_id;
>>>> >> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>>>> >> + static bool check_storm = true;
>>>> >> + static unsigned int check_start;
>>>> >> u32 interrupt;
>>>> >> int i, rc;
>>>> >>
>>>> >> + if (unlikely(check_storm)) {
>>>> >> + if (!check_start) {
>>>> >> + check_start = jiffies_to_msecs(jiffies);
>>>> >> + } else if ((kstat_irqs(priv->irq) > 1000) &&
>>>> >> + (jiffies_to_msecs(jiffies) - check_start <
>>>> >> 500)) {
>>>> >> + check_storm = false;
>>>> >> + schedule_work(&priv->storm_work);
>>>> >> + } else if (jiffies_to_msecs(jiffies) - check_start >=
>>>> >> 500) {
>>>> >> + check_storm = false;
>>>> >> + }
>>>> >> + }
>>>> >> +
>>>> >> rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality),
>>>> >> &interrupt);
>>>> >> if (rc < 0)
>>>> >> return IRQ_NONE;
>>>> >> @@ -987,6 +1003,14 @@ static const struct tpm_class_ops tpm_tis = {
>>>> >> .clk_enable = tpm_tis_clkrun_enable,
>>>> >> };
>>>> >>
>>>> >> +static void tpm_tis_storm_work(struct work_struct *work)
>>>> >> +{
>>>> >> + struct tpm_tis_data *priv = container_of(work, struct
>>>> >> tpm_tis_data, storm_work);
>>>> >> +
>>>> >> + disable_interrupts(priv->chip);
>>>> >> + dev_warn(&priv->chip->dev, "Interrupt storm detected, using
>>>> >> polling.\n");
>>>> >> +}
>>>> >> +
>>>> >> int tpm_tis_core_init(struct de
Re: Question about domain_init (v5.3-v5.7)
Lu Baolu @ 2020-11-26 04:01 MST: > Hi Jerry, > > On 2020/11/26 4:27, Jerry Snitselaar wrote: >> Is there a reason we check the requested guest address width against >> the >> iommu's mgaw, instead of the agaw that we already know for the iommu? >> I've run into a case with a new system where the mgaw reported is 57, >> but if they set PAE to 46 instead of 52 in the bios, then sagaw reports >> the highest supported agaw is 48 and the domain_init code fails here. In > > Isn't this a platform bug? If it's too late to fix it in the BIOS, you > maybe have to add a platform specific quirk to set mgaw to the highest > supported agaw? > > Best regards, > baolu Is there somewhere you can point me to that discusses how they should be setting the mgaw? I misunderstood when I previously asked you about whether the mgaw could be a value that was greater than any of sagaw. If it is a bios issue, then they should fix it there. > >> other places like prepare_domain_attach_device, the dmar domain agaw >> gets adjusted down to the iommu agaw. The agaw of the iommu gets >> determined based off what is reported for sagaw. I'm wondering if it >> can't instead do: >> --- >> drivers/iommu/intel-iommu.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> diff --git a/drivers/iommu/intel-iommu.c >> b/drivers/iommu/intel-iommu.c >> index 6ca5c92ef2e5..a8e41ec36d9e 100644 >> --- a/drivers/iommu/intel-iommu.c >> +++ b/drivers/iommu/intel-iommu.c >> @@ -1862,8 +1862,8 @@ static int domain_init(struct dmar_domain *domain, >> struct intel_iommu *iommu, >> domain_reserve_special_ranges(domain); >> /* calculate AGAW */ >> -if (guest_width > cap_mgaw(iommu->cap)) >> -guest_width = cap_mgaw(iommu->cap); >> +if (guest_width > agaw_to_width(iommu->agaw)) >> +guest_width = agaw_to_width(iommu->agaw); >> domain->gaw = guest_width; >> adjust_width = guestwidth_to_adjustwidth(guest_width); >> agaw = width_to_agaw(adjust_width); >> -- >> 2.27.0 >> >> Thoughts? With the former code the ehci device for the ilo fails when >> trying to get a private domain. >> Thanks, >> Jerry >>
Re: Question about domain_init (v5.3-v5.7)
Lu Baolu @ 2020-11-26 19:12 MST: > Hi Jerry, > > On 11/27/20 5:35 AM, Jerry Snitselaar wrote: >> Lu Baolu @ 2020-11-26 04:01 MST: >> >>> Hi Jerry, >>> >>> On 2020/11/26 4:27, Jerry Snitselaar wrote: >>>> Is there a reason we check the requested guest address width against >>>> the >>>> iommu's mgaw, instead of the agaw that we already know for the iommu? >>>> I've run into a case with a new system where the mgaw reported is 57, >>>> but if they set PAE to 46 instead of 52 in the bios, then sagaw reports >>>> the highest supported agaw is 48 and the domain_init code fails here. In >>> >>> Isn't this a platform bug? If it's too late to fix it in the BIOS, you >>> maybe have to add a platform specific quirk to set mgaw to the highest >>> supported agaw? >>> >>> Best regards, >>> baolu >> Is there somewhere you can point me to that discusses how they >> should be >> setting the mgaw? I misunderstood when I previously asked you about >> whether the mgaw could be a value that was greater than any of sagaw. >> If it is a bios issue, then they should fix it there. > > MGAW indicates the max gpa width supported by 2nd translation. The VT-d > spec requires that this value must be at least equal to the host > physical addressibility. According to this, BIOS is good, right? > Yes, the host address width is 46. MGAW reports 57 (56+1), and highest sagaw bit is for 48. > For this failure case, domain_init() just wants to find a suitable agaw > for the private domain. I think it makes sense to check against > iommu->agaw instead of cap_mgaw. > > Best regards, > baolu > >> >>> >>>> other places like prepare_domain_attach_device, the dmar domain agaw >>>> gets adjusted down to the iommu agaw. The agaw of the iommu gets >>>> determined based off what is reported for sagaw. I'm wondering if it >>>> can't instead do: >>>> --- >>>>drivers/iommu/intel-iommu.c | 4 ++-- >>>>1 file changed, 2 insertions(+), 2 deletions(-) >>>> diff --git a/drivers/iommu/intel-iommu.c >>>> b/drivers/iommu/intel-iommu.c >>>> index 6ca5c92ef2e5..a8e41ec36d9e 100644 >>>> --- a/drivers/iommu/intel-iommu.c >>>> +++ b/drivers/iommu/intel-iommu.c >>>> @@ -1862,8 +1862,8 @@ static int domain_init(struct dmar_domain *domain, >>>> struct intel_iommu *iommu, >>>>domain_reserve_special_ranges(domain); >>>>/* calculate AGAW */ >>>> - if (guest_width > cap_mgaw(iommu->cap)) >>>> - guest_width = cap_mgaw(iommu->cap); >>>> + if (guest_width > agaw_to_width(iommu->agaw)) >>>> + guest_width = agaw_to_width(iommu->agaw); >>>>domain->gaw = guest_width; >>>>adjust_width = guestwidth_to_adjustwidth(guest_width); >>>>agaw = width_to_agaw(adjust_width); >>>> -- >>>> 2.27.0 >>>> >>>> Thoughts? With the former code the ehci device for the ilo fails when >>>> trying to get a private domain. >>>> Thanks, >>>> Jerry >>>> >>
[PATCH] staging/line6: blank line after declaration cleanup
Fix coding style warnings reported by checkpath.
Signed-off-by: Jerry Snitselaar
---
drivers/staging/line6/capture.c | 3 +++
drivers/staging/line6/midi.c | 2 ++
drivers/staging/line6/playback.c | 6 +-
drivers/staging/line6/pod.c | 5 +
drivers/staging/line6/toneport.c | 2 ++
drivers/staging/line6/variax.c | 2 ++
6 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/line6/capture.c b/drivers/staging/line6/capture.c
index 0eda51d..e6ca631 100644
--- a/drivers/staging/line6/capture.c
+++ b/drivers/staging/line6/capture.c
@@ -97,6 +97,7 @@ void line6_unlink_audio_in_urbs(struct snd_line6_pcm
*line6pcm)
if (test_bit(i, &line6pcm->active_urb_in)) {
if (!test_and_set_bit(i, &line6pcm->unlink_urb_in)) {
struct urb *u = line6pcm->urb_audio_in[i];
+
usb_unlink_urb(u);
}
}
@@ -334,6 +335,7 @@ static int snd_line6_capture_hw_params(struct
snd_pcm_substream *substream,
static int snd_line6_capture_hw_free(struct snd_pcm_substream *substream)
{
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
+
line6_pcm_release(line6pcm, LINE6_BIT_PCM_ALSA_CAPTURE_BUFFER);
return snd_pcm_lib_free_pages(substream);
}
@@ -380,6 +382,7 @@ static snd_pcm_uframes_t
snd_line6_capture_pointer(struct snd_pcm_substream *substream)
{
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
+
return line6pcm->pos_in_done;
}
diff --git a/drivers/staging/line6/midi.c b/drivers/staging/line6/midi.c
index 02345fb..1ac343b 100644
--- a/drivers/staging/line6/midi.c
+++ b/drivers/staging/line6/midi.c
@@ -183,6 +183,7 @@ static void line6_midi_output_drain(struct
snd_rawmidi_substream *substream)
struct usb_line6 *line6 =
line6_rawmidi_substream_midi(substream)->line6;
struct snd_line6_midi *midi = line6->line6midi;
+
wait_event_interruptible(midi->send_wait,
midi->num_active_send_urbs == 0);
}
@@ -260,6 +261,7 @@ static int snd_line6_new_midi(struct snd_line6_midi
*line6midi)
static int snd_line6_midi_free(struct snd_device *device)
{
struct snd_line6_midi *line6midi = device->device_data;
+
line6_midibuf_destroy(&line6midi->midibuf_in);
line6_midibuf_destroy(&line6midi->midibuf_out);
return 0;
diff --git a/drivers/staging/line6/playback.c b/drivers/staging/line6/playback.c
index 0f72db5..2ca8900 100644
--- a/drivers/staging/line6/playback.c
+++ b/drivers/staging/line6/playback.c
@@ -44,6 +44,7 @@ static void change_volume(struct urb *urb_out, int volume[],
}
} else if (bytes_per_frame == 6) {
unsigned char *p, *buf_end;
+
p = (unsigned char *)urb_out->transfer_buffer;
buf_end = p + urb_out->transfer_buffer_length;
@@ -310,6 +311,7 @@ void line6_unlink_audio_out_urbs(struct snd_line6_pcm
*line6pcm)
if (test_bit(i, &line6pcm->active_urb_out)) {
if (!test_and_set_bit(i, &line6pcm->unlink_urb_out)) {
struct urb *u = line6pcm->urb_audio_out[i];
+
usb_unlink_urb(u);
}
}
@@ -363,7 +365,6 @@ static void audio_out_callback(struct urb *urb)
{
int i, index, length = 0, shutdown = 0;
unsigned long flags;
-
struct snd_line6_pcm *line6pcm = (struct snd_line6_pcm *)urb->context;
struct snd_pcm_substream *substream =
get_substream(line6pcm, SNDRV_PCM_STREAM_PLAYBACK);
@@ -389,6 +390,7 @@ static void audio_out_callback(struct urb *urb)
if (test_bit(LINE6_INDEX_PCM_ALSA_PLAYBACK_STREAM, &line6pcm->flags)) {
struct snd_pcm_runtime *runtime = substream->runtime;
+
line6pcm->pos_out_done +=
length / line6pcm->properties->bytes_per_frame;
@@ -485,6 +487,7 @@ static int snd_line6_playback_hw_params(struct
snd_pcm_substream *substream,
static int snd_line6_playback_hw_free(struct snd_pcm_substream *substream)
{
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
+
line6_pcm_release(line6pcm, LINE6_BIT_PCM_ALSA_PLAYBACK_BUFFER);
return snd_pcm_lib_free_pages(substream);
}
@@ -539,6 +542,7 @@ static snd_pcm_uframes_t
snd_line6_playback_pointer(struct snd_pcm_substream *substream)
{
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
+
return line6pcm->pos_out_done;
}
diff --git a/drivers/staging/line6/pod.c b/drivers/staging/line6/pod.c
index f4e95a6..44f4b2f 100644
--- a/drivers/staging/line6/pod.c
+++ b/drivers/staging/line6/pod.c
@@ -197,6 +197,7 @@ static ssize_t serial_number_show(struct device *dev,
[PATCH 1/2] virtio_scsi: blank line after declaration cleanup
Clean up of coding style warnings from checkpatch
Signed-off-by: Jerry Snitselaar
---
drivers/scsi/virtio_scsi.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 16bfd50..fa0b25f 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -498,8 +498,8 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
{
struct virtio_scsi_cmd *cmd;
int ret;
-
struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev);
+
BUG_ON(scsi_sg_count(sc) > shost->sg_tablesize);
/* TODO: check feature bit and fail if unsupported? */
@@ -661,6 +661,7 @@ static int virtscsi_target_alloc(struct scsi_target
*starget)
{
struct virtio_scsi_target_state *tgt =
kmalloc(sizeof(*tgt), GFP_KERNEL);
+
if (!tgt)
return -ENOMEM;
@@ -675,6 +676,7 @@ static int virtscsi_target_alloc(struct scsi_target
*starget)
static void virtscsi_target_destroy(struct scsi_target *starget)
{
struct virtio_scsi_target_state *tgt = starget->hostdata;
+
kfree(tgt);
}
@@ -768,6 +770,7 @@ static int virtscsi_cpu_callback(struct notifier_block *nfb,
unsigned long action, void *hcpu)
{
struct virtio_scsi *vscsi = container_of(nfb, struct virtio_scsi, nb);
+
switch(action) {
case CPU_ONLINE:
case CPU_ONLINE_FROZEN:
--
2.0.0.rc0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] virtio_scsi: space required before open parenthesis
Fix coding style warnings from checkpatch
Signed-off-by: Jerry Snitselaar
---
drivers/scsi/virtio_scsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 9f2fccc..9248a1e 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -723,7 +723,7 @@ static struct scsi_host_template
virtscsi_host_template_multi = {
do { \
typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \
virtio_cwrite(vdev, struct virtio_scsi_config, fld, &__val); \
- } while(0)
+ } while (0)
static void __virtscsi_set_affinity(struct virtio_scsi *vscsi, bool affinity)
{
@@ -771,7 +771,7 @@ static int virtscsi_cpu_callback(struct notifier_block *nfb,
{
struct virtio_scsi *vscsi = container_of(nfb, struct virtio_scsi, nb);
- switch(action) {
+ switch (action) {
case CPU_ONLINE:
case CPU_ONLINE_FROZEN:
case CPU_DEAD:
--
2.0.0.rc0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: binder: cleanup dereference of noderef expressions
Clean up sparse warnings for cred struct dereference.
Signed-off-by: Jerry Snitselaar
---
drivers/staging/android/binder.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index fc59281..6279d82 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -1313,6 +1313,7 @@ static void binder_transaction(struct binder_proc *proc,
struct binder_transaction *in_reply_to = NULL;
struct binder_transaction_log_entry *e;
uint32_t return_error;
+ const struct cred *cred = __task_cred(proc->tsk);
e = binder_transaction_log_add(&binder_transaction_log);
e->call_type = reply ? 2 : !!(tr->flags & TF_ONE_WAY);
@@ -1452,7 +1453,7 @@ static void binder_transaction(struct binder_proc *proc,
t->from = thread;
else
t->from = NULL;
- t->sender_euid = proc->tsk->cred->euid;
+ t->sender_euid = cred->euid;
t->to_proc = target_proc;
t->to_thread = target_thread;
t->code = tr->code;
@@ -2574,6 +2575,7 @@ static long binder_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
struct binder_thread *thread;
unsigned int size = _IOC_SIZE(cmd);
void __user *ubuf = (void __user *)arg;
+ const struct cred *cred = current_cred();
/*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd,
arg);*/
@@ -2658,15 +2660,15 @@ static long binder_ioctl(struct file *filp, unsigned
int cmd, unsigned long arg)
goto err;
}
if (uid_valid(binder_context_mgr_uid)) {
- if (!uid_eq(binder_context_mgr_uid,
current->cred->euid)) {
+ if (!uid_eq(binder_context_mgr_uid, cred->euid)) {
pr_err("BINDER_SET_CONTEXT_MGR bad uid %d !=
%d\n",
- from_kuid(&init_user_ns,
current->cred->euid),
+ from_kuid(&init_user_ns, cred->euid),
from_kuid(&init_user_ns,
binder_context_mgr_uid));
ret = -EPERM;
goto err;
}
} else
- binder_context_mgr_uid = current->cred->euid;
+ binder_context_mgr_uid = cred->euid;
binder_context_mgr_node = binder_new_node(proc, 0, 0);
if (binder_context_mgr_node == NULL) {
ret = -ENOMEM;
--
2.0.0.rc0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2] staging: Check against NULL in fw_download_code
On Mon Aug 11 14, Nicholas Krause wrote: > I am fixing the bug entry , https://bugzilla.kernel.org/show_bug.cgi?id=60461. > This entry states that we are not checking the skb allocated in > fw_download_code > and after checking I fixed it to check for the NULL value before using the > allocate > skb. > > Signed-off-by: Nicholas Krause > --- > drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c > b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c > index 1a95d1f..817e50e 100644 > --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c > +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c > @@ -61,6 +61,8 @@ static bool fw_download_code(struct net_device *dev, u8 > *code_virtual_address, > } > > skb = dev_alloc_skb(frag_length + 4); > + if (skb == NULL) > + return !rt_status; > memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev)); > tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE); > tcb_desc->queue_index = TXCMD_QUEUE; > -- > 1.9.1 Look at fw_download_code() in drivers/staging/rtl8192u/r819xU_firmware.c -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] tpm: remove unneeded include of actbl2.h
tpm_tis.c already gets actbl2.h via linux/acpi.h -> acpi/acpi.h ->
acpi/actbl.h -> acpi/actbl2.h, so the direct include in tpm_tis.c
is not needed.
Signed-off-by: Jerry Snitselaar
---
Jarrko, this is a trivial thing I noticed while doing some work.
Not sure if you guys care, but thought I'd send it in anyways.
drivers/char/tpm/tpm_tis.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 65f7eec..461d486 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -28,7 +28,6 @@
#include
#include
#include
-#include
#include "tpm.h"
enum tis_access {
--
2.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [tpmdd-devel] [PATCH] tpm: remove unneeded include of actbl2.h
On Mon Jan 04 16, Jerry Snitselaar wrote:
tpm_tis.c already gets actbl2.h via linux/acpi.h -> acpi/acpi.h ->
acpi/actbl.h -> acpi/actbl2.h, so the direct include in tpm_tis.c
is not needed.
Signed-off-by: Jerry Snitselaar
---
Jarrko, this is a trivial thing I noticed while doing some work.
Not sure if you guys care, but thought I'd send it in anyways.
drivers/char/tpm/tpm_tis.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 65f7eec..461d486 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -28,7 +28,6 @@
#include
#include
#include
-#include
#include "tpm.h"
enum tis_access {
--
2.6.1
Not sure what happened, but it doesn't look like it CC'd Jarkko even
though it shows it in the git send-email output. Adding the other
maintainers as well.
0001-tpm-remove-unneeded-include-of-actbl2.h.patch
From: Jerry Snitselaar
To: tpmdd-de...@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org,
jarkko.sakki...@linux.intel.com
Subject: [PATCH] tpm: remove unneeded include of actbl2.h
Date: Mon, 4 Jan 2016 12:19:43 -0700
Message-Id: <1451935183-2514-1-git-send-email-jsnit...@redhat.com>
X-Mailer: git-send-email 2.6.1
Send this email? ([y]es|[n]o|[q]uit|[a]ll): a
OK. Log says:
Server: smtp.corp.redhat.com
MAIL FROM:
RCPT TO:
RCPT TO:
RCPT TO:
From: Jerry Snitselaar
To: tpmdd-de...@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org,
jarkko.sakki...@linux.intel.com
Subject: [PATCH] tpm: remove unneeded include of actbl2.h
Date: Mon, 4 Jan 2016 12:19:43 -0700
Message-Id: <1451935183-2514-1-git-send-email-jsnit...@redhat.com>
X-Mailer: git-send-email 2.6.1
Result: 250 2.0.0 u04JJkA6027832 Message accepted for delivery
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [tpmdd-devel] [GIT PULL] remaining tpmdd fixes for Linux 4.5
On Mon Feb 22 16, Jarkko Sakkinen wrote: On Mon, Feb 22, 2016 at 12:56:53PM +1100, James Morris wrote: On Sat, 20 Feb 2016, Jarkko Sakkinen wrote: > Hi James, > > I'm sorry for the late pull request for 4.5. The reason for this was > the latency in my previous one. I picked with care the absolutely > critical fixes so that we can make a sound tpmdd release. > > I really hope you can still pick these as one of them is absolutely > critical to get authorization policy sealing API right (kernel keeps > it finger out of user space created objects). Pushed to next for more testing and review. This really is getting too late in the development cycle for so many fixes. It means the code was not ready to be merged in the first place. I fully agree what you're saying. I'll learn the lesson here and take factors more conservative attitude from now on. No excuses. I'm sorry about this. Partly the reason for recent increase in regressions has been increased real-world use of TPM2 and thus issues have started to pop up that's a lame excuse anyway. Would it be worthwhile to have a tpm branch that gets pulled by -next directly so changes will have already been going through the paces in -next prior to the pull reuqest to James? snits
[PATCH] hv_netvsc: remove unused variable in netvsc_send()
With commit b56fc3c53654 ("hv_netvsc: Fix a bug in netvsc_start_xmit()"),
skb variable is no longer used in netvsc_send. Remove variable and dead
code that depended on it.
Cc: Haiyang Zhang
Cc: K. Y. Srinivasan
Signed-off-by: Jerry Snitselaar
---
drivers/net/hyperv/netvsc.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 2d9ef53..ea091bc 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -826,7 +826,6 @@ int netvsc_send(struct hv_device *device,
u16 q_idx = packet->q_idx;
u32 pktlen = packet->total_data_buflen, msd_len = 0;
unsigned int section_index = NETVSC_INVALID_INDEX;
- struct sk_buff *skb = NULL;
unsigned long flag;
struct multi_send_data *msdp;
struct hv_netvsc_packet *msd_send = NULL, *cur_send = NULL;
@@ -924,12 +923,8 @@ int netvsc_send(struct hv_device *device,
if (cur_send)
ret = netvsc_send_pkt(cur_send, net_device);
- if (ret != 0) {
- if (section_index != NETVSC_INVALID_INDEX)
- netvsc_free_send_slot(net_device, section_index);
- } else if (skb) {
- dev_kfree_skb_any(skb);
- }
+ if (ret != 0 && section_index != NETVSC_INVALID_INDEX)
+ netvsc_free_send_slot(net_device, section_index);
return ret;
}
--
2.4.0.rc3.3.g6eb1401
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] tracing: use seq_buf_used() in seq_buf_to_user() instead of len
commit 5ac48378414d ("tracing: Use trace_seq_used() and seq_buf_used()
instead of len") changed the tracing code to use trace_seq_used() and
seq_buf_used() instead of using the seq_buf len directly to avoid
overflow issues, but missed a spot in seq_buf_to_user() that makes use
of s->len.
Cleaned up the code a bit as well per suggestion of Steve Rostedt.
Cc: "Steven Rostedt (Red Hat)"
Signed-off-by: Jerry Snitselaar
---
lib/seq_buf.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 5c94e10..cb18469 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -306,10 +306,12 @@ int seq_buf_to_user(struct seq_buf *s, char __user *ubuf,
int cnt)
if (!cnt)
return 0;
- if (s->len <= s->readpos)
+ len = seq_buf_used(s);
+
+ if (len <= s->readpos)
return -EBUSY;
- len = seq_buf_used(s) - s->readpos;
+ len -= s->readpos;
if (cnt > len)
cnt = len;
ret = copy_to_user(ubuf, s->buffer + s->readpos, cnt);
--
2.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28
On Thu Aug 29 19, Jarkko Sakkinen wrote: On Thu, Aug 29, 2019 at 05:40:40PM +0300, Jarkko Sakkinen wrote: On Tue, Aug 27, 2019 at 05:46:21PM -0700, Jerry Snitselaar wrote: > There was revealed a bug in the STM TPM chipset used in Dell R415s. > Bug is observed so far only on chipset firmware 1.2.8.28 > (1.2 TPM, device-id 0x0, rev-id 78). After some number of > operations chipset hangs and stays in inconsistent state: > > tpm_tis 00:09: Operation Timed out > tpm_tis 00:09: tpm_transmit: tpm_send: error -5 > > Durations returned by the chip are the same like on other > firmware revisions but apparently with specifically 1.2.8.28 fw > durations should be reset to 2 minutes to enable tpm chip work > properly. No working way of updating firmware was found. > > This patch adds implementation of ->update_durations method > that matches only STM devices with specific firmware version. > > Cc: Peter Huewe > Cc: Jarkko Sakkinen > Cc: Jason Gunthorpe > Signed-off-by: Alexey Klimov > Signed-off-by: Jerry Snitselaar > --- > v2: Make suggested changes from Jarkko > - change struct field name to durations from durs > - formatting cleanups > - turn into void function like update_timeouts and > use chip->duration_adjusted to track whether adjustment occurred. The code repetition looks horrible so I wrote a patch that should help: https://patchwork.kernel.org/patch/11121475/ Read the remar that prepends the diffstat. Forgot from that remark that I did not have TPM 1.x available at hand (WFH today) so please also review and test it. /Jrakko I will test it this morning, and once that is done I'll submit a v3 that cleans up the version comparison in the update_durations function.
Re: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28
On Thu Aug 29 19, Jarkko Sakkinen wrote: On Tue, Aug 27, 2019 at 05:46:21PM -0700, Jerry Snitselaar wrote: There was revealed a bug in the STM TPM chipset used in Dell R415s. Bug is observed so far only on chipset firmware 1.2.8.28 (1.2 TPM, device-id 0x0, rev-id 78). After some number of operations chipset hangs and stays in inconsistent state: tpm_tis 00:09: Operation Timed out tpm_tis 00:09: tpm_transmit: tpm_send: error -5 Durations returned by the chip are the same like on other firmware revisions but apparently with specifically 1.2.8.28 fw durations should be reset to 2 minutes to enable tpm chip work properly. No working way of updating firmware was found. This patch adds implementation of ->update_durations method that matches only STM devices with specific firmware version. Cc: Peter Huewe Cc: Jarkko Sakkinen Cc: Jason Gunthorpe Signed-off-by: Alexey Klimov Signed-off-by: Jerry Snitselaar --- v2: Make suggested changes from Jarkko - change struct field name to durations from durs - formatting cleanups - turn into void function like update_timeouts and use chip->duration_adjusted to track whether adjustment occurred. The code repetition looks horrible so I wrote a patch that should help: https://patchwork.kernel.org/patch/11121475/ Read the remar that prepends the diffstat. /Jarkko LGTM, and testing it on a 1.2 tpm system here worked fine. You can add my Reviewed-by and Tested-by. I have reworked this 2/2 patch to make use of these new structs and pull the tpm1_getcap calls out of the for loop. So I will submit a v3 to go on top of your patch.
Re: [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
On Thu Aug 29 19, Jarkko Sakkinen wrote:
Replace existing TPM 1.x version structs with new structs that consolidate
the common parts into a single struct so that code duplication is no longer
needed in caps_show().
Cc: Alexey Klimov
Cc: Jerry Snitselaar
Signed-off-by: Jarkko Sakkinen
---
Jerry, Alexey: Plese include this to the next version of your patches.
This a low priority patch alone so it does not need to be merge upfront.
Will do. I'll add my Reviewed-by and Tested-by to it and submit it with v3.
drivers/char/tpm/tpm-sysfs.c | 44 ++--
drivers/char/tpm/tpm.h | 23 ---
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index edfa89160010..8064fea2de59 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
char *buf)
{
struct tpm_chip *chip = to_tpm_chip(dev);
+ struct tpm1_version *version;
ssize_t rc = 0;
char *str = buf;
cap_t cap;
@@ -232,31 +233,30 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
str += sprintf(str, "Manufacturer: 0x%x\n",
be32_to_cpu(cap.manufacturer_id));
- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+ /* TPM 1.2 */
+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
"attempting to determine the 1.2 version",
-sizeof(cap.tpm_version_1_2));
- if (!rc) {
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version_1_2.Major,
- cap.tpm_version_1_2.Minor,
- cap.tpm_version_1_2.revMajor,
- cap.tpm_version_1_2.revMinor);
- } else {
- /* Otherwise just use TPM_STRUCT_VER */
- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
- "attempting to determine the 1.1 version",
- sizeof(cap.tpm_version)))
- goto out_ops;
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version.Major,
- cap.tpm_version.Minor,
- cap.tpm_version.revMajor,
- cap.tpm_version.revMinor);
+sizeof(cap.version2))) {
+ version = &cap.version2.version;
+ goto out_print;
}
+
+ /* TPM 1.1 */
+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.version1))) {
+ version = &cap.version1;
+ goto out_ops;
+ }
+
+out_print:
+ str += sprintf(str,
+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
+ version->major, version->minor,
+ version->rev_major, version->rev_minor);
+
rc = str - buf;
+
out_ops:
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..a4f74dd02a35 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,19 +186,16 @@ structstclear_flags_t {
u8 bGlobalLock;
} __packed;
-struct tpm_version_t {
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version {
+ u8 major;
+ u8 minor;
+ u8 rev_major;
+ u8 rev_minor;
} __packed;
-struct tpm_version_1_2_t {
- __be16 tag;
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version2 {
+ __be16 tag;
+ struct tpm1_version version;
} __packed;
struct timeout_t {
@@ -243,8 +240,8 @@ typedef union {
struct stclear_flags_t stclear_flags;
__u8owned;
__be32 num_pcrs;
- struct tpm_version_t tpm_version;
- struct tpm_version_1_2_t tpm_version_1_2;
+ struct tpm1_version version1;
+ struct tpm1_version2 version2;
__be32 manufacturer_id;
struct timeout_t timeout;
struct duration_t duration;
--
2.20.1
[PATCH 0/3 v3] tpm: add update_durations class op to allow override of chip supplied values
We've run into a case where a customer has an STM TPM 1.2 chip (version 1.2.8.28), that is getting into an inconsistent state and they end up getting tpm transmit errors. In really old tpm code this wasn't seen because the code that grabbed the duration values from the chip could fail silently, and would proceed to just use default values and move forward. More recent code though successfully gets the duration values from the chip, and using those values this particular chip version gets into the state seen by the customer. The idea with this patchset is to provide a facility like the update_timeouts operation to allow the override of chip supplied values. v3: Rework update_durations to make use of new version structs and pull tpm1_getcap calls out of loop. v2: Update Alexey's v1 submission with suggestions made by Jarkko
[PATCH 3/3 v3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28
There was revealed a bug in the STM TPM chipset used in Dell R415s.
Bug is observed so far only on chipset firmware 1.2.8.28
(1.2 TPM, device-id 0x0, rev-id 78). After some number of
operations chipset hangs and stays in inconsistent state:
tpm_tis 00:09: Operation Timed out
tpm_tis 00:09: tpm_transmit: tpm_send: error -5
Durations returned by the chip are the same like on other
firmware revisions but apparently with specifically 1.2.8.28 fw
durations should be reset to 2 minutes to enable tpm chip work
properly. No working way of updating firmware was found.
This patch adds implementation of ->update_durations method
that matches only STM devices with specific firmware version.
Cc: Peter Huewe
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Signed-off-by: Alexey Klimov
Signed-off-by: Jerry Snitselaar
---
v3: Rework update_durations to make use of new version structs
from 1/3 patch, and move tpm1_getcap calls out of the loop.
v2: Make suggested changes from Jarkko
- change struct field name to durations from durs
- formatting cleanups
- turn into void function like update_timeouts and
use chip->duration_adjusted to track whether adjustment occurred.
drivers/char/tpm/tpm_tis_core.c | 79 +
1 file changed, 79 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c3181ea9f271..27c6ca031e23 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -506,6 +506,84 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf,
size_t len)
return rc;
}
+struct tis_vendor_durations_override {
+ u32 did_vid;
+ struct tpm1_version version;
+ unsigned long durations[3];
+};
+
+static const struct tis_vendor_durations_override vendor_dur_overrides[] = {
+ /* STMicroelectronics 0x104a */
+ { 0x104a,
+ { 1, 2, 8, 28 },
+ { (2 * 60 * HZ), (2 * 60 * HZ), (2 * 60 * HZ) } },
+};
+
+static void tpm_tis_update_durations(struct tpm_chip *chip,
+unsigned long *duration_cap)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ struct tpm1_version *version;
+ u32 did_vid;
+ int i, rc;
+ cap_t cap;
+
+ chip->duration_adjusted = false;
+
+ if (chip->ops->clk_enable != NULL)
+ chip->ops->clk_enable(chip, true);
+
+ rc = tpm_tis_read32(priv, TPM_DID_VID(0), &did_vid);
+ if (rc < 0) {
+ dev_warn(&chip->dev, "%s: failed to read did_vid. %d\n",
+__func__, rc);
+ goto out;
+ }
+
+ /* Try to get a TPM version 1.2 or 1.1 TPM_CAP_VERSION_INFO */
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+"attempting to determine the 1.2 version",
+sizeof(cap.version2));
+ if (!rc) {
+ version = &cap.version2.version;
+ } else {
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+"attempting to determine the 1.1 version",
+sizeof(cap.version1));
+
+ if (rc)
+ goto out;
+
+ version = &cap.version1;
+ }
+
+ for (i = 0; i != ARRAY_SIZE(vendor_dur_overrides); i++) {
+ if (vendor_dur_overrides[i].did_vid != did_vid)
+ continue;
+
+ if ((version->major ==
+vendor_dur_overrides[i].version.major) &&
+ (version->minor ==
+vendor_dur_overrides[i].version.minor) &&
+ (version->rev_major ==
+vendor_dur_overrides[i].version.rev_major) &&
+ (version->rev_minor ==
+vendor_dur_overrides[i].version.rev_minor)) {
+
+ memcpy(duration_cap,
+ vendor_dur_overrides[i].durations,
+ sizeof(vendor_dur_overrides[i].durations));
+
+ chip->duration_adjusted = true;
+ goto out;
+ }
+ }
+
+out:
+ if (chip->ops->clk_enable != NULL)
+ chip->ops->clk_enable(chip, false);
+}
+
struct tis_vendor_timeout_override {
u32 did_vid;
unsigned long timeout_us[4];
@@ -842,6 +920,7 @@ static const struct tpm_class_ops tpm_tis = {
.send = tpm_tis_send,
.cancel = tpm_tis_ready,
.update_timeouts = tpm_tis_update_timeouts,
+ .update_durations = tpm_tis_update_durations,
.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_canceled = tpm_tis_req_canceled,
--
2.21.0
[PATCH 1/3 v3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
From: Jarkko Sakkinen
Replace existing TPM 1.x version structs with new structs that consolidate
the common parts into a single struct so that code duplication is no longer
needed in caps_show().
Cc: Peter Huewe
Cc: Jason Gunthorpe
Cc: Alexey Klimov
Signed-off-by: Jarkko Sakkinen
Reviewed-by: Jerry Snitselaar
Tested-by: Jerry Snitselaar
---
drivers/char/tpm/tpm-sysfs.c | 44 ++--
drivers/char/tpm/tpm.h | 23 ---
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index d9caedda075b..e0550f0cfd8f 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
char *buf)
{
struct tpm_chip *chip = to_tpm_chip(dev);
+ struct tpm1_version *version;
ssize_t rc = 0;
char *str = buf;
cap_t cap;
@@ -232,31 +233,30 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
str += sprintf(str, "Manufacturer: 0x%x\n",
be32_to_cpu(cap.manufacturer_id));
- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+ /* TPM 1.2 */
+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
"attempting to determine the 1.2 version",
-sizeof(cap.tpm_version_1_2));
- if (!rc) {
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version_1_2.Major,
- cap.tpm_version_1_2.Minor,
- cap.tpm_version_1_2.revMajor,
- cap.tpm_version_1_2.revMinor);
- } else {
- /* Otherwise just use TPM_STRUCT_VER */
- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
- "attempting to determine the 1.1 version",
- sizeof(cap.tpm_version)))
- goto out_ops;
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version.Major,
- cap.tpm_version.Minor,
- cap.tpm_version.revMajor,
- cap.tpm_version.revMinor);
+sizeof(cap.version2))) {
+ version = &cap.version2.version;
+ goto out_print;
}
+
+ /* TPM 1.1 */
+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.version1))) {
+ version = &cap.version1;
+ goto out_ops;
+ }
+
+out_print:
+ str += sprintf(str,
+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
+ version->major, version->minor,
+ version->rev_major, version->rev_minor);
+
rc = str - buf;
+
out_ops:
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..a4f74dd02a35 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,19 +186,16 @@ structstclear_flags_t {
u8 bGlobalLock;
} __packed;
-struct tpm_version_t {
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version {
+ u8 major;
+ u8 minor;
+ u8 rev_major;
+ u8 rev_minor;
} __packed;
-struct tpm_version_1_2_t {
- __be16 tag;
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version2 {
+ __be16 tag;
+ struct tpm1_version version;
} __packed;
struct timeout_t {
@@ -243,8 +240,8 @@ typedef union {
struct stclear_flags_t stclear_flags;
__u8owned;
__be32 num_pcrs;
- struct tpm_version_t tpm_version;
- struct tpm_version_1_2_t tpm_version_1_2;
+ struct tpm1_version version1;
+ struct tpm1_version2 version2;
__be32 manufacturer_id;
struct timeout_t timeout;
struct duration_t duration;
--
2.21.0
[PATCH 2/3 v3] tpm: provide a way to override the chip returned durations
Patch adds method ->update_durations to override returned
durations in case TPM chip misbehaves for TPM 1.2 drivers.
Cc: Peter Huewe
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Signed-off-by: Alexey Klimov
Signed-off-by: Jerry Snitselaar
---
v3: no changes
v2: newline cleanup as requested by Jarkko
drivers/char/tpm/tpm1-cmd.c | 15 +++
include/linux/tpm.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 149e953ca369..ca7158fa6e6c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -343,6 +343,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
{
cap_t cap;
unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
+ unsigned long durations[3];
ssize_t rc;
rc = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
@@ -427,6 +428,20 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
usecs_to_jiffies(be32_to_cpu(cap.duration.tpm_long));
chip->duration[TPM_LONG_LONG] = 0; /* not used under 1.2 */
+ /*
+* Provide the ability for vendor overrides of duration values in case
+* of misreporting.
+*/
+ if (chip->ops->update_durations)
+ chip->ops->update_durations(chip, durations);
+
+ if (chip->duration_adjusted) {
+ dev_info(&chip->dev, HW_ERR "Adjusting reported durations.");
+ chip->duration[TPM_SHORT] = durations[0];
+ chip->duration[TPM_MEDIUM] = durations[1];
+ chip->duration[TPM_LONG] = durations[2];
+ }
+
/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
* value wrong and apparently reports msecs rather than usecs. So we
* fix up the resulting too-small TPM_SHORT value to make things work.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 53c0ea9ec9df..bb1d1ac7081d 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -67,6 +67,8 @@ struct tpm_class_ops {
u8 (*status) (struct tpm_chip *chip);
void (*update_timeouts)(struct tpm_chip *chip,
unsigned long *timeout_cap);
+ void (*update_durations)(struct tpm_chip *chip,
+unsigned long *duration_cap);
int (*go_idle)(struct tpm_chip *chip);
int (*cmd_ready)(struct tpm_chip *chip);
int (*request_locality)(struct tpm_chip *chip, int loc);
--
2.21.0
Re: [PATCH] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
On Thu Aug 29 19, Jarkko Sakkinen wrote:
Replace existing TPM 1.x version structs with new structs that consolidate
the common parts into a single struct so that code duplication is no longer
needed in caps_show().
Cc: Alexey Klimov
Cc: Jerry Snitselaar
Signed-off-by: Jarkko Sakkinen
---
Jerry, Alexey: Plese include this to the next version of your patches.
This a low priority patch alone so it does not need to be merge upfront.
drivers/char/tpm/tpm-sysfs.c | 44 ++--
drivers/char/tpm/tpm.h | 23 ---
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index edfa89160010..8064fea2de59 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
char *buf)
{
struct tpm_chip *chip = to_tpm_chip(dev);
+ struct tpm1_version *version;
ssize_t rc = 0;
char *str = buf;
cap_t cap;
@@ -232,31 +233,30 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
str += sprintf(str, "Manufacturer: 0x%x\n",
be32_to_cpu(cap.manufacturer_id));
- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+ /* TPM 1.2 */
+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
"attempting to determine the 1.2 version",
-sizeof(cap.tpm_version_1_2));
- if (!rc) {
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version_1_2.Major,
- cap.tpm_version_1_2.Minor,
- cap.tpm_version_1_2.revMajor,
- cap.tpm_version_1_2.revMinor);
- } else {
- /* Otherwise just use TPM_STRUCT_VER */
- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
- "attempting to determine the 1.1 version",
- sizeof(cap.tpm_version)))
- goto out_ops;
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version.Major,
- cap.tpm_version.Minor,
- cap.tpm_version.revMajor,
- cap.tpm_version.revMinor);
+sizeof(cap.version2))) {
+ version = &cap.version2.version;
+ goto out_print;
}
+
+ /* TPM 1.1 */
+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.version1))) {
+ version = &cap.version1;
Actually looking at this again, this seems wrong. The version assignment here
should be below this if, or in an else block, yes?
+ goto out_ops;
+ }
+
+out_print:
+ str += sprintf(str,
+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
+ version->major, version->minor,
+ version->rev_major, version->rev_minor);
+
rc = str - buf;
+
out_ops:
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..a4f74dd02a35 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,19 +186,16 @@ structstclear_flags_t {
u8 bGlobalLock;
} __packed;
-struct tpm_version_t {
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version {
+ u8 major;
+ u8 minor;
+ u8 rev_major;
+ u8 rev_minor;
} __packed;
-struct tpm_version_1_2_t {
- __be16 tag;
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version2 {
+ __be16 tag;
+ struct tpm1_version version;
} __packed;
struct timeout_t {
@@ -243,8 +240,8 @@ typedef union {
struct stclear_flags_t stclear_flags;
__u8owned;
__be32 num_pcrs;
- struct tpm_version_t tpm_version;
- struct tpm_version_1_2_t tpm_version_1_2;
+ struct tpm1_version version1;
+ struct tpm1_version2 version2;
__be32 manufacturer_id;
struct timeout_t timeout;
struct duration_t duration;
--
2.20.1
Re: [PATCH 1/3 v3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
On Thu Aug 29 19, Jerry Snitselaar wrote:
From: Jarkko Sakkinen
Replace existing TPM 1.x version structs with new structs that consolidate
the common parts into a single struct so that code duplication is no longer
needed in caps_show().
Cc: Peter Huewe
Cc: Jason Gunthorpe
Cc: Alexey Klimov
Signed-off-by: Jarkko Sakkinen
Reviewed-by: Jerry Snitselaar
Tested-by: Jerry Snitselaar
---
drivers/char/tpm/tpm-sysfs.c | 44 ++--
drivers/char/tpm/tpm.h | 23 ---
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index d9caedda075b..e0550f0cfd8f 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
char *buf)
{
struct tpm_chip *chip = to_tpm_chip(dev);
+ struct tpm1_version *version;
ssize_t rc = 0;
char *str = buf;
cap_t cap;
@@ -232,31 +233,30 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
str += sprintf(str, "Manufacturer: 0x%x\n",
be32_to_cpu(cap.manufacturer_id));
- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+ /* TPM 1.2 */
+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
"attempting to determine the 1.2 version",
-sizeof(cap.tpm_version_1_2));
- if (!rc) {
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version_1_2.Major,
- cap.tpm_version_1_2.Minor,
- cap.tpm_version_1_2.revMajor,
- cap.tpm_version_1_2.revMinor);
- } else {
- /* Otherwise just use TPM_STRUCT_VER */
- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
- "attempting to determine the 1.1 version",
- sizeof(cap.tpm_version)))
- goto out_ops;
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version.Major,
- cap.tpm_version.Minor,
- cap.tpm_version.revMajor,
- cap.tpm_version.revMinor);
+sizeof(cap.version2))) {
+ version = &cap.version2.version;
+ goto out_print;
}
+
+ /* TPM 1.1 */
+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.version1))) {
+ version = &cap.version1;
+ goto out_ops;
+ }
Jarkko, does the following change to the above block look good? I'll
submit a v4. With the current patch it is setting version when the
tpm1_getcap fails for 1.1 instead of when it succeeds. I only have
1.2 and 2.0 devices, so I didn't hit it when testing your patch.
+ /* TPM 1.1 */
+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.version1)))
+ goto out_ops;
+
+ version = &cap.version1;
+
+out_print:
+ str += sprintf(str,
+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
+ version->major, version->minor,
+ version->rev_major, version->rev_minor);
+
rc = str - buf;
+
out_ops:
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..a4f74dd02a35 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,19 +186,16 @@ structstclear_flags_t {
u8 bGlobalLock;
} __packed;
-struct tpm_version_t {
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version {
+ u8 major;
+ u8 minor;
+ u8 rev_major;
+ u8 rev_minor;
} __packed;
-struct tpm_version_1_2_t {
- __be16 tag;
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version2 {
+ __be16 tag;
+ struct tpm1_version version;
} __packed;
struct timeout_t {
@@ -243,8 +240,8 @@ typedef union {
struct stclear_flags_t stclear_flags;
__u8owned;
__be32 num_pcrs;
- struct tpm_version_t tpm_version;
- struct tpm_version_1_2_t tpm_version_1_2;
+ s
[PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values
We've run into a case where a customer has an STM TPM 1.2 chip (version 1.2.8.28), that is getting into an inconsistent state and they end up getting tpm transmit errors. In really old tpm code this wasn't seen because the code that grabbed the duration values from the chip could fail silently, and would proceed to just use default values and move forward. More recent code though successfully gets the duration values from the chip, and using those values this particular chip version gets into the state seen by the customer. The idea with this patchset is to provide a facility like the update_timeouts operation to allow the override of chip supplied values. changes from v3: * Assign value to version when tpm1_getcap is successful for TPM 1.1 device not when it fails. changes from v2: * Added patch 1/3 * Rework tpm_tis_update_durations to make use of new version structs and pull tpm1_getcap calls out of loop. changes from v1: * Remove unneeded newline * Formatting cleanups * Change tpm_tis_update_durations to be a void function, and use chip->duration_adjusted to track whether adjustment was made. Jarkko Sakkinen (1): tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jerry Snitselaar (2): tpm: provide a way to override the chip returned durations tpm_tis: override durations for STM tpm with firmware 1.2.8.28
[PATCH v4 2/3] tpm: provide a way to override the chip returned durations
Patch adds method ->update_durations to override returned
durations in case TPM chip misbehaves for TPM 1.2 drivers.
Cc: Peter Huewe
Cc: Jason Gunthorpe
Signed-off-by: Alexey Klimov
Signed-off-by: Jerry Snitselaar
Reviewed-by: Jarkko Sakkinen
---
drivers/char/tpm/tpm1-cmd.c | 15 +++
include/linux/tpm.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 149e953ca369..ca7158fa6e6c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -343,6 +343,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
{
cap_t cap;
unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
+ unsigned long durations[3];
ssize_t rc;
rc = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
@@ -427,6 +428,20 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
usecs_to_jiffies(be32_to_cpu(cap.duration.tpm_long));
chip->duration[TPM_LONG_LONG] = 0; /* not used under 1.2 */
+ /*
+* Provide the ability for vendor overrides of duration values in case
+* of misreporting.
+*/
+ if (chip->ops->update_durations)
+ chip->ops->update_durations(chip, durations);
+
+ if (chip->duration_adjusted) {
+ dev_info(&chip->dev, HW_ERR "Adjusting reported durations.");
+ chip->duration[TPM_SHORT] = durations[0];
+ chip->duration[TPM_MEDIUM] = durations[1];
+ chip->duration[TPM_LONG] = durations[2];
+ }
+
/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
* value wrong and apparently reports msecs rather than usecs. So we
* fix up the resulting too-small TPM_SHORT value to make things work.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 53c0ea9ec9df..bb1d1ac7081d 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -67,6 +67,8 @@ struct tpm_class_ops {
u8 (*status) (struct tpm_chip *chip);
void (*update_timeouts)(struct tpm_chip *chip,
unsigned long *timeout_cap);
+ void (*update_durations)(struct tpm_chip *chip,
+unsigned long *duration_cap);
int (*go_idle)(struct tpm_chip *chip);
int (*cmd_ready)(struct tpm_chip *chip);
int (*request_locality)(struct tpm_chip *chip, int loc);
--
2.21.0
[PATCH v4 3/3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28
There was revealed a bug in the STM TPM chipset used in Dell R415s.
Bug is observed so far only on chipset firmware 1.2.8.28
(1.2 TPM, device-id 0x0, rev-id 78). After some number of
operations chipset hangs and stays in inconsistent state:
tpm_tis 00:09: Operation Timed out
tpm_tis 00:09: tpm_transmit: tpm_send: error -5
Durations returned by the chip are the same like on other
firmware revisions but apparently with specifically 1.2.8.28 fw
durations should be reset to 2 minutes to enable tpm chip work
properly. No working way of updating firmware was found.
This patch adds implementation of ->update_durations method
that matches only STM devices with specific firmware version.
Cc: Peter Huewe
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Signed-off-by: Alexey Klimov
Signed-off-by: Jerry Snitselaar
Reviewed-by: Jarkko Sakkinen
---
drivers/char/tpm/tpm_tis_core.c | 79 +
1 file changed, 79 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c3181ea9f271..27c6ca031e23 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -506,6 +506,84 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf,
size_t len)
return rc;
}
+struct tis_vendor_durations_override {
+ u32 did_vid;
+ struct tpm1_version version;
+ unsigned long durations[3];
+};
+
+static const struct tis_vendor_durations_override vendor_dur_overrides[] = {
+ /* STMicroelectronics 0x104a */
+ { 0x104a,
+ { 1, 2, 8, 28 },
+ { (2 * 60 * HZ), (2 * 60 * HZ), (2 * 60 * HZ) } },
+};
+
+static void tpm_tis_update_durations(struct tpm_chip *chip,
+unsigned long *duration_cap)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ struct tpm1_version *version;
+ u32 did_vid;
+ int i, rc;
+ cap_t cap;
+
+ chip->duration_adjusted = false;
+
+ if (chip->ops->clk_enable != NULL)
+ chip->ops->clk_enable(chip, true);
+
+ rc = tpm_tis_read32(priv, TPM_DID_VID(0), &did_vid);
+ if (rc < 0) {
+ dev_warn(&chip->dev, "%s: failed to read did_vid. %d\n",
+__func__, rc);
+ goto out;
+ }
+
+ /* Try to get a TPM version 1.2 or 1.1 TPM_CAP_VERSION_INFO */
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+"attempting to determine the 1.2 version",
+sizeof(cap.version2));
+ if (!rc) {
+ version = &cap.version2.version;
+ } else {
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+"attempting to determine the 1.1 version",
+sizeof(cap.version1));
+
+ if (rc)
+ goto out;
+
+ version = &cap.version1;
+ }
+
+ for (i = 0; i != ARRAY_SIZE(vendor_dur_overrides); i++) {
+ if (vendor_dur_overrides[i].did_vid != did_vid)
+ continue;
+
+ if ((version->major ==
+vendor_dur_overrides[i].version.major) &&
+ (version->minor ==
+vendor_dur_overrides[i].version.minor) &&
+ (version->rev_major ==
+vendor_dur_overrides[i].version.rev_major) &&
+ (version->rev_minor ==
+vendor_dur_overrides[i].version.rev_minor)) {
+
+ memcpy(duration_cap,
+ vendor_dur_overrides[i].durations,
+ sizeof(vendor_dur_overrides[i].durations));
+
+ chip->duration_adjusted = true;
+ goto out;
+ }
+ }
+
+out:
+ if (chip->ops->clk_enable != NULL)
+ chip->ops->clk_enable(chip, false);
+}
+
struct tis_vendor_timeout_override {
u32 did_vid;
unsigned long timeout_us[4];
@@ -842,6 +920,7 @@ static const struct tpm_class_ops tpm_tis = {
.send = tpm_tis_send,
.cancel = tpm_tis_ready,
.update_timeouts = tpm_tis_update_timeouts,
+ .update_durations = tpm_tis_update_durations,
.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_canceled = tpm_tis_req_canceled,
--
2.21.0
[PATCH v4 1/3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
From: Jarkko Sakkinen
Replace existing TPM 1.x version structs with new structs that consolidate
the common parts into a single struct so that code duplication is no longer
needed in caps_show().
Cc: Peter Huewe
Cc: Jason Gunthorpe
Cc: Alexey Klimov
Signed-off-by: Jarkko Sakkinen
Reviewed-by: Jerry Snitselaar
Tested-by: Jerry Snitselaar
---
drivers/char/tpm/tpm-sysfs.c | 45 ++--
drivers/char/tpm/tpm.h | 23 --
2 files changed, 33 insertions(+), 35 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index d9caedda075b..eb05d5df4759 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
char *buf)
{
struct tpm_chip *chip = to_tpm_chip(dev);
+ struct tpm1_version *version;
ssize_t rc = 0;
char *str = buf;
cap_t cap;
@@ -232,31 +233,31 @@ static ssize_t caps_show(struct device *dev, struct
device_attribute *attr,
str += sprintf(str, "Manufacturer: 0x%x\n",
be32_to_cpu(cap.manufacturer_id));
- /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
- rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+ /* TPM 1.2 */
+ if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
"attempting to determine the 1.2 version",
-sizeof(cap.tpm_version_1_2));
- if (!rc) {
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version_1_2.Major,
- cap.tpm_version_1_2.Minor,
- cap.tpm_version_1_2.revMajor,
- cap.tpm_version_1_2.revMinor);
- } else {
- /* Otherwise just use TPM_STRUCT_VER */
- if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
- "attempting to determine the 1.1 version",
- sizeof(cap.tpm_version)))
- goto out_ops;
- str += sprintf(str,
- "TCG version: %d.%d\nFirmware version: %d.%d\n",
- cap.tpm_version.Major,
- cap.tpm_version.Minor,
- cap.tpm_version.revMajor,
- cap.tpm_version.revMinor);
+sizeof(cap.version2))) {
+ version = &cap.version2.version;
+ goto out_print;
}
+
+ /* TPM 1.1 */
+ if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.version1))) {
+ goto out_ops;
+ }
+
+ version = &cap.version1;
+
+out_print:
+ str += sprintf(str,
+ "TCG version: %d.%d\nFirmware version: %d.%d\n",
+ version->major, version->minor,
+ version->rev_major, version->rev_minor);
+
rc = str - buf;
+
out_ops:
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..a4f74dd02a35 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,19 +186,16 @@ structstclear_flags_t {
u8 bGlobalLock;
} __packed;
-struct tpm_version_t {
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version {
+ u8 major;
+ u8 minor;
+ u8 rev_major;
+ u8 rev_minor;
} __packed;
-struct tpm_version_1_2_t {
- __be16 tag;
- u8 Major;
- u8 Minor;
- u8 revMajor;
- u8 revMinor;
+struct tpm1_version2 {
+ __be16 tag;
+ struct tpm1_version version;
} __packed;
struct timeout_t {
@@ -243,8 +240,8 @@ typedef union {
struct stclear_flags_t stclear_flags;
__u8owned;
__be32 num_pcrs;
- struct tpm_version_t tpm_version;
- struct tpm_version_1_2_t tpm_version_1_2;
+ struct tpm1_version version1;
+ struct tpm1_version2 version2;
__be32 manufacturer_id;
struct timeout_t timeout;
struct duration_t duration;
--
2.21.0
Re: [PATCH AUTOSEL 4.19 126/167] tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations
On Tue Sep 03 19, Doug Anderson wrote:
Hi,
On Tue, Sep 3, 2019 at 9:28 AM Sasha Levin wrote:
From: Vadim Sukhomlinov
[ Upstream commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 ]
TPM 2.0 Shutdown involve sending TPM2_Shutdown to TPM chip and disabling
future TPM operations. TPM 1.2 behavior was different, future TPM
operations weren't disabled, causing rare issues. This patch ensures
that future TPM operations are disabled.
Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
Cc: sta...@vger.kernel.org
Signed-off-by: Vadim Sukhomlinov
[dianders: resolved merge conflicts with mainline]
Signed-off-by: Douglas Anderson
Reviewed-by: Jarkko Sakkinen
Signed-off-by: Jarkko Sakkinen
Signed-off-by: Sasha Levin
---
drivers/char/tpm/tpm-chip.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Jarkko: did you deal with the issues that came up in response to my
post? Are you happy with this going into 4.19 stable at this point?
I notice this has your Signed-off-by so maybe?
I think that is just the signed-off-by chain coming from the upstream patch.
Jarkko mentioned getting to the backports after Linux Plumbers, which is next
week.
-Doug
Re: [PATCH v4] tpm: Parse event log from TPM2 ACPI table
On Fri Aug 30 19, Jordan Hand wrote:
For systems with a TPM2 chip which use ACPI to expose event logs, retrieve the
crypto-agile event log from the TPM2 ACPI table. The TPM2 table is defined
in section 7.3 of the TCG ACPI Specification (see link).
The TPM2 table is used by SeaBIOS in place of the TCPA table when the system's
TPM is version 2.0 to denote (among other metadata) the location of the
crypto-agile log.
Link: https://trustedcomputinggroup.org/resource/tcg-acpi-specification/
Signed-off-by: Jordan Hand
---
drivers/char/tpm/eventlog/acpi.c | 60 ++--
1 file changed, 41 insertions(+), 19 deletions(-)
diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index 63ada5e53f13..38a8bcec1dd5 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -41,17 +41,23 @@ struct acpi_tcpa {
};
};
+/* If an event log is present, the TPM2 ACPI table will contain the full
+ * trailer
+ */
+
/* read binary bios log */
int tpm_read_log_acpi(struct tpm_chip *chip)
{
- struct acpi_tcpa *buff;
+ struct acpi_table_header *buff;
+ struct acpi_tcpa *tcpa;
+ struct acpi_tpm2_trailer *tpm2_trailer;
acpi_status status;
void __iomem *virt;
u64 len, start;
+ int log_type;
struct tpm_bios_log *log;
-
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- return -ENODEV;
+ bool is_tpm2 = chip->flags & TPM_CHIP_FLAG_TPM2;
+ acpi_string table_sig;
log = &chip->log;
@@ -61,26 +67,42 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
if (!chip->acpi_dev_handle)
return -ENODEV;
- /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
- status = acpi_get_table(ACPI_SIG_TCPA, 1,
- (struct acpi_table_header **)&buff);
+ /* Find TCPA or TPM2 entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
+ table_sig = is_tpm2 ? ACPI_SIG_TPM2 : ACPI_SIG_TCPA;
+ status = acpi_get_table(table_sig, 1, &buff);
if (ACPI_FAILURE(status))
return -ENODEV;
- switch(buff->platform_class) {
- case BIOS_SERVER:
- len = buff->server.log_max_len;
- start = buff->server.log_start_addr;
- break;
- case BIOS_CLIENT:
- default:
- len = buff->client.log_max_len;
- start = buff->client.log_start_addr;
- break;
+ if (!is_tpm2) {
+ tcpa = (struct acpi_tcpa *)buff;
+ switch (tcpa->platform_class) {
+ case BIOS_SERVER:
+ len = tcpa->server.log_max_len;
+ start = tcpa->server.log_start_addr;
+ break;
+ case BIOS_CLIENT:
+ default:
+ len = tcpa->client.log_max_len;
+ start = tcpa->client.log_start_addr;
+ break;
+ }
+ log_type = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+ } else if (buff->length ==
+ sizeof(struct acpi_table_tpm2) +
+ sizeof(struct acpi_tpm2_trailer)) {
+ tpm2_trailer = (struct acpi_tpm2_trailer *)buff;
+
+ len = tpm2_trailer.minimum_log_length;
+ start = tpm2_trailer.log_address;
Are your builds not failing here? Both v3 and v4 have this.
+ log_type = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
+ } else {
+ return -ENODEV;
}
+
if (!len) {
- dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
+ dev_warn(&chip->dev, "%s: %s log area empty\n",
+__func__, table_sig);
return -EIO;
}
@@ -98,7 +120,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
memcpy_fromio(log->bios_event_log, virt, len);
acpi_os_unmap_iomem(virt, len);
- return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+ return log_type;
err:
kfree(log->bios_event_log);
--
2.20.1
[PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28
There was revealed a bug in the STM TPM chipset used in Dell R415s.
Bug is observed so far only on chipset firmware 1.2.8.28
(1.2 TPM, device-id 0x0, rev-id 78). After some number of
operations chipset hangs and stays in inconsistent state:
tpm_tis 00:09: Operation Timed out
tpm_tis 00:09: tpm_transmit: tpm_send: error -5
Durations returned by the chip are the same like on other
firmware revisions but apparently with specifically 1.2.8.28 fw
durations should be reset to 2 minutes to enable tpm chip work
properly. No working way of updating firmware was found.
This patch adds implementation of ->update_durations method
that matches only STM devices with specific firmware version.
Cc: Peter Huewe
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Signed-off-by: Alexey Klimov
Signed-off-by: Jerry Snitselaar
---
v2: Make suggested changes from Jarkko
- change struct field name to durations from durs
- formatting cleanups
- turn into void function like update_timeouts and
use chip->duration_adjusted to track whether adjustment occurred.
drivers/char/tpm/tpm_tis_core.c | 91 +
1 file changed, 91 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c3181ea9f271..81b65ec2a41b 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -506,6 +506,96 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf,
size_t len)
return rc;
}
+struct tis_vendor_durations_override {
+ u32 did_vid;
+ struct tpm_version_t tpm_version;
+ unsigned long durations[3];
+};
+
+static const struct tis_vendor_durations_override vendor_dur_overrides[] = {
+ /* STMicroelectronics 0x104a */
+ { 0x104a,
+ { 1, 2, 8, 28 },
+ { (2 * 60 * HZ), (2 * 60 * HZ), (2 * 60 * HZ) } },
+};
+
+static void tpm_tis_update_durations(struct tpm_chip *chip,
+unsigned long *duration_cap)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ u32 did_vid;
+ int i, rc;
+ cap_t cap;
+
+ chip->duration_adjusted = false;
+
+ if (chip->ops->clk_enable != NULL)
+ chip->ops->clk_enable(chip, true);
+
+ rc = tpm_tis_read32(priv, TPM_DID_VID(0), &did_vid);
+ if (rc < 0) {
+ dev_warn(&chip->dev, "%s: failed to read did_vid. %d\n",
+__func__, rc);
+ goto out;
+ }
+
+ for (i = 0; i != ARRAY_SIZE(vendor_dur_overrides); i++) {
+ if (vendor_dur_overrides[i].did_vid != did_vid)
+ continue;
+
+ /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+"attempting to determine the 1.2 version",
+sizeof(cap.tpm_version_1_2));
+ if (!rc) {
+ if ((cap.tpm_version_1_2.Major ==
+vendor_dur_overrides[i].tpm_version.Major) &&
+ (cap.tpm_version_1_2.Minor ==
+vendor_dur_overrides[i].tpm_version.Minor) &&
+ (cap.tpm_version_1_2.revMajor ==
+vendor_dur_overrides[i].tpm_version.revMajor) &&
+ (cap.tpm_version_1_2.revMinor ==
+vendor_dur_overrides[i].tpm_version.revMinor)) {
+
+ memcpy(duration_cap,
+ vendor_dur_overrides[i].durations,
+
sizeof(vendor_dur_overrides[i].durations));
+
+ chip->duration_adjusted = true;
+ goto out;
+ }
+ } else {
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+"attempting to determine the 1.1
version",
+sizeof(cap.tpm_version));
+
+ if (rc)
+ goto out;
+
+ if ((cap.tpm_version.Major ==
+vendor_dur_overrides[i].tpm_version.Major) &&
+ (cap.tpm_version.Minor ==
+vendor_dur_overrides[i].tpm_version.Minor) &&
+ (cap.tpm_version.revMajor ==
+vendor_dur_overrides[i].tpm_version.revMajor) &&
+ (cap.tpm_version.revMinor ==
+vendor_dur_overrides[i].tpm_version.revMinor)) {
+
+ memcpy(duration_cap,
+ vendor_dur_overrides[i].durations,
+
[PATCH 1/2 v2] tpm: provide a way to override the chip returned durations
Patch adds method ->update_durations to override returned
durations in case TPM chip misbehaves for TPM 1.2 drivers.
Cc: Peter Huewe
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Signed-off-by: Alexey Klimov
Signed-off-by: Jerry Snitselaar
---
v2: newline cleanup as requested by Jarkko
drivers/char/tpm/tpm1-cmd.c | 15 +++
include/linux/tpm.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 149e953ca369..ca7158fa6e6c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -343,6 +343,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
{
cap_t cap;
unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
+ unsigned long durations[3];
ssize_t rc;
rc = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
@@ -427,6 +428,20 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
usecs_to_jiffies(be32_to_cpu(cap.duration.tpm_long));
chip->duration[TPM_LONG_LONG] = 0; /* not used under 1.2 */
+ /*
+* Provide the ability for vendor overrides of duration values in case
+* of misreporting.
+*/
+ if (chip->ops->update_durations)
+ chip->ops->update_durations(chip, durations);
+
+ if (chip->duration_adjusted) {
+ dev_info(&chip->dev, HW_ERR "Adjusting reported durations.");
+ chip->duration[TPM_SHORT] = durations[0];
+ chip->duration[TPM_MEDIUM] = durations[1];
+ chip->duration[TPM_LONG] = durations[2];
+ }
+
/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
* value wrong and apparently reports msecs rather than usecs. So we
* fix up the resulting too-small TPM_SHORT value to make things work.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 53c0ea9ec9df..bb1d1ac7081d 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -67,6 +67,8 @@ struct tpm_class_ops {
u8 (*status) (struct tpm_chip *chip);
void (*update_timeouts)(struct tpm_chip *chip,
unsigned long *timeout_cap);
+ void (*update_durations)(struct tpm_chip *chip,
+unsigned long *duration_cap);
int (*go_idle)(struct tpm_chip *chip);
int (*cmd_ready)(struct tpm_chip *chip);
int (*request_locality)(struct tpm_chip *chip, int loc);
--
2.21.0
[PATCH 0/2 v2] tpm: add update_durations class op to allow override of chip supplied values
We've run into a case where a customer has an STM TPM 1.2 chip (version 1.2.8.28) that is getting into an inconsistent state and they end up getting tpm transmit errors. In really old tpm code this wasn't seen because the code that grabbed the duration values from the chip could fail silently, and would proceed to just use default values and move forward. More recent code though successfully gets the duration values from the chip, and using those values this particular chip version gets into the state seen by the customer. The idea with this patchset is to provide a facility like the update_timeouts operation to allow the override of chip supplied values. I went back and looked at the original submission thread, and updated Alexey's patches based on Jarkko's suggestions for v2.
Re: [PATCH] tpm: Call tpm_put_ops() when the validation for @digests fails
On Tue Sep 10 19, Jarkko Sakkinen wrote:
The chip is not released when the validation for @digests fails. Add
tpm_put_ops() to the failure path.
Cc: sta...@vger.kernel.org
Reported-by: Roberto Sassu
Signed-off-by: Jarkko Sakkinen
Reviewed-by: Jerry Snitselaar
---
drivers/char/tpm/tpm-interface.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 208e5ba40e6e..c7eeb40feac8 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -320,18 +320,22 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
if (!chip)
return -ENODEV;
- for (i = 0; i < chip->nr_allocated_banks; i++)
- if (digests[i].alg_id != chip->allocated_banks[i].alg_id)
- return -EINVAL;
+ for (i = 0; i < chip->nr_allocated_banks; i++) {
+ if (digests[i].alg_id != chip->allocated_banks[i].alg_id) {
+ rc = EINVAL;
+ goto out;
+ }
+ }
if (chip->flags & TPM_CHIP_FLAG_TPM2) {
rc = tpm2_pcr_extend(chip, pcr_idx, digests);
- tpm_put_ops(chip);
- return rc;
+ goto out;
}
rc = tpm1_pcr_extend(chip, pcr_idx, digests[0].digest,
"attempting extend a PCR value");
+
+out:
tpm_put_ops(chip);
return rc;
}
--
2.20.1
Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm_send()
On Mon Sep 16 19, Jarkko Sakkinen wrote:
tpm_send() does not give anymore the result back to the caller. This
would require another memcpy(), which kind of tells that the whole
approach is somewhat broken. Instead, as Mimi suggested, this commit
just wraps the data to the tpm_buf, and thus the result will not go to
the garbage.
Obviously this assumes from the caller that it passes large enough
buffer, which makes the whole API somewhat broken because it could be
different size than @buflen but since trusted keys is the only module
using this API right now I think that this fix is sufficient for the
moment.
In the near future the plan is to replace the parameters with a tpm_buf
created by the caller.
Reported-by: Mimi Zohar
Suggested-by: Mimi Zohar
Cc: sta...@vger.kernel.org
Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO parameter")
Signed-off-by: Jarkko Sakkinen
---
drivers/char/tpm/tpm-interface.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9ace5480665..2459d36dd8cc 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t
buflen)
if (!chip)
return -ENODEV;
- rc = tpm_buf_init(&buf, 0, 0);
- if (rc)
- goto out;
-
- memcpy(buf.data, cmd, buflen);
+ buf.data = cmd;
rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to a send a command");
- tpm_buf_destroy(&buf);
+
out:
tpm_put_ops(chip);
return rc;
--
2.20.1
Nothing uses the out label any longer so it should be dropped as well, but
other than that...
Acked-by: Jerry Snitselaar
Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm_send()
On Mon Sep 16 19, Jerry Snitselaar wrote:
On Mon Sep 16 19, Jarkko Sakkinen wrote:
tpm_send() does not give anymore the result back to the caller. This
would require another memcpy(), which kind of tells that the whole
approach is somewhat broken. Instead, as Mimi suggested, this commit
just wraps the data to the tpm_buf, and thus the result will not go to
the garbage.
Obviously this assumes from the caller that it passes large enough
buffer, which makes the whole API somewhat broken because it could be
different size than @buflen but since trusted keys is the only module
using this API right now I think that this fix is sufficient for the
moment.
In the near future the plan is to replace the parameters with a tpm_buf
created by the caller.
Reported-by: Mimi Zohar
Suggested-by: Mimi Zohar
Cc: sta...@vger.kernel.org
Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO parameter")
Signed-off-by: Jarkko Sakkinen
---
drivers/char/tpm/tpm-interface.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9ace5480665..2459d36dd8cc 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t
buflen)
if (!chip)
return -ENODEV;
- rc = tpm_buf_init(&buf, 0, 0);
- if (rc)
- goto out;
-
- memcpy(buf.data, cmd, buflen);
+ buf.data = cmd;
rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to a send a command");
- tpm_buf_destroy(&buf);
+
out:
tpm_put_ops(chip);
return rc;
--
2.20.1
Nothing uses the out label any longer so it should be dropped as well, but
other than that...
Acked-by: Jerry Snitselaar
sigh (wrong emacs macro hit), that should be:
Reviewed-by: Jerry Snitselaar
Re: [PATCH 3/3] tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations
On Wed Oct 02 19, Sasha Levin wrote:
On Wed, Oct 02, 2019 at 03:57:58PM +0200, Greg KH wrote:
On Wed, Oct 02, 2019 at 04:14:44PM +0300, Jarkko Sakkinen wrote:
From: Vadim Sukhomlinov
commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 upstream
TPM 2.0 Shutdown involve sending TPM2_Shutdown to TPM chip and disabling
future TPM operations. TPM 1.2 behavior was different, future TPM
operations weren't disabled, causing rare issues. This patch ensures
that future TPM operations are disabled.
Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
Cc: sta...@vger.kernel.org
Signed-off-by: Vadim Sukhomlinov
[dianders: resolved merge conflicts with mainline]
Signed-off-by: Douglas Anderson
Reviewed-by: Jarkko Sakkinen
Signed-off-by: Jarkko Sakkinen
---
drivers/char/tpm/tpm-chip.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
What kernel version(s) is this for?
It would go to 4.19, we've recently reverted an incorrect backport of
this patch.
Jarkko, why is this patch 3/3? We haven't seen the first two on the
mailing list, do we need anything besides this patch?
--
Thanks,
Sasha
It looks like there was a problem mailing the earlier patchset, and patches 1
and 2
weren't cc'd to stable, but patch 3 was.
Re: [PATCH 3/3] tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations
On Wed Oct 02 19, Jerry Snitselaar wrote:
On Wed Oct 02 19, Sasha Levin wrote:
On Wed, Oct 02, 2019 at 03:57:58PM +0200, Greg KH wrote:
On Wed, Oct 02, 2019 at 04:14:44PM +0300, Jarkko Sakkinen wrote:
From: Vadim Sukhomlinov
commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 upstream
TPM 2.0 Shutdown involve sending TPM2_Shutdown to TPM chip and disabling
future TPM operations. TPM 1.2 behavior was different, future TPM
operations weren't disabled, causing rare issues. This patch ensures
that future TPM operations are disabled.
Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
Cc: sta...@vger.kernel.org
Signed-off-by: Vadim Sukhomlinov
[dianders: resolved merge conflicts with mainline]
Signed-off-by: Douglas Anderson
Reviewed-by: Jarkko Sakkinen
Signed-off-by: Jarkko Sakkinen
---
drivers/char/tpm/tpm-chip.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
What kernel version(s) is this for?
It would go to 4.19, we've recently reverted an incorrect backport of
this patch.
Jarkko, why is this patch 3/3? We haven't seen the first two on the
mailing list, do we need anything besides this patch?
--
Thanks,
Sasha
It looks like there was a problem mailing the earlier patchset, and patches 1
and 2
weren't cc'd to stable, but patch 3 was.
Is linux-stab...@vger.kernel.org a valid address?
Re: [PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values
On Wed Oct 02 19, Jarkko Sakkinen wrote: On Mon, Sep 02, 2019 at 07:27:32AM -0700, Jerry Snitselaar wrote: We've run into a case where a customer has an STM TPM 1.2 chip (version 1.2.8.28), that is getting into an inconsistent state and they end up getting tpm transmit errors. In really old tpm code this wasn't seen because the code that grabbed the duration values from the chip could fail silently, and would proceed to just use default values and move forward. More recent code though successfully gets the duration values from the chip, and using those values this particular chip version gets into the state seen by the customer. The idea with this patchset is to provide a facility like the update_timeouts operation to allow the override of chip supplied values. changes from v3: * Assign value to version when tpm1_getcap is successful for TPM 1.1 device not when it fails. changes from v2: * Added patch 1/3 * Rework tpm_tis_update_durations to make use of new version structs and pull tpm1_getcap calls out of loop. changes from v1: * Remove unneeded newline * Formatting cleanups * Change tpm_tis_update_durations to be a void function, and use chip->duration_adjusted to track whether adjustment was made. Jarkko Sakkinen (1): tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jerry Snitselaar (2): tpm: provide a way to override the chip returned durations tpm_tis: override durations for STM tpm with firmware 1.2.8.28 I applied to my master branch. Probably hard to get wide testing given the "niche" case when the issue happens. Should be sufficient that the commonc case still works. /Jarkko Yeah, it is a pain. The people with the problem systems tested an earlier version of Alexey's patches. I have a system with a different rev STM device, so I did some testing with a modified patch that keyed off that revision, but it will be hard to get it wide exposure.
Re: [PATCH] tpm: Detach page allocation from tpm_buf
On Thu Sep 26 19, Jarkko Sakkinen wrote:
As has been seen recently, binding the buffer allocation and tpm_buf
together is sometimes far from optimal. The buffer might come from the
caller namely when tpm_send() is used by another subsystem. In addition we
can stability in call sites w/o rollback (e.g. power events)>
Take allocation out of the tpm_buf framework and make it purely a wrapper
for the data buffer.
Link: https://patchwork.kernel.org/patch/11146585/
Cc: Mimi Zohar
Cc: Jerry Snitselaar
Cc: James Bottomley
Cc: Sumit Garg
Cc: Stefan Berger
Signed-off-by: Jarkko Sakkinen
---
v2:
* In tpm2_get_random(), TPM2_CC_GET_RANDOM was accidently switch to
TPM2_CC_PCR_EXTEND. Now it has been switched back.
drivers/char/tpm/tpm-sysfs.c | 19 ++-
drivers/char/tpm/tpm.h| 40 ++---
drivers/char/tpm/tpm1-cmd.c | 114 +
drivers/char/tpm/tpm2-cmd.c | 265 +++---
drivers/char/tpm/tpm2-space.c | 64 +---
drivers/char/tpm/tpm_vtpm_proxy.c | 24 +--
6 files changed, 333 insertions(+), 193 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index edfa89160010..eeb90c9225b9 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -32,21 +32,26 @@ struct tpm_readpubek_out {
static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- struct tpm_buf tpm_buf;
- struct tpm_readpubek_out *out;
- int i;
- char *str = buf;
struct tpm_chip *chip = to_tpm_chip(dev);
+ struct tpm_readpubek_out *out;
+ struct page *data_page;
+ struct tpm_buf tpm_buf;
char anti_replay[20];
+ char *str = buf;
+ int i;
memset(&anti_replay, 0, sizeof(anti_replay));
if (tpm_try_get_ops(chip))
return 0;
- if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK))
+ data_page = alloc_page(GFP_HIGHUSER);
+ if (!data_page)
goto out_ops;
+ tpm_buf_reset(&tpm_buf, kmap(data_page), PAGE_SIZE, TPM_TAG_RQU_COMMAND,
+ TPM_ORD_READPUBEK);
+
tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE,
@@ -83,7 +88,9 @@ static ssize_t pubek_show(struct device *dev, struct
device_attribute *attr,
}
out_buf:
- tpm_buf_destroy(&tpm_buf);
+ kunmap(data_page);
+ __free_page(data_page);
+
out_ops:
tpm_put_ops(chip);
return str - buf;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..45316e5d2d36 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -284,36 +284,30 @@ enum tpm_buf_flags {
};
struct tpm_buf {
- struct page *data_page;
- unsigned int flags;
u8 *data;
+ unsigned int size;
+ unsigned int flags;
};
-static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static inline void tpm_buf_reset(struct tpm_buf *buf, u8 *data,
+ unsigned int size, u16 tag, u32 ordinal)
{
- struct tpm_header *head = (struct tpm_header *)buf->data;
+ struct tpm_header *head = (struct tpm_header *)data;
- head->tag = cpu_to_be16(tag);
- head->length = cpu_to_be32(sizeof(*head));
- head->ordinal = cpu_to_be32(ordinal);
-}
-
-static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
- buf->data_page = alloc_page(GFP_HIGHUSER);
- if (!buf->data_page)
- return -ENOMEM;
+ /* sanity check */
+ if (size < TPM_HEADER_SIZE) {
+ WARN(1, "tpm_buf: overflow\n");
+ buf->flags |= TPM_BUF_OVERFLOW;
+ return;
+ }
+ buf->data = data;
+ buf->size = size;
buf->flags = 0;
- buf->data = kmap(buf->data_page);
- tpm_buf_reset(buf, tag, ordinal);
- return 0;
-}
-static inline void tpm_buf_destroy(struct tpm_buf *buf)
-{
- kunmap(buf->data_page);
- __free_page(buf->data_page);
+ head->tag = cpu_to_be16(tag);
+ head->length = cpu_to_be32(sizeof(*head));
+ head->ordinal = cpu_to_be32(ordinal);
}
static inline u32 tpm_buf_length(struct tpm_buf *buf)
@@ -341,7 +335,7 @@ static inline void tpm_buf_append(struct tpm_buf *buf,
if (buf->flags & TPM_BUF_OVERFLOW)
return;
- if ((len + new_len) > PAGE_SIZE) {
+ if ((len + new_len) > buf->size) {
WARN(1, "tpm_buf: overflow\n");
buf->flags |= TPM_BUF_OVERFLOW;
return;
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 149e953ca369..2753699454ab 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -323,19 +323,25 @@ unsigne
Re: [PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values
On Mon Sep 02 19, Jerry Snitselaar wrote: We've run into a case where a customer has an STM TPM 1.2 chip (version 1.2.8.28), that is getting into an inconsistent state and they end up getting tpm transmit errors. In really old tpm code this wasn't seen because the code that grabbed the duration values from the chip could fail silently, and would proceed to just use default values and move forward. More recent code though successfully gets the duration values from the chip, and using those values this particular chip version gets into the state seen by the customer. The idea with this patchset is to provide a facility like the update_timeouts operation to allow the override of chip supplied values. changes from v3: * Assign value to version when tpm1_getcap is successful for TPM 1.1 device not when it fails. changes from v2: * Added patch 1/3 * Rework tpm_tis_update_durations to make use of new version structs and pull tpm1_getcap calls out of loop. changes from v1: * Remove unneeded newline * Formatting cleanups * Change tpm_tis_update_durations to be a void function, and use chip->duration_adjusted to track whether adjustment was made. Jarkko Sakkinen (1): tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jerry Snitselaar (2): tpm: provide a way to override the chip returned durations tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Anyone else have any feedback on this patchset?
