| 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: <20250313160409.GS9682@e132581.arm.com>
Date: Thu, 13 Mar 2025 16:04:09 +0000
From: Leo Yan <leo.yan@....com>
To: James Clark <james.clark@...aro.org>
Cc: lcherian@...vell.com, coresight@...ts.linaro.org,
Mike Leach <mike.leach@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH 5/7] coresight: Clear self hosted claim tag on probe
On Tue, Feb 11, 2025 at 10:39:41AM +0000, James Clark wrote:
>
> This can be left behind from a crashed kernel after a kexec so clear it
> when probing each device. Similarly to
> coresight_disclaim_device_unlocked(), only clear it if it's already set
> to avoid races with an external debugger.
>
> We need a csdev_access struct in etm_init_arch_data() so just replace
> the iomem pointer with a full csdev_access struct. This means all usages
> need to be updated to go through csa->base.
>
> Signed-off-by: James Clark <james.clark@...aro.org>
> ---
> drivers/hwtracing/coresight/coresight-catu.c | 1 +
> drivers/hwtracing/coresight/coresight-core.c | 48 +++++++++++++++----
> .../hwtracing/coresight/coresight-cti-core.c | 2 +
> drivers/hwtracing/coresight/coresight-etb10.c | 2 +
> drivers/hwtracing/coresight/coresight-etm.h | 6 +--
> .../coresight/coresight-etm3x-core.c | 28 +++++------
> .../coresight/coresight-etm3x-sysfs.c | 8 ++--
> .../coresight/coresight-etm4x-core.c | 2 +
> .../hwtracing/coresight/coresight-funnel.c | 2 +
> .../coresight/coresight-replicator.c | 1 +
> .../hwtracing/coresight/coresight-tmc-core.c | 1 +
> include/linux/coresight.h | 3 ++
> 12 files changed, 73 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index d9259c0b6e64..575c2d247a90 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -558,6 +558,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
> catu_desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
> catu_desc.ops = &catu_ops;
>
> + coresight_reset_claim(&catu_desc.access);
> drvdata->csdev = coresight_register(&catu_desc);
> if (IS_ERR(drvdata->csdev))
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 7fe5d5d432c4..97f33ffad05e 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -212,20 +212,48 @@ int coresight_claim_device(struct coresight_device *csdev)
> EXPORT_SYMBOL_GPL(coresight_claim_device);
>
> /*
> - * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
> + * Clear the claim tag for the device.
> + * Returns an error if the device wasn't already claimed.
> + */
> +int coresight_reset_claim(struct csdev_access *csa)
> +{
> + int ret;
> +
> + CS_UNLOCK(csa->base);
> + ret = coresight_reset_claim_unlocked(csa);
> + CS_LOCK(csa->base);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(coresight_reset_claim);
Maybe my question is overlapping with Mike's comment.
Callers never check the return values from coresight_reset_claim(). I am
wandering if coresight_reset_claim() can directly call
coresight_clear_self_claim_tag() for _trying_ to clear self-host tag in
probe phase. Any self claim tag issues will be deferred to detect until
enable the component.
For consistent, we might rename coresight_reset_claim() to
coresight_reset_self_claim_tag(), which acquires CS lock and clear
self claim tag.
> +/*
> + * Clear the claim tag for the device. Called with CS_UNLOCKed for the component.
> + * Returns an error if the device wasn't already claimed.
> + */
> +int coresight_reset_claim_unlocked(struct csdev_access *csa)
> +{
> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED) {
> + coresight_clear_self_claim_tag(csa);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_reset_claim_unlocked);
> +
> +/*
> + * coresight_disclaim_device_unlocked : Clear the claim tag for the device
> + * and warn if the device wasn't already claimed.
> * Called with CS_UNLOCKed for the component.
> */
> void coresight_disclaim_device_unlocked(struct csdev_access *csa)
> {
> - if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
> - coresight_clear_self_claim_tag(csa);
> - else
> - /*
> - * The external agent may have not honoured our claim
> - * and has manipulated it. Or something else has seriously
> - * gone wrong in our driver.
> - */
> - WARN_ON_ONCE(1);
> + /*
> + * Warn if the external agent hasn't honoured our claim
> + * and has manipulated it. Or something else has seriously
> + * gone wrong in our driver.
> + */
> + WARN_ON_ONCE(coresight_reset_claim_unlocked(csa));
> }
> EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 073f67a41af9..389a72362f0c 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -931,6 +931,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
> cti_desc.ops = &cti_ops;
> cti_desc.groups = drvdata->ctidev.con_groups;
> cti_desc.dev = dev;
> +
> + coresight_reset_claim(&cti_desc.access);
> drvdata->csdev = coresight_register(&cti_desc);
> if (IS_ERR(drvdata->csdev)) {
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index d8bc3e776c88..b598b2c0c9bb 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -772,6 +772,8 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
> desc.pdata = pdata;
> desc.dev = dev;
> desc.groups = coresight_etb_groups;
> +
> + coresight_reset_claim(&desc.access);
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev))
> return PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
> index e02c3ea972c9..a89736309c27 100644
> --- a/drivers/hwtracing/coresight/coresight-etm.h
> +++ b/drivers/hwtracing/coresight/coresight-etm.h
> @@ -229,7 +229,7 @@ struct etm_config {
> * @config: structure holding configuration parameters.
> */
> struct etm_drvdata {
> - void __iomem *base;
> + struct csdev_access csa;
I would like to extract the change for using `csdev_access` in the
ETMv3 driver into a new patch, which is irrelevant to reset self claim
tags and would significantly reduce the complexity in this patch.
Thanks,
Leo
Powered by blists - more mailing lists