Message ID | 1521232280-13089-1-git-send-email-alindsay@codeaurora.org |
---|---|
Headers | show |
Series | More fully implement ARM PMUv3 | expand |
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-alindsay@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-alindsay@codeaurora.org -> patchew/1521232280-13089-1-git-send-email-alindsay@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... Checking PATCH 21/22: target/arm: PMU: Set PMCR.N to 4... Checking PATCH 22/22: target/arm: Implement PMSWINC... === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@freelists.org
My apologies for the below style issues - I've already fixed them up for v4... -Aaron On Mar 16 13:58, no-reply@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-alindsay@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-alindsay@codeaurora.org -> patchew/1521232280-13089-1-git-send-email-alindsay@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... > Checking PATCH 21/22: target/arm: PMU: Set PMCR.N to 4... > Checking PATCH 22/22: target/arm: Implement PMSWINC... > === OUTPUT END === > > Test command exited with code: 1 > > > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-devel@freelists.org
On 16 March 2018 at 20:30, Aaron Lindsay <alindsay@codeaurora.org> 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
On Apr 12 18:32, Peter Maydell wrote: > On 16 March 2018 at 20:30, Aaron Lindsay <alindsay@codeaurora.org> 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