Re: [Qemu-devel] [PATCH v3 00/22] More fully implement ARM PMUv3

2018-04-12 Thread Aaron Lindsay 
On Apr 12 18:32, Peter Maydell wrote:
> On 16 March 2018 at 20:30, Aaron Lindsay  wrote:
> > The ARM PMU implementation currently contains a basic cycle counter, but it 
> > is
> > often useful to gather counts of other events and filter them based on
> > execution mode. These patches flesh out the implementations of various PMU
> > registers including PM[X]EVCNTR and PM[X]EVTYPER, add a struct definition to
> > represent arbitrary counter types, implement mode filtering, and add
> > instruction, cycle, and software increment events.
> 
> Hi; sorry it's taken me a while to get to this (I've been focusing
> on for-2.12 work). I've now reviewed most of the patches in this
> set. My suggestion is that you fix the stuff I've commented on
> and send out a v4, and then I can take some of the patches from
> the start of that into target-arm.next, and I'll review the tail
> end of the set then.

Thanks for the review! I'll see if I can get a v4 out late this week or
early next.

I'll make a note of this when I send it out, but one thing to look out
for in the next version is that I had to reorganize a few things to make
the interrupt-on-overflow functionality work better. I think the changes
are fairly minor and limited to the later patches, with the possible
exception of using two variables per counter (because you have to know
what the previous counter value was in addition to the current value to
know if you've overflowed since your last check).

-Aaron

-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [Qemu-devel] [PATCH v3 00/22] More fully implement ARM PMUv3

2018-04-12 Thread Peter Maydell 
On 16 March 2018 at 20:30, Aaron Lindsay  wrote:
> The ARM PMU implementation currently contains a basic cycle counter, but it is
> often useful to gather counts of other events and filter them based on
> execution mode. These patches flesh out the implementations of various PMU
> registers including PM[X]EVCNTR and PM[X]EVTYPER, add a struct definition to
> represent arbitrary counter types, implement mode filtering, and add
> instruction, cycle, and software increment events.

Hi; sorry it's taken me a while to get to this (I've been focusing
on for-2.12 work). I've now reviewed most of the patches in this
set. My suggestion is that you fix the stuff I've commented on
and send out a v4, and then I can take some of the patches from
the start of that into target-arm.next, and I'll review the tail
end of the set then.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/22] More fully implement ARM PMUv3

2018-03-16 Thread Aaron Lindsay 
My apologies for the below style issues - I've already fixed them up for
v4...

-Aaron

On Mar 16 13:58, no-re...@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 1521232280-13089-1-git-send-email-alind...@codeaurora.org
> Subject: [Qemu-devel] [PATCH v3 00/22] More fully implement ARM PMUv3
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> 
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
> then
> failed=1
> echo
> fi
> n=$((n+1))
> done
> 
> exit $failed
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]   
> patchew/1521232280-13089-1-git-send-email-alind...@codeaurora.org -> 
> patchew/1521232280-13089-1-git-send-email-alind...@codeaurora.org
> Switched to a new branch 'test'
> d81791b184 target/arm: Implement PMSWINC
> 512124d018 target/arm: PMU: Set PMCR.N to 4
> b748e97306 target/arm: PMU: Add instruction and cycle events
> 7e46a77f89 target/arm: Finish implementation of PM[X]EVCNTR and PM[X]EVTYPER
> 82cffef855 target/arm: Add array for supported PMU events, generate PMCEID[01]
> d635021d0a target/arm: Split arm_ccnt_enabled into generic pmu_counter_enabled
> 3e1b438deb target/arm: Implement PMOVSSET
> c13c832988 target/arm: Add ARM_FEATURE_V7VE for v7 Virtualization Extensions
> 9c80af4d82 target/arm: Make PMOVSCLR 64 bits wide
> 8ffbcd3dd8 target/arm: Allow AArch32 access for PMCCFILTR
> 7dc4ec9715 target/arm: Filter cycle counter based on PMCCFILTR_EL0
> f1e40f9fff target/arm: Fix bitmask for PMCCFILTR writes
> eb142b42b4 target/arm: Allow EL change hooks to do IO
> d37186abdd target/arm: Add pre-EL change hooks
> ae12e161da target/arm: Support multiple EL change hooks
> 9bfa99805d target/arm: Fetch GICv3 state directly from CPUARMState
> 881bee5e96 target/arm: Mask PMU register writes based on PMCR_EL0.N
> 890ad6472b target/arm: Reorganize PMCCNTR read, write, sync
> 922eec023b target/arm: Treat PMCCNTR as alias of PMCCNTR_EL0
> 5b1ce33c0b target/arm: Check PMCNTEN for whether PMCCNTR is enabled
> 2ecb09ae04 target/arm: A15 PMCEID0 initialization style nit
> 0f26a1568a target/arm: A53: Initialize PMCEID[01]
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/22: target/arm: A53: Initialize PMCEID[01]...
> Checking PATCH 2/22: target/arm: A15 PMCEID0 initialization style nit...
> Checking PATCH 3/22: target/arm: Check PMCNTEN for whether PMCCNTR is 
> enabled...
> Checking PATCH 4/22: target/arm: Treat PMCCNTR as alias of PMCCNTR_EL0...
> Checking PATCH 5/22: target/arm: Reorganize PMCCNTR read, write, sync...
> Checking PATCH 6/22: target/arm: Mask PMU register writes based on 
> PMCR_EL0.N...
> Checking PATCH 7/22: target/arm: Fetch GICv3 state directly from 
> CPUARMState...
> Checking PATCH 8/22: target/arm: Support multiple EL change hooks...
> ERROR: space prohibited between function name and open parenthesis '('
> #26: FILE: target/arm/cpu.c:62:
> +entry = g_malloc0(sizeof (*entry));
> 
> total: 1 errors, 0 warnings, 87 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 9/22: target/arm: Add pre-EL change hooks...
> ERROR: space prohibited between function name and open parenthesis '('
> #26: FILE: target/arm/cpu.c:62:
> +entry = g_malloc0(sizeof (*entry));
> 
> total: 1 errors, 0 warnings, 110 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 10/22: target/arm: Allow EL change hooks to do IO...
> Checking PATCH 11/22: target/arm: Fix bitmask for PMCCFILTR writes...
> Checking PATCH 12/22: target/arm: Filter cycle counter based on 
> PMCCFILTR_EL0...
> Checking PATCH 13/22: target/arm: Allow AArch32 access for PMCCFILTR...
> Checking PATCH 14/22: target/arm: Make PMOVSCLR 64 bits wide...
> Checking PATCH 15/22: target/arm: Add ARM_FEATURE_V7VE for v7 Virtualization 
> Extensions...
> Checking PATCH 16/22: target/arm: Implement PMOVSSET...
> WARNING: line over 80 characters
> #39: FILE: target/arm/helper.c:1417:
> +  .access = PL0_RW, .fieldoffset = offsetoflow32(CPUARMState, 
> cp15.c9_pmovsr),
> 
> total: 0 errors, 1 warnings, 59 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in

Re: [Qemu-devel] [PATCH v3 00/22] More fully implement ARM PMUv3

2018-03-16 Thread no-reply 
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1521232280-13089-1-git-send-email-alind...@codeaurora.org
Subject: [Qemu-devel] [PATCH v3 00/22] More fully implement ARM PMUv3

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/1521232280-13089-1-git-send-email-alind...@codeaurora.org -> 
patchew/1521232280-13089-1-git-send-email-alind...@codeaurora.org
Switched to a new branch 'test'
d81791b184 target/arm: Implement PMSWINC
512124d018 target/arm: PMU: Set PMCR.N to 4
b748e97306 target/arm: PMU: Add instruction and cycle events
7e46a77f89 target/arm: Finish implementation of PM[X]EVCNTR and PM[X]EVTYPER
82cffef855 target/arm: Add array for supported PMU events, generate PMCEID[01]
d635021d0a target/arm: Split arm_ccnt_enabled into generic pmu_counter_enabled
3e1b438deb target/arm: Implement PMOVSSET
c13c832988 target/arm: Add ARM_FEATURE_V7VE for v7 Virtualization Extensions
9c80af4d82 target/arm: Make PMOVSCLR 64 bits wide
8ffbcd3dd8 target/arm: Allow AArch32 access for PMCCFILTR
7dc4ec9715 target/arm: Filter cycle counter based on PMCCFILTR_EL0
f1e40f9fff target/arm: Fix bitmask for PMCCFILTR writes
eb142b42b4 target/arm: Allow EL change hooks to do IO
d37186abdd target/arm: Add pre-EL change hooks
ae12e161da target/arm: Support multiple EL change hooks
9bfa99805d target/arm: Fetch GICv3 state directly from CPUARMState
881bee5e96 target/arm: Mask PMU register writes based on PMCR_EL0.N
890ad6472b target/arm: Reorganize PMCCNTR read, write, sync
922eec023b target/arm: Treat PMCCNTR as alias of PMCCNTR_EL0
5b1ce33c0b target/arm: Check PMCNTEN for whether PMCCNTR is enabled
2ecb09ae04 target/arm: A15 PMCEID0 initialization style nit
0f26a1568a target/arm: A53: Initialize PMCEID[01]

=== OUTPUT BEGIN ===
Checking PATCH 1/22: target/arm: A53: Initialize PMCEID[01]...
Checking PATCH 2/22: target/arm: A15 PMCEID0 initialization style nit...
Checking PATCH 3/22: target/arm: Check PMCNTEN for whether PMCCNTR is enabled...
Checking PATCH 4/22: target/arm: Treat PMCCNTR as alias of PMCCNTR_EL0...
Checking PATCH 5/22: target/arm: Reorganize PMCCNTR read, write, sync...
Checking PATCH 6/22: target/arm: Mask PMU register writes based on PMCR_EL0.N...
Checking PATCH 7/22: target/arm: Fetch GICv3 state directly from CPUARMState...
Checking PATCH 8/22: target/arm: Support multiple EL change hooks...
ERROR: space prohibited between function name and open parenthesis '('
#26: FILE: target/arm/cpu.c:62:
+entry = g_malloc0(sizeof (*entry));

total: 1 errors, 0 warnings, 87 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 9/22: target/arm: Add pre-EL change hooks...
ERROR: space prohibited between function name and open parenthesis '('
#26: FILE: target/arm/cpu.c:62:
+entry = g_malloc0(sizeof (*entry));

total: 1 errors, 0 warnings, 110 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 10/22: target/arm: Allow EL change hooks to do IO...
Checking PATCH 11/22: target/arm: Fix bitmask for PMCCFILTR writes...
Checking PATCH 12/22: target/arm: Filter cycle counter based on PMCCFILTR_EL0...
Checking PATCH 13/22: target/arm: Allow AArch32 access for PMCCFILTR...
Checking PATCH 14/22: target/arm: Make PMOVSCLR 64 bits wide...
Checking PATCH 15/22: target/arm: Add ARM_FEATURE_V7VE for v7 Virtualization 
Extensions...
Checking PATCH 16/22: target/arm: Implement PMOVSSET...
WARNING: line over 80 characters
#39: FILE: target/arm/helper.c:1417:
+  .access = PL0_RW, .fieldoffset = offsetoflow32(CPUARMState, 
cp15.c9_pmovsr),

total: 0 errors, 1 warnings, 59 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 17/22: target/arm: Split arm_ccnt_enabled into generic 
pmu_counter_enabled...
Checking PATCH 18/22: target/arm: Add array for supported PMU events, generate 
PMCEID[01]...
Checking PATCH 19/22: target/arm: Finish implementation of PM[X]EVCNTR and 
PM[X]EVTYPER...
Checking PATCH 20/22: target/arm: PMU: Add instruction and cycle events...