[Qemu-devel] [PATCH 0/3] target/arm: pmu fixes

Andrew Jones posted 3 patches 5 years, 1 month ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190322162333.17159-1-drjones@redhat.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/cpu.c    |  3 +++
target/arm/cpu.h    | 11 -----------
target/arm/helper.c |  8 ++++++--
3 files changed, 9 insertions(+), 13 deletions(-)
[Qemu-devel] [PATCH 0/3] target/arm: pmu fixes
Posted by Andrew Jones 5 years, 1 month ago
The first two patches fix a regression I found when running
the arm (as opposed to arm64) pmu kvm-unit-tests test on tcg,

  $ git clone git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
  $ cd kvm-unit-tests
  $ ./configure --arch=arm --cross-prefix=arm-linux-gnu-
  $ make -j
  $ export QEMU=/path/to/qemu-system-arm or qemu-system-aarch64
  $ arm/run arm/pmu.flat

After checking the QEMU code I found it's also reproducible with
the arm64 test if the PMU is removed,

  $ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
  $ make -j
  $ export QEMU=/path/to/qemu-system-aarch64
  $ arm/run arm/pmu.flat -cpu cortex-a57,pmu=off

Those tests used to pass, but now QEMU was crashing. I've broken
the fix into two patches because the second patch is a bit of
an RFC since I don't know if it's safe to enable all
ARM_FEATURE_PMU code for PMUv2. Maybe that feature is only for
PMUv3? This patch also enables the pmu cpu property to work with
those cpu types, i.e. we can now do '-cpu cortex-a15,pmu=off' to
remove the pmu. Although it wasn't clear to me if the PMU is
optional (permitted to be removed) on those cpu types from the
manuals.

The last patch is just a trivial cleanup.

Andrew Jones (3):
  target/arm: fix crash on pmu register access
  target/arm: cortex-a7 and cortex-a15 have pmus
  target/arm: make pmccntr_op_start/finish static

 target/arm/cpu.c    |  3 +++
 target/arm/cpu.h    | 11 -----------
 target/arm/helper.c |  8 ++++++--
 3 files changed, 9 insertions(+), 13 deletions(-)

-- 
2.17.2


Re: [Qemu-devel] [PATCH 0/3] target/arm: pmu fixes
Posted by Andrew Jones 5 years, 1 month ago
On Fri, Mar 22, 2019 at 05:23:30PM +0100, Andrew Jones wrote:
> The first two patches fix a regression I found when running
> the arm (as opposed to arm64) pmu kvm-unit-tests test on tcg,
> 
>   $ git clone git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
>   $ cd kvm-unit-tests
>   $ ./configure --arch=arm --cross-prefix=arm-linux-gnu-
>   $ make -j
>   $ export QEMU=/path/to/qemu-system-arm or qemu-system-aarch64
>   $ arm/run arm/pmu.flat
> 
> After checking the QEMU code I found it's also reproducible with
> the arm64 test if the PMU is removed,
> 
>   $ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
>   $ make -j
>   $ export QEMU=/path/to/qemu-system-aarch64
>   $ arm/run arm/pmu.flat -cpu cortex-a57,pmu=off

Small correction here: I had to also hack the unit test to not
bail out when it didn't see a PMU advertised in the ID register.

diff --git a/arm/pmu.c b/arm/pmu.c
index 1de7d77d184f..185ff77244ed 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -291,8 +291,9 @@ int main(int argc, char *argv[])
		cpi = atol(argv[1]);
 
	if (!pmu_probe()) {
-	       printf("No PMU found, test skipped...\n");
-	       return report_summary();
+//	     printf("No PMU found, test skipped...\n");
+	       printf("No PMU found, let's crash!\n");
+//	     return report_summary();
	}
 
	report_prefix_push("pmu");

> 
> Those tests used to pass, but now QEMU was crashing. I've broken
> the fix into two patches because the second patch is a bit of
> an RFC since I don't know if it's safe to enable all
> ARM_FEATURE_PMU code for PMUv2. Maybe that feature is only for
> PMUv3? This patch also enables the pmu cpu property to work with
> those cpu types, i.e. we can now do '-cpu cortex-a15,pmu=off' to
> remove the pmu. Although it wasn't clear to me if the PMU is
> optional (permitted to be removed) on those cpu types from the
> manuals.
> 
> The last patch is just a trivial cleanup.
> 
> Andrew Jones (3):
>   target/arm: fix crash on pmu register access
>   target/arm: cortex-a7 and cortex-a15 have pmus
>   target/arm: make pmccntr_op_start/finish static
> 
>  target/arm/cpu.c    |  3 +++
>  target/arm/cpu.h    | 11 -----------
>  target/arm/helper.c |  8 ++++++--
>  3 files changed, 9 insertions(+), 13 deletions(-)
> 
> -- 
> 2.17.2
> 

Re: [Qemu-devel] [PATCH 0/3] target/arm: pmu fixes
Posted by Richard Henderson 5 years, 1 month ago
On 3/22/19 9:23 AM, Andrew Jones wrote:
> Andrew Jones (3):
>   target/arm: fix crash on pmu register access
>   target/arm: cortex-a7 and cortex-a15 have pmus
>   target/arm: make pmccntr_op_start/finish static

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 0/3] target/arm: pmu fixes
Posted by Peter Maydell 5 years, 1 month ago
On Fri, 22 Mar 2019 at 16:23, Andrew Jones <drjones@redhat.com> wrote:
>
> The first two patches fix a regression I found when running
> the arm (as opposed to arm64) pmu kvm-unit-tests test on tcg,
>
>   $ git clone git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
>   $ cd kvm-unit-tests
>   $ ./configure --arch=arm --cross-prefix=arm-linux-gnu-
>   $ make -j
>   $ export QEMU=/path/to/qemu-system-arm or qemu-system-aarch64
>   $ arm/run arm/pmu.flat
>
> After checking the QEMU code I found it's also reproducible with
> the arm64 test if the PMU is removed,
>
>   $ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
>   $ make -j
>   $ export QEMU=/path/to/qemu-system-aarch64
>   $ arm/run arm/pmu.flat -cpu cortex-a57,pmu=off
>
> Those tests used to pass, but now QEMU was crashing. I've broken
> the fix into two patches because the second patch is a bit of
> an RFC since I don't know if it's safe to enable all
> ARM_FEATURE_PMU code for PMUv2. Maybe that feature is only for
> PMUv3? This patch also enables the pmu cpu property to work with
> those cpu types, i.e. we can now do '-cpu cortex-a15,pmu=off' to
> remove the pmu. Although it wasn't clear to me if the PMU is
> optional (permitted to be removed) on those cpu types from the
> manuals.
>
> The last patch is just a trivial cleanup.
>
> Andrew Jones (3):
>   target/arm: fix crash on pmu register access
>   target/arm: cortex-a7 and cortex-a15 have pmus
>   target/arm: make pmccntr_op_start/finish static
>



Applied to target-arm.next, thanks.

-- PMM