Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c981692b-af7b-453d-39af-402221e174f5@arm.com>
Date:   Fri, 24 Jun 2022 19:09:08 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH RESEND] perf/marvell_cn10k: Add MPAM support for TAD PMU
Content-Language: en-GB
To:     Will Deacon <will@kernel.org>, Tanmay Jagdale <tanmay@marvell.com>
Cc:     mark.rutland@arm.com, robh@kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        sgoutham@marvell.com, lcherian@marvell.com, bbhushan2@marvell.com,
        amitsinght@marvell.com, james.morse@arm.com
References: <20220527185647.3138084-1-tanmay@marvell.com>
 <20220624121435.GA18561@willie-the-truck>
From:   Robin Murphy <robin.murphy@arm.com>
In-Reply-To: <20220624121435.GA18561@willie-the-truck>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@marvell.com>
>> ---
>>   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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel