[PATCH] powerpc/perf/hv-gpci: Fix the logic to compute counter value from the hcall result buffer.

[PATCH] powerpc/perf/hv-gpci: Fix the logic to compute counter value from the hcall result buffer.

[Kajol Jain]

H_GetPerformanceCounterInfo (0xF080) hcall returns the counter data in the
result buffer. Result buffer has specific format defined in the PAPR
specification. One of the field is counter offset and width of the counter
data returned.

Counter data are returned in a unsigned char array. To
get the final counter data, these values should be left shifted
byte at a time. But commit 220a0c609ad17 ("powerpc/perf: Add support 
for the hv gpci (get performance counter info) interface") made the
shifting bitwise. Because of this, hcall counters values could end up
in lower side, which messes the counter prev vs now calculation. This
lead to huge counter value reporting

[command]#: perf stat -e hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
           -C 0 -I 1000
        time             counts unit events
     1.000078854 18,446,744,073,709,535,232      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     2.000213293                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     3.000320107                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     4.000428392                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     5.000537864                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     6.000649087                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     7.000760312                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     8.000865218             16,448      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
     9.000978985 18,446,744,073,709,535,232      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
    10.001088891             16,384      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
    11.001201435                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
    12.001307937 18,446,744,073,709,535,232      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/

Patch here fixes the shifting logic to make is byte-wise with which no more the issue seen. 

Fixes: e4f226b1580b3 ("powerpc/perf/hv-gpci: Increase request buffer size")
Reported-by: Nageswara R Sastry<rnsastry@linux.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
 arch/powerpc/perf/hv-gpci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index d48413e28c39..c756228a081f 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -175,7 +175,7 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index,
 	 */
 	count = 0;
 	for (i = offset; i < offset + length; i++)
-		count |= arg->bytes[i] << (i - offset);
+		count |= (u64)(arg->bytes[i]) << ((length - 1 - (i - offset)) * 8);
 
 	*value = count;
 out:
-- 
2.26.2

Re: [PATCH] powerpc/perf/hv-gpci: Fix the logic to compute counter value from the hcall result buffer.

[Nageswara Sastry]

On 13/08/21 1:51 pm, Kajol Jain wrote:
> H_GetPerformanceCounterInfo (0xF080) hcall returns the counter data in the
> result buffer. Result buffer has specific format defined in the PAPR
> specification. One of the field is counter offset and width of the counter
> data returned.
> 
> Counter data are returned in a unsigned char array. To
> get the final counter data, these values should be left shifted
> byte at a time. But commit 220a0c609ad17 ("powerpc/perf: Add support
> for the hv gpci (get performance counter info) interface") made the
> shifting bitwise. Because of this, hcall counters values could end up
> in lower side, which messes the counter prev vs now calculation. This
> lead to huge counter value reporting
> 
> [command]#: perf stat -e hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>             -C 0 -I 1000
>          time             counts unit events
>       1.000078854 18,446,744,073,709,535,232      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       2.000213293                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       3.000320107                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       4.000428392                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       5.000537864                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       6.000649087                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       7.000760312                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       8.000865218             16,448      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>       9.000978985 18,446,744,073,709,535,232      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>      10.001088891             16,384      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>      11.001201435                  0      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
>      12.001307937 18,446,744,073,709,535,232      hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
> 
> Patch here fixes the shifting logic to make is byte-wise with which no more the issue seen.
> 
> Fixes: e4f226b1580b3 ("powerpc/perf/hv-gpci: Increase request buffer size")
> Reported-by: Nageswara R Sastry<rnsastry@linux.ibm.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>

Tested-by: Nageswara R Sastry<rnsastry@linux.ibm.com>

Now not seeing huge numbers.

# perf stat -e 
hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/ -C 0 -I 1000
#           time             counts unit events
      1.001023931             26,624 
hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
      2.002176767                  0 
hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
      3.003296382                  0 
hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/
      4.004385311             33,280 
hv_gpci/system_tlbie_count_and_time_tlbie_instructions_issued/


> ---
>   arch/powerpc/perf/hv-gpci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
> index d48413e28c39..c756228a081f 100644
> --- a/arch/powerpc/perf/hv-gpci.c
> +++ b/arch/powerpc/perf/hv-gpci.c
> @@ -175,7 +175,7 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index,
>   	 */
>   	count = 0;
>   	for (i = offset; i < offset + length; i++)
> -		count |= arg->bytes[i] << (i - offset);
> +		count |= (u64)(arg->bytes[i]) << ((length - 1 - (i - offset)) * 8);
>   
>   	*value = count;
>   out:
> 

-- 
Thanks and Regards
R.Nageswara Sastry

Re: [PATCH] powerpc/perf/hv-gpci: Fix the logic to compute counter value from the hcall result buffer.

[Michael Ellerman]

On Fri, 13 Aug 2021 13:51:58 +0530, Kajol Jain wrote:
> H_GetPerformanceCounterInfo (0xF080) hcall returns the counter data in the
> result buffer. Result buffer has specific format defined in the PAPR
> specification. One of the field is counter offset and width of the counter
> data returned.
> 
> Counter data are returned in a unsigned char array. To
> get the final counter data, these values should be left shifted
> byte at a time. But commit 220a0c609ad17 ("powerpc/perf: Add support
> for the hv gpci (get performance counter info) interface") made the
> shifting bitwise. Because of this, hcall counters values could end up
> in lower side, which messes the counter prev vs now calculation. This
> lead to huge counter value reporting
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/perf/hv-gpci: Fix the logic to compute counter value from the hcall result buffer.
      https://git.kernel.org/powerpc/c/f9addd85fbfacf0d155e83dbee8696d6df5ed0c7

cheers