Messages in this thread | | | Date | Wed, 8 Jun 2022 11:47:12 +0300 | From | Nikita Shubin <> | Subject | Re: [PATCH v3 1/4] RISC-V: Create unique identification for SoC PMU |
| |
Hello Atish!
On Tue, 7 Jun 2022 09:37:19 -0700 Atish Patra <atishp@atishpatra.org> wrote:
> On Tue, Jun 7, 2022 at 6:17 AM Nikita Shubin > <nikita.shubin@maquefel.me> wrote: > > > > From: João Mário Domingos <joao.mario@tecnico.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]; > > > > This is not entirely correct as marchid or mimpid can be MXLEN. The > encoding scheme is left upto the > vendor. We can not assume anything about it. > > The purpose of the PMU ID is to distinguish between different > vendors/generations. The perf tool expects > a json string. > I think you can just keep all these 3 registers into the JSON string > as it is to avoid any pitfalls with vendor weirdness.
This make sense to me. I'll rework this patch according your suggestions.
Yours, Nikita Shubin.
> > > 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@tecnico.ulisboa.pt> > > Tested-by: Nikita Shubin <n.shubin@yadro.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); > > + 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 > > > > > -- > Regards, > Atish
| |