Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] Fix debugfs register access while suspended
@ 2020-01-24 13:29 Geert Uytterhoeven
  2020-01-24 13:29 ` [PATCH 1/2] debugfs: regset32: Add Runtime PM support Geert Uytterhoeven
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2020-01-24 13:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Gilad Ben-Yossef, Herbert Xu,
	David S . Miller, Rafael J . Wysocki
  Cc: Rob Clark, Sean Paul, Felipe Balbi, Alan Stern, Thierry Reding,
	Jonathan Hunter, Mathias Nyman, Matthias Brugger, Chunfeng Yun,
	Bin Liu, linux-crypto, linux-pm, linux-renesas-soc, linux-usb,
	linux-kernel, Geert Uytterhoeven

	Hi all,

While comparing register values read from debugfs files under
/sys/kernel/debug/ccree/, I noticed some oddities.
Apparently there is no guarantee these registers are read from the
device while it is resumed.  This may lead to bogus values, or crashes
and lock-ups.

This patch series:
  1. Allows debugfs_create_regset32() to be used for devices whose
     registers must be accessed when resumed,
  2. Fixes the CCREE driver to make use of this.

I have identified several other drivers that may be affected (i.e.
using debugfs_create_regset32() and pm_runtime_*()):
  - drivers/gpu/drm/msm/disp/dpu1
  - drivers/usb/dwc3
  - drivers/usb/host/ehci-omap.c
  - drivers/usb/host/ehci-tegra.c
  - drivers/usb/host/ohci-platform.c
  - drivers/usb/host/xhci.c
  - drivers/usb/host/xhci-dbgcap.c
  - drivers/usb/host/xhci-histb.c
  - drivers/usb/host/xhci-hub.c
  - drivers/usb/host/xhci-mtk.c
  - drivers/usb/host/xhci-pci.c
  - drivers/usb/host/xhci-plat.c
  - drivers/usb/host/xhci-tegra.c
  - drivers/usb/mtu3
  - drivers/usb/musb

Some of these call pm_runtime_forbid(), but given the comment "users
should enable runtime pm using power/control in sysfs", this can be
overridden from userspace, so these are unsafe, too?

Thanks for your comments!

Geert Uytterhoeven (2):
  debugfs: regset32: Add Runtime PM support
  crypto: ccree - fix debugfs register access while suspended

 drivers/crypto/ccree/cc_debugfs.c | 2 ++
 fs/debugfs/file.c                 | 8 ++++++++
 include/linux/debugfs.h           | 1 +
 3 files changed, 11 insertions(+)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

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

* [PATCH 1/2] debugfs: regset32: Add Runtime PM support
  2020-01-24 13:29 [PATCH 0/2] Fix debugfs register access while suspended Geert Uytterhoeven
@ 2020-01-24 13:29 ` Geert Uytterhoeven
  2020-01-31 10:13   ` Rafael J. Wysocki
  2020-02-10 19:01   ` Greg Kroah-Hartman
  2020-01-24 13:29 ` [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended Geert Uytterhoeven
  2020-01-24 16:24 ` [PATCH 0/2] Fix " Niklas Söderlund
  2 siblings, 2 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2020-01-24 13:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Gilad Ben-Yossef, Herbert Xu,
	David S . Miller, Rafael J . Wysocki
  Cc: Rob Clark, Sean Paul, Felipe Balbi, Alan Stern, Thierry Reding,
	Jonathan Hunter, Mathias Nyman, Matthias Brugger, Chunfeng Yun,
	Bin Liu, linux-crypto, linux-pm, linux-renesas-soc, linux-usb,
	linux-kernel, Geert Uytterhoeven

Hardware registers of devices under control of power management cannot
be accessed at all times.  If such a device is suspended, register
accesses may lead to undefined behavior, like reading bogus values, or
causing exceptions or system locks.

Extend struct debugfs_regset32 with an optional field to let device
drivers specify the device the registers in the set belong to.  This
allows debugfs_show_regset32() to make sure the device is resumed while
its registers are being read.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 fs/debugfs/file.c       | 8 ++++++++
 include/linux/debugfs.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index dede25247b81f72a..5e52d68421c678f2 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -18,6 +18,7 @@
 #include <linux/slab.h>
 #include <linux/atomic.h>
 #include <linux/device.h>
+#include <linux/pm_runtime.h>
 #include <linux/poll.h>
 #include <linux/security.h>
 
@@ -1057,7 +1058,14 @@ static int debugfs_show_regset32(struct seq_file *s, void *data)
 {
 	struct debugfs_regset32 *regset = s->private;
 
+	if (regset->dev)
+		pm_runtime_get_sync(regset->dev);
+
 	debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
+
+	if (regset->dev)
+		pm_runtime_put(regset->dev);
+
 	return 0;
 }
 
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index bf9b6cafa4c26a68..5d0783ae09f365ac 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -35,6 +35,7 @@ struct debugfs_regset32 {
 	const struct debugfs_reg32 *regs;
 	int nregs;
 	void __iomem *base;
+	struct device *dev;	/* Optional device for Runtime PM */
 };
 
 extern struct dentry *arch_debugfs_dir;
-- 
2.17.1


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

* [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended
  2020-01-24 13:29 [PATCH 0/2] Fix debugfs register access while suspended Geert Uytterhoeven
  2020-01-24 13:29 ` [PATCH 1/2] debugfs: regset32: Add Runtime PM support Geert Uytterhoeven
@ 2020-01-24 13:29 ` Geert Uytterhoeven
  2020-01-26 13:32   ` Gilad Ben-Yossef
  2020-01-31 10:11   ` Rafael J. Wysocki
  2020-01-24 16:24 ` [PATCH 0/2] Fix " Niklas Söderlund
  2 siblings, 2 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2020-01-24 13:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Gilad Ben-Yossef, Herbert Xu,
	David S . Miller, Rafael J . Wysocki
  Cc: Rob Clark, Sean Paul, Felipe Balbi, Alan Stern, Thierry Reding,
	Jonathan Hunter, Mathias Nyman, Matthias Brugger, Chunfeng Yun,
	Bin Liu, linux-crypto, linux-pm, linux-renesas-soc, linux-usb,
	linux-kernel, Geert Uytterhoeven

Reading the debugfs files under /sys/kernel/debug/ccree/ can be done by
the user at any time.  On R-Car SoCs, the CCREE device is power-managed
using a moduile clock, and if this clock is not running, bogus register
values may be read.

Fix this by filling in the debugfs_regset32.dev field, so debugfs will
make sure the device is resumed while its registers are being read.

This fixes the bogus values (0x00000260) in the register dumps on R-Car
H3 ES1.0:

    -e6601000.crypto/regs:HOST_IRR = 0x00000260
    -e6601000.crypto/regs:HOST_POWER_DOWN_EN = 0x00000260
    +e6601000.crypto/regs:HOST_IRR = 0x00000038
    +e6601000.crypto/regs:HOST_POWER_DOWN_EN = 0x00000038
     e6601000.crypto/regs:AXIM_MON_ERR = 0x00000000
     e6601000.crypto/regs:DSCRPTR_QUEUE_CONTENT = 0x000002aa
    -e6601000.crypto/regs:HOST_IMR = 0x00000260
    +e6601000.crypto/regs:HOST_IMR = 0x017ffeff
     e6601000.crypto/regs:AXIM_CFG = 0x001f0007
     e6601000.crypto/regs:AXIM_CACHE_PARAMS = 0x00000000
    -e6601000.crypto/regs:GPR_HOST = 0x00000260
    +e6601000.crypto/regs:GPR_HOST = 0x017ffeff
     e6601000.crypto/regs:AXIM_MON_COMP = 0x00000000
    -e6601000.crypto/version:SIGNATURE = 0x00000260
    -e6601000.crypto/version:VERSION = 0x00000260
    +e6601000.crypto/version:SIGNATURE = 0xdcc63000
    +e6601000.crypto/version:VERSION = 0xaf400001

Note that this behavior is system-dependent, and the issue does not show
up on all R-Car Gen3 SoCs and boards.  Even when the device is
suspended, the module clock may be left enabled, if configured by the
firmware for Secure Mode, or when controlled by the Real-Time Core.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/crypto/ccree/cc_debugfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/ccree/cc_debugfs.c b/drivers/crypto/ccree/cc_debugfs.c
index 5669997386988055..35f3a2137502bd96 100644
--- a/drivers/crypto/ccree/cc_debugfs.c
+++ b/drivers/crypto/ccree/cc_debugfs.c
@@ -81,6 +81,7 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
 	regset->regs = debug_regs;
 	regset->nregs = ARRAY_SIZE(debug_regs);
 	regset->base = drvdata->cc_base;
+	regset->dev = dev;
 
 	ctx->dir = debugfs_create_dir(drvdata->plat_dev->name, cc_debugfs_dir);
 
@@ -102,6 +103,7 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
 		verset->nregs = ARRAY_SIZE(pid_cid_regs);
 	}
 	verset->base = drvdata->cc_base;
+	verset->dev = dev;
 
 	debugfs_create_regset32("version", 0400, ctx->dir, verset);
 
-- 
2.17.1


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

* Re: [PATCH 0/2] Fix debugfs register access while suspended
  2020-01-24 13:29 [PATCH 0/2] Fix debugfs register access while suspended Geert Uytterhoeven
  2020-01-24 13:29 ` [PATCH 1/2] debugfs: regset32: Add Runtime PM support Geert Uytterhoeven
  2020-01-24 13:29 ` [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended Geert Uytterhoeven
@ 2020-01-24 16:24 ` " Niklas Söderlund
  2 siblings, 0 replies; 8+ messages in thread
From: Niklas Söderlund @ 2020-01-24 16:24 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Gilad Ben-Yossef, Herbert Xu,
	David S . Miller, Rafael J . Wysocki, Rob Clark, Sean Paul,
	Felipe Balbi, Alan Stern, Thierry Reding, Jonathan Hunter,
	Mathias Nyman, Matthias Brugger, Chunfeng Yun, Bin Liu,
	linux-crypto, linux-pm, linux-renesas-soc, linux-usb,
	linux-kernel

Hi Geert,

Thanks for your series.

On 2020-01-24 14:29:55 +0100, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> While comparing register values read from debugfs files under
> /sys/kernel/debug/ccree/, I noticed some oddities.
> Apparently there is no guarantee these registers are read from the
> device while it is resumed.  This may lead to bogus values, or crashes
> and lock-ups.
> 
> This patch series:
>   1. Allows debugfs_create_regset32() to be used for devices whose
>      registers must be accessed when resumed,
>   2. Fixes the CCREE driver to make use of this.
> 
> I have identified several other drivers that may be affected (i.e.
> using debugfs_create_regset32() and pm_runtime_*()):
>   - drivers/gpu/drm/msm/disp/dpu1
>   - drivers/usb/dwc3
>   - drivers/usb/host/ehci-omap.c
>   - drivers/usb/host/ehci-tegra.c
>   - drivers/usb/host/ohci-platform.c
>   - drivers/usb/host/xhci.c
>   - drivers/usb/host/xhci-dbgcap.c
>   - drivers/usb/host/xhci-histb.c
>   - drivers/usb/host/xhci-hub.c
>   - drivers/usb/host/xhci-mtk.c
>   - drivers/usb/host/xhci-pci.c
>   - drivers/usb/host/xhci-plat.c
>   - drivers/usb/host/xhci-tegra.c
>   - drivers/usb/mtu3
>   - drivers/usb/musb
> 
> Some of these call pm_runtime_forbid(), but given the comment "users
> should enable runtime pm using power/control in sysfs", this can be
> overridden from userspace, so these are unsafe, too?
> 
> Thanks for your comments!

Looks good to me,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> 
> Geert Uytterhoeven (2):
>   debugfs: regset32: Add Runtime PM support
>   crypto: ccree - fix debugfs register access while suspended
> 
>  drivers/crypto/ccree/cc_debugfs.c | 2 ++
>  fs/debugfs/file.c                 | 8 ++++++++
>  include/linux/debugfs.h           | 1 +
>  3 files changed, 11 insertions(+)
> 
> -- 
> 2.17.1
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended
  2020-01-24 13:29 ` [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended Geert Uytterhoeven
@ 2020-01-26 13:32   ` Gilad Ben-Yossef
  2020-01-31 10:11   ` Rafael J. Wysocki
  1 sibling, 0 replies; 8+ messages in thread
From: Gilad Ben-Yossef @ 2020-01-26 13:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Herbert Xu, David S . Miller,
	Rafael J . Wysocki, Rob Clark, Sean Paul, Felipe Balbi,
	Alan Stern, Thierry Reding, Jonathan Hunter, Mathias Nyman,
	Matthias Brugger, Chunfeng Yun, Bin Liu,
	Linux Crypto Mailing List, open list:THERMAL, Linux-Renesas,
	linux-usb, Linux kernel mailing list

On Fri, Jan 24, 2020 at 3:30 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> Reading the debugfs files under /sys/kernel/debug/ccree/ can be done by
> the user at any time.  On R-Car SoCs, the CCREE device is power-managed
> using a moduile clock, and if this clock is not running, bogus register
> values may be read.
>
> Fix this by filling in the debugfs_regset32.dev field, so debugfs will
> make sure the device is resumed while its registers are being read.
>
> This fixes the bogus values (0x00000260) in the register dumps on R-Car
> H3 ES1.0:
>
>     -e6601000.crypto/regs:HOST_IRR = 0x00000260
>     -e6601000.crypto/regs:HOST_POWER_DOWN_EN = 0x00000260
>     +e6601000.crypto/regs:HOST_IRR = 0x00000038
>     +e6601000.crypto/regs:HOST_POWER_DOWN_EN = 0x00000038
>      e6601000.crypto/regs:AXIM_MON_ERR = 0x00000000
>      e6601000.crypto/regs:DSCRPTR_QUEUE_CONTENT = 0x000002aa
>     -e6601000.crypto/regs:HOST_IMR = 0x00000260
>     +e6601000.crypto/regs:HOST_IMR = 0x017ffeff
>      e6601000.crypto/regs:AXIM_CFG = 0x001f0007
>      e6601000.crypto/regs:AXIM_CACHE_PARAMS = 0x00000000
>     -e6601000.crypto/regs:GPR_HOST = 0x00000260
>     +e6601000.crypto/regs:GPR_HOST = 0x017ffeff
>      e6601000.crypto/regs:AXIM_MON_COMP = 0x00000000
>     -e6601000.crypto/version:SIGNATURE = 0x00000260
>     -e6601000.crypto/version:VERSION = 0x00000260
>     +e6601000.crypto/version:SIGNATURE = 0xdcc63000
>     +e6601000.crypto/version:VERSION = 0xaf400001
>
> Note that this behavior is system-dependent, and the issue does not show
> up on all R-Car Gen3 SoCs and boards.  Even when the device is
> suspended, the module clock may be left enabled, if configured by the
> firmware for Secure Mode, or when controlled by the Real-Time Core.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/crypto/ccree/cc_debugfs.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/crypto/ccree/cc_debugfs.c b/drivers/crypto/ccree/cc_debugfs.c
> index 5669997386988055..35f3a2137502bd96 100644
> --- a/drivers/crypto/ccree/cc_debugfs.c
> +++ b/drivers/crypto/ccree/cc_debugfs.c
> @@ -81,6 +81,7 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
>         regset->regs = debug_regs;
>         regset->nregs = ARRAY_SIZE(debug_regs);
>         regset->base = drvdata->cc_base;
> +       regset->dev = dev;
>
>         ctx->dir = debugfs_create_dir(drvdata->plat_dev->name, cc_debugfs_dir);
>
> @@ -102,6 +103,7 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
>                 verset->nregs = ARRAY_SIZE(pid_cid_regs);
>         }
>         verset->base = drvdata->cc_base;
> +       verset->dev = dev;
>
>         debugfs_create_regset32("version", 0400, ctx->dir, verset);
>
> --
> 2.17.1
>


Acked-by: Gilad Ben-Yossef <gilad@benyossef.com>

Thanks,
Gilad
-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

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

* Re: [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended
  2020-01-24 13:29 ` [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended Geert Uytterhoeven
  2020-01-26 13:32   ` Gilad Ben-Yossef
@ 2020-01-31 10:11   ` Rafael J. Wysocki
  1 sibling, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2020-01-31 10:11 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Gilad Ben-Yossef, Herbert Xu,
	David S . Miller, Rafael J . Wysocki, Rob Clark, Sean Paul,
	Felipe Balbi, Alan Stern, Thierry Reding, Jonathan Hunter,
	Mathias Nyman, Matthias Brugger, Chunfeng Yun, Bin Liu,
	linux-crypto, Linux PM, Linux-Renesas,
	open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:,
	Linux Kernel Mailing List

On Fri, Jan 24, 2020 at 2:30 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> Reading the debugfs files under /sys/kernel/debug/ccree/ can be done by
> the user at any time.  On R-Car SoCs, the CCREE device is power-managed
> using a moduile clock, and if this clock is not running, bogus register
> values may be read.
>
> Fix this by filling in the debugfs_regset32.dev field, so debugfs will
> make sure the device is resumed while its registers are being read.
>
> This fixes the bogus values (0x00000260) in the register dumps on R-Car
> H3 ES1.0:
>
>     -e6601000.crypto/regs:HOST_IRR = 0x00000260
>     -e6601000.crypto/regs:HOST_POWER_DOWN_EN = 0x00000260
>     +e6601000.crypto/regs:HOST_IRR = 0x00000038
>     +e6601000.crypto/regs:HOST_POWER_DOWN_EN = 0x00000038
>      e6601000.crypto/regs:AXIM_MON_ERR = 0x00000000
>      e6601000.crypto/regs:DSCRPTR_QUEUE_CONTENT = 0x000002aa
>     -e6601000.crypto/regs:HOST_IMR = 0x00000260
>     +e6601000.crypto/regs:HOST_IMR = 0x017ffeff
>      e6601000.crypto/regs:AXIM_CFG = 0x001f0007
>      e6601000.crypto/regs:AXIM_CACHE_PARAMS = 0x00000000
>     -e6601000.crypto/regs:GPR_HOST = 0x00000260
>     +e6601000.crypto/regs:GPR_HOST = 0x017ffeff
>      e6601000.crypto/regs:AXIM_MON_COMP = 0x00000000
>     -e6601000.crypto/version:SIGNATURE = 0x00000260
>     -e6601000.crypto/version:VERSION = 0x00000260
>     +e6601000.crypto/version:SIGNATURE = 0xdcc63000
>     +e6601000.crypto/version:VERSION = 0xaf400001
>
> Note that this behavior is system-dependent, and the issue does not show
> up on all R-Car Gen3 SoCs and boards.  Even when the device is
> suspended, the module clock may be left enabled, if configured by the
> firmware for Secure Mode, or when controlled by the Real-Time Core.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

LGTM:

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/crypto/ccree/cc_debugfs.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/crypto/ccree/cc_debugfs.c b/drivers/crypto/ccree/cc_debugfs.c
> index 5669997386988055..35f3a2137502bd96 100644
> --- a/drivers/crypto/ccree/cc_debugfs.c
> +++ b/drivers/crypto/ccree/cc_debugfs.c
> @@ -81,6 +81,7 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
>         regset->regs = debug_regs;
>         regset->nregs = ARRAY_SIZE(debug_regs);
>         regset->base = drvdata->cc_base;
> +       regset->dev = dev;
>
>         ctx->dir = debugfs_create_dir(drvdata->plat_dev->name, cc_debugfs_dir);
>
> @@ -102,6 +103,7 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
>                 verset->nregs = ARRAY_SIZE(pid_cid_regs);
>         }
>         verset->base = drvdata->cc_base;
> +       verset->dev = dev;
>
>         debugfs_create_regset32("version", 0400, ctx->dir, verset);
>
> --
> 2.17.1
>

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

* Re: [PATCH 1/2] debugfs: regset32: Add Runtime PM support
  2020-01-24 13:29 ` [PATCH 1/2] debugfs: regset32: Add Runtime PM support Geert Uytterhoeven
@ 2020-01-31 10:13   ` Rafael J. Wysocki
  2020-02-10 19:01   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2020-01-31 10:13 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Gilad Ben-Yossef, Herbert Xu,
	David S . Miller, Rafael J . Wysocki, Rob Clark, Sean Paul,
	Felipe Balbi, Alan Stern, Thierry Reding, Jonathan Hunter,
	Mathias Nyman, Matthias Brugger, Chunfeng Yun, Bin Liu,
	linux-crypto, Linux PM, Linux-Renesas,
	open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:,
	Linux Kernel Mailing List

On Fri, Jan 24, 2020 at 2:30 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> Hardware registers of devices under control of power management cannot
> be accessed at all times.  If such a device is suspended, register
> accesses may lead to undefined behavior, like reading bogus values, or
> causing exceptions or system locks.
>
> Extend struct debugfs_regset32 with an optional field to let device
> drivers specify the device the registers in the set belong to.  This
> allows debugfs_show_regset32() to make sure the device is resumed while
> its registers are being read.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

LGTM:

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  fs/debugfs/file.c       | 8 ++++++++
>  include/linux/debugfs.h | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index dede25247b81f72a..5e52d68421c678f2 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -18,6 +18,7 @@
>  #include <linux/slab.h>
>  #include <linux/atomic.h>
>  #include <linux/device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/poll.h>
>  #include <linux/security.h>
>
> @@ -1057,7 +1058,14 @@ static int debugfs_show_regset32(struct seq_file *s, void *data)
>  {
>         struct debugfs_regset32 *regset = s->private;
>
> +       if (regset->dev)
> +               pm_runtime_get_sync(regset->dev);
> +
>         debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
> +
> +       if (regset->dev)
> +               pm_runtime_put(regset->dev);
> +
>         return 0;
>  }
>
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index bf9b6cafa4c26a68..5d0783ae09f365ac 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -35,6 +35,7 @@ struct debugfs_regset32 {
>         const struct debugfs_reg32 *regs;
>         int nregs;
>         void __iomem *base;
> +       struct device *dev;     /* Optional device for Runtime PM */
>  };
>
>  extern struct dentry *arch_debugfs_dir;
> --
> 2.17.1
>

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

* Re: [PATCH 1/2] debugfs: regset32: Add Runtime PM support
  2020-01-24 13:29 ` [PATCH 1/2] debugfs: regset32: Add Runtime PM support Geert Uytterhoeven
  2020-01-31 10:13   ` Rafael J. Wysocki
@ 2020-02-10 19:01   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-10 19:01 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Gilad Ben-Yossef, Herbert Xu, David S . Miller,
	Rafael J . Wysocki, Rob Clark, Sean Paul, Felipe Balbi,
	Alan Stern, Thierry Reding, Jonathan Hunter, Mathias Nyman,
	Matthias Brugger, Chunfeng Yun, Bin Liu, linux-crypto, linux-pm,
	linux-renesas-soc, linux-usb, linux-kernel

On Fri, Jan 24, 2020 at 02:29:56PM +0100, Geert Uytterhoeven wrote:
> Hardware registers of devices under control of power management cannot
> be accessed at all times.  If such a device is suspended, register
> accesses may lead to undefined behavior, like reading bogus values, or
> causing exceptions or system locks.
> 
> Extend struct debugfs_regset32 with an optional field to let device
> drivers specify the device the registers in the set belong to.  This
> allows debugfs_show_regset32() to make sure the device is resumed while
> its registers are being read.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  fs/debugfs/file.c       | 8 ++++++++
>  include/linux/debugfs.h | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index dede25247b81f72a..5e52d68421c678f2 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -18,6 +18,7 @@
>  #include <linux/slab.h>
>  #include <linux/atomic.h>
>  #include <linux/device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/poll.h>
>  #include <linux/security.h>
>  
> @@ -1057,7 +1058,14 @@ static int debugfs_show_regset32(struct seq_file *s, void *data)
>  {
>  	struct debugfs_regset32 *regset = s->private;
>  
> +	if (regset->dev)
> +		pm_runtime_get_sync(regset->dev);
> +
>  	debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
> +
> +	if (regset->dev)
> +		pm_runtime_put(regset->dev);
> +
>  	return 0;
>  }
>  
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index bf9b6cafa4c26a68..5d0783ae09f365ac 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -35,6 +35,7 @@ struct debugfs_regset32 {
>  	const struct debugfs_reg32 *regs;
>  	int nregs;
>  	void __iomem *base;
> +	struct device *dev;	/* Optional device for Runtime PM */
>  };
>  
>  extern struct dentry *arch_debugfs_dir;

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-24 13:29 [PATCH 0/2] Fix debugfs register access while suspended Geert Uytterhoeven
2020-01-24 13:29 ` [PATCH 1/2] debugfs: regset32: Add Runtime PM support Geert Uytterhoeven
2020-01-31 10:13   ` Rafael J. Wysocki
2020-02-10 19:01   ` Greg Kroah-Hartman
2020-01-24 13:29 ` [PATCH 2/2] crypto: ccree - fix debugfs register access while suspended Geert Uytterhoeven
2020-01-26 13:32   ` Gilad Ben-Yossef
2020-01-31 10:11   ` Rafael J. Wysocki
2020-01-24 16:24 ` [PATCH 0/2] Fix " Niklas Söderlund

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org
	public-inbox-index linux-crypto

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git