| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJ9a7ViT6vTdq8BMt0nv1CK1FNDOAF8r2Ca8HPWuKeeEXRSRVg@mail.gmail.com>
Date: Thu, 12 Jan 2023 14:43:19 +0000
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@....com>
Cc: coresight@...ts.linaro.org, quic_jinlmao@...cinc.com,
suzuki.poulose@....com,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Leo Yan <leo.yan@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] coresight: cti: Remove atomic type from enable_req_count
On Tue, 10 Jan 2023 at 11:08, James Clark <james.clark@....com> wrote:
>
> enable_req_count is only ever accessed inside the spinlock, so to avoid
> confusion that there are concurrent accesses and simplify the code,
> change it to an int.
>
> One access outside of the spinlock is in enable_show() which appears to
> allow partially written data to be displayed between enable_req_count,
> powered and enabled so move this one inside the spin lock too.
>
> Signed-off-by: James Clark <james.clark@....com>
> ---
> drivers/hwtracing/coresight/coresight-cti-core.c | 14 +++++++-------
> drivers/hwtracing/coresight/coresight-cti-sysfs.c | 2 +-
> drivers/hwtracing/coresight/coresight-cti.h | 2 +-
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 838872f2484d..277c890a1f1f 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -107,12 +107,12 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
> cti_write_all_hw_regs(drvdata);
>
> config->hw_enabled = true;
> - atomic_inc(&drvdata->config.enable_req_count);
> + drvdata->config.enable_req_count++;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> return rc;
>
> cti_state_unchanged:
> - atomic_inc(&drvdata->config.enable_req_count);
> + drvdata->config.enable_req_count++;
>
> /* cannot enable due to error */
> cti_err_not_enabled:
> @@ -129,7 +129,7 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata)
> config->hw_powered = true;
>
> /* no need to do anything if no enable request */
> - if (!atomic_read(&drvdata->config.enable_req_count))
> + if (!drvdata->config.enable_req_count)
> goto cti_hp_not_enabled;
>
> /* try to claim the device */
> @@ -156,13 +156,13 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
> spin_lock(&drvdata->spinlock);
>
> /* don't allow negative refcounts, return an error */
> - if (!atomic_read(&drvdata->config.enable_req_count)) {
> + if (!drvdata->config.enable_req_count) {
> ret = -EINVAL;
> goto cti_not_disabled;
> }
>
> /* check refcount - disable on 0 */
> - if (atomic_dec_return(&drvdata->config.enable_req_count) > 0)
> + if (--drvdata->config.enable_req_count > 0)
> goto cti_not_disabled;
>
> /* no need to do anything if disabled or cpu unpowered */
> @@ -239,7 +239,7 @@ static void cti_set_default_config(struct device *dev,
> /* Most regs default to 0 as zalloc'ed except...*/
> config->trig_filter_enable = true;
> config->ctigate = GENMASK(config->nr_ctm_channels - 1, 0);
> - atomic_set(&config->enable_req_count, 0);
> + config->enable_req_count = 0;
> }
>
> /*
> @@ -696,7 +696,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
> drvdata->config.hw_enabled = false;
>
> /* check enable reference count to enable HW */
> - if (atomic_read(&drvdata->config.enable_req_count)) {
> + if (drvdata->config.enable_req_count) {
> /* check we can claim the device as we re-power */
> if (coresight_claim_device(csdev))
> goto cti_notify_exit;
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index 71e7a8266bb3..e528cff9d4e2 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -84,8 +84,8 @@ static ssize_t enable_show(struct device *dev,
> bool enabled, powered;
> struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>
> - enable_req = atomic_read(&drvdata->config.enable_req_count);
> spin_lock(&drvdata->spinlock);
> + enable_req = drvdata->config.enable_req_count;
> powered = drvdata->config.hw_powered;
> enabled = drvdata->config.hw_enabled;
> spin_unlock(&drvdata->spinlock);
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
> index acf7b545e6b9..8b106b13a244 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -141,7 +141,7 @@ struct cti_config {
> int nr_trig_max;
>
> /* cti enable control */
> - atomic_t enable_req_count;
> + int enable_req_count;
> bool hw_enabled;
> bool hw_powered;
>
> --
> 2.25.1
>
Reviewed-by: Mike Leach <mike.leach@...aro.org>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists