All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Stephen Warren <swarren@wwwdotorg.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Alexandre Courbot <gnurou@gmail.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Kevin Hilman <khilman@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>
Cc: linux-tegra@vger.kernel.org, linux-pm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jon Hunter <jonathanh@nvidia.com>
Subject: [PATCH V5 02/14] soc: tegra: pmc: Protect public functions from potential race conditions
Date: Thu, 28 Jan 2016 16:33:40 +0000	[thread overview]
Message-ID: <1453998832-27383-3-git-send-email-jonathanh@nvidia.com> (raw)
In-Reply-To: <1453998832-27383-1-git-send-email-jonathanh@nvidia.com>

The PMC base address pointer is initialised during early boot so that
early platform code may used the PMC public functions. During the probe
of the PMC driver the base address pointer is mapped again and the initial
mapping is freed. This exposes a window where a device accessing the PMC
registers via one of the public functions, could race with the updating
of the pointer and lead to a invalid access. Furthermore, the only
protection between multiple devices attempting to access the PMC registers
is when setting the powergate state to on or off. None of the other public
functions that access the PMC registers are protected.

Use the existing mutex to protect paths that may race with regard to
accessing the PMC registers.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 44 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 85b4e166273a..f8cdb7ce9755 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -235,7 +235,10 @@ int tegra_powergate_is_powered(int id)
 	if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
 		return -EINVAL;
 
+	mutex_lock(&pmc->powergates_lock);
 	status = tegra_pmc_readl(PWRGATE_STATUS) & (1 << id);
+	mutex_unlock(&pmc->powergates_lock);
+
 	return !!status;
 }
 
@@ -250,6 +253,8 @@ int tegra_powergate_remove_clamping(int id)
 	if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
 		return -EINVAL;
 
+	mutex_lock(&pmc->powergates_lock);
+
 	/*
 	 * On Tegra124 and later, the clamps for the GPU are controlled by a
 	 * separate register (with different semantics).
@@ -257,7 +262,7 @@ int tegra_powergate_remove_clamping(int id)
 	if (id == TEGRA_POWERGATE_3D) {
 		if (pmc->soc->has_gpu_clamps) {
 			tegra_pmc_writel(0, GPU_RG_CNTRL);
-			return 0;
+			goto out;
 		}
 	}
 
@@ -274,6 +279,9 @@ int tegra_powergate_remove_clamping(int id)
 
 	tegra_pmc_writel(mask, REMOVE_CLAMPING);
 
+out:
+	mutex_unlock(&pmc->powergates_lock);
+
 	return 0;
 }
 EXPORT_SYMBOL(tegra_powergate_remove_clamping);
@@ -520,9 +528,11 @@ int tegra_io_rail_power_on(int id)
 	unsigned int bit, mask;
 	int err;
 
+	mutex_lock(&pmc->powergates_lock);
+
 	err = tegra_io_rail_prepare(id, &request, &status, &bit);
 	if (err < 0)
-		return err;
+		goto error;
 
 	mask = 1 << bit;
 
@@ -535,12 +545,15 @@ int tegra_io_rail_power_on(int id)
 	err = tegra_io_rail_poll(status, mask, 0, 250);
 	if (err < 0) {
 		pr_info("tegra_io_rail_poll() failed: %d\n", err);
-		return err;
+		goto error;
 	}
 
 	tegra_io_rail_unprepare();
 
-	return 0;
+error:
+	mutex_unlock(&pmc->powergates_lock);
+
+	return err < 0 ? err : 0;
 }
 EXPORT_SYMBOL(tegra_io_rail_power_on);
 
@@ -550,10 +563,12 @@ int tegra_io_rail_power_off(int id)
 	unsigned int bit, mask;
 	int err;
 
+	mutex_lock(&pmc->powergates_lock);
+
 	err = tegra_io_rail_prepare(id, &request, &status, &bit);
 	if (err < 0) {
 		pr_info("tegra_io_rail_prepare() failed: %d\n", err);
-		return err;
+		goto error;
 	}
 
 	mask = 1 << bit;
@@ -566,11 +581,14 @@ int tegra_io_rail_power_off(int id)
 
 	err = tegra_io_rail_poll(status, mask, mask, 250);
 	if (err < 0)
-		return err;
+		goto error;
 
 	tegra_io_rail_unprepare();
 
-	return 0;
+error:
+	mutex_unlock(&pmc->powergates_lock);
+
+	return err < 0 ? err : 0;
 }
 EXPORT_SYMBOL(tegra_io_rail_power_off);
 
@@ -817,9 +835,15 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	/* take over the memory region from the early initialization */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	mutex_lock(&pmc->powergates_lock);
 	pmc->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(pmc->base))
-		return PTR_ERR(pmc->base);
+	mutex_unlock(&pmc->powergates_lock);
+
+	if (IS_ERR(pmc->base)) {
+		err = PTR_ERR(pmc->base);
+		goto error;
+	}
 
 	pmc->clk = devm_clk_get(&pdev->dev, "pclk");
 	if (IS_ERR(pmc->clk)) {
@@ -853,7 +877,9 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 	return 0;
 
 error:
+	mutex_lock(&pmc->powergates_lock);
 	pmc->base = base;
+	mutex_unlock(&pmc->powergates_lock);
 
 	return err;
 }
-- 
2.1.4


WARNING: multiple messages have this Message-ID (diff)
From: jonathanh@nvidia.com (Jon Hunter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V5 02/14] soc: tegra: pmc: Protect public functions from potential race conditions
Date: Thu, 28 Jan 2016 16:33:40 +0000	[thread overview]
Message-ID: <1453998832-27383-3-git-send-email-jonathanh@nvidia.com> (raw)
In-Reply-To: <1453998832-27383-1-git-send-email-jonathanh@nvidia.com>

The PMC base address pointer is initialised during early boot so that
early platform code may used the PMC public functions. During the probe
of the PMC driver the base address pointer is mapped again and the initial
mapping is freed. This exposes a window where a device accessing the PMC
registers via one of the public functions, could race with the updating
of the pointer and lead to a invalid access. Furthermore, the only
protection between multiple devices attempting to access the PMC registers
is when setting the powergate state to on or off. None of the other public
functions that access the PMC registers are protected.

Use the existing mutex to protect paths that may race with regard to
accessing the PMC registers.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 44 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 85b4e166273a..f8cdb7ce9755 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -235,7 +235,10 @@ int tegra_powergate_is_powered(int id)
 	if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
 		return -EINVAL;
 
+	mutex_lock(&pmc->powergates_lock);
 	status = tegra_pmc_readl(PWRGATE_STATUS) & (1 << id);
+	mutex_unlock(&pmc->powergates_lock);
+
 	return !!status;
 }
 
@@ -250,6 +253,8 @@ int tegra_powergate_remove_clamping(int id)
 	if (!pmc->soc || id < 0 || id >= pmc->soc->num_powergates)
 		return -EINVAL;
 
+	mutex_lock(&pmc->powergates_lock);
+
 	/*
 	 * On Tegra124 and later, the clamps for the GPU are controlled by a
 	 * separate register (with different semantics).
@@ -257,7 +262,7 @@ int tegra_powergate_remove_clamping(int id)
 	if (id == TEGRA_POWERGATE_3D) {
 		if (pmc->soc->has_gpu_clamps) {
 			tegra_pmc_writel(0, GPU_RG_CNTRL);
-			return 0;
+			goto out;
 		}
 	}
 
@@ -274,6 +279,9 @@ int tegra_powergate_remove_clamping(int id)
 
 	tegra_pmc_writel(mask, REMOVE_CLAMPING);
 
+out:
+	mutex_unlock(&pmc->powergates_lock);
+
 	return 0;
 }
 EXPORT_SYMBOL(tegra_powergate_remove_clamping);
@@ -520,9 +528,11 @@ int tegra_io_rail_power_on(int id)
 	unsigned int bit, mask;
 	int err;
 
+	mutex_lock(&pmc->powergates_lock);
+
 	err = tegra_io_rail_prepare(id, &request, &status, &bit);
 	if (err < 0)
-		return err;
+		goto error;
 
 	mask = 1 << bit;
 
@@ -535,12 +545,15 @@ int tegra_io_rail_power_on(int id)
 	err = tegra_io_rail_poll(status, mask, 0, 250);
 	if (err < 0) {
 		pr_info("tegra_io_rail_poll() failed: %d\n", err);
-		return err;
+		goto error;
 	}
 
 	tegra_io_rail_unprepare();
 
-	return 0;
+error:
+	mutex_unlock(&pmc->powergates_lock);
+
+	return err < 0 ? err : 0;
 }
 EXPORT_SYMBOL(tegra_io_rail_power_on);
 
@@ -550,10 +563,12 @@ int tegra_io_rail_power_off(int id)
 	unsigned int bit, mask;
 	int err;
 
+	mutex_lock(&pmc->powergates_lock);
+
 	err = tegra_io_rail_prepare(id, &request, &status, &bit);
 	if (err < 0) {
 		pr_info("tegra_io_rail_prepare() failed: %d\n", err);
-		return err;
+		goto error;
 	}
 
 	mask = 1 << bit;
@@ -566,11 +581,14 @@ int tegra_io_rail_power_off(int id)
 
 	err = tegra_io_rail_poll(status, mask, mask, 250);
 	if (err < 0)
-		return err;
+		goto error;
 
 	tegra_io_rail_unprepare();
 
-	return 0;
+error:
+	mutex_unlock(&pmc->powergates_lock);
+
+	return err < 0 ? err : 0;
 }
 EXPORT_SYMBOL(tegra_io_rail_power_off);
 
@@ -817,9 +835,15 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	/* take over the memory region from the early initialization */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	mutex_lock(&pmc->powergates_lock);
 	pmc->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(pmc->base))
-		return PTR_ERR(pmc->base);
+	mutex_unlock(&pmc->powergates_lock);
+
+	if (IS_ERR(pmc->base)) {
+		err = PTR_ERR(pmc->base);
+		goto error;
+	}
 
 	pmc->clk = devm_clk_get(&pdev->dev, "pclk");
 	if (IS_ERR(pmc->clk)) {
@@ -853,7 +877,9 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 	return 0;
 
 error:
+	mutex_lock(&pmc->powergates_lock);
 	pmc->base = base;
+	mutex_unlock(&pmc->powergates_lock);
 
 	return err;
 }
-- 
2.1.4

  reply	other threads:[~2016-01-28 16:33 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-28 16:33 [PATCH V5 00/14] Add generic PM domain support for Tegra Jon Hunter
2016-01-28 16:33 ` Jon Hunter
2016-01-28 16:33 ` Jon Hunter [this message]
2016-01-28 16:33   ` [PATCH V5 02/14] soc: tegra: pmc: Protect public functions from potential race conditions Jon Hunter
2016-01-29 16:20   ` Mathieu Poirier
2016-01-29 16:20     ` Mathieu Poirier
2016-02-01 13:42     ` Jon Hunter
2016-02-01 13:42       ` Jon Hunter
2016-01-28 16:33 ` [PATCH V5 03/14] soc: tegra: pmc: Change powergate and rail IDs to be an unsigned type Jon Hunter
2016-01-28 16:33   ` Jon Hunter
2016-01-28 16:33 ` [PATCH V5 04/14] soc: tegra: pmc: Fix testing of powergate state Jon Hunter
2016-01-28 16:33   ` Jon Hunter
2016-01-28 16:33 ` [PATCH V5 07/14] soc: tegra: pmc: Ensure partitions can be toggled on/off by PMC Jon Hunter
2016-01-28 16:33   ` Jon Hunter
2016-01-28 16:33 ` [PATCH V5 08/14] PM / Domains: Add function to remove a pm-domain Jon Hunter
2016-01-28 16:33   ` Jon Hunter
     [not found]   ` <1453998832-27383-9-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-02 15:35     ` Ulf Hansson
2016-02-02 15:35       ` Ulf Hansson
     [not found]       ` <CAPDyKFqJLdoee4a9819XukXTmYyd3pue452K_zbiV6XhfA=fTw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-03 10:51         ` Jon Hunter
2016-02-03 10:51           ` Jon Hunter
     [not found] ` <1453998832-27383-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-01-28 16:33   ` [PATCH V5 01/14] soc: tegra: pmc: Restore base address on probe failure Jon Hunter
2016-01-28 16:33     ` Jon Hunter
2016-01-28 16:33   ` [PATCH V5 05/14] soc: tegra: pmc: Wait for powergate state to change Jon Hunter
2016-01-28 16:33     ` Jon Hunter
2016-01-29 16:58     ` Mathieu Poirier
2016-01-29 16:58       ` Mathieu Poirier
     [not found]       ` <CANLsYkycbEo+wyMX8RJ9H-S5kDTjQR4nnDZc5gvf2kShOZAv9w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-01 13:44         ` Jon Hunter
2016-02-01 13:44           ` Jon Hunter
     [not found]           ` <56AF613A.1000909-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-03  9:20             ` Jon Hunter
2016-02-03  9:20               ` Jon Hunter
     [not found]               ` <56B1C647.4060504-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-03 15:58                 ` Mathieu Poirier
2016-02-03 15:58                   ` Mathieu Poirier
2016-01-28 16:33   ` [PATCH V5 06/14] soc: tegra: pmc: Fix checking of valid partitions Jon Hunter
2016-01-28 16:33     ` Jon Hunter
     [not found]     ` <1453998832-27383-7-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-01-29 17:08       ` Mathieu Poirier
2016-01-29 17:08         ` Mathieu Poirier
     [not found]         ` <CANLsYkxY5P2wQxGev0veN39nD-1cTVkZCVpX9jca7da39JJpWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-01 13:45           ` Jon Hunter
2016-02-01 13:45             ` Jon Hunter
2016-01-28 16:33   ` [PATCH V5 09/14] Documentation: DT: bindings: Update NVIDIA PMC for Tegra Jon Hunter
2016-01-28 16:33     ` Jon Hunter
2016-01-29 16:08     ` Rob Herring
2016-01-29 16:08       ` Rob Herring
2016-01-28 16:33   ` [PATCH V5 10/14] Documentation: DT: bindings: Add power domain info for NVIDIA PMC Jon Hunter
2016-01-28 16:33     ` Jon Hunter
2016-01-29 16:06     ` Rob Herring
2016-01-29 16:06       ` Rob Herring
2016-02-03 11:02       ` Jon Hunter
2016-02-03 11:02         ` Jon Hunter
     [not found]         ` <56B1DE40.7080403-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-03 15:48           ` Rob Herring
2016-02-03 15:48             ` Rob Herring
     [not found]             ` <CAL_JsqLcoKW2znNNvM=sYLmZ6O6ZWqn7+aXspkXoONw6-O1ygg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-10 10:57               ` Jon Hunter
2016-02-10 10:57                 ` Jon Hunter
     [not found]                 ` <56BB1787.4050801-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-10 14:06                   ` Rob Herring
2016-02-10 14:06                     ` Rob Herring
2016-01-28 16:33   ` [PATCH V5 11/14] soc: tegra: pmc: Add generic PM domain support Jon Hunter
2016-01-28 16:33     ` Jon Hunter
2016-02-04 15:44     ` Ulf Hansson
2016-02-04 15:44       ` Ulf Hansson
2016-02-10 18:01       ` Jon Hunter
2016-02-10 18:01         ` Jon Hunter
     [not found]         ` <56BB7AF4.8040708-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-10 18:25           ` Ulf Hansson
2016-02-10 18:25             ` Ulf Hansson
     [not found]             ` <CAPDyKFrZ6tWBsQC0tyWWeChiZja3h_zcbaiX25ak-Zyp4MzqVw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-11  9:13               ` Jon Hunter
2016-02-11  9:13                 ` Jon Hunter
2016-02-11  9:57                 ` Ulf Hansson
2016-02-11  9:57                   ` Ulf Hansson
     [not found]                   ` <CAPDyKFrdmufsMqNL0U7q5gPEUqsg3SrkrNChcziQjEOjvd30Ng-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-11 10:13                     ` Jon Hunter
2016-02-11 10:13                       ` Jon Hunter
     [not found]                       ` <56BC5EE0.2040804-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-11 10:26                         ` Jon Hunter
2016-02-11 10:26                           ` Jon Hunter
2016-02-11 10:37                           ` Ulf Hansson
2016-02-11 10:37                             ` Ulf Hansson
2016-02-11 10:52                             ` Jon Hunter
2016-02-11 10:52                               ` Jon Hunter
2016-02-11 10:28                       ` Ulf Hansson
2016-02-11 10:28                         ` Ulf Hansson
     [not found]                         ` <CAPDyKFq_0t4tcvkgMBW8p8ubJDALWMjdhgGM+_Z6auRxEkSPdA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-11 16:38                           ` Jon Hunter
2016-02-11 16:38                             ` Jon Hunter
     [not found]                             ` <56BCB90C.8000302-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-18 15:06                               ` Ulf Hansson
2016-02-18 15:06                                 ` Ulf Hansson
2016-02-12 23:14       ` Kevin Hilman
2016-02-12 23:14         ` Kevin Hilman
     [not found]         ` <7hh9hdzflv.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-02-15 11:27           ` Jon Hunter
2016-02-15 11:27             ` Jon Hunter
     [not found]             ` <56C1B62B.5060708-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-02-18 16:00               ` Ulf Hansson
2016-02-18 16:00                 ` Ulf Hansson
     [not found]                 ` <CAPDyKFoPrFoMOFxC37zXX4L3VdLKknaw_LUTw7ycr9mfa_=7_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-18 16:31                   ` Jon Hunter
2016-02-18 16:31                     ` Jon Hunter
2016-02-24  0:03                     ` Kevin Hilman
2016-02-24  0:03                       ` Kevin Hilman
2016-01-28 16:33   ` [PATCH V5 12/14] clk: tegra210: Add the APB2APE audio clock Jon Hunter
2016-01-28 16:33     ` Jon Hunter
2016-02-02 14:37     ` Thierry Reding
2016-02-02 14:37       ` Thierry Reding
2016-01-28 16:33   ` [PATCH V5 13/14] ARM64: tegra: Add audio PM domain device node for Tegra210 Jon Hunter
2016-01-28 16:33     ` Jon Hunter
2016-01-28 16:33 ` [PATCH V5 14/14] ARM64: tegra: select PM_GENERIC_DOMAINS Jon Hunter
2016-01-28 16:33   ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1453998832-27383-3-git-send-email-jonathanh@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=gnurou@gmail.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=khilman@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@kernel.org \
    --cc=swarren@wwwdotorg.org \
    --cc=thierry.reding@gmail.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.