On 2/8/23 12:06, Thomas Huth wrote:
On 02/02/2023 10.28, Pierre Morel 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@xxxxxxxxxxxxx> --- s390x/Makefile | 1 + s390x/topology.c | 155 ++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 3 + 3 files changed, 159 insertions(+) create mode 100644 s390x/topology.c diff --git a/s390x/Makefile b/s390x/Makefile index 52a9d82..b5fe8a3 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -39,6 +39,7 @@ tests += $(TEST_DIR)/panic-loop-extint.elf tests += $(TEST_DIR)/panic-loop-pgm.elf tests += $(TEST_DIR)/migration-sck.elf tests += $(TEST_DIR)/exittime.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..20f7ba2 --- /dev/null +++ b/s390x/topology.c @@ -0,0 +1,155 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * CPU Topology + * + * Copyright IBM Corp. 2022 + * + * Authors: + * Pierre Morel <pmorel@xxxxxxxxxxxxx> + */ + +#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( + " .insn rre,0xb9a20000,%1,0\n"Why are you specifying the instruction manually? I think both, GCC and Clang should know the "ptf" mnemonic, shouldn't they?
:D right !
+ " ipm %0\n" + " srl %0,28\n" + : "=d" (cc), "+d" (fc) + : + : "cc"); + + *rc = fc >> 8; + return cc; +} + +static void test_ptf(void) +{ + unsigned long rc; + int cc; + + /* PTF is a privilege instruction */s/privilege/privileged/ ?
Yes, thanks
+ report_prefix_push("Privilege"); + enter_pstate(); + expect_pgm_int(); + ptf(PTF_REQ_CHECK, &rc); + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); + report_prefix_pop(); + + report_prefix_push("Wrong fc"); + expect_pgm_int(); + ptf(0xff, &rc); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_push("Reserved bits"); + expect_pgm_int(); + ptf(0xffffffffffffff00UL, &rc); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop();This function is quite big ... I'd maybe group the above checks for error conditions into a separate function instead.
OK
+ 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(); + + report_prefix_push("Topology polarisation check"); + /* + * We can not assume the state of the polarization fors/can not/cannot/ ?
OK
Also, you sometimes write polarization with "z" and sometimes with "s". I'd suggest to standardize on "z" (as in "IBM Z" ;-))
OK
+ * any Virtual Machine but KVM. + * Let's skip the polarisation tests for other VMs. + */ + if (!host_is_kvm()) { + report_skip("Topology polarisation check is done for KVM only"); + goto end; + } + + /* + * 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."); + +end: + report_prefix_pop(); +}Apart from the nits, the patch looks fine to me. Thomas
Thanks, Regards. Pierre -- Pierre Morel IBM Lab Boeblingen
