Re: [PATCH RESEND] perf/marvell_cn10k: Add MPAM support for TAD PMU

[Robin Murphy]

On 2022-06-24 13:14, Will Deacon wrote:
> On Sat, May 28, 2022 at 12:26:47AM +0530, Tanmay Jagdale wrote:
> > The TAD PMU supports following counters that can be filtered by MPAM
> > partition id.
> >      - (0x1a) tad_alloc_dtg : Allocations to DTG.
> >      - (0x1b) tad_alloc_ltg : Allocations to LTG.
> >      - (0x1c) tad_alloc_any : Total allocations to DTG/LTG.
> >      - (0x1d) tad_hit_dtg   : DTG hits.
> >      - (0x1e) tad_hit_ltg   : LTG hits.
> >      - (0x1f) tad_hit_any   : Hit in LTG/DTG.
> >      - (0x20) tad_tag_rd    : Total tag reads.
> >
> > Add a new 'partid' attribute of 16-bits to get the partition id
> > passed from perf tool. This value would be stored in config1 field
> > of perf_event_attr structure.
> >
> > Example:
> > perf stat -e tad/tad_alloc_any,partid=0x12/ <program>
> >
> > - Drop read of TAD_PRF since we don't have to preserve any
> >    bit fields and always write an updated value.
> > - Update register offsets of TAD_PRF and TAD_PFC.
>
> It would be great if you could document some of this under
> Documentation/admin-guide/perf like many of the other PMU drivers have
> done.

Especially documenting how the user obtains the required partid value to 
pass.

Thanks,
Robin.

> > Signed-off-by: Tanmay Jagdale <tanmay@xxxxxxxxxxx>
> > ---
> >   drivers/perf/marvell_cn10k_tad_pmu.c | 23 ++++++++++++++++++-----
> >   1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/perf/marvell_cn10k_tad_pmu.c b/drivers/perf/marvell_cn10k_tad_pmu.c
> > index 282d3a071a67..f552e6bffcac 100644
> > --- a/drivers/perf/marvell_cn10k_tad_pmu.c
> > +++ b/drivers/perf/marvell_cn10k_tad_pmu.c
> > @@ -18,10 +18,12 @@
> >   #include <linux/perf_event.h>
> >   #include <linux/platform_device.h>
> >   
> > -#define TAD_PFC_OFFSET		0x0
> > +#define TAD_PFC_OFFSET		0x800
> >   #define TAD_PFC(counter)	(TAD_PFC_OFFSET | (counter << 3))
> > -#define TAD_PRF_OFFSET		0x100
> > +#define TAD_PRF_OFFSET		0x900
> >   #define TAD_PRF(counter)	(TAD_PRF_OFFSET | (counter << 3))
> > +#define TAD_PRF_MATCH_PARTID	(1 << 8)
> > +#define TAD_PRF_PARTID_NS	(1 << 10)
> >   #define TAD_PRF_CNTSEL_MASK	0xFF
> >   #define TAD_MAX_COUNTERS	8
> >   
> > @@ -86,23 +88,32 @@ static void tad_pmu_event_counter_start(struct perf_event *event, int flags)
> >   	struct hw_perf_event *hwc = &event->hw;
> >   	u32 event_idx = event->attr.config;
> >   	u32 counter_idx = hwc->idx;
> > +	u32 partid_filter = 0;
> >   	u64 reg_val;
> > +	u32 partid;
> >   	int i;
> >   
> >   	hwc->state = 0;
> >   
> > +	/* Extract the partid (if any) passed by user */
> > +	partid = event->attr.config1 & 0x3f;
>
> [...]
>
> >   PMU_FORMAT_ATTR(event, "config:0-7");
> > +PMU_FORMAT_ATTR(partid, "config1:0-15");
>
> This doesn't seem to match the mask used above?
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel