[Skiboot] [PATCH] cpufeatures: Add tm-suspend-hypervisor-assist and tm-suspend-xer-so-bug node

Michael Neuling mikey at neuling.org
Thu Apr 4 17:30:38 AEDT 2019


On Wed, 2019-04-03 at 16:48 +1100, Stewart Smith wrote:
> tm-suspend-hypervisor-assist for P9 >=DD2.2
> And a tm-suspend-xer-so-bug node for P9 DD2.2 only.
> 
> I also treat P9P as P9 DD2.3 and add a unit test for the cpufeatures
> infrastructure.
> 
> Fixes: https://github.com/open-power/skiboot/issues/233
> Suggested-by: Paul Mackerras <paulus at ozlabs.org>
> Signed-off-by: Stewart Smith <stewart at linux.ibm.com>

Tested-by: Michael Neuling <mikey at neuling.org>

FWIW, tested in mambo by playing with the PVR and looking at what cpu features
bits get set in the early kernel boot log. Checked 2.1, 2.2 and 2.3 did what was
expected.

Mikey

> ---
>  core/cpufeatures.c          |  56 ++++++++++++-
>  core/test/Makefile.check    |   1 +
>  core/test/run-cpufeatures.c | 155 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 209 insertions(+), 3 deletions(-)
>  create mode 100644 core/test/run-cpufeatures.c
> 
> diff --git a/core/cpufeatures.c b/core/cpufeatures.c
> index 070419d9cfb7..530dc77f73c4 100644
> --- a/core/cpufeatures.c
> +++ b/core/cpufeatures.c
> @@ -1,4 +1,4 @@
> -/* Copyright 2017-2018 IBM Corp.
> +/* Copyright 2017-2019 IBM Corp.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -56,8 +56,12 @@
>  #define CPU_P8_DD1	(1U << 0)
>  #define CPU_P8_DD2	(1U << 1)
>  #define CPU_P9_DD1	(1U << 2)
> -#define CPU_P9_DD2	(1U << 3)
> +#define CPU_P9_DD2_0_1	(1U << 3) // 2.01 or 2.1
>  #define CPU_P9P		(1U << 4)
> +#define CPU_P9_DD2_2    (1U << 5)
> +#define CPU_P9_DD2_3    (1U << 6)
> +
> +#define CPU_P9_DD2      (CPU_P9_DD2_0_1|CPU_P9_DD2_2|CPU_P9_DD2_3|CPU_P9P)
>  
>  #define CPU_P8		(CPU_P8_DD1|CPU_P8_DD2)
>  #define CPU_P9		(CPU_P9_DD1|CPU_P9_DD2|CPU_P9P)
> @@ -721,6 +725,39 @@ static const struct cpu_feature cpu_features_table[] = {
>  	HV_NONE, OS_NONE,
>  	-1, -1, -1,
>  	NULL, },
> +
> +	/*
> +	 * Due to hardware bugs in POWER9, the hypervisor needs to assist
> +	 * guests.
> +	 *
> +	 * Presence of this feature indicates presence of the bug.
> +	 *
> +	 * See linux kernel commit 4bb3c7a0208f
> +	 * and linux Documentation/powerpc/transactional_memory.txt
> +	 */
> +	{ "tm-suspend-hypervisor-assist",
> +	CPU_P9_DD2_2|CPU_P9_DD2_3|CPU_P9P,
> +	ISA_V3_0B, USABLE_HV|USABLE_OS,
> +	HV_CUSTOM, OS_NONE,
> +	-1, -1, -1,
> +	NULL, },
> +
> +	/*
> +	 * Due to hardware bugs in POWER9, the hypervisor can hit
> +	 * CPU bugs in the operations it needs to do for
> +	 * tm-suspend-hypervisor-assist.
> +	 *
> +	 * Presence of this "feature" means processor is affected by the bug.
> +	 *
> +	 * See linux kernel commit 4bb3c7a0208f
> +	 * and linux Documentation/powerpc/transactional_memory.txt
> +	 */
> +	{ "tm-suspend-xer-so-bug",
> +	CPU_P9_DD2_2,
> +	ISA_V3_0B, USABLE_HV,
> +	HV_CUSTOM, OS_NONE,
> +	-1, -1, -1,
> +	NULL, },
>  };
>  
>  static void add_cpu_feature_nodeps(struct dt_node *features,
> @@ -905,7 +942,20 @@ void dt_add_cpufeatures(struct dt_node *root)
>  		if (is_power9n(version) &&
>  			   (PVR_VERS_MAJ(version) == 2)) {
>  			/* P9N DD2.x */
> -			cpu_feature_cpu = CPU_P9_DD2;
> +			switch (PVR_VERS_MIN(version)) {
> +			case 0:
> +			case 1:
> +				cpu_feature_cpu = CPU_P9_DD2_0_1;
> +				break;
> +			case 2:
> +				cpu_feature_cpu = CPU_P9_DD2_2;
> +				break;
> +			case 3:
> +				cpu_feature_cpu = CPU_P9_DD2_3;
> +				break;
> +			default:
> +				assert(0);
> +			}
>  		} else {
>  			assert(0);
>  		}
> diff --git a/core/test/Makefile.check b/core/test/Makefile.check
> index 0fb585e38d2d..8e59ef00e7b6 100644
> --- a/core/test/Makefile.check
> +++ b/core/test/Makefile.check
> @@ -1,6 +1,7 @@
>  # -*-Makefile-*-
>  CORE_TEST := \
>  	core/test/run-bitmap \
> +	core/test/run-cpufeatures \
>  	core/test/run-device \
>  	core/test/run-flash-subpartition \
>  	core/test/run-flash-firmware-versions \
> diff --git a/core/test/run-cpufeatures.c b/core/test/run-cpufeatures.c
> new file mode 100644
> index 000000000000..9db21440d6eb
> --- /dev/null
> +++ b/core/test/run-cpufeatures.c
> @@ -0,0 +1,155 @@
> +/* Copyright 2019 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * 	http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <skiboot.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +
> +/* Override this for testing. */
> +#define is_rodata(p) fake_is_rodata(p)
> +
> +char __rodata_start[16];
> +#define __rodata_end (__rodata_start + sizeof(__rodata_start))
> +
> +static inline bool fake_is_rodata(const void *p)
> +{
> +	return ((char *)p >= __rodata_start && (char *)p < __rodata_end);
> +}
> +
> +#define zalloc(bytes) calloc((bytes), 1)
> +
> +#include "../device.c"
> +#include <assert.h>
> +#include "../../test/dt_common.c"
> +
> +#define __TEST__
> +
> +static inline unsigned long mfspr(unsigned int spr);
> +
> +#include <ccan/str/str.c>
> +
> +#include "../cpufeatures.c"
> +
> +static unsigned long fake_pvr = PVR_TYPE_P7;
> +
> +static inline unsigned long mfspr(unsigned int spr)
> +{
> +	assert(spr == SPR_PVR);
> +	return fake_pvr;
> +}
> +
> +int main(void)
> +{
> +	struct dt_node *dt_root;
> +
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, true);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P8E << 16) | 0x100; // P8E DD1.0
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P8E << 16) | 0x200; // P8E DD2.0
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P8 << 16) | 0x100; // P8 DD1.0
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P8 << 16) | 0x200; // P8 DD2.0
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P8NVL << 16) | 0x100; // P8NVL DD1.0
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P9 << 16) | 0x200; // P9 DD2.0
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix"));
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P9 << 16) | 0x201; // P9 DD2.1
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix"));
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") == 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P9 << 16) | 0x202; // P9 DD2.2
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix"));
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") != 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") != 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P9 << 16) | 0x203; // P9 DD2.3
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix"));
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") != 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	fake_pvr = (PVR_TYPE_P9P << 16) | 0x100; // P9P DD1.0
> +	dt_root = dt_new_root("");
> +	dt_add_cpufeatures(dt_root);
> +	dump_dt(dt_root, 0, false);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/mmu-
> radix"));
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-hypervisor-assist") != 0);
> +	assert(dt_find_by_path(dt_root, "cpus/ibm,powerpc-cpu-features/tm-
> suspend-xer-so-bug") == 0);
> +	dt_free(dt_root);
> +
> +	exit(EXIT_SUCCESS);
> +}



More information about the Skiboot mailing list