| Message ID | 20201027171944.13933-4-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Statistical Profiling Extension Tests | expand |
Hi Alexandru, On 10/27/20 6:19 PM, Alexandru Elisei wrote: > From: Eric Auger <eric.auger@redhat.com> > > Probe the DTB and the ID registers to get information about SPE, then > compare the register fields with the valid values as defined by ARM DDI > 0487F.b. > > SPE is supported only on AArch64, so make the test exclusive to the > arm64 architecture. > > [ Alexandru E: Removed aarch32 compilation support, added DTB probing, > reworded commit, mostly cosmetic changes to the code ]all those changes make sense to me. + spe_buffer not allocated in spe_probe() anymore. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > arm/Makefile.arm64 | 1 + > lib/libcflat.h | 1 + > arm/spe.c | 172 +++++++++++++++++++++++++++++++++++++++++++++ > arm/unittests.cfg | 7 ++ > 4 files changed, 181 insertions(+) > create mode 100644 arm/spe.c > > diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64 > index dbc7524d3070..94b9c63f0b05 100644 > --- a/arm/Makefile.arm64 > +++ b/arm/Makefile.arm64 > @@ -30,6 +30,7 @@ OBJDIRS += lib/arm64 > tests = $(TEST_DIR)/timer.flat > tests += $(TEST_DIR)/micro-bench.flat > tests += $(TEST_DIR)/cache.flat > +tests += $(TEST_DIR)/spe.flat > > include $(SRCDIR)/$(TEST_DIR)/Makefile.common > > diff --git a/lib/libcflat.h b/lib/libcflat.h > index ec0f58b05701..37550c99ffb6 100644 > --- a/lib/libcflat.h > +++ b/lib/libcflat.h > @@ -37,6 +37,7 @@ > #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) > > #define SZ_256 (1 << 8) > +#define SZ_2K (1 << 11) > #define SZ_4K (1 << 12) > #define SZ_8K (1 << 13) > #define SZ_16K (1 << 14) > diff --git a/arm/spe.c b/arm/spe.c > new file mode 100644 > index 000000000000..c199cd239194 > --- /dev/null > +++ b/arm/spe.c > @@ -0,0 +1,172 @@ > +/* > + * Copyright (C) 2020, Red Hat Inc, Eric Auger <eric.auger@redhat.com> > + * > + * 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 <stdint.h> > + > +#include <bitops.h> > +#include <devicetree.h> > +#include <libcflat.h> > + > +#include <asm/gic.h> > +#include <asm/processor.h> > +#include <asm/sysreg.h> > + > +#define ID_AA64DFR0_PMSVER_SHIFT 32 > +#define ID_AA64DFR0_PMSVER_MASK 0xf > + > +#define SYS_PMBIDR_EL1 sys_reg(3, 0, 9, 10, 7) > +#define SYS_PMBIDR_EL1_F_SHIFT 5 > +#define SYS_PMBIDR_EL1_P_SHIFT 4 > +#define SYS_PMBIDR_EL1_ALIGN_MASK 0xfUL > +#define SYS_PMBIDR_EL1_ALIGN_SHIFT 0 > + > +#define SYS_PMSIDR_EL1 sys_reg(3, 0, 9, 9, 7) > +#define SYS_PMSIDR_EL1_FE_SHIFT 0 > +#define SYS_PMSIDR_EL1_FT_SHIFT 1 > +#define SYS_PMSIDR_EL1_FL_SHIFT 2 > +#define SYS_PMSIDR_EL1_INTERVAL_SHIFT 8 > +#define SYS_PMSIDR_EL1_INTERVAL_MASK 0xfUL > +#define SYS_PMSIDR_EL1_MAXSIZE_SHIFT 12 > +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL > +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL > +#define SYS_PMSIDR_EL1_COUNTSIZE_SHIFT 16 > +#define SYS_PMSIDR_EL1_COUNTSIZE_MASK 0xfUL > + > +struct spe { > + uint32_t intid; > + int min_interval; > + int max_record_size; > + int countsize; > + bool fl_cap; > + bool ft_cap; > + bool fe_cap; > + int align; > +}; > +static struct spe spe; > + > +static int spe_min_interval(uint8_t interval) > +{ > + switch (interval) { > + case 0x0: > + return 256; > + case 0x2: > + return 512; > + case 0x3: > + return 768; > + case 0x4: > + return 1024; > + case 0x5: > + return 1536; > + case 0x6: > + return 2048; > + case 0x7: > + return 3072; > + case 0x8: > + return 4096; > + default: > + return 0; > + } > +} > + > +static bool spe_probe(void) > +{ > + const struct fdt_property *prop; > + const void *fdt = dt_fdt(); > + int node, len; > + uint32_t *data; > + uint64_t pmbidr, pmsidr; > + uint64_t aa64dfr0 = get_id_aa64dfr0(); > + uint8_t pmsver, interval; > + > + node = fdt_node_offset_by_compatible(fdt, -1, "arm,statistical-profiling-extension-v1"); > + assert(node >= 0); > + prop = fdt_get_property(fdt, node, "interrupts", &len); > + assert(prop && len == 3 * sizeof(u32)); > + > + data = (u32 *)prop->data; > + /* SPE interrupt is required to be a PPI. */ > + assert(fdt32_to_cpu(data[0]) == 1); > + spe.intid = fdt32_to_cpu(data[1]); > + > + pmsver = (aa64dfr0 >> ID_AA64DFR0_PMSVER_SHIFT) & ID_AA64DFR0_PMSVER_MASK; > + if (!pmsver || pmsver > 2) { > + report_info("Unknown SPE version: 0x%x", pmsver); > + return false; > + } > + > + pmbidr = read_sysreg_s(SYS_PMBIDR_EL1); > + if (pmbidr & BIT(SYS_PMBIDR_EL1_P_SHIFT)) { > + report_info("Profiling buffer owned by higher exception level"); > + return false; > + } > + > + spe.align = (pmbidr >> SYS_PMBIDR_EL1_ALIGN_SHIFT) & SYS_PMBIDR_EL1_ALIGN_MASK; you can remove ">> SYS_PMBIDR_EL1_ALIGN_SHIFT" here and SYS_PMBIDR_EL1_ALIGN_SHIFT (0) > + spe.align = 1 << spe.align; > + > + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1); > + > + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK; > + spe.min_interval = spe_min_interval(interval); > + > + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \ > + SYS_PMSIDR_EL1_MAXSIZE_MASK; > + spe.max_record_size = 1 << spe.max_record_size; > + > + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \ > + SYS_PMSIDR_EL1_COUNTSIZE_MASK; > + > + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT); > + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT); > + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT); Why did you remove the report_info() section? I think those human readable info can be useful. > + > + return true; > +} > + > +static void spe_test_introspection(void) > +{ > + report_prefix_push("spe-introspection"); I see you moved this here. gic and pmu tests seem to leave them in the main, as done originally. > + > + report(spe.align <= SZ_2K, "PMBIDR_E1.Align"); good check added > + report(spe.countsize == 0x2, "PMSIDR_EL1.CountSize"); > + report(spe.max_record_size >= 16 && spe.max_record_size <= 2048, > + "PMSIDR_EL1 maximum record size"); > + report(spe.min_interval >= 256 && spe.min_interval <= 4096, > + "PMSIDR_EL1 minimum sampling interval"); > + report(spe.fl_cap && spe.ft_cap && spe.fe_cap, "PMSIDR_EL1 sampling filters"); indeed all bits read as 1 as the spec says. which is bit weird by the way. > + > + report_prefix_pop(); > +} > + > +int main(int argc, char *argv[]) > +{ > + if (!spe_probe()) { > + report_skip("SPE not supported"); > + return report_summary(); > + } > + > + printf("intid: %u\n", PPI(spe.intid)); > + printf("align: %d\n", spe.align); > + printf("min_interval: %d\n", spe.min_interval); > + printf("max_record_size: %d\n", spe.max_record_size); > + > + if (argc < 2) > + report_abort("no test specified"); > + > + report_prefix_push("spe"); > + > + if (strcmp(argv[1], "spe-introspection") == 0) > + spe_test_introspection(); > + else > + report_abort("Unknown subtest '%s'", argv[1]); > + > + return report_summary(); > +} > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > index f776b66ef96d..ad10be123774 100644 > --- a/arm/unittests.cfg > +++ b/arm/unittests.cfg > @@ -134,6 +134,13 @@ extra_params = -append 'pmu-overflow-interrupt' > #groups = pmu > #accel = tcg > > +[spe-introspection] > +file = spe.flat > +groups = spe > +arch = arm64 > +accel = kvm > +extra_params = -append 'spe-introspection' > + > # Test GIC emulation > [gicv2-ipi] > file = gic.flat > Thanks Eric
Hi Eric, On 10/30/20 3:29 PM, Auger Eric wrote: > Hi Alexandru, > > On 10/27/20 6:19 PM, Alexandru Elisei wrote: >> From: Eric Auger <eric.auger@redhat.com> >> >> Probe the DTB and the ID registers to get information about SPE, then >> compare the register fields with the valid values as defined by ARM DDI >> 0487F.b. >> >> SPE is supported only on AArch64, so make the test exclusive to the >> arm64 architecture. >> >> [ Alexandru E: Removed aarch32 compilation support, added DTB probing, >> reworded commit, mostly cosmetic changes to the code ]all those changes make sense to me. > + spe_buffer not allocated in spe_probe() anymore. Sure, will add that. >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >> --- >> arm/Makefile.arm64 | 1 + >> lib/libcflat.h | 1 + >> arm/spe.c | 172 +++++++++++++++++++++++++++++++++++++++++++++ >> arm/unittests.cfg | 7 ++ >> 4 files changed, 181 insertions(+) >> create mode 100644 arm/spe.c >> >> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64 >> index dbc7524d3070..94b9c63f0b05 100644 >> --- a/arm/Makefile.arm64 >> +++ b/arm/Makefile.arm64 >> @@ -30,6 +30,7 @@ OBJDIRS += lib/arm64 >> tests = $(TEST_DIR)/timer.flat >> tests += $(TEST_DIR)/micro-bench.flat >> tests += $(TEST_DIR)/cache.flat >> +tests += $(TEST_DIR)/spe.flat >> >> include $(SRCDIR)/$(TEST_DIR)/Makefile.common >> >> diff --git a/lib/libcflat.h b/lib/libcflat.h >> index ec0f58b05701..37550c99ffb6 100644 >> --- a/lib/libcflat.h >> +++ b/lib/libcflat.h >> @@ -37,6 +37,7 @@ >> #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) >> >> #define SZ_256 (1 << 8) >> +#define SZ_2K (1 << 11) >> #define SZ_4K (1 << 12) >> #define SZ_8K (1 << 13) >> #define SZ_16K (1 << 14) >> diff --git a/arm/spe.c b/arm/spe.c >> new file mode 100644 >> index 000000000000..c199cd239194 >> --- /dev/null >> +++ b/arm/spe.c >> @@ -0,0 +1,172 @@ >> +/* >> + * Copyright (C) 2020, Red Hat Inc, Eric Auger <eric.auger@redhat.com> >> + * >> + * 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 <stdint.h> >> + >> +#include <bitops.h> >> +#include <devicetree.h> >> +#include <libcflat.h> >> + >> +#include <asm/gic.h> >> +#include <asm/processor.h> >> +#include <asm/sysreg.h> >> + >> +#define ID_AA64DFR0_PMSVER_SHIFT 32 >> +#define ID_AA64DFR0_PMSVER_MASK 0xf >> + >> +#define SYS_PMBIDR_EL1 sys_reg(3, 0, 9, 10, 7) >> +#define SYS_PMBIDR_EL1_F_SHIFT 5 >> +#define SYS_PMBIDR_EL1_P_SHIFT 4 >> +#define SYS_PMBIDR_EL1_ALIGN_MASK 0xfUL >> +#define SYS_PMBIDR_EL1_ALIGN_SHIFT 0 >> + >> +#define SYS_PMSIDR_EL1 sys_reg(3, 0, 9, 9, 7) >> +#define SYS_PMSIDR_EL1_FE_SHIFT 0 >> +#define SYS_PMSIDR_EL1_FT_SHIFT 1 >> +#define SYS_PMSIDR_EL1_FL_SHIFT 2 >> +#define SYS_PMSIDR_EL1_INTERVAL_SHIFT 8 >> +#define SYS_PMSIDR_EL1_INTERVAL_MASK 0xfUL >> +#define SYS_PMSIDR_EL1_MAXSIZE_SHIFT 12 >> +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL >> +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL >> +#define SYS_PMSIDR_EL1_COUNTSIZE_SHIFT 16 >> +#define SYS_PMSIDR_EL1_COUNTSIZE_MASK 0xfUL >> + >> +struct spe { >> + uint32_t intid; >> + int min_interval; >> + int max_record_size; >> + int countsize; >> + bool fl_cap; >> + bool ft_cap; >> + bool fe_cap; >> + int align; >> +}; >> +static struct spe spe; >> + >> +static int spe_min_interval(uint8_t interval) >> +{ >> + switch (interval) { >> + case 0x0: >> + return 256; >> + case 0x2: >> + return 512; >> + case 0x3: >> + return 768; >> + case 0x4: >> + return 1024; >> + case 0x5: >> + return 1536; >> + case 0x6: >> + return 2048; >> + case 0x7: >> + return 3072; >> + case 0x8: >> + return 4096; >> + default: >> + return 0; >> + } >> +} >> + >> +static bool spe_probe(void) >> +{ >> + const struct fdt_property *prop; >> + const void *fdt = dt_fdt(); >> + int node, len; >> + uint32_t *data; >> + uint64_t pmbidr, pmsidr; >> + uint64_t aa64dfr0 = get_id_aa64dfr0(); >> + uint8_t pmsver, interval; >> + >> + node = fdt_node_offset_by_compatible(fdt, -1, "arm,statistical-profiling-extension-v1"); >> + assert(node >= 0); >> + prop = fdt_get_property(fdt, node, "interrupts", &len); >> + assert(prop && len == 3 * sizeof(u32)); >> + >> + data = (u32 *)prop->data; >> + /* SPE interrupt is required to be a PPI. */ >> + assert(fdt32_to_cpu(data[0]) == 1); >> + spe.intid = fdt32_to_cpu(data[1]); >> + >> + pmsver = (aa64dfr0 >> ID_AA64DFR0_PMSVER_SHIFT) & ID_AA64DFR0_PMSVER_MASK; >> + if (!pmsver || pmsver > 2) { >> + report_info("Unknown SPE version: 0x%x", pmsver); >> + return false; >> + } >> + >> + pmbidr = read_sysreg_s(SYS_PMBIDR_EL1); >> + if (pmbidr & BIT(SYS_PMBIDR_EL1_P_SHIFT)) { >> + report_info("Profiling buffer owned by higher exception level"); >> + return false; >> + } >> + >> + spe.align = (pmbidr >> SYS_PMBIDR_EL1_ALIGN_SHIFT) & SYS_PMBIDR_EL1_ALIGN_MASK; > you can remove ">> SYS_PMBIDR_EL1_ALIGN_SHIFT" here and > SYS_PMBIDR_EL1_ALIGN_SHIFT (0) I prefer to keep it to match the way we extract the values for the other fields, even though I realize it's unnecessary (the compiler will optimize it anyway). >> + spe.align = 1 << spe.align; >> + >> + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1); >> + >> + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK; >> + spe.min_interval = spe_min_interval(interval); >> + >> + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \ >> + SYS_PMSIDR_EL1_MAXSIZE_MASK; >> + spe.max_record_size = 1 << spe.max_record_size; >> + >> + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \ >> + SYS_PMSIDR_EL1_COUNTSIZE_MASK; >> + >> + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT); >> + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT); >> + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT); > Why did you remove the report_info() section? I think those human > readable info can be useful. I made them part of the test. Since the architecture says they are 1, I think making sure their value matches is more useful than printing something that the architecture guarantees. >> + >> + return true; >> +} >> + >> +static void spe_test_introspection(void) >> +{ >> + report_prefix_push("spe-introspection"); > I see you moved this here. gic and pmu tests seem to leave them in the > main, as done originally. You're right, consistency is good, will put them in main before calling the test function. >> + >> + report(spe.align <= SZ_2K, "PMBIDR_E1.Align"); > good check added >> + report(spe.countsize == 0x2, "PMSIDR_EL1.CountSize"); >> + report(spe.max_record_size >= 16 && spe.max_record_size <= 2048, >> + "PMSIDR_EL1 maximum record size"); >> + report(spe.min_interval >= 256 && spe.min_interval <= 4096, >> + "PMSIDR_EL1 minimum sampling interval"); >> + report(spe.fl_cap && spe.ft_cap && spe.fe_cap, "PMSIDR_EL1 sampling filters"); > indeed all bits read as 1 as the spec says. which is bit weird by the way. I think they added the fields because the architects wanted the flexibility to allow changes in the future. Thanks, Alex >> + >> + report_prefix_pop(); >> +} >> + >> +int main(int argc, char *argv[]) >> +{ >> + if (!spe_probe()) { >> + report_skip("SPE not supported"); >> + return report_summary(); >> + } >> + >> + printf("intid: %u\n", PPI(spe.intid)); >> + printf("align: %d\n", spe.align); >> + printf("min_interval: %d\n", spe.min_interval); >> + printf("max_record_size: %d\n", spe.max_record_size); >> + >> + if (argc < 2) >> + report_abort("no test specified"); >> + >> + report_prefix_push("spe"); >> + >> + if (strcmp(argv[1], "spe-introspection") == 0) >> + spe_test_introspection(); >> + else >> + report_abort("Unknown subtest '%s'", argv[1]); >> + >> + return report_summary(); >> +} >> diff --git a/arm/unittests.cfg b/arm/unittests.cfg >> index f776b66ef96d..ad10be123774 100644 >> --- a/arm/unittests.cfg >> +++ b/arm/unittests.cfg >> @@ -134,6 +134,13 @@ extra_params = -append 'pmu-overflow-interrupt' >> #groups = pmu >> #accel = tcg >> >> +[spe-introspection] >> +file = spe.flat >> +groups = spe >> +arch = arm64 >> +accel = kvm >> +extra_params = -append 'spe-introspection' >> + >> # Test GIC emulation >> [gicv2-ipi] >> file = gic.flat >> > Thanks > > Eric >
Hi Alexandru, On 10/30/20 4:59 PM, Alexandru Elisei wrote: > Hi Eric, > > On 10/30/20 3:29 PM, Auger Eric wrote: >> Hi Alexandru, >> >> On 10/27/20 6:19 PM, Alexandru Elisei wrote: >>> From: Eric Auger <eric.auger@redhat.com> >>> >>> Probe the DTB and the ID registers to get information about SPE, then >>> compare the register fields with the valid values as defined by ARM DDI >>> 0487F.b. >>> >>> SPE is supported only on AArch64, so make the test exclusive to the >>> arm64 architecture. >>> >>> [ Alexandru E: Removed aarch32 compilation support, added DTB probing, >>> reworded commit, mostly cosmetic changes to the code ]all those changes make sense to me. all those changes make sense to me (missing returned line above) >> + spe_buffer not allocated in spe_probe() anymore. > > Sure, will add that. > >>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >>> --- >>> arm/Makefile.arm64 | 1 + >>> lib/libcflat.h | 1 + >>> arm/spe.c | 172 +++++++++++++++++++++++++++++++++++++++++++++ >>> arm/unittests.cfg | 7 ++ >>> 4 files changed, 181 insertions(+) >>> create mode 100644 arm/spe.c >>> >>> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64 >>> index dbc7524d3070..94b9c63f0b05 100644 >>> --- a/arm/Makefile.arm64 >>> +++ b/arm/Makefile.arm64 >>> @@ -30,6 +30,7 @@ OBJDIRS += lib/arm64 >>> tests = $(TEST_DIR)/timer.flat >>> tests += $(TEST_DIR)/micro-bench.flat >>> tests += $(TEST_DIR)/cache.flat >>> +tests += $(TEST_DIR)/spe.flat >>> >>> include $(SRCDIR)/$(TEST_DIR)/Makefile.common >>> >>> diff --git a/lib/libcflat.h b/lib/libcflat.h >>> index ec0f58b05701..37550c99ffb6 100644 >>> --- a/lib/libcflat.h >>> +++ b/lib/libcflat.h >>> @@ -37,6 +37,7 @@ >>> #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) >>> >>> #define SZ_256 (1 << 8) >>> +#define SZ_2K (1 << 11) >>> #define SZ_4K (1 << 12) >>> #define SZ_8K (1 << 13) >>> #define SZ_16K (1 << 14) >>> diff --git a/arm/spe.c b/arm/spe.c >>> new file mode 100644 >>> index 000000000000..c199cd239194 >>> --- /dev/null >>> +++ b/arm/spe.c >>> @@ -0,0 +1,172 @@ >>> +/* >>> + * Copyright (C) 2020, Red Hat Inc, Eric Auger <eric.auger@redhat.com> >>> + * >>> + * 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 <stdint.h> >>> + >>> +#include <bitops.h> >>> +#include <devicetree.h> >>> +#include <libcflat.h> >>> + >>> +#include <asm/gic.h> >>> +#include <asm/processor.h> >>> +#include <asm/sysreg.h> >>> + >>> +#define ID_AA64DFR0_PMSVER_SHIFT 32 >>> +#define ID_AA64DFR0_PMSVER_MASK 0xf >>> + >>> +#define SYS_PMBIDR_EL1 sys_reg(3, 0, 9, 10, 7) >>> +#define SYS_PMBIDR_EL1_F_SHIFT 5 >>> +#define SYS_PMBIDR_EL1_P_SHIFT 4 >>> +#define SYS_PMBIDR_EL1_ALIGN_MASK 0xfUL >>> +#define SYS_PMBIDR_EL1_ALIGN_SHIFT 0 >>> + >>> +#define SYS_PMSIDR_EL1 sys_reg(3, 0, 9, 9, 7) >>> +#define SYS_PMSIDR_EL1_FE_SHIFT 0 >>> +#define SYS_PMSIDR_EL1_FT_SHIFT 1 >>> +#define SYS_PMSIDR_EL1_FL_SHIFT 2 >>> +#define SYS_PMSIDR_EL1_INTERVAL_SHIFT 8 >>> +#define SYS_PMSIDR_EL1_INTERVAL_MASK 0xfUL >>> +#define SYS_PMSIDR_EL1_MAXSIZE_SHIFT 12 >>> +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL >>> +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL >>> +#define SYS_PMSIDR_EL1_COUNTSIZE_SHIFT 16 >>> +#define SYS_PMSIDR_EL1_COUNTSIZE_MASK 0xfUL >>> + >>> +struct spe { >>> + uint32_t intid; >>> + int min_interval; >>> + int max_record_size; >>> + int countsize; >>> + bool fl_cap; >>> + bool ft_cap; >>> + bool fe_cap; >>> + int align; >>> +}; >>> +static struct spe spe; >>> + >>> +static int spe_min_interval(uint8_t interval) >>> +{ >>> + switch (interval) { >>> + case 0x0: >>> + return 256; >>> + case 0x2: >>> + return 512; >>> + case 0x3: >>> + return 768; >>> + case 0x4: >>> + return 1024; >>> + case 0x5: >>> + return 1536; >>> + case 0x6: >>> + return 2048; >>> + case 0x7: >>> + return 3072; >>> + case 0x8: >>> + return 4096; >>> + default: >>> + return 0; >>> + } >>> +} >>> + >>> +static bool spe_probe(void) >>> +{ >>> + const struct fdt_property *prop; >>> + const void *fdt = dt_fdt(); >>> + int node, len; >>> + uint32_t *data; >>> + uint64_t pmbidr, pmsidr; >>> + uint64_t aa64dfr0 = get_id_aa64dfr0(); >>> + uint8_t pmsver, interval; >>> + >>> + node = fdt_node_offset_by_compatible(fdt, -1, "arm,statistical-profiling-extension-v1"); >>> + assert(node >= 0); >>> + prop = fdt_get_property(fdt, node, "interrupts", &len); >>> + assert(prop && len == 3 * sizeof(u32)); >>> + >>> + data = (u32 *)prop->data; >>> + /* SPE interrupt is required to be a PPI. */ >>> + assert(fdt32_to_cpu(data[0]) == 1); >>> + spe.intid = fdt32_to_cpu(data[1]); >>> + >>> + pmsver = (aa64dfr0 >> ID_AA64DFR0_PMSVER_SHIFT) & ID_AA64DFR0_PMSVER_MASK; >>> + if (!pmsver || pmsver > 2) { >>> + report_info("Unknown SPE version: 0x%x", pmsver); >>> + return false; >>> + } >>> + >>> + pmbidr = read_sysreg_s(SYS_PMBIDR_EL1); >>> + if (pmbidr & BIT(SYS_PMBIDR_EL1_P_SHIFT)) { >>> + report_info("Profiling buffer owned by higher exception level"); >>> + return false; >>> + } >>> + >>> + spe.align = (pmbidr >> SYS_PMBIDR_EL1_ALIGN_SHIFT) & SYS_PMBIDR_EL1_ALIGN_MASK; >> you can remove ">> SYS_PMBIDR_EL1_ALIGN_SHIFT" here and >> SYS_PMBIDR_EL1_ALIGN_SHIFT (0) > > I prefer to keep it to match the way we extract the values for the other fields, > even though I realize it's unnecessary (the compiler will optimize it anyway). ok > >>> + spe.align = 1 << spe.align; >>> + >>> + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1); >>> + >>> + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK; >>> + spe.min_interval = spe_min_interval(interval); >>> + >>> + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \ >>> + SYS_PMSIDR_EL1_MAXSIZE_MASK; >>> + spe.max_record_size = 1 << spe.max_record_size; >>> + >>> + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \ >>> + SYS_PMSIDR_EL1_COUNTSIZE_MASK; >>> + >>> + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT); >>> + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT); >>> + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT); >> Why did you remove the report_info() section? I think those human >> readable info can be useful. > > I made them part of the test. Since the architecture says they are 1, I think > making sure their value matches is more useful than printing something that the > architecture guarantees. OK for those caps which are always 1 anyway but I was more thinking about report_info("Align= %d bytes, Min Interval=%d Single record Max Size = %d bytes", spe.align, spe.min_interval, spe.maxsize); I'd prefer to keep it. > >>> + >>> + return true; >>> +} >>> + >>> +static void spe_test_introspection(void) >>> +{ >>> + report_prefix_push("spe-introspection"); >> I see you moved this here. gic and pmu tests seem to leave them in the >> main, as done originally. > > You're right, consistency is good, will put them in main before calling the test > function. > >>> + >>> + report(spe.align <= SZ_2K, "PMBIDR_E1.Align"); >> good check added >>> + report(spe.countsize == 0x2, "PMSIDR_EL1.CountSize"); >>> + report(spe.max_record_size >= 16 && spe.max_record_size <= 2048, >>> + "PMSIDR_EL1 maximum record size"); >>> + report(spe.min_interval >= 256 && spe.min_interval <= 4096, >>> + "PMSIDR_EL1 minimum sampling interval"); >>> + report(spe.fl_cap && spe.ft_cap && spe.fe_cap, "PMSIDR_EL1 sampling filters"); >> indeed all bits read as 1 as the spec says. which is bit weird by the way. > > I think they added the fields because the architects wanted the flexibility to > allow changes in the future. OK Thanks Eric > > Thanks, > Alex >>> + >>> + report_prefix_pop(); >>> +} >>> + >>> +int main(int argc, char *argv[]) >>> +{ >>> + if (!spe_probe()) { >>> + report_skip("SPE not supported"); >>> + return report_summary(); >>> + } >>> + >>> + printf("intid: %u\n", PPI(spe.intid)); >>> + printf("align: %d\n", spe.align); >>> + printf("min_interval: %d\n", spe.min_interval); >>> + printf("max_record_size: %d\n", spe.max_record_size); >>> + >>> + if (argc < 2) >>> + report_abort("no test specified"); >>> + >>> + report_prefix_push("spe"); >>> + >>> + if (strcmp(argv[1], "spe-introspection") == 0) >>> + spe_test_introspection(); >>> + else >>> + report_abort("Unknown subtest '%s'", argv[1]); >>> + >>> + return report_summary(); >>> +} >>> diff --git a/arm/unittests.cfg b/arm/unittests.cfg >>> index f776b66ef96d..ad10be123774 100644 >>> --- a/arm/unittests.cfg >>> +++ b/arm/unittests.cfg >>> @@ -134,6 +134,13 @@ extra_params = -append 'pmu-overflow-interrupt' >>> #groups = pmu >>> #accel = tcg >>> >>> +[spe-introspection] >>> +file = spe.flat >>> +groups = spe >>> +arch = arm64 >>> +accel = kvm >>> +extra_params = -append 'spe-introspection' >>> + >>> # Test GIC emulation >>> [gicv2-ipi] >>> file = gic.flat >>> >> Thanks >> >> Eric >> >
Hi Eric, On 10/30/20 5:09 PM, Auger Eric wrote: > Hi Alexandru, > > On 10/30/20 4:59 PM, Alexandru Elisei wrote: > [..] >>>> + spe.align = 1 << spe.align; >>>> + >>>> + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1); >>>> + >>>> + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK; >>>> + spe.min_interval = spe_min_interval(interval); >>>> + >>>> + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \ >>>> + SYS_PMSIDR_EL1_MAXSIZE_MASK; >>>> + spe.max_record_size = 1 << spe.max_record_size; >>>> + >>>> + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \ >>>> + SYS_PMSIDR_EL1_COUNTSIZE_MASK; >>>> + >>>> + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT); >>>> + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT); >>>> + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT); >>> Why did you remove the report_info() section? I think those human >>> readable info can be useful. >> I made them part of the test. Since the architecture says they are 1, I think >> making sure their value matches is more useful than printing something that the >> architecture guarantees. > OK for those caps which are always 1 anyway but I was more thinking about > > report_info("Align= %d bytes, Min Interval=%d Single record Max Size = > %d bytes", spe.align, spe.min_interval, spe.maxsize); > > I'd prefer to keep it. Oh, I think I see what you mean, I chose to print them using printf in main(). This is very similar to what the timer test does, only it does it directly in main(), instead of calling another function (print_timer_info(), in the case of the timer test). I can move the printfs to spe_probe() if that's what you prefer. Thanks, Alex
Hi Alexandru, On 10/30/20 6:50 PM, Alexandru Elisei wrote: > Hi Eric, > > On 10/30/20 5:09 PM, Auger Eric wrote: >> Hi Alexandru, >> >> On 10/30/20 4:59 PM, Alexandru Elisei wrote: >> [..] >>>>> + spe.align = 1 << spe.align; >>>>> + >>>>> + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1); >>>>> + >>>>> + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK; >>>>> + spe.min_interval = spe_min_interval(interval); >>>>> + >>>>> + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \ >>>>> + SYS_PMSIDR_EL1_MAXSIZE_MASK; >>>>> + spe.max_record_size = 1 << spe.max_record_size; >>>>> + >>>>> + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \ >>>>> + SYS_PMSIDR_EL1_COUNTSIZE_MASK; >>>>> + >>>>> + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT); >>>>> + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT); >>>>> + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT); >>>> Why did you remove the report_info() section? I think those human >>>> readable info can be useful. >>> I made them part of the test. Since the architecture says they are 1, I think >>> making sure their value matches is more useful than printing something that the >>> architecture guarantees. >> OK for those caps which are always 1 anyway but I was more thinking about >> >> report_info("Align= %d bytes, Min Interval=%d Single record Max Size = >> %d bytes", spe.align, spe.min_interval, spe.maxsize); >> >> I'd prefer to keep it. > > Oh, I think I see what you mean, I chose to print them using printf in main(). > This is very similar to what the timer test does, only it does it directly in > main(), instead of calling another function (print_timer_info(), in the case of > the timer test). I can move the printfs to spe_probe() if that's what you prefer. Ah OK I did not notice. Whatever the place if those traces are there I am fine. Thanks Eric > > Thanks, > > Alex >
On Fri, Oct 30, 2020 at 05:50:35PM +0000, Alexandru Elisei wrote: > Hi Eric, > > On 10/30/20 5:09 PM, Auger Eric wrote: > > Hi Alexandru, > > > > On 10/30/20 4:59 PM, Alexandru Elisei wrote: > > [..] > >>>> + spe.align = 1 << spe.align; > >>>> + > >>>> + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1); > >>>> + > >>>> + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK; > >>>> + spe.min_interval = spe_min_interval(interval); > >>>> + > >>>> + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \ > >>>> + SYS_PMSIDR_EL1_MAXSIZE_MASK; > >>>> + spe.max_record_size = 1 << spe.max_record_size; > >>>> + > >>>> + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \ > >>>> + SYS_PMSIDR_EL1_COUNTSIZE_MASK; > >>>> + > >>>> + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT); > >>>> + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT); > >>>> + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT); > >>> Why did you remove the report_info() section? I think those human > >>> readable info can be useful. > >> I made them part of the test. Since the architecture says they are 1, I think > >> making sure their value matches is more useful than printing something that the > >> architecture guarantees. > > OK for those caps which are always 1 anyway but I was more thinking about > > > > report_info("Align= %d bytes, Min Interval=%d Single record Max Size = > > %d bytes", spe.align, spe.min_interval, spe.maxsize); > > > > I'd prefer to keep it. > > Oh, I think I see what you mean, I chose to print them using printf in main(). > This is very similar to what the timer test does, only it does it directly in > main(), instead of calling another function (print_timer_info(), in the case of > the timer test). I can move the printfs to spe_probe() if that's what you prefer. Please use report_info(). When tests fail it's popular to diff the failing results against the passing results. Some output changes each run, even in the same environment, and some output really isn't that important. We should keep important, non-test test results in INFO messages. Then, when diffing, we can filter out anything without a prefix. (BTW, I was cc'ed, or maybe bcc'ed, on this series from the beginning.) Thanks, drew > > Thanks, > > Alex >
diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64 index dbc7524d3070..94b9c63f0b05 100644 --- a/arm/Makefile.arm64 +++ b/arm/Makefile.arm64 @@ -30,6 +30,7 @@ OBJDIRS += lib/arm64 tests = $(TEST_DIR)/timer.flat tests += $(TEST_DIR)/micro-bench.flat tests += $(TEST_DIR)/cache.flat +tests += $(TEST_DIR)/spe.flat include $(SRCDIR)/$(TEST_DIR)/Makefile.common diff --git a/lib/libcflat.h b/lib/libcflat.h index ec0f58b05701..37550c99ffb6 100644 --- a/lib/libcflat.h +++ b/lib/libcflat.h @@ -37,6 +37,7 @@ #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) #define SZ_256 (1 << 8) +#define SZ_2K (1 << 11) #define SZ_4K (1 << 12) #define SZ_8K (1 << 13) #define SZ_16K (1 << 14) diff --git a/arm/spe.c b/arm/spe.c new file mode 100644 index 000000000000..c199cd239194 --- /dev/null +++ b/arm/spe.c @@ -0,0 +1,172 @@ +/* + * Copyright (C) 2020, Red Hat Inc, Eric Auger <eric.auger@redhat.com> + * + * 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 <stdint.h> + +#include <bitops.h> +#include <devicetree.h> +#include <libcflat.h> + +#include <asm/gic.h> +#include <asm/processor.h> +#include <asm/sysreg.h> + +#define ID_AA64DFR0_PMSVER_SHIFT 32 +#define ID_AA64DFR0_PMSVER_MASK 0xf + +#define SYS_PMBIDR_EL1 sys_reg(3, 0, 9, 10, 7) +#define SYS_PMBIDR_EL1_F_SHIFT 5 +#define SYS_PMBIDR_EL1_P_SHIFT 4 +#define SYS_PMBIDR_EL1_ALIGN_MASK 0xfUL +#define SYS_PMBIDR_EL1_ALIGN_SHIFT 0 + +#define SYS_PMSIDR_EL1 sys_reg(3, 0, 9, 9, 7) +#define SYS_PMSIDR_EL1_FE_SHIFT 0 +#define SYS_PMSIDR_EL1_FT_SHIFT 1 +#define SYS_PMSIDR_EL1_FL_SHIFT 2 +#define SYS_PMSIDR_EL1_INTERVAL_SHIFT 8 +#define SYS_PMSIDR_EL1_INTERVAL_MASK 0xfUL +#define SYS_PMSIDR_EL1_MAXSIZE_SHIFT 12 +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL +#define SYS_PMSIDR_EL1_MAXSIZE_MASK 0xfUL +#define SYS_PMSIDR_EL1_COUNTSIZE_SHIFT 16 +#define SYS_PMSIDR_EL1_COUNTSIZE_MASK 0xfUL + +struct spe { + uint32_t intid; + int min_interval; + int max_record_size; + int countsize; + bool fl_cap; + bool ft_cap; + bool fe_cap; + int align; +}; +static struct spe spe; + +static int spe_min_interval(uint8_t interval) +{ + switch (interval) { + case 0x0: + return 256; + case 0x2: + return 512; + case 0x3: + return 768; + case 0x4: + return 1024; + case 0x5: + return 1536; + case 0x6: + return 2048; + case 0x7: + return 3072; + case 0x8: + return 4096; + default: + return 0; + } +} + +static bool spe_probe(void) +{ + const struct fdt_property *prop; + const void *fdt = dt_fdt(); + int node, len; + uint32_t *data; + uint64_t pmbidr, pmsidr; + uint64_t aa64dfr0 = get_id_aa64dfr0(); + uint8_t pmsver, interval; + + node = fdt_node_offset_by_compatible(fdt, -1, "arm,statistical-profiling-extension-v1"); + assert(node >= 0); + prop = fdt_get_property(fdt, node, "interrupts", &len); + assert(prop && len == 3 * sizeof(u32)); + + data = (u32 *)prop->data; + /* SPE interrupt is required to be a PPI. */ + assert(fdt32_to_cpu(data[0]) == 1); + spe.intid = fdt32_to_cpu(data[1]); + + pmsver = (aa64dfr0 >> ID_AA64DFR0_PMSVER_SHIFT) & ID_AA64DFR0_PMSVER_MASK; + if (!pmsver || pmsver > 2) { + report_info("Unknown SPE version: 0x%x", pmsver); + return false; + } + + pmbidr = read_sysreg_s(SYS_PMBIDR_EL1); + if (pmbidr & BIT(SYS_PMBIDR_EL1_P_SHIFT)) { + report_info("Profiling buffer owned by higher exception level"); + return false; + } + + spe.align = (pmbidr >> SYS_PMBIDR_EL1_ALIGN_SHIFT) & SYS_PMBIDR_EL1_ALIGN_MASK; + spe.align = 1 << spe.align; + + pmsidr = read_sysreg_s(SYS_PMSIDR_EL1); + + interval = (pmsidr >> SYS_PMSIDR_EL1_INTERVAL_SHIFT) & SYS_PMSIDR_EL1_INTERVAL_MASK; + spe.min_interval = spe_min_interval(interval); + + spe.max_record_size = (pmsidr >> SYS_PMSIDR_EL1_MAXSIZE_SHIFT) & \ + SYS_PMSIDR_EL1_MAXSIZE_MASK; + spe.max_record_size = 1 << spe.max_record_size; + + spe.countsize = (pmsidr >> SYS_PMSIDR_EL1_COUNTSIZE_SHIFT) & \ + SYS_PMSIDR_EL1_COUNTSIZE_MASK; + + spe.fl_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FL_SHIFT); + spe.ft_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FT_SHIFT); + spe.fe_cap = pmsidr & BIT(SYS_PMSIDR_EL1_FE_SHIFT); + + return true; +} + +static void spe_test_introspection(void) +{ + report_prefix_push("spe-introspection"); + + report(spe.align <= SZ_2K, "PMBIDR_E1.Align"); + report(spe.countsize == 0x2, "PMSIDR_EL1.CountSize"); + report(spe.max_record_size >= 16 && spe.max_record_size <= 2048, + "PMSIDR_EL1 maximum record size"); + report(spe.min_interval >= 256 && spe.min_interval <= 4096, + "PMSIDR_EL1 minimum sampling interval"); + report(spe.fl_cap && spe.ft_cap && spe.fe_cap, "PMSIDR_EL1 sampling filters"); + + report_prefix_pop(); +} + +int main(int argc, char *argv[]) +{ + if (!spe_probe()) { + report_skip("SPE not supported"); + return report_summary(); + } + + printf("intid: %u\n", PPI(spe.intid)); + printf("align: %d\n", spe.align); + printf("min_interval: %d\n", spe.min_interval); + printf("max_record_size: %d\n", spe.max_record_size); + + if (argc < 2) + report_abort("no test specified"); + + report_prefix_push("spe"); + + if (strcmp(argv[1], "spe-introspection") == 0) + spe_test_introspection(); + else + report_abort("Unknown subtest '%s'", argv[1]); + + return report_summary(); +} diff --git a/arm/unittests.cfg b/arm/unittests.cfg index f776b66ef96d..ad10be123774 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -134,6 +134,13 @@ extra_params = -append 'pmu-overflow-interrupt' #groups = pmu #accel = tcg +[spe-introspection] +file = spe.flat +groups = spe +arch = arm64 +accel = kvm +extra_params = -append 'spe-introspection' + # Test GIC emulation [gicv2-ipi] file = gic.flat