Re: [Qemu-devel] [kvm-unit-tests PATCH v13 2/4] arm: Add PMU test
On 1 December 2016 at 12:19, Andre Przywarawrote: > Hi, > > On 01/12/16 12:02, Peter Maydell wrote: >> On 1 December 2016 at 11:28, Andre Przywara wrote: >>> I don't think so. At least here as a _variable_ type uint32_t is >>> probably the right one, as the ARMv8 ARM explicitly says that PMCR is a >>> 32-bit register, for both bitnesses. >> >> For 64-bit ARM this is strictly speaking just shorthand for "64-bit >> register with the top 32-bit being RES0". It is in theory possible that >> a future architecture extension might define uses for those RES0 >> bits. > > I trade: "in theory possible that a future architecture extension might" > (that's four speculative terms, right?) against: > > ARMv8 ARM, D7.4.7 PMCR_EL0, Performance Monitors Control Register: > Attributes > PMCR_EL0 is a 32-bit register. As I say, this just means "64 bit with 32 RES0 bits". See DDI0487A.k C5.1.1 "System register width": # In AArch64 state, each encoding in the System instruction space can # provide access to a 64-bit register. An AArch64 System register is # described as either a 32-bit register or a 64-bit register. For # a 32-bit register, the upper bits, bits[63:32], are RES0. (ie the register is 64-bits, it's just "described as" 32-bits for convenience.) thanks -- PMM ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [Qemu-devel] [kvm-unit-tests PATCH v13 2/4] arm: Add PMU test
Hi, On 01/12/16 12:02, Peter Maydell wrote: > On 1 December 2016 at 11:28, Andre Przywarawrote: >> I don't think so. At least here as a _variable_ type uint32_t is >> probably the right one, as the ARMv8 ARM explicitly says that PMCR is a >> 32-bit register, for both bitnesses. > > For 64-bit ARM this is strictly speaking just shorthand for "64-bit > register with the top 32-bit being RES0". It is in theory possible that > a future architecture extension might define uses for those RES0 > bits. I trade: "in theory possible that a future architecture extension might" (that's four speculative terms, right?) against: ARMv8 ARM, D7.4.7 PMCR_EL0, Performance Monitors Control Register: Attributes PMCR_EL0 is a 32-bit register. If this ever gets extended, we would need extra code to deal with the new bits, so we would need to touch the code anyway. And again, it's just a local variable, not an interface. Cheers, Andre. P.S. We really should save this discussion for a Friday afternoon ;-) ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [Qemu-devel] [kvm-unit-tests PATCH v13 2/4] arm: Add PMU test
On 1 December 2016 at 11:28, Andre Przywarawrote: > I don't think so. At least here as a _variable_ type uint32_t is > probably the right one, as the ARMv8 ARM explicitly says that PMCR is a > 32-bit register, for both bitnesses. For 64-bit ARM this is strictly speaking just shorthand for "64-bit register with the top 32-bit being RES0". It is in theory possible that a future architecture extension might define uses for those RES0 bits. thanks -- PMM ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [Qemu-devel] [kvm-unit-tests PATCH v13 2/4] arm: Add PMU test
Hi, On 01/12/16 09:03, Andrew Jones wrote: > On Wed, Nov 30, 2016 at 11:16:40PM -0600, Wei Huang wrote: >> From: Christopher Covington>> >> Beginning with a simple sanity check of the control register, add >> a unit test for the ARM Performance Monitors Unit (PMU). PMU register >> was read using the newly defined macros. >> >> Signed-off-by: Christopher Covington >> Signed-off-by: Wei Huang >> Reviewed-by: Andrew Jones >> --- >> arm/Makefile.common | 3 ++- >> arm/pmu.c | 62 >> + >> arm/unittests.cfg | 5 + >> 3 files changed, 69 insertions(+), 1 deletion(-) >> create mode 100644 arm/pmu.c >> >> diff --git a/arm/Makefile.common b/arm/Makefile.common >> index f37b5c2..5da2fdd 100644 >> --- a/arm/Makefile.common >> +++ b/arm/Makefile.common >> @@ -12,7 +12,8 @@ endif >> tests-common = \ >> $(TEST_DIR)/selftest.flat \ >> $(TEST_DIR)/spinlock-test.flat \ >> -$(TEST_DIR)/pci-test.flat >> +$(TEST_DIR)/pci-test.flat \ >> +$(TEST_DIR)/pmu.flat >> >> all: test_cases >> >> diff --git a/arm/pmu.c b/arm/pmu.c >> new file mode 100644 >> index 000..1fe2b1a >> --- /dev/null >> +++ b/arm/pmu.c >> @@ -0,0 +1,62 @@ >> +/* >> + * Test the ARM Performance Monitors Unit (PMU). >> + * >> + * Copyright (c) 2015-2016, The Linux Foundation. All rights reserved. >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU Lesser General Public License version 2.1 and >> + * only version 2.1 as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, but >> WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public >> License >> + * for more details. >> + */ >> +#include "libcflat.h" >> +#include "asm/barrier.h" >> +#include "asm/processor.h" >> + >> +#define PMU_PMCR_N_SHIFT 11 >> +#define PMU_PMCR_N_MASK0x1f >> +#define PMU_PMCR_ID_SHIFT 16 >> +#define PMU_PMCR_ID_MASK 0xff >> +#define PMU_PMCR_IMP_SHIFT 24 >> +#define PMU_PMCR_IMP_MASK 0xff >> + >> +#if defined(__arm__) >> +DEFINE_GET_SYSREG32(pmcr, 0, c9, c12, 0) >> +#elif defined(__aarch64__) >> +DEFINE_GET_SYSREG32(pmcr, el0) >> +#endif >> + >> +/* >> + * As a simple sanity check on the PMCR_EL0, ensure the implementer field >> isn't >> + * null. Also print out a couple other interesting fields for diagnostic >> + * purposes. For example, as of fall 2016, QEMU TCG mode doesn't implement >> + * event counters and therefore reports zero event counters, but hopefully >> + * support for at least the instructions event will be added in the future >> and >> + * the reported number of event counters will become nonzero. >> + */ >> +static bool check_pmcr(void) >> +{ >> +uint32_t pmcr; > > So based on my comments from the previous patch, pmcr should be > 'unsigned int' I don't think so. At least here as a _variable_ type uint32_t is probably the right one, as the ARMv8 ARM explicitly says that PMCR is a 32-bit register, for both bitnesses. I find it only natural to express it here accordingly. I believe this is a different (though related) discussion from the return type of the accessor functions. Cheers, Andre. >> + >> +pmcr = get_pmcr(); >> + >> +report_info("PMU implementer/ID code/counters: 0x%x(\"%c\")/0x%x/%d", >> +(pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK, >> +((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) ? : ' ', >> +(pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK, >> +(pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK); >> + >> +return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0; >> +} >> + >> +int main(void) >> +{ >> +report_prefix_push("pmu"); >> + >> +report("Control register", check_pmcr()); >> + >> +return report_summary(); >> +} >> diff --git a/arm/unittests.cfg b/arm/unittests.cfg >> index ae32a42..816f494 100644 >> --- a/arm/unittests.cfg >> +++ b/arm/unittests.cfg >> @@ -58,3 +58,8 @@ groups = selftest >> [pci-test] >> file = pci-test.flat >> groups = pci >> + >> +# Test PMU support >> +[pmu] >> +file = pmu.flat >> +groups = pmu >> -- >> 1.8.3.1 >> >> > > drew > ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [Qemu-devel] [kvm-unit-tests PATCH v13 2/4] arm: Add PMU test
On Wed, Nov 30, 2016 at 11:16:40PM -0600, Wei Huang wrote: > From: Christopher Covington> > Beginning with a simple sanity check of the control register, add > a unit test for the ARM Performance Monitors Unit (PMU). PMU register > was read using the newly defined macros. > > Signed-off-by: Christopher Covington > Signed-off-by: Wei Huang > Reviewed-by: Andrew Jones > --- > arm/Makefile.common | 3 ++- > arm/pmu.c | 62 > + > arm/unittests.cfg | 5 + > 3 files changed, 69 insertions(+), 1 deletion(-) > create mode 100644 arm/pmu.c > > diff --git a/arm/Makefile.common b/arm/Makefile.common > index f37b5c2..5da2fdd 100644 > --- a/arm/Makefile.common > +++ b/arm/Makefile.common > @@ -12,7 +12,8 @@ endif > tests-common = \ > $(TEST_DIR)/selftest.flat \ > $(TEST_DIR)/spinlock-test.flat \ > - $(TEST_DIR)/pci-test.flat > + $(TEST_DIR)/pci-test.flat \ > + $(TEST_DIR)/pmu.flat > > all: test_cases > > diff --git a/arm/pmu.c b/arm/pmu.c > new file mode 100644 > index 000..1fe2b1a > --- /dev/null > +++ b/arm/pmu.c > @@ -0,0 +1,62 @@ > +/* > + * Test the ARM Performance Monitors Unit (PMU). > + * > + * Copyright (c) 2015-2016, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU Lesser General Public License version 2.1 and > + * only version 2.1 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public > License > + * for more details. > + */ > +#include "libcflat.h" > +#include "asm/barrier.h" > +#include "asm/processor.h" > + > +#define PMU_PMCR_N_SHIFT 11 > +#define PMU_PMCR_N_MASK0x1f > +#define PMU_PMCR_ID_SHIFT 16 > +#define PMU_PMCR_ID_MASK 0xff > +#define PMU_PMCR_IMP_SHIFT 24 > +#define PMU_PMCR_IMP_MASK 0xff > + > +#if defined(__arm__) > +DEFINE_GET_SYSREG32(pmcr, 0, c9, c12, 0) > +#elif defined(__aarch64__) > +DEFINE_GET_SYSREG32(pmcr, el0) > +#endif > + > +/* > + * As a simple sanity check on the PMCR_EL0, ensure the implementer field > isn't > + * null. Also print out a couple other interesting fields for diagnostic > + * purposes. For example, as of fall 2016, QEMU TCG mode doesn't implement > + * event counters and therefore reports zero event counters, but hopefully > + * support for at least the instructions event will be added in the future > and > + * the reported number of event counters will become nonzero. > + */ > +static bool check_pmcr(void) > +{ > + uint32_t pmcr; So based on my comments from the previous patch, pmcr should be 'unsigned int' > + > + pmcr = get_pmcr(); > + > + report_info("PMU implementer/ID code/counters: 0x%x(\"%c\")/0x%x/%d", > + (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK, > + ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) ? : ' ', > + (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK, > + (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK); > + > + return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0; > +} > + > +int main(void) > +{ > + report_prefix_push("pmu"); > + > + report("Control register", check_pmcr()); > + > + return report_summary(); > +} > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > index ae32a42..816f494 100644 > --- a/arm/unittests.cfg > +++ b/arm/unittests.cfg > @@ -58,3 +58,8 @@ groups = selftest > [pci-test] > file = pci-test.flat > groups = pci > + > +# Test PMU support > +[pmu] > +file = pmu.flat > +groups = pmu > -- > 1.8.3.1 > > drew ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[kvm-unit-tests PATCH v13 2/4] arm: Add PMU test
From: Christopher CovingtonBeginning with a simple sanity check of the control register, add a unit test for the ARM Performance Monitors Unit (PMU). PMU register was read using the newly defined macros. Signed-off-by: Christopher Covington Signed-off-by: Wei Huang Reviewed-by: Andrew Jones --- arm/Makefile.common | 3 ++- arm/pmu.c | 62 + arm/unittests.cfg | 5 + 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 arm/pmu.c diff --git a/arm/Makefile.common b/arm/Makefile.common index f37b5c2..5da2fdd 100644 --- a/arm/Makefile.common +++ b/arm/Makefile.common @@ -12,7 +12,8 @@ endif tests-common = \ $(TEST_DIR)/selftest.flat \ $(TEST_DIR)/spinlock-test.flat \ - $(TEST_DIR)/pci-test.flat + $(TEST_DIR)/pci-test.flat \ + $(TEST_DIR)/pmu.flat all: test_cases diff --git a/arm/pmu.c b/arm/pmu.c new file mode 100644 index 000..1fe2b1a --- /dev/null +++ b/arm/pmu.c @@ -0,0 +1,62 @@ +/* + * Test the ARM Performance Monitors Unit (PMU). + * + * Copyright (c) 2015-2016, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License version 2.1 and + * only version 2.1 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + */ +#include "libcflat.h" +#include "asm/barrier.h" +#include "asm/processor.h" + +#define PMU_PMCR_N_SHIFT 11 +#define PMU_PMCR_N_MASK0x1f +#define PMU_PMCR_ID_SHIFT 16 +#define PMU_PMCR_ID_MASK 0xff +#define PMU_PMCR_IMP_SHIFT 24 +#define PMU_PMCR_IMP_MASK 0xff + +#if defined(__arm__) +DEFINE_GET_SYSREG32(pmcr, 0, c9, c12, 0) +#elif defined(__aarch64__) +DEFINE_GET_SYSREG32(pmcr, el0) +#endif + +/* + * As a simple sanity check on the PMCR_EL0, ensure the implementer field isn't + * null. Also print out a couple other interesting fields for diagnostic + * purposes. For example, as of fall 2016, QEMU TCG mode doesn't implement + * event counters and therefore reports zero event counters, but hopefully + * support for at least the instructions event will be added in the future and + * the reported number of event counters will become nonzero. + */ +static bool check_pmcr(void) +{ + uint32_t pmcr; + + pmcr = get_pmcr(); + + report_info("PMU implementer/ID code/counters: 0x%x(\"%c\")/0x%x/%d", + (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK, + ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) ? : ' ', + (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK, + (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK); + + return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0; +} + +int main(void) +{ + report_prefix_push("pmu"); + + report("Control register", check_pmcr()); + + return report_summary(); +} diff --git a/arm/unittests.cfg b/arm/unittests.cfg index ae32a42..816f494 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -58,3 +58,8 @@ groups = selftest [pci-test] file = pci-test.flat groups = pci + +# Test PMU support +[pmu] +file = pmu.flat +groups = pmu -- 1.8.3.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm