Message ID | 20230320085642.12251-2-pmorel@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | S390x: CPU Topology Information | expand |
On Mon, 20 Mar 2023 09:56:41 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > We check that the PTF instruction is working correctly when > the cpu topology facility is available. > > For KVM only, we test changing of the polarity between horizontal > and vertical and that a reset set the horizontal polarity. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > s390x/Makefile | 1 + > s390x/topology.c | 180 ++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 3 + > 3 files changed, 184 insertions(+) > create mode 100644 s390x/topology.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index e94b720..05dac04 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -40,6 +40,7 @@ tests += $(TEST_DIR)/panic-loop-pgm.elf > tests += $(TEST_DIR)/migration-sck.elf > tests += $(TEST_DIR)/exittime.elf > tests += $(TEST_DIR)/ex.elf > +tests += $(TEST_DIR)/topology.elf > > pv-tests += $(TEST_DIR)/pv-diags.elf > > diff --git a/s390x/topology.c b/s390x/topology.c > new file mode 100644 > index 0000000..ce248f1 > --- /dev/null > +++ b/s390x/topology.c > @@ -0,0 +1,180 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * CPU Topology > + * > + * Copyright IBM Corp. 2022 > + * > + * Authors: > + * Pierre Morel <pmorel@linux.ibm.com> > + */ > + > +#include <libcflat.h> > +#include <asm/page.h> > +#include <asm/asm-offsets.h> > +#include <asm/interrupt.h> > +#include <asm/facility.h> > +#include <smp.h> > +#include <sclp.h> > +#include <s390x/hardware.h> > + > +#define PTF_REQ_HORIZONTAL 0 > +#define PTF_REQ_VERTICAL 1 > +#define PTF_REQ_CHECK 2 > + > +#define PTF_ERR_NO_REASON 0 > +#define PTF_ERR_ALRDY_POLARIZED 1 > +#define PTF_ERR_IN_PROGRESS 2 > + > +extern int diag308_load_reset(u64); > + > +static int ptf(unsigned long fc, unsigned long *rc) > +{ > + int cc; > + > + asm volatile( > + " ptf %1 \n" > + " ipm %0 \n" > + " srl %0,28 \n" > + : "=d" (cc), "+d" (fc) > + : > + : "cc"); > + > + *rc = fc >> 8; > + return cc; > +} > + > +static void check_privilege(int fc) > +{ > + unsigned long rc; > + > + report_prefix_push("Privilege"); > + report_info("function code %d", fc); > + enter_pstate(); > + expect_pgm_int(); > + ptf(fc, &rc); > + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); > + report_prefix_pop(); > +} > + > +static void check_function_code(void) > +{ > + unsigned long rc; > + > + report_prefix_push("Undefined fc"); > + expect_pgm_int(); > + ptf(0xff, &rc); please don't use magic numbers, add a new macro PTF_INVALID_FUNCTION (or something like that) > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > +} > + > +static void check_reserved_bits(void) > +{ > + unsigned long rc; > + > + report_prefix_push("Reserved bits"); > + expect_pgm_int(); > + ptf(0xffffffffffffff00UL, &rc); I would like every single bit to be tested, since all of them are required to be zero. make a loop and test each, but please report success of failure only once at the end. use a report_info in case of failure to indicate which bit failed > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > +} > + > +static void check_mtcr_pending(void) > +{ > + unsigned long rc; > + int cc; > + > + report_prefix_push("Topology Report pending"); > + /* > + * At this moment the topology may already have changed > + * since the VM has been started. > + * However, we can test if a second PTF instruction > + * reports that the topology did not change since the > + * preceding PFT instruction. > + */ > + ptf(PTF_REQ_CHECK, &rc); > + cc = ptf(PTF_REQ_CHECK, &rc); > + report(cc == 0, "PTF check should clear topology report"); > + report_prefix_pop(); > +} > + > +static void check_polarization_change(void) > +{ > + unsigned long rc; > + int cc; > + > + report_prefix_push("Topology polarization check"); > + > + /* We expect a clean state through reset */ > + report(diag308_load_reset(1), "load normal reset done"); > + > + /* > + * Set vertical polarization to verify that RESET sets > + * horizontal polarization back. > + */ > + cc = ptf(PTF_REQ_VERTICAL, &rc); > + report(cc == 0, "Set vertical polarization."); > + > + report(diag308_load_reset(1), "load normal reset done"); > + > + cc = ptf(PTF_REQ_CHECK, &rc); > + report(cc == 0, "Reset should clear topology report"); > + > + cc = ptf(PTF_REQ_HORIZONTAL, &rc); > + report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED, > + "After RESET polarization is horizontal"); > + > + /* Flip between vertical and horizontal polarization */ > + cc = ptf(PTF_REQ_VERTICAL, &rc); > + report(cc == 0, "Change to vertical polarization."); either here or in a new block, test that setting vertical twice in a row will also result in a cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED > + > + cc = ptf(PTF_REQ_CHECK, &rc); > + report(cc == 1, "Polarization change should set topology report"); > + > + cc = ptf(PTF_REQ_HORIZONTAL, &rc); > + report(cc == 0, "Change to horizontal polarization."); it cannot hurt to add here another check for pending reports > + > + report_prefix_pop(); > +} > + > +static void test_ptf(void) > +{ > + check_privilege(PTF_REQ_HORIZONTAL); > + check_privilege(PTF_REQ_VERTICAL); > + check_privilege(PTF_REQ_CHECK); > + check_function_code(); > + check_reserved_bits(); > + check_mtcr_pending(); > + check_polarization_change(); > +} > + > +static struct { > + const char *name; > + void (*func)(void); > +} tests[] = { > + { "PTF", test_ptf}, > + { NULL, NULL } > +}; > + > +int main(int argc, char *argv[]) > +{ > + int i; > + > + report_prefix_push("CPU Topology"); > + > + if (!test_facility(11)) { > + report_skip("Topology facility not present"); > + goto end; > + } > + > + report_info("Virtual machine level %ld", stsi_get_fc()); > + > + for (i = 0; tests[i].name; i++) { > + report_prefix_push(tests[i].name); > + tests[i].func(); > + report_prefix_pop(); > + } > + > +end: > + report_prefix_pop(); > + return report_summary(); > +} > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg > index 453ee9c..d0ac683 100644 > --- a/s390x/unittests.cfg > +++ b/s390x/unittests.cfg > @@ -233,3 +233,6 @@ extra_params = -append '--parallel' > > [execute] > file = ex.elf > + > +[topology] > +file = topology.elf
Quoting Pierre Morel (2023-03-20 09:56:41) [...] > diff --git a/s390x/topology.c b/s390x/topology.c > new file mode 100644 > index 0000000..ce248f1 > --- /dev/null > +++ b/s390x/topology.c > @@ -0,0 +1,180 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * CPU Topology > + * > + * Copyright IBM Corp. 2022 > + * > + * Authors: > + * Pierre Morel <pmorel@linux.ibm.com> > + */ > + > +#include <libcflat.h> > +#include <asm/page.h> > +#include <asm/asm-offsets.h> > +#include <asm/interrupt.h> > +#include <asm/facility.h> > +#include <smp.h> > +#include <sclp.h> > +#include <s390x/hardware.h> > + > +#define PTF_REQ_HORIZONTAL 0 > +#define PTF_REQ_VERTICAL 1 > +#define PTF_REQ_CHECK 2 These are all function codes, so how about we name these defines PTF_FC_... and since PTF_REQ_CHECK doesn't request anything we should rename to PTF_FC_CHECK [...] > +static struct { > + const char *name; > + void (*func)(void); > +} tests[] = { > + { "PTF", test_ptf}, missing space ^
On 3/23/23 16:45, Claudio Imbrenda wrote: > On Mon, 20 Mar 2023 09:56:41 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> We check that the PTF instruction is working correctly when >> the cpu topology facility is available. >> >> For KVM only, we test changing of the polarity between horizontal >> and vertical and that a reset set the horizontal polarity. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> s390x/Makefile | 1 + >> s390x/topology.c | 180 ++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 3 + >> 3 files changed, 184 insertions(+) >> create mode 100644 s390x/topology.c >> >> diff --git a/s390x/Makefile b/s390x/Makefile >> index e94b720..05dac04 100644 >> --- a/s390x/Makefile >> +++ b/s390x/Makefile >> @@ -40,6 +40,7 @@ tests += $(TEST_DIR)/panic-loop-pgm.elf >> tests += $(TEST_DIR)/migration-sck.elf >> tests += $(TEST_DIR)/exittime.elf >> tests += $(TEST_DIR)/ex.elf >> +tests += $(TEST_DIR)/topology.elf >> >> pv-tests += $(TEST_DIR)/pv-diags.elf >> >> diff --git a/s390x/topology.c b/s390x/topology.c >> new file mode 100644 >> index 0000000..ce248f1 >> --- /dev/null >> +++ b/s390x/topology.c >> @@ -0,0 +1,180 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * CPU Topology >> + * >> + * Copyright IBM Corp. 2022 >> + * >> + * Authors: >> + * Pierre Morel <pmorel@linux.ibm.com> >> + */ >> + >> +#include <libcflat.h> >> +#include <asm/page.h> >> +#include <asm/asm-offsets.h> >> +#include <asm/interrupt.h> >> +#include <asm/facility.h> >> +#include <smp.h> >> +#include <sclp.h> >> +#include <s390x/hardware.h> >> + >> +#define PTF_REQ_HORIZONTAL 0 >> +#define PTF_REQ_VERTICAL 1 >> +#define PTF_REQ_CHECK 2 >> + >> +#define PTF_ERR_NO_REASON 0 >> +#define PTF_ERR_ALRDY_POLARIZED 1 >> +#define PTF_ERR_IN_PROGRESS 2 >> + >> +extern int diag308_load_reset(u64); >> + >> +static int ptf(unsigned long fc, unsigned long *rc) >> +{ >> + int cc; >> + >> + asm volatile( >> + " ptf %1 \n" >> + " ipm %0 \n" >> + " srl %0,28 \n" >> + : "=d" (cc), "+d" (fc) >> + : >> + : "cc"); >> + >> + *rc = fc >> 8; >> + return cc; >> +} >> + >> +static void check_privilege(int fc) >> +{ >> + unsigned long rc; >> + >> + report_prefix_push("Privilege"); >> + report_info("function code %d", fc); >> + enter_pstate(); >> + expect_pgm_int(); >> + ptf(fc, &rc); >> + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); >> + report_prefix_pop(); >> +} >> + >> +static void check_function_code(void) >> +{ >> + unsigned long rc; >> + >> + report_prefix_push("Undefined fc"); >> + expect_pgm_int(); >> + ptf(0xff, &rc); > please don't use magic numbers, add a new macro PTF_INVALID_FUNCTION > (or something like that) OK > >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> +} >> + >> +static void check_reserved_bits(void) >> +{ >> + unsigned long rc; >> + >> + report_prefix_push("Reserved bits"); >> + expect_pgm_int(); >> + ptf(0xffffffffffffff00UL, &rc); > I would like every single bit to be tested, since all of them are > required to be zero. > > make a loop and test each, but please report success of failure only > once at the end. > use a report_info in case of failure to indicate which bit failed OK > >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> +} >> + >> +static void check_mtcr_pending(void) >> +{ >> + unsigned long rc; >> + int cc; >> + >> + report_prefix_push("Topology Report pending"); >> + /* >> + * At this moment the topology may already have changed >> + * since the VM has been started. >> + * However, we can test if a second PTF instruction >> + * reports that the topology did not change since the >> + * preceding PFT instruction. >> + */ >> + ptf(PTF_REQ_CHECK, &rc); >> + cc = ptf(PTF_REQ_CHECK, &rc); >> + report(cc == 0, "PTF check should clear topology report"); >> + report_prefix_pop(); >> +} >> + >> +static void check_polarization_change(void) >> +{ >> + unsigned long rc; >> + int cc; >> + >> + report_prefix_push("Topology polarization check"); >> + >> + /* We expect a clean state through reset */ >> + report(diag308_load_reset(1), "load normal reset done"); >> + >> + /* >> + * Set vertical polarization to verify that RESET sets >> + * horizontal polarization back. >> + */ >> + cc = ptf(PTF_REQ_VERTICAL, &rc); >> + report(cc == 0, "Set vertical polarization."); >> + >> + report(diag308_load_reset(1), "load normal reset done"); >> + >> + cc = ptf(PTF_REQ_CHECK, &rc); >> + report(cc == 0, "Reset should clear topology report"); >> + >> + cc = ptf(PTF_REQ_HORIZONTAL, &rc); >> + report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED, >> + "After RESET polarization is horizontal"); >> + >> + /* Flip between vertical and horizontal polarization */ >> + cc = ptf(PTF_REQ_VERTICAL, &rc); >> + report(cc == 0, "Change to vertical polarization."); > either here or in a new block, test that setting vertical twice in > a row will also result in a cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED OK > >> + >> + cc = ptf(PTF_REQ_CHECK, &rc); >> + report(cc == 1, "Polarization change should set topology report"); >> + >> + cc = ptf(PTF_REQ_HORIZONTAL, &rc); >> + report(cc == 0, "Change to horizontal polarization."); > it cannot hurt to add here another check for pending reports OK Thanks for the comments, Regards, Pierre
On 3/24/23 11:11, Nico Boehr wrote: > Quoting Pierre Morel (2023-03-20 09:56:41) > [...] >> diff --git a/s390x/topology.c b/s390x/topology.c >> new file mode 100644 >> index 0000000..ce248f1 >> --- /dev/null >> +++ b/s390x/topology.c >> @@ -0,0 +1,180 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * CPU Topology >> + * >> + * Copyright IBM Corp. 2022 >> + * >> + * Authors: >> + * Pierre Morel <pmorel@linux.ibm.com> >> + */ >> + >> +#include <libcflat.h> >> +#include <asm/page.h> >> +#include <asm/asm-offsets.h> >> +#include <asm/interrupt.h> >> +#include <asm/facility.h> >> +#include <smp.h> >> +#include <sclp.h> >> +#include <s390x/hardware.h> >> + >> +#define PTF_REQ_HORIZONTAL 0 >> +#define PTF_REQ_VERTICAL 1 >> +#define PTF_REQ_CHECK 2 > These are all function codes, so how about we name these defines PTF_FC_... > > and since PTF_REQ_CHECK doesn't request anything we should rename to PTF_FC_CHECK OK > > [...] >> +static struct { >> + const char *name; >> + void (*func)(void); >> +} tests[] = { >> + { "PTF", test_ptf}, > missing space ^ Yes, thanks. Regards, Pierre
diff --git a/s390x/Makefile b/s390x/Makefile index e94b720..05dac04 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -40,6 +40,7 @@ tests += $(TEST_DIR)/panic-loop-pgm.elf tests += $(TEST_DIR)/migration-sck.elf tests += $(TEST_DIR)/exittime.elf tests += $(TEST_DIR)/ex.elf +tests += $(TEST_DIR)/topology.elf pv-tests += $(TEST_DIR)/pv-diags.elf diff --git a/s390x/topology.c b/s390x/topology.c new file mode 100644 index 0000000..ce248f1 --- /dev/null +++ b/s390x/topology.c @@ -0,0 +1,180 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * CPU Topology + * + * Copyright IBM Corp. 2022 + * + * Authors: + * Pierre Morel <pmorel@linux.ibm.com> + */ + +#include <libcflat.h> +#include <asm/page.h> +#include <asm/asm-offsets.h> +#include <asm/interrupt.h> +#include <asm/facility.h> +#include <smp.h> +#include <sclp.h> +#include <s390x/hardware.h> + +#define PTF_REQ_HORIZONTAL 0 +#define PTF_REQ_VERTICAL 1 +#define PTF_REQ_CHECK 2 + +#define PTF_ERR_NO_REASON 0 +#define PTF_ERR_ALRDY_POLARIZED 1 +#define PTF_ERR_IN_PROGRESS 2 + +extern int diag308_load_reset(u64); + +static int ptf(unsigned long fc, unsigned long *rc) +{ + int cc; + + asm volatile( + " ptf %1 \n" + " ipm %0 \n" + " srl %0,28 \n" + : "=d" (cc), "+d" (fc) + : + : "cc"); + + *rc = fc >> 8; + return cc; +} + +static void check_privilege(int fc) +{ + unsigned long rc; + + report_prefix_push("Privilege"); + report_info("function code %d", fc); + enter_pstate(); + expect_pgm_int(); + ptf(fc, &rc); + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); + report_prefix_pop(); +} + +static void check_function_code(void) +{ + unsigned long rc; + + report_prefix_push("Undefined fc"); + expect_pgm_int(); + ptf(0xff, &rc); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); +} + +static void check_reserved_bits(void) +{ + unsigned long rc; + + report_prefix_push("Reserved bits"); + expect_pgm_int(); + ptf(0xffffffffffffff00UL, &rc); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); +} + +static void check_mtcr_pending(void) +{ + unsigned long rc; + int cc; + + report_prefix_push("Topology Report pending"); + /* + * At this moment the topology may already have changed + * since the VM has been started. + * However, we can test if a second PTF instruction + * reports that the topology did not change since the + * preceding PFT instruction. + */ + ptf(PTF_REQ_CHECK, &rc); + cc = ptf(PTF_REQ_CHECK, &rc); + report(cc == 0, "PTF check should clear topology report"); + report_prefix_pop(); +} + +static void check_polarization_change(void) +{ + unsigned long rc; + int cc; + + report_prefix_push("Topology polarization check"); + + /* We expect a clean state through reset */ + report(diag308_load_reset(1), "load normal reset done"); + + /* + * Set vertical polarization to verify that RESET sets + * horizontal polarization back. + */ + cc = ptf(PTF_REQ_VERTICAL, &rc); + report(cc == 0, "Set vertical polarization."); + + report(diag308_load_reset(1), "load normal reset done"); + + cc = ptf(PTF_REQ_CHECK, &rc); + report(cc == 0, "Reset should clear topology report"); + + cc = ptf(PTF_REQ_HORIZONTAL, &rc); + report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED, + "After RESET polarization is horizontal"); + + /* Flip between vertical and horizontal polarization */ + cc = ptf(PTF_REQ_VERTICAL, &rc); + report(cc == 0, "Change to vertical polarization."); + + cc = ptf(PTF_REQ_CHECK, &rc); + report(cc == 1, "Polarization change should set topology report"); + + cc = ptf(PTF_REQ_HORIZONTAL, &rc); + report(cc == 0, "Change to horizontal polarization."); + + report_prefix_pop(); +} + +static void test_ptf(void) +{ + check_privilege(PTF_REQ_HORIZONTAL); + check_privilege(PTF_REQ_VERTICAL); + check_privilege(PTF_REQ_CHECK); + check_function_code(); + check_reserved_bits(); + check_mtcr_pending(); + check_polarization_change(); +} + +static struct { + const char *name; + void (*func)(void); +} tests[] = { + { "PTF", test_ptf}, + { NULL, NULL } +}; + +int main(int argc, char *argv[]) +{ + int i; + + report_prefix_push("CPU Topology"); + + if (!test_facility(11)) { + report_skip("Topology facility not present"); + goto end; + } + + report_info("Virtual machine level %ld", stsi_get_fc()); + + for (i = 0; tests[i].name; i++) { + report_prefix_push(tests[i].name); + tests[i].func(); + report_prefix_pop(); + } + +end: + report_prefix_pop(); + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index 453ee9c..d0ac683 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -233,3 +233,6 @@ extra_params = -append '--parallel' [execute] file = ex.elf + +[topology] +file = topology.elf
We check that the PTF instruction is working correctly when the cpu topology facility is available. For KVM only, we test changing of the polarity between horizontal and vertical and that a reset set the horizontal polarity. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- s390x/Makefile | 1 + s390x/topology.c | 180 ++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 3 + 3 files changed, 184 insertions(+) create mode 100644 s390x/topology.c