[PATCH 1/4] RISC-V: Create unique identification for SoC PMU

[Nikita Shubin]

Hello Mário.

On Tue,  9 Nov 2021 10:25:52 +0000
João Mário Domingos <joao.mario at tecnico.ulisboa.pt> wrote:

> 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.
> 

This patch doesn't compile and also has a warning:
```
drivers/perf/riscv_pmu_sbi.c:239:43: error: 'pmu_sbi_id_show'
undeclared here (not in a function)
  239 | static DEVICE_ATTR(id, S_IRUGO | S_IWUSR, pmu_sbi_id_show, 0);
      |                                           ^~~~~~~~~~~~~~~
include/linux/sysfs.h:104:19: note: in definition of macro '__ATTR'
  104 |         .show   = _show,
        \
      |                   ^~~~~
drivers/perf/riscv_pmu_sbi.c:239:8: note: in expansion of macro
'DEVICE_ATTR'
  239 | static DEVICE_ATTR(id, S_IRUGO | S_IWUSR, pmu_sbi_id_show, 0);
      |        ^~~~~~~~~~~
drivers/perf/riscv_pmu_sbi.c: In function 'pmu_sbi_id_show':
drivers/perf/riscv_pmu_sbi.c:675:29: warning: format '%lx' expects
argument of type 'long unsigned int', 
but argument 3 has type 'uint64_t' {aka 'long long unsigned int'}
[-Wformat=]
  675 |     len = sprintf(buf, "0x%lx\n", pmu_sbi_get_pmu_id());
      |                           ~~^     ~~~~~~~~~~~~~~~~~~~~
      |                             |     |
      |                             |     uint64_t {aka long long
unsigned int}
      |                             long unsigned int
      |                           %llx
```

May be you wanted to place DEVICE_ATTR after pmu_sbi_id_show function
declaration ?

Please check with a clean build.

Yours,
Nikita Shubin

> [1] https://github.com/riscv/riscv-isa-manual
> 
> Signed-off-by: João Mário Domingos <joao.mario at tecnico.ulisboa.pt>
> ---
>  arch/riscv/kernel/sbi.c      |  3 +++
>  drivers/perf/riscv_pmu_sbi.c | 46
> ++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+)
> 
> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
> index 7402a417f38e..4e4f5671b864 100644
> --- a/arch/riscv/kernel/sbi.c
> +++ b/arch/riscv/kernel/sbi.c
> @@ -551,16 +551,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 7a68dfa89f6f..f913d8ddfe73 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -236,6 +236,15 @@ static const struct pmu_event_data
> pmu_cache_event_map[PERF_COUNT_HW_CACHE_MAX] },
>  };
>  
> +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_ctr_get_width(int idx)
>  {
>  	return pmu_ctr_list[idx].width;
> @@ -642,6 +651,36 @@ 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 );
> +	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%lx\n", pmu_sbi_get_pmu_id());
> +    if (len <= 0)
> +        dev_err(dev, "mydrv: Invalid sprintf len: %dn", len);
> +
> +    return len;
> +}
> +
>  static int pmu_sbi_device_probe(struct platform_device *pdev)
>  {
>  	struct riscv_pmu *pmu = NULL;
> @@ -680,6 +719,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;