| Message ID | 20220208132709.48291-4-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | S390x: CPU Topology Information | expand |
On Tue, 2022-02-08 at 14:27 +0100, Pierre Morel wrote: > We check the PTF instruction. You could test some very basic things as well: - you get a privileged pgm int in problem state, - reserved bits in first operand cause specification pgm int, - reserved FC values result in a specification pgm int, - second operand is ignored. > > - We do not expect to support vertical polarization. > > - We do not expect the Modified Topology Change Report to be [...] Forgive me if I'm missing something, but why _Modified_ Topology Change Report? > diff --git a/s390x/topology.c b/s390x/topology.c > new file mode 100644 > index 00000000..a1f9ce51 > --- /dev/null > +++ b/s390x/topology.c [...] > +static int ptf(unsigned long fc, unsigned long *rc) > +{ > + int cc; > + > + asm volatile( > + " .insn rre,0xb9a20000,%1,0\n" > + " ipm %0\n" > + " srl %0,28\n" > + : "=d" (cc), "+d" (fc) > + : "d" (fc) Why list fc here again?
On 2/9/22 12:37, Nico Boehr wrote: > On Tue, 2022-02-08 at 14:27 +0100, Pierre Morel wrote: >> We check the PTF instruction. > > You could test some very basic things as well: > > - you get a privileged pgm int in problem state, > - reserved bits in first operand cause specification pgm int, > - reserved FC values result in a specification pgm int, > - second operand is ignored. right, I will add these, I was more focused on the functionalities > >> >> - We do not expect to support vertical polarization. >> >> - We do not expect the Modified Topology Change Report to be > [...] > > Forgive me if I'm missing something, but why _Modified_ Topology Change > Report? > >> diff --git a/s390x/topology.c b/s390x/topology.c >> new file mode 100644 >> index 00000000..a1f9ce51 >> --- /dev/null >> +++ b/s390x/topology.c > > [...] > >> +static int ptf(unsigned long fc, unsigned long *rc) >> +{ >> + int cc; >> + >> + asm volatile( >> + " .insn rre,0xb9a20000,%1,0\n" >> + " ipm %0\n" >> + " srl %0,28\n" >> + : "=d" (cc), "+d" (fc) >> + : "d" (fc) > > Why list fc here again? > > No reason, I suppress it. Thanks for the review pierre
On 2/9/22 12:37, Nico Boehr wrote: > On Tue, 2022-02-08 at 14:27 +0100, Pierre Morel wrote: >> We check the PTF instruction. > > You could test some very basic things as well: > > - you get a privileged pgm int in problem state, > - reserved bits in first operand cause specification pgm int, > - reserved FC values result in a specification pgm int, > - second operand is ignored. > >> >> - We do not expect to support vertical polarization. >> >> - We do not expect the Modified Topology Change Report to be > [...] > > Forgive me if I'm missing something, but why _Modified_ Topology Change > Report? This does only exist in my mind the real name is indeed only Topology change report. > >> diff --git a/s390x/topology.c b/s390x/topology.c >> new file mode 100644 >> index 00000000..a1f9ce51 >> --- /dev/null >> +++ b/s390x/topology.c > > [...] > >> +static int ptf(unsigned long fc, unsigned long *rc) >> +{ >> + int cc; >> + >> + asm volatile( >> + " .insn rre,0xb9a20000,%1,0\n" >> + " ipm %0\n" >> + " srl %0,28\n" >> + : "=d" (cc), "+d" (fc) >> + : "d" (fc) > > Why list fc here again? > >
On 2/9/22 12:37, Nico Boehr wrote: > On Tue, 2022-02-08 at 14:27 +0100, Pierre Morel wrote: >> We check the PTF instruction. > > You could test some very basic things as well: > > - you get a privileged pgm int in problem state, > - reserved bits in first operand cause specification pgm int, > - reserved FC values result in a specification pgm int, > - second operand is ignored. Which second operand? > >> >> - We do not expect to support vertical polarization. >> >> - We do not expect the Modified Topology Change Report to be > [...] > > Forgive me if I'm missing something, but why _Modified_ Topology Change > Report? > >> diff --git a/s390x/topology.c b/s390x/topology.c >> new file mode 100644 >> index 00000000..a1f9ce51 >> --- /dev/null >> +++ b/s390x/topology.c > > [...] > >> +static int ptf(unsigned long fc, unsigned long *rc) >> +{ >> + int cc; >> + >> + asm volatile( >> + " .insn rre,0xb9a20000,%1,0\n" >> + " ipm %0\n" >> + " srl %0,28\n" >> + : "=d" (cc), "+d" (fc) >> + : "d" (fc) > > Why list fc here again? > >
On 2/15/22 09:50, Pierre Morel wrote: > > > On 2/9/22 12:37, Nico Boehr wrote: >> On Tue, 2022-02-08 at 14:27 +0100, Pierre Morel wrote: >>> We check the PTF instruction. >> >> You could test some very basic things as well: >> >> - you get a privileged pgm int in problem state, >> - reserved bits in first operand cause specification pgm int, >> - reserved FC values result in a specification pgm int, >> - second operand is ignored. > > Which second operand? Sorry got it > >> >>> >>> - We do not expect to support vertical polarization. >>> >>> - We do not expect the Modified Topology Change Report to be >> [...] >> >> Forgive me if I'm missing something, but why _Modified_ Topology Change >> Report? >> >>> diff --git a/s390x/topology.c b/s390x/topology.c >>> new file mode 100644 >>> index 00000000..a1f9ce51 >>> --- /dev/null >>> +++ b/s390x/topology.c >> >> [...] >> >>> +static int ptf(unsigned long fc, unsigned long *rc) >>> +{ >>> + int cc; >>> + >>> + asm volatile( >>> + " .insn rre,0xb9a20000,%1,0\n" >>> + " ipm %0\n" >>> + " srl %0,28\n" >>> + : "=d" (cc), "+d" (fc) >>> + : "d" (fc) >> >> Why list fc here again? >> >> >
On 2/15/22 10:21, Pierre Morel wrote: > > > On 2/15/22 09:50, Pierre Morel wrote: >> >> >> On 2/9/22 12:37, Nico Boehr wrote: >>> On Tue, 2022-02-08 at 14:27 +0100, Pierre Morel wrote: >>>> We check the PTF instruction. >>> >>> You could test some very basic things as well: >>> >>> - you get a privileged pgm int in problem state, >>> - reserved bits in first operand cause specification pgm int, >>> - reserved FC values result in a specification pgm int, >>> - second operand is ignored. >> >> Which second operand? > > Sorry got it I was a little fast in my answer, twice. If the second operand is ignored, how would you like to check something like that? We can check that the result of the instruction is identical for the known effects the user can check what ever we put in there but how can we know if it is really ignored? >> >>> >>>> >>>> - We do not expect to support vertical polarization. >>>> >>>> - We do not expect the Modified Topology Change Report to be >>> [...] >>> >>> Forgive me if I'm missing something, but why _Modified_ Topology Change >>> Report? >>> >>>> diff --git a/s390x/topology.c b/s390x/topology.c >>>> new file mode 100644 >>>> index 00000000..a1f9ce51 >>>> --- /dev/null >>>> +++ b/s390x/topology.c >>> >>> [...] >>> >>>> +static int ptf(unsigned long fc, unsigned long *rc) >>>> +{ >>>> + int cc; >>>> + >>>> + asm volatile( >>>> + " .insn rre,0xb9a20000,%1,0\n" >>>> + " ipm %0\n" >>>> + " srl %0,28\n" >>>> + : "=d" (cc), "+d" (fc) >>>> + : "d" (fc) >>> >>> Why list fc here again? >>> >>> >> >
On Tue, 2022-02-15 at 10:44 +0100, Pierre Morel wrote: > > > > - second operand is ignored. > > > > > > Which second operand? > > > > Sorry got it > > I was a little fast in my answer, twice. > If the second operand is ignored, how would you like to check > something > like that? > We can check that the result of the instruction is identical for the > known effects the user can check what ever we put in there but how > can > we know if it is really ignored? Yes, there is no 100% guarantee. If you think there is no value or it's too difficult to test for the value it adds, it is fine for me if you leave it out. Your suggestion sounds fine, though.
diff --git a/s390x/Makefile b/s390x/Makefile index 53b0fe04..d833d6a3 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -26,6 +26,7 @@ tests += $(TEST_DIR)/edat.elf tests += $(TEST_DIR)/mvpg-sie.elf tests += $(TEST_DIR)/spec_ex-sie.elf tests += $(TEST_DIR)/firq.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 00000000..a1f9ce51 --- /dev/null +++ b/s390x/topology.c @@ -0,0 +1,115 @@ +/* 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/vm.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 + +static int ptf(unsigned long fc, unsigned long *rc) +{ + int cc; + + asm volatile( + " .insn rre,0xb9a20000,%1,0\n" + " ipm %0\n" + " srl %0,28\n" + : "=d" (cc), "+d" (fc) + : "d" (fc) + : "cc"); + + *rc = fc >> 8; + return cc; +} + +static void test_ptf(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(); + + report_prefix_push("Topology polarisation check"); + /* + * We can not assume the state of the polarization for + * any Virtual Machine but KVM. + * Let's skip the polarisation tests for other VMs. + */ + if (!vm_is_kvm()) { + report_skip("Topology polarisation check is done for KVM only"); + goto end; + } + + cc = ptf(PTF_REQ_HORIZONTAL, &rc); + report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED, + "KVM always provides horizontal polarization"); + + cc = ptf(PTF_REQ_VERTICAL, &rc); + report(cc == 2 && rc == PTF_ERR_NO_REASON, + "KVM doesn't support vertical polarization."); + +end: + report_prefix_pop(); +} + +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("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 054560c2..e2d3e6a5 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -122,3 +122,6 @@ extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=1 -devi file = firq.elf timeout = 20 extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=2 -device qemu-s390x-cpu,core-id=1 + +[topology] +file = topology.elf
We check the PTF instruction. - We do not expect to support vertical polarization. - We do not expect the Modified Topology Change Report to be pending or not at the moment the first PTF instruction with PTF_CHECK function code is done as some code already did run a polarization change may have occur. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- s390x/Makefile | 1 + s390x/topology.c | 115 ++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 3 ++ 3 files changed, 119 insertions(+) create mode 100644 s390x/topology.c