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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56d56f85-6172-4fb1-a002-fe82ffc8a66a@arm.com>
Date: Thu, 15 Feb 2024 10:55:32 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Anshuman Khandual <anshuman.khandual@....com>,
 linux-arm-kernel@...ts.infradead.org
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Sudeep Holla <sudeep.holla@....com>, Mike Leach <mike.leach@...aro.org>,
 James Clark <james.clark@....com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>, linux-acpi@...r.kernel.org,
 linux-kernel@...r.kernel.org, coresight@...ts.linaro.org,
 linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH V4 02/11] coresight: stm: Extract device name from AMBA
 pid based table lookup

On 23/01/2024 05:45, Anshuman Khandual wrote:
> Instead of using AMBA private data field, extract the device name from AMBA
> pid based table lookup using new coresight_get_uci_data_from_amba() helper.
> 
> Cc: Suzuki K Poulose <suzuki.poulose@....com>
> Cc: Mike Leach <mike.leach@...aro.org>
> Cc: James Clark <james.clark@....com>
> Cc: coresight@...ts.linaro.org
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-stm32@...md-mailman.stormreply.com
> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
> ---
>   drivers/hwtracing/coresight/coresight-priv.h | 10 ++++++++++
>   drivers/hwtracing/coresight/coresight-stm.c  | 14 +++++++++++++-
>   2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 767076e07970..68cbb036cec8 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -221,6 +221,16 @@ static inline void *coresight_get_uci_data(const struct amba_id *id)
>   	return uci_id->data;
>   }
>   
> +static inline void *coresight_get_uci_data_from_amba(const struct amba_id *table, u32 pid)
> +{
> +	while (table->mask) {
> +		if ((table->id & table->mask) == pid)

Why are we masking table->id ? table->id is a static value that the
driver wants to check for "variants" of a given device. The table->mask
is there to filter out the "irrelevant" bits of the PID that we read
from the device. So this should instead be:

		if ((table->mask & pid) == table->id)

> +			return coresight_get_uci_data(table);
> +		table++;
> +	};
> +	return NULL;
> +}
> +
>   void coresight_release_platform_data(struct coresight_device *csdev,
>   				     struct device *dev,
>   				     struct coresight_platform_data *pdata);
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index a1c27c901ad1..9cdca4f86cab 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -804,6 +804,18 @@ static void stm_init_generic_data(struct stm_drvdata *drvdata,
>   	drvdata->stm.set_options = stm_generic_set_options;
>   }
>   
> +#define STM_AMBA_MASK 0xfffff
> +
> +static const struct amba_id stm_ids[];
> +
> +static char *stm_csdev_name(struct coresight_device *csdev)
> +{
> +	u32 stm_pid = coresight_get_pid(&csdev->access) & STM_AMBA_MASK;

Similar to above:

Why do we apply a "custom" mask to the PID and later check the PID with
that of the table->pid.

The way it is supposed work is :

	(table->mask & dev_pid) == table->pid

the table->mask is there for a reason: i.e., to get the relevant bits 
from the device_pid and compare it against "the" expected value 
(table->pid).


Suzuki

> +	void *uci_data = coresight_get_uci_data_from_amba(stm_ids, stm_pid);
> +
> +	return uci_data ? (char *)uci_data : "STM";
> +}
> +
>   static int stm_probe(struct amba_device *adev, const struct amba_id *id)
>   {
>   	int ret, trace_id;
> @@ -900,7 +912,7 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id)
>   	pm_runtime_put(&adev->dev);
>   
>   	dev_info(&drvdata->csdev->dev, "%s initialized\n",
> -		 (char *)coresight_get_uci_data(id));
> +		 stm_csdev_name(drvdata->csdev));
>   	return 0;
>   
>   cs_unregister:


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ