Re: [kvm-unit-tests PATCH v8 2/2] s390x: topology: Checking Configuration Topology Information

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Pierre Morel (2023-04-26 10:34:26)
> diff --git a/s390x/topology.c b/s390x/topology.c
> index 07f1650..42a9cc9 100644
> --- a/s390x/topology.c
> +++ b/s390x/topology.c
> @@ -17,6 +17,20 @@
[...]
> +/**
> + * check_tle:
> + * @tc: pointer to first TLE
> + *
> + * Recursively check the containers TLEs until we
> + * find a CPU TLE.
> + */
> +static uint8_t *check_tle(void *tc)
> +{
[...]
> +
> +       report(!cpus->d || (cpus->pp == 3 || cpus->pp == 0),
> +              "Dedication versus entitlement");

Again, I would prefer something like this:

if (!cpus->d)
    report_skip("Not dedicated")
else
    report(cpus->pp == 3 || cpus->pp == 0, "Dedicated CPUs are either vertically polarized or have high entitlement")

No?

[...]

> +/**
> + * check_sysinfo_15_1_x:
> + * @info: pointer to the STSI info structure
> + * @sel2: the selector giving the topology level to check
> + *
> + * Check if the validity of the STSI instruction and then
> + * calls specific checks on the information buffer.
> + */
> +static void check_sysinfo_15_1_x(struct sysinfo_15_1_x *info, int sel2)
> +{
> +       int ret;
> +       int cc;
> +       unsigned long rc;
> +
> +       report_prefix_pushf("15_1_%d", sel2);
> +
> +       ret = stsi_get_sysib(info, sel2);
> +       if (ret) {
> +               report_skip("Selector 2 not supported by architecture");
> +               goto end;
> +       }
> +
> +       report_prefix_pushf("H");
> +       cc = ptf(PTF_REQ_HORIZONTAL, &rc);
> +       if (cc != 0 && rc != PTF_ERR_ALRDY_POLARIZED) {
> +               report(0, "Unable to set horizontal polarization");

report_fail() please

> +               goto vertical;
> +       }
> +
> +       stsi_check_mag(info);
> +       stsi_check_tle_coherency(info);
> +
> +vertical:
> +       report_prefix_pop();
> +       report_prefix_pushf("V");
> +
> +       cc = ptf(PTF_REQ_VERTICAL, &rc);
> +       if (cc != 0 && rc != PTF_ERR_ALRDY_POLARIZED) {
> +               report(0, "Unable to set vertical polarization");

report_fail() please

[...]
> +static int arch_max_cpus(void)

Does the name arch_max_cpus() make sense? Maybe expected_num_cpus()?

> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index fc3666b..375e6ce 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -221,3 +221,6 @@ file = ex.elf
>  
>  [topology]
>  file = topology.elf
> +# 3 CPUs on socket 0 with different CPU TLE (standard, dedicated, origin)
> +# 1 CPU on socket 2
> +extra_params = -smp 1,drawers=3,books=3,sockets=4,cores=4,maxcpus=144 -cpu z14,ctop=on -device z14-s390x-cpu,core-id=1,entitlement=low -device z14-s390x-cpu,core-id=2,dedicated=on -device z14-s390x-cpu,core-id=10 -device z14-s390x-cpu,core-id=20 -device z14-s390x-cpu,core-id=130,socket-id=0,book-id=0,drawer-id=0 -append '-drawers 3 -books 3 -sockets 4 -cores 4'

If I got the command line right, all CPUs are on the same drawer with this command line, aren't they? If so, does it make sense to run with different combinations, i.e. CPUs on different drawers, books etc?




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux