| Message ID | 1480569402-8848-3-git-send-email-wei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 30, 2016 at 11:16:40PM -0600, Wei Huang wrote: > From: Christopher Covington <cov@codeaurora.org> > > 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 <cov@codeaurora.org> > Signed-off-by: Wei Huang <wei@redhat.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > --- > 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 0000000..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_MASK 0x1f > +#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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <cov@codeaurora.org> >> >> 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 <cov@codeaurora.org> >> Signed-off-by: Wei Huang <wei@redhat.com> >> Reviewed-by: Andrew Jones <drjones@redhat.com> >> --- >> 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 0000000..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_MASK 0x1f >> +#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 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1 December 2016 at 11:28, Andre Przywara <andre.przywara@arm.com> 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. thanks -- PMM -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 01/12/16 12:02, Peter Maydell wrote: > On 1 December 2016 at 11:28, Andre Przywara <andre.przywara@arm.com> 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. 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 ;-) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1 December 2016 at 12:19, Andre Przywara <andre.przywara@arm.com> wrote: > Hi, > > On 01/12/16 12:02, Peter Maydell wrote: >> On 1 December 2016 at 11:28, Andre Przywara <andre.przywara@arm.com> 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 0000000..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_MASK 0x1f +#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