On Wed, Jun 06, 2018 at 10:36:44AM -0400, Babu Moger wrote:
> topoext feature cannot be supported in certain cases
> with large number of cores or threads. Add the check.
>
> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> ---
> target/i386/cpu.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 86fb1a4..fc5c66d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -509,6 +509,20 @@ static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
> }
>
> /*
> + * Check if we can support this topology
> + * Fail if number of cores are beyond the supported config
> + * or nr_threads is more than 2
> + */
> +static int topology_supports_topoext(int nr_cores, int nr_threads)
> +{
> + if ((nr_cores > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET)) ||
> + (nr_threads > 2)) {
> + return 0;
> + }
> + return 1;
> +}
> +
> +/*
> * Definitions of the hardcoded cache entries we expose:
> * These are legacy cache values. If there is a need to change any
> * of these values please use builtin_x86_defs
> @@ -4941,6 +4955,19 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>
> qemu_init_vcpu(cs);
>
> + /* On AMD systems, check if we can support topoext feature */
> + if (IS_AMD_CPU(env) &&
> + (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT)) {
> + if (!topology_supports_topoext(cs->nr_cores, cs->nr_threads)) {
> + /* Cannot support topoext */
> + error_setg(errp, "CPU model does not support topoext feature "
> + "with number of cores(%d) and threads(%d). "
> + "Please configure -smp options properly.",
> + cs->nr_cores, cs->nr_threads);
See error.h documentation:
* Error reporting system loosely patterned after Glib's GError.
*
* Create an error:
* error_setg(&err, "situation normal, all fouled up");
*
* Create an error and add additional explanation:
* error_setg(&err, "invalid quark");
* error_append_hint(&err, "Valid quarks are up, down, strange, "
* "charm, top, bottom.\n");
*
* Do *not* contract this to
* error_setg(&err, "invalid quark\n"
* "Valid quarks are up, down, strange, charm, top, bottom.");
*
I suggest something like this:
static bool topology_supports_topoext(int nr_cores, int nr_threads, Error **errp)
{
if (nr_cores > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET))) {
error_setg(errp, "TOPOEXT unsupported with %d cores per socket", nr_cores);
error_append_hint(errp, "TOPOEXT supports only up to %d cores per socket",
(MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
return false;
}
if (nr_threads > 2) {
error_setg(errp, "TOPOEXT unsupported with %d threads per core", nr_threads);
error_append_hint(errp, "TOPOEXT supports only up to 2 threads per core");
(MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
return false;
}
return true;
}
static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
{
/* ... */
if (IS_AMD_CPU(env) &&
(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
!topology_supports_topoext(cs->nr_cores, cs->nr_threads, errp)) {
return;
}
/* ... */
}
--
Eduardo
