Date:   Tue, 14 Jun 2022 15:46:00 +0530
From:   Sunil V L <sunilvl@...tanamicro.com>
To:     Nikita Shubin <nikita.shubin@...uefel.me>
Cc:     Genevieve Chan <genevieve.chan@...rfivetech.com>,
        João Mário Domingos 
        <joao.mario@...nico.ulisboa.pt>,
        Nikita Shubin <n.shubin@...ro.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Atish Patra <atishp@...shpatra.org>,
        Anup Patel <anup@...infault.org>,
        Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        "open list:RISC-V ARCHITECTURE" <linux-riscv@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM PMU PROFILING AND DEBUGGING" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 1/4] RISC-V: Create unique identification for SoC PMU

Hi Nikita,

On Tue, Jun 07, 2022 at 04:16:44PM +0300, Nikita Shubin wrote:
> From: João Mário Domingos <joao.mario@...nico.ulisboa.pt>
> 
> The SBI PMU platform driver did not provide any identification for
> perf events matching. This patch introduces a new sysfs file inside the
> platform device (soc:pmu/id) for pmu identification.
> 
> The identification is a 64-bit value generated as:
> [63-32]: mvendorid;
> [31]: marchid[MSB];
> [30-16]: marchid[15-0];
> [15-0]: mimpid[15MSBs];
> 
> The CSRs are detailed in the RISC-V privileged spec [1].
> The marchid is split in MSB + 15LSBs, due to the MSB being used for
> open-source architecture identification.
> 
> [1] https://github.com/riscv/riscv-isa-manual
> 
> Signed-off-by: João Mário Domingos <joao.mario@...nico.ulisboa.pt>
> Tested-by: Nikita Shubin <n.shubin@...ro.com>
> ---
>  arch/riscv/kernel/sbi.c      |  3 +++
>  drivers/perf/riscv_pmu_sbi.c | 47 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
> index 775d3322b422..50dd9b6ecc9e 100644
> --- a/arch/riscv/kernel/sbi.c
> +++ b/arch/riscv/kernel/sbi.c
> @@ -627,16 +627,19 @@ long sbi_get_mvendorid(void)
>  {
>  	return __sbi_base_ecall(SBI_EXT_BASE_GET_MVENDORID);
>  }
> +EXPORT_SYMBOL(sbi_get_mvendorid);
>  
>  long sbi_get_marchid(void)
>  {
>  	return __sbi_base_ecall(SBI_EXT_BASE_GET_MARCHID);
>  }
> +EXPORT_SYMBOL(sbi_get_marchid);
>  
>  long sbi_get_mimpid(void)
>  {
>  	return __sbi_base_ecall(SBI_EXT_BASE_GET_MIMPID);
>  }
> +EXPORT_SYMBOL(sbi_get_mimpid);
>  
>  static void sbi_send_cpumask_ipi(const struct cpumask *target)
>  {
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index a1317a483512..15ab3dc68e7a 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -693,6 +693,46 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
>  	return 0;
>  }
>  
> +static uint64_t pmu_sbi_get_pmu_id(void)
> +{
> +	union sbi_pmu_id {
> +		uint64_t value;
> +		struct {
> +			uint16_t imp:16;
> +			uint16_t arch:16;
> +			uint32_t vendor:32;
> +		};
> +	} pmuid;
> +
> +	pmuid.value = 0;
> +	pmuid.vendor = (uint32_t) sbi_get_mvendorid();
> +	pmuid.arch = (sbi_get_marchid() >> (63 - 15) & (1 << 15)) | (sbi_get_marchid() & 0x7FFF);

This statement generates below warning in rv32 build.

drivers/perf/riscv_pmu_sbi.c: In function 'pmu_sbi_get_pmu_id':
 drivers/perf/riscv_pmu_sbi.c:715:41: warning: right shift count >=
 width of type [-Wshift-count-overflow]
  715 |        pmuid.arch = (sbi_get_marchid() >> (63 - 15) & (1 << 15))
  | (sbi_get_marchid() & 0x7FFF);
     

Please take care of this when you rework this patch.

Thanks
Sunil

> +	pmuid.imp = (sbi_get_mimpid() >> 16);
> +
> +	return pmuid.value;
> +}
> +
> +static ssize_t pmu_sbi_id_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	int len;
> +
> +	len = sprintf(buf, "0x%llx\n", pmu_sbi_get_pmu_id());
> +	if (len <= 0)
> +		dev_err(dev, "mydrv: Invalid sprintf len: %dn", len);
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR(id, S_IRUGO | S_IWUSR, pmu_sbi_id_show, 0);
> +
> +static struct attribute *pmu_sbi_attrs[] = {
> +	&dev_attr_id.attr,
> +	NULL
> +};
> +
> +ATTRIBUTE_GROUPS(pmu_sbi);
> +
>  static int pmu_sbi_device_probe(struct platform_device *pdev)
>  {
>  	struct riscv_pmu *pmu = NULL;
> @@ -729,6 +769,13 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
>  	pmu->ctr_clear_idx = pmu_sbi_ctr_clear_idx;
>  	pmu->ctr_read = pmu_sbi_ctr_read;
>  
> +	ret = sysfs_create_group(&pdev->dev.kobj, &pmu_sbi_group);
> +	if (ret) {
> +		dev_err(&pdev->dev, "sysfs creation failed\n");
> +		return ret;
> +	}
> +	pdev->dev.groups = pmu_sbi_groups;
> +
>  	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_RISCV_STARTING, &pmu->node);
>  	if (ret)
>  		return ret;
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists