* [PATCH v6 01/10] Revert "i386: Fix pkg_id offset for EPYC cpu models"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:11 ` Igor Mammedov
2020-08-31 18:42 ` [PATCH v6 02/10] Revert "target/i386: Enable new apic id encoding for EPYC based cpus models" Babu Moger
` (9 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit 7b225762c8c05fd31d4c2be116aedfbc00383f8b.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Also fix all the references of pkg_offset.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
hw/i386/pc.c | 1 -
target/i386/cpu.c | 9 ++++-----
target/i386/cpu.h | 1 -
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5d8d5ef8b3..6b708f4341 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1502,7 +1502,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
env->nr_dies = x86ms->smp_dies;
env->nr_nodes = topo_info.nodes_per_pkg;
- env->pkg_offset = x86ms->apicid_pkg_offset(&topo_info);
/*
* If APIC ID is not set,
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 588f32e136..173e6f4a07 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5678,7 +5678,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
break;
case 1:
- *eax = env->pkg_offset;
+ *eax = apicid_pkg_offset(&topo_info);
*ebx = cs->nr_cores * cs->nr_threads;
*ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
break;
@@ -5712,7 +5712,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
break;
case 2:
- *eax = env->pkg_offset;
+ *eax = apicid_pkg_offset(&topo_info);
*ebx = env->nr_dies * cs->nr_cores * cs->nr_threads;
*ecx |= CPUID_TOPOLOGY_LEVEL_DIE;
break;
@@ -5889,11 +5889,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
/*
* Bits 15:12 is "The number of bits in the initial
* Core::X86::Apic::ApicId[ApicId] value that indicate
- * thread ID within a package". This is already stored at
- * CPUX86State::pkg_offset.
+ * thread ID within a package".
* Bits 7:0 is "The number of threads in the package is NC+1"
*/
- *ecx = (env->pkg_offset << 12) |
+ *ecx = (apicid_pkg_offset(&topo_info) << 12) |
((cs->nr_cores * cs->nr_threads) - 1);
} else {
*ecx = 0;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index e1a5c174dc..d5ad42d694 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1630,7 +1630,6 @@ typedef struct CPUX86State {
unsigned nr_dies;
unsigned nr_nodes;
- unsigned pkg_offset;
} CPUX86State;
struct kvm_msrs;
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 01/10] Revert "i386: Fix pkg_id offset for EPYC cpu models"
2020-08-31 18:42 ` [PATCH v6 01/10] Revert "i386: Fix pkg_id offset for EPYC cpu models" Babu Moger
@ 2020-09-01 12:11 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:11 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:11 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit 7b225762c8c05fd31d4c2be116aedfbc00383f8b.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Also fix all the references of pkg_offset.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/pc.c | 1 -
> target/i386/cpu.c | 9 ++++-----
> target/i386/cpu.h | 1 -
> 3 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5d8d5ef8b3..6b708f4341 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1502,7 +1502,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>
> env->nr_dies = x86ms->smp_dies;
> env->nr_nodes = topo_info.nodes_per_pkg;
> - env->pkg_offset = x86ms->apicid_pkg_offset(&topo_info);
>
> /*
> * If APIC ID is not set,
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 588f32e136..173e6f4a07 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5678,7 +5678,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
> break;
> case 1:
> - *eax = env->pkg_offset;
> + *eax = apicid_pkg_offset(&topo_info);
> *ebx = cs->nr_cores * cs->nr_threads;
> *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
> break;
> @@ -5712,7 +5712,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
> break;
> case 2:
> - *eax = env->pkg_offset;
> + *eax = apicid_pkg_offset(&topo_info);
> *ebx = env->nr_dies * cs->nr_cores * cs->nr_threads;
> *ecx |= CPUID_TOPOLOGY_LEVEL_DIE;
> break;
> @@ -5889,11 +5889,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> /*
> * Bits 15:12 is "The number of bits in the initial
> * Core::X86::Apic::ApicId[ApicId] value that indicate
> - * thread ID within a package". This is already stored at
> - * CPUX86State::pkg_offset.
> + * thread ID within a package".
> * Bits 7:0 is "The number of threads in the package is NC+1"
> */
> - *ecx = (env->pkg_offset << 12) |
> + *ecx = (apicid_pkg_offset(&topo_info) << 12) |
> ((cs->nr_cores * cs->nr_threads) - 1);
> } else {
> *ecx = 0;
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index e1a5c174dc..d5ad42d694 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1630,7 +1630,6 @@ typedef struct CPUX86State {
>
> unsigned nr_dies;
> unsigned nr_nodes;
> - unsigned pkg_offset;
> } CPUX86State;
>
> struct kvm_msrs;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 02/10] Revert "target/i386: Enable new apic id encoding for EPYC based cpus models"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
2020-08-31 18:42 ` [PATCH v6 01/10] Revert "i386: Fix pkg_id offset for EPYC cpu models" Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:12 ` Igor Mammedov
2020-08-31 18:42 ` [PATCH v6 03/10] Revert "hw/i386: Move arch_id decode inside x86_cpus_init" Babu Moger
` (8 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit 247b18c593ec298446645af8d5d28911daf653b1.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 173e6f4a07..c9c1e681c2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3995,7 +3995,6 @@ static X86CPUDefinition builtin_x86_defs[] = {
.xlevel = 0x8000001E,
.model_id = "AMD EPYC Processor",
.cache_info = &epyc_cache_info,
- .use_epyc_apic_id_encoding = 1,
.versions = (X86CPUVersionDefinition[]) {
{ .version = 1 },
{
@@ -4123,7 +4122,6 @@ static X86CPUDefinition builtin_x86_defs[] = {
.xlevel = 0x8000001E,
.model_id = "AMD EPYC-Rome Processor",
.cache_info = &epyc_rome_cache_info,
- .use_epyc_apic_id_encoding = 1,
},
};
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 02/10] Revert "target/i386: Enable new apic id encoding for EPYC based cpus models"
2020-08-31 18:42 ` [PATCH v6 02/10] Revert "target/i386: Enable new apic id encoding for EPYC based cpus models" Babu Moger
@ 2020-09-01 12:12 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:12 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:17 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit 247b18c593ec298446645af8d5d28911daf653b1.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> target/i386/cpu.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 173e6f4a07..c9c1e681c2 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3995,7 +3995,6 @@ static X86CPUDefinition builtin_x86_defs[] = {
> .xlevel = 0x8000001E,
> .model_id = "AMD EPYC Processor",
> .cache_info = &epyc_cache_info,
> - .use_epyc_apic_id_encoding = 1,
> .versions = (X86CPUVersionDefinition[]) {
> { .version = 1 },
> {
> @@ -4123,7 +4122,6 @@ static X86CPUDefinition builtin_x86_defs[] = {
> .xlevel = 0x8000001E,
> .model_id = "AMD EPYC-Rome Processor",
> .cache_info = &epyc_rome_cache_info,
> - .use_epyc_apic_id_encoding = 1,
> },
> };
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 03/10] Revert "hw/i386: Move arch_id decode inside x86_cpus_init"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
2020-08-31 18:42 ` [PATCH v6 01/10] Revert "i386: Fix pkg_id offset for EPYC cpu models" Babu Moger
2020-08-31 18:42 ` [PATCH v6 02/10] Revert "target/i386: Enable new apic id encoding for EPYC based cpus models" Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:13 ` Igor Mammedov
2020-08-31 18:42 ` [PATCH v6 04/10] Revert "i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition" Babu Moger
` (7 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit 2e26f4ab3bf8390a2677d3afd9b1a04f015d7721.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
hw/i386/pc.c | 6 +++---
hw/i386/x86.c | 37 +++++++------------------------------
2 files changed, 10 insertions(+), 33 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6b708f4341..0677d6a272 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1556,14 +1556,14 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
topo_ids.die_id = cpu->die_id;
topo_ids.core_id = cpu->core_id;
topo_ids.smt_id = cpu->thread_id;
- cpu->apic_id = x86ms->apicid_from_topo_ids(&topo_info, &topo_ids);
+ cpu->apic_id = x86_apicid_from_topo_ids(&topo_info, &topo_ids);
}
cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
if (!cpu_slot) {
MachineState *ms = MACHINE(pcms);
- x86ms->topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
+ x86_topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
error_setg(errp,
"Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
" APIC ID %" PRIu32 ", valid index range 0:%d",
@@ -1584,7 +1584,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
/* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()
* once -smp refactoring is complete and there will be CPU private
* CPUState::nr_cores and CPUState::nr_threads fields instead of globals */
- x86ms->topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
+ x86_topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
if (cpu->socket_id != -1 && cpu->socket_id != topo_ids.pkg_id) {
error_setg(errp, "property socket-id: %u doesn't match set apic-id:"
" 0x%x (socket-id: %u)", cpu->socket_id, cpu->apic_id,
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index cf384b9743..3cc2318212 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -68,22 +68,6 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
topo_info->threads_per_core = ms->smp.threads;
}
-/*
- * Set up with the new EPYC topology handlers
- *
- * AMD uses different apic id encoding for EPYC based cpus. Override
- * the default topo handlers with EPYC encoding handlers.
- */
-static void x86_set_epyc_topo_handlers(MachineState *machine)
-{
- X86MachineState *x86ms = X86_MACHINE(machine);
-
- x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx_epyc;
- x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid_epyc;
- x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids_epyc;
- x86ms->apicid_pkg_offset = apicid_pkg_offset_epyc;
-}
-
/*
* Calculates initial APIC ID for a specific CPU index
*
@@ -102,7 +86,7 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
init_topo_info(&topo_info, x86ms);
- correct_id = x86ms->apicid_from_cpu_idx(&topo_info, cpu_index);
+ correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
if (x86mc->compat_apic_id_mode) {
if (cpu_index != correct_id && !warned && !qtest_enabled()) {
error_report("APIC IDs set in compatibility mode, "
@@ -136,11 +120,6 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
MachineState *ms = MACHINE(x86ms);
MachineClass *mc = MACHINE_GET_CLASS(x86ms);
- /* Check for apicid encoding */
- if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) {
- x86_set_epyc_topo_handlers(ms);
- }
-
x86_cpu_set_default_version(default_cpu_version);
/*
@@ -154,12 +133,6 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms,
ms->smp.max_cpus - 1) + 1;
possible_cpus = mc->possible_cpu_arch_ids(ms);
-
- for (i = 0; i < ms->possible_cpus->len; i++) {
- ms->possible_cpus->cpus[i].arch_id =
- x86_cpu_apic_id_from_index(x86ms, i);
- }
-
for (i = 0; i < ms->smp.cpus; i++) {
x86_cpu_new(x86ms, possible_cpus->cpus[i].arch_id, &error_fatal);
}
@@ -184,7 +157,8 @@ int64_t x86_get_default_cpu_node_id(const MachineState *ms, int idx)
init_topo_info(&topo_info, x86ms);
assert(idx < ms->possible_cpus->len);
- x86_topo_ids_from_idx(&topo_info, idx, &topo_ids);
+ x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
+ &topo_info, &topo_ids);
return topo_ids.pkg_id % ms->numa_state->num_nodes;
}
@@ -215,7 +189,10 @@ const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms)
ms->possible_cpus->cpus[i].type = ms->cpu_type;
ms->possible_cpus->cpus[i].vcpus_count = 1;
- x86_topo_ids_from_idx(&topo_info, i, &topo_ids);
+ ms->possible_cpus->cpus[i].arch_id =
+ x86_cpu_apic_id_from_index(x86ms, i);
+ x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
+ &topo_info, &topo_ids);
ms->possible_cpus->cpus[i].props.has_socket_id = true;
ms->possible_cpus->cpus[i].props.socket_id = topo_ids.pkg_id;
if (x86ms->smp_dies > 1) {
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 03/10] Revert "hw/i386: Move arch_id decode inside x86_cpus_init"
2020-08-31 18:42 ` [PATCH v6 03/10] Revert "hw/i386: Move arch_id decode inside x86_cpus_init" Babu Moger
@ 2020-09-01 12:13 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:13 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:23 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit 2e26f4ab3bf8390a2677d3afd9b1a04f015d7721.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/pc.c | 6 +++---
> hw/i386/x86.c | 37 +++++++------------------------------
> 2 files changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6b708f4341..0677d6a272 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1556,14 +1556,14 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
> topo_ids.die_id = cpu->die_id;
> topo_ids.core_id = cpu->core_id;
> topo_ids.smt_id = cpu->thread_id;
> - cpu->apic_id = x86ms->apicid_from_topo_ids(&topo_info, &topo_ids);
> + cpu->apic_id = x86_apicid_from_topo_ids(&topo_info, &topo_ids);
> }
>
> cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
> if (!cpu_slot) {
> MachineState *ms = MACHINE(pcms);
>
> - x86ms->topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
> + x86_topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
> error_setg(errp,
> "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
> " APIC ID %" PRIu32 ", valid index range 0:%d",
> @@ -1584,7 +1584,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
> /* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()
> * once -smp refactoring is complete and there will be CPU private
> * CPUState::nr_cores and CPUState::nr_threads fields instead of globals */
> - x86ms->topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
> + x86_topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
> if (cpu->socket_id != -1 && cpu->socket_id != topo_ids.pkg_id) {
> error_setg(errp, "property socket-id: %u doesn't match set apic-id:"
> " 0x%x (socket-id: %u)", cpu->socket_id, cpu->apic_id,
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index cf384b9743..3cc2318212 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -68,22 +68,6 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
> topo_info->threads_per_core = ms->smp.threads;
> }
>
> -/*
> - * Set up with the new EPYC topology handlers
> - *
> - * AMD uses different apic id encoding for EPYC based cpus. Override
> - * the default topo handlers with EPYC encoding handlers.
> - */
> -static void x86_set_epyc_topo_handlers(MachineState *machine)
> -{
> - X86MachineState *x86ms = X86_MACHINE(machine);
> -
> - x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx_epyc;
> - x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid_epyc;
> - x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids_epyc;
> - x86ms->apicid_pkg_offset = apicid_pkg_offset_epyc;
> -}
> -
> /*
> * Calculates initial APIC ID for a specific CPU index
> *
> @@ -102,7 +86,7 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
>
> init_topo_info(&topo_info, x86ms);
>
> - correct_id = x86ms->apicid_from_cpu_idx(&topo_info, cpu_index);
> + correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
> if (x86mc->compat_apic_id_mode) {
> if (cpu_index != correct_id && !warned && !qtest_enabled()) {
> error_report("APIC IDs set in compatibility mode, "
> @@ -136,11 +120,6 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
> MachineState *ms = MACHINE(x86ms);
> MachineClass *mc = MACHINE_GET_CLASS(x86ms);
>
> - /* Check for apicid encoding */
> - if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) {
> - x86_set_epyc_topo_handlers(ms);
> - }
> -
> x86_cpu_set_default_version(default_cpu_version);
>
> /*
> @@ -154,12 +133,6 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
> x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms,
> ms->smp.max_cpus - 1) + 1;
> possible_cpus = mc->possible_cpu_arch_ids(ms);
> -
> - for (i = 0; i < ms->possible_cpus->len; i++) {
> - ms->possible_cpus->cpus[i].arch_id =
> - x86_cpu_apic_id_from_index(x86ms, i);
> - }
> -
> for (i = 0; i < ms->smp.cpus; i++) {
> x86_cpu_new(x86ms, possible_cpus->cpus[i].arch_id, &error_fatal);
> }
> @@ -184,7 +157,8 @@ int64_t x86_get_default_cpu_node_id(const MachineState *ms, int idx)
> init_topo_info(&topo_info, x86ms);
>
> assert(idx < ms->possible_cpus->len);
> - x86_topo_ids_from_idx(&topo_info, idx, &topo_ids);
> + x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
> + &topo_info, &topo_ids);
> return topo_ids.pkg_id % ms->numa_state->num_nodes;
> }
>
> @@ -215,7 +189,10 @@ const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms)
>
> ms->possible_cpus->cpus[i].type = ms->cpu_type;
> ms->possible_cpus->cpus[i].vcpus_count = 1;
> - x86_topo_ids_from_idx(&topo_info, i, &topo_ids);
> + ms->possible_cpus->cpus[i].arch_id =
> + x86_cpu_apic_id_from_index(x86ms, i);
> + x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
> + &topo_info, &topo_ids);
> ms->possible_cpus->cpus[i].props.has_socket_id = true;
> ms->possible_cpus->cpus[i].props.socket_id = topo_ids.pkg_id;
> if (x86ms->smp_dies > 1) {
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 04/10] Revert "i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (2 preceding siblings ...)
2020-08-31 18:42 ` [PATCH v6 03/10] Revert "hw/i386: Move arch_id decode inside x86_cpus_init" Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:14 ` Igor Mammedov
2020-08-31 18:42 ` [PATCH v6 05/10] Revert "hw/i386: Introduce apicid functions inside X86MachineState" Babu Moger
` (6 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit 0c1538cb1a26287c072645f4759b9872b1596d79.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 16 ----------------
target/i386/cpu.h | 1 -
2 files changed, 17 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c9c1e681c2..b72b4b08ac 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1638,10 +1638,6 @@ typedef struct X86CPUDefinition {
FeatureWordArray features;
const char *model_id;
CPUCaches *cache_info;
-
- /* Use AMD EPYC encoding for apic id */
- bool use_epyc_apic_id_encoding;
-
/*
* Definitions for alternative versions of CPU model.
* List is terminated by item with version == 0.
@@ -1683,18 +1679,6 @@ static const X86CPUVersionDefinition *x86_cpu_def_get_versions(X86CPUDefinition
return def->versions ?: default_version_list;
}
-bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type)
-{
- X86CPUClass *xcc = X86_CPU_CLASS(object_class_by_name(cpu_type));
-
- assert(xcc);
- if (xcc->model && xcc->model->cpudef) {
- return xcc->model->cpudef->use_epyc_apic_id_encoding;
- } else {
- return false;
- }
-}
-
static CPUCaches epyc_cache_info = {
.l1d_cache = &(CPUCacheInfo) {
.type = DATA_CACHE,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d5ad42d694..5ff8ad8427 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1918,7 +1918,6 @@ void cpu_clear_apic_feature(CPUX86State *env);
void host_cpuid(uint32_t function, uint32_t count,
uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
void host_vendor_fms(char *vendor, int *family, int *model, int *stepping);
-bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type);
/* helper.c */
bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 04/10] Revert "i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition"
2020-08-31 18:42 ` [PATCH v6 04/10] Revert "i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition" Babu Moger
@ 2020-09-01 12:14 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:14 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:30 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit 0c1538cb1a26287c072645f4759b9872b1596d79.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> target/i386/cpu.c | 16 ----------------
> target/i386/cpu.h | 1 -
> 2 files changed, 17 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index c9c1e681c2..b72b4b08ac 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1638,10 +1638,6 @@ typedef struct X86CPUDefinition {
> FeatureWordArray features;
> const char *model_id;
> CPUCaches *cache_info;
> -
> - /* Use AMD EPYC encoding for apic id */
> - bool use_epyc_apic_id_encoding;
> -
> /*
> * Definitions for alternative versions of CPU model.
> * List is terminated by item with version == 0.
> @@ -1683,18 +1679,6 @@ static const X86CPUVersionDefinition *x86_cpu_def_get_versions(X86CPUDefinition
> return def->versions ?: default_version_list;
> }
>
> -bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type)
> -{
> - X86CPUClass *xcc = X86_CPU_CLASS(object_class_by_name(cpu_type));
> -
> - assert(xcc);
> - if (xcc->model && xcc->model->cpudef) {
> - return xcc->model->cpudef->use_epyc_apic_id_encoding;
> - } else {
> - return false;
> - }
> -}
> -
> static CPUCaches epyc_cache_info = {
> .l1d_cache = &(CPUCacheInfo) {
> .type = DATA_CACHE,
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index d5ad42d694..5ff8ad8427 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1918,7 +1918,6 @@ void cpu_clear_apic_feature(CPUX86State *env);
> void host_cpuid(uint32_t function, uint32_t count,
> uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> void host_vendor_fms(char *vendor, int *family, int *model, int *stepping);
> -bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type);
>
> /* helper.c */
> bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 05/10] Revert "hw/i386: Introduce apicid functions inside X86MachineState"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (3 preceding siblings ...)
2020-08-31 18:42 ` [PATCH v6 04/10] Revert "i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition" Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:15 ` Igor Mammedov
2020-08-31 18:42 ` [PATCH v6 06/10] Revert "target/i386: Cleanup and use the EPYC mode topology functions" Babu Moger
` (5 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit 6121c7fbfd98dbc3af1b00b56ff2eef66df87828.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
hw/i386/x86.c | 5 -----
include/hw/i386/x86.h | 9 ---------
2 files changed, 14 deletions(-)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 3cc2318212..727c4a0f1d 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -896,11 +896,6 @@ static void x86_machine_initfn(Object *obj)
x86ms->smm = ON_OFF_AUTO_AUTO;
x86ms->acpi = ON_OFF_AUTO_AUTO;
x86ms->smp_dies = 1;
-
- x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx;
- x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid;
- x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids;
- x86ms->apicid_pkg_offset = apicid_pkg_offset;
}
static void x86_machine_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index b79f24e285..4d9a26326d 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -63,15 +63,6 @@ typedef struct {
OnOffAuto smm;
OnOffAuto acpi;
- /* Apic id specific handlers */
- uint32_t (*apicid_from_cpu_idx)(X86CPUTopoInfo *topo_info,
- unsigned cpu_index);
- void (*topo_ids_from_apicid)(apic_id_t apicid, X86CPUTopoInfo *topo_info,
- X86CPUTopoIDs *topo_ids);
- apic_id_t (*apicid_from_topo_ids)(X86CPUTopoInfo *topo_info,
- const X86CPUTopoIDs *topo_ids);
- uint32_t (*apicid_pkg_offset)(X86CPUTopoInfo *topo_info);
-
/*
* Address space used by IOAPIC device. All IOAPIC interrupts
* will be translated to MSI messages in the address space.
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 05/10] Revert "hw/i386: Introduce apicid functions inside X86MachineState"
2020-08-31 18:42 ` [PATCH v6 05/10] Revert "hw/i386: Introduce apicid functions inside X86MachineState" Babu Moger
@ 2020-09-01 12:15 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:15 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:36 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit 6121c7fbfd98dbc3af1b00b56ff2eef66df87828.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/x86.c | 5 -----
> include/hw/i386/x86.h | 9 ---------
> 2 files changed, 14 deletions(-)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 3cc2318212..727c4a0f1d 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -896,11 +896,6 @@ static void x86_machine_initfn(Object *obj)
> x86ms->smm = ON_OFF_AUTO_AUTO;
> x86ms->acpi = ON_OFF_AUTO_AUTO;
> x86ms->smp_dies = 1;
> -
> - x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx;
> - x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid;
> - x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids;
> - x86ms->apicid_pkg_offset = apicid_pkg_offset;
> }
>
> static void x86_machine_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index b79f24e285..4d9a26326d 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -63,15 +63,6 @@ typedef struct {
> OnOffAuto smm;
> OnOffAuto acpi;
>
> - /* Apic id specific handlers */
> - uint32_t (*apicid_from_cpu_idx)(X86CPUTopoInfo *topo_info,
> - unsigned cpu_index);
> - void (*topo_ids_from_apicid)(apic_id_t apicid, X86CPUTopoInfo *topo_info,
> - X86CPUTopoIDs *topo_ids);
> - apic_id_t (*apicid_from_topo_ids)(X86CPUTopoInfo *topo_info,
> - const X86CPUTopoIDs *topo_ids);
> - uint32_t (*apicid_pkg_offset)(X86CPUTopoInfo *topo_info);
> -
> /*
> * Address space used by IOAPIC device. All IOAPIC interrupts
> * will be translated to MSI messages in the address space.
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 06/10] Revert "target/i386: Cleanup and use the EPYC mode topology functions"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (4 preceding siblings ...)
2020-08-31 18:42 ` [PATCH v6 05/10] Revert "hw/i386: Introduce apicid functions inside X86MachineState" Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:15 ` Igor Mammedov
2020-08-31 18:42 ` [PATCH v6 07/10] Revert "hw/386: Add EPYC mode topology decoding functions" Babu Moger
` (4 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit dd08ef0318e2b61d14bc069590d174913f7f437a.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 161 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 127 insertions(+), 34 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b72b4b08ac..256bfa669f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -338,15 +338,68 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
}
}
+/*
+ * Definitions used for building CPUID Leaf 0x8000001D and 0x8000001E
+ * Please refer to the AMD64 Architecture Programmer’s Manual Volume 3.
+ * Define the constants to build the cpu topology. Right now, TOPOEXT
+ * feature is enabled only on EPYC. So, these constants are based on
+ * EPYC supported configurations. We may need to handle the cases if
+ * these values change in future.
+ */
+/* Maximum core complexes in a node */
+#define MAX_CCX 2
+/* Maximum cores in a core complex */
+#define MAX_CORES_IN_CCX 4
+/* Maximum cores in a node */
+#define MAX_CORES_IN_NODE 8
+/* Maximum nodes in a socket */
+#define MAX_NODES_PER_SOCKET 4
+
+/*
+ * Figure out the number of nodes required to build this config.
+ * Max cores in a node is 8
+ */
+static int nodes_in_socket(int nr_cores)
+{
+ int nodes;
+
+ nodes = DIV_ROUND_UP(nr_cores, MAX_CORES_IN_NODE);
+
+ /* Hardware does not support config with 3 nodes, return 4 in that case */
+ return (nodes == 3) ? 4 : nodes;
+}
+
+/*
+ * Decide the number of cores in a core complex with the given nr_cores using
+ * following set constants MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE and
+ * MAX_NODES_PER_SOCKET. Maintain symmetry as much as possible
+ * L3 cache is shared across all cores in a core complex. So, this will also
+ * tell us how many cores are sharing the L3 cache.
+ */
+static int cores_in_core_complex(int nr_cores)
+{
+ int nodes;
+
+ /* Check if we can fit all the cores in one core complex */
+ if (nr_cores <= MAX_CORES_IN_CCX) {
+ return nr_cores;
+ }
+ /* Get the number of nodes required to build this config */
+ nodes = nodes_in_socket(nr_cores);
+
+ /*
+ * Divide the cores accros all the core complexes
+ * Return rounded up value
+ */
+ return DIV_ROUND_UP(nr_cores, nodes * MAX_CCX);
+}
+
/* Encode cache info for CPUID[8000001D] */
-static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
- X86CPUTopoInfo *topo_info,
- uint32_t *eax, uint32_t *ebx,
- uint32_t *ecx, uint32_t *edx)
+static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
+ uint32_t *eax, uint32_t *ebx,
+ uint32_t *ecx, uint32_t *edx)
{
uint32_t l3_cores;
- unsigned nodes = MAX(topo_info->nodes_per_pkg, 1);
-
assert(cache->size == cache->line_size * cache->associativity *
cache->partitions * cache->sets);
@@ -355,13 +408,10 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
/* L3 is shared among multiple cores */
if (cache->level == 3) {
- l3_cores = DIV_ROUND_UP((topo_info->dies_per_pkg *
- topo_info->cores_per_die *
- topo_info->threads_per_core),
- nodes);
- *eax |= (l3_cores - 1) << 14;
+ l3_cores = cores_in_core_complex(cs->nr_cores);
+ *eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
} else {
- *eax |= ((topo_info->threads_per_core - 1) << 14);
+ *eax |= ((cs->nr_threads - 1) << 14);
}
assert(cache->line_size > 0);
@@ -381,17 +431,55 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
(cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
}
+/* Data structure to hold the configuration info for a given core index */
+struct core_topology {
+ /* core complex id of the current core index */
+ int ccx_id;
+ /*
+ * Adjusted core index for this core in the topology
+ * This can be 0,1,2,3 with max 4 cores in a core complex
+ */
+ int core_id;
+ /* Node id for this core index */
+ int node_id;
+ /* Number of nodes in this config */
+ int num_nodes;
+};
+
+/*
+ * Build the configuration closely match the EPYC hardware. Using the EPYC
+ * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE)
+ * right now. This could change in future.
+ * nr_cores : Total number of cores in the config
+ * core_id : Core index of the current CPU
+ * topo : Data structure to hold all the config info for this core index
+ */
+static void build_core_topology(int nr_cores, int core_id,
+ struct core_topology *topo)
+{
+ int nodes, cores_in_ccx;
+
+ /* First get the number of nodes required */
+ nodes = nodes_in_socket(nr_cores);
+
+ cores_in_ccx = cores_in_core_complex(nr_cores);
+
+ topo->node_id = core_id / (cores_in_ccx * MAX_CCX);
+ topo->ccx_id = (core_id % (cores_in_ccx * MAX_CCX)) / cores_in_ccx;
+ topo->core_id = core_id % cores_in_ccx;
+ topo->num_nodes = nodes;
+}
+
/* Encode cache info for CPUID[8000001E] */
-static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
+static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx)
{
- X86CPUTopoIDs topo_ids = {0};
- unsigned long nodes = MAX(topo_info->nodes_per_pkg, 1);
+ struct core_topology topo = {0};
+ unsigned long nodes;
int shift;
- x86_topo_ids_from_apicid_epyc(cpu->apic_id, topo_info, &topo_ids);
-
+ build_core_topology(cs->nr_cores, cpu->core_id, &topo);
*eax = cpu->apic_id;
/*
* CPUID_Fn8000001E_EBX
@@ -408,8 +496,12 @@ static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
* 3 Core complex id
* 1:0 Core id
*/
- *ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.node_id << 3) |
- (topo_ids.core_id);
+ if (cs->nr_threads - 1) {
+ *ebx = ((cs->nr_threads - 1) << 8) | (topo.node_id << 3) |
+ (topo.ccx_id << 2) | topo.core_id;
+ } else {
+ *ebx = (topo.node_id << 4) | (topo.ccx_id << 3) | topo.core_id;
+ }
/*
* CPUID_Fn8000001E_ECX
* 31:11 Reserved
@@ -418,8 +510,9 @@ static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
* 2 Socket id
* 1:0 Node id
*/
- if (nodes <= 4) {
- *ecx = ((nodes - 1) << 8) | (topo_ids.pkg_id << 2) | topo_ids.node_id;
+ if (topo.num_nodes <= 4) {
+ *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
+ topo.node_id;
} else {
/*
* Node id fix up. Actual hardware supports up to 4 nodes. But with
@@ -434,10 +527,10 @@ static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
* number of nodes. find_last_bit returns last set bit(0 based). Left
* shift(+1) the socket id to represent all the nodes.
*/
- nodes -= 1;
+ nodes = topo.num_nodes - 1;
shift = find_last_bit(&nodes, 8);
- *ecx = (nodes << 8) | (topo_ids.pkg_id << (shift + 1)) |
- topo_ids.node_id;
+ *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << (shift + 1)) |
+ topo.node_id;
}
*edx = 0;
}
@@ -5471,7 +5564,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t signature[3];
X86CPUTopoInfo topo_info;
- topo_info.nodes_per_pkg = env->nr_nodes;
topo_info.dies_per_pkg = env->nr_dies;
topo_info.cores_per_die = cs->nr_cores;
topo_info.threads_per_core = cs->nr_threads;
@@ -5902,20 +5994,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
switch (count) {
case 0: /* L1 dcache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache,
- &topo_info, eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, cs,
+ eax, ebx, ecx, edx);
break;
case 1: /* L1 icache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache,
- &topo_info, eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, cs,
+ eax, ebx, ecx, edx);
break;
case 2: /* L2 cache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache,
- &topo_info, eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, cs,
+ eax, ebx, ecx, edx);
break;
case 3: /* L3 cache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache,
- &topo_info, eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, cs,
+ eax, ebx, ecx, edx);
break;
default: /* end of info */
*eax = *ebx = *ecx = *edx = 0;
@@ -5924,7 +6016,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
break;
case 0x8000001E:
assert(cpu->core_id <= 255);
- encode_topo_cpuid8000001e(&topo_info, cpu, eax, ebx, ecx, edx);
+ encode_topo_cpuid8000001e(cs, cpu,
+ eax, ebx, ecx, edx);
break;
case 0xC0000000:
*eax = env->cpuid_xlevel2;
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 06/10] Revert "target/i386: Cleanup and use the EPYC mode topology functions"
2020-08-31 18:42 ` [PATCH v6 06/10] Revert "target/i386: Cleanup and use the EPYC mode topology functions" Babu Moger
@ 2020-09-01 12:15 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:15 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:42 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit dd08ef0318e2b61d14bc069590d174913f7f437a.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> target/i386/cpu.c | 161 ++++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 127 insertions(+), 34 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b72b4b08ac..256bfa669f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -338,15 +338,68 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
> }
> }
>
> +/*
> + * Definitions used for building CPUID Leaf 0x8000001D and 0x8000001E
> + * Please refer to the AMD64 Architecture Programmer’s Manual Volume 3.
> + * Define the constants to build the cpu topology. Right now, TOPOEXT
> + * feature is enabled only on EPYC. So, these constants are based on
> + * EPYC supported configurations. We may need to handle the cases if
> + * these values change in future.
> + */
> +/* Maximum core complexes in a node */
> +#define MAX_CCX 2
> +/* Maximum cores in a core complex */
> +#define MAX_CORES_IN_CCX 4
> +/* Maximum cores in a node */
> +#define MAX_CORES_IN_NODE 8
> +/* Maximum nodes in a socket */
> +#define MAX_NODES_PER_SOCKET 4
> +
> +/*
> + * Figure out the number of nodes required to build this config.
> + * Max cores in a node is 8
> + */
> +static int nodes_in_socket(int nr_cores)
> +{
> + int nodes;
> +
> + nodes = DIV_ROUND_UP(nr_cores, MAX_CORES_IN_NODE);
> +
> + /* Hardware does not support config with 3 nodes, return 4 in that case */
> + return (nodes == 3) ? 4 : nodes;
> +}
> +
> +/*
> + * Decide the number of cores in a core complex with the given nr_cores using
> + * following set constants MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE and
> + * MAX_NODES_PER_SOCKET. Maintain symmetry as much as possible
> + * L3 cache is shared across all cores in a core complex. So, this will also
> + * tell us how many cores are sharing the L3 cache.
> + */
> +static int cores_in_core_complex(int nr_cores)
> +{
> + int nodes;
> +
> + /* Check if we can fit all the cores in one core complex */
> + if (nr_cores <= MAX_CORES_IN_CCX) {
> + return nr_cores;
> + }
> + /* Get the number of nodes required to build this config */
> + nodes = nodes_in_socket(nr_cores);
> +
> + /*
> + * Divide the cores accros all the core complexes
> + * Return rounded up value
> + */
> + return DIV_ROUND_UP(nr_cores, nodes * MAX_CCX);
> +}
> +
> /* Encode cache info for CPUID[8000001D] */
> -static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
> - X86CPUTopoInfo *topo_info,
> - uint32_t *eax, uint32_t *ebx,
> - uint32_t *ecx, uint32_t *edx)
> +static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
> + uint32_t *eax, uint32_t *ebx,
> + uint32_t *ecx, uint32_t *edx)
> {
> uint32_t l3_cores;
> - unsigned nodes = MAX(topo_info->nodes_per_pkg, 1);
> -
> assert(cache->size == cache->line_size * cache->associativity *
> cache->partitions * cache->sets);
>
> @@ -355,13 +408,10 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
>
> /* L3 is shared among multiple cores */
> if (cache->level == 3) {
> - l3_cores = DIV_ROUND_UP((topo_info->dies_per_pkg *
> - topo_info->cores_per_die *
> - topo_info->threads_per_core),
> - nodes);
> - *eax |= (l3_cores - 1) << 14;
> + l3_cores = cores_in_core_complex(cs->nr_cores);
> + *eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
> } else {
> - *eax |= ((topo_info->threads_per_core - 1) << 14);
> + *eax |= ((cs->nr_threads - 1) << 14);
> }
>
> assert(cache->line_size > 0);
> @@ -381,17 +431,55 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
> (cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
> }
>
> +/* Data structure to hold the configuration info for a given core index */
> +struct core_topology {
> + /* core complex id of the current core index */
> + int ccx_id;
> + /*
> + * Adjusted core index for this core in the topology
> + * This can be 0,1,2,3 with max 4 cores in a core complex
> + */
> + int core_id;
> + /* Node id for this core index */
> + int node_id;
> + /* Number of nodes in this config */
> + int num_nodes;
> +};
> +
> +/*
> + * Build the configuration closely match the EPYC hardware. Using the EPYC
> + * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE)
> + * right now. This could change in future.
> + * nr_cores : Total number of cores in the config
> + * core_id : Core index of the current CPU
> + * topo : Data structure to hold all the config info for this core index
> + */
> +static void build_core_topology(int nr_cores, int core_id,
> + struct core_topology *topo)
> +{
> + int nodes, cores_in_ccx;
> +
> + /* First get the number of nodes required */
> + nodes = nodes_in_socket(nr_cores);
> +
> + cores_in_ccx = cores_in_core_complex(nr_cores);
> +
> + topo->node_id = core_id / (cores_in_ccx * MAX_CCX);
> + topo->ccx_id = (core_id % (cores_in_ccx * MAX_CCX)) / cores_in_ccx;
> + topo->core_id = core_id % cores_in_ccx;
> + topo->num_nodes = nodes;
> +}
> +
> /* Encode cache info for CPUID[8000001E] */
> -static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
> +static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
> uint32_t *eax, uint32_t *ebx,
> uint32_t *ecx, uint32_t *edx)
> {
> - X86CPUTopoIDs topo_ids = {0};
> - unsigned long nodes = MAX(topo_info->nodes_per_pkg, 1);
> + struct core_topology topo = {0};
> + unsigned long nodes;
> int shift;
>
> - x86_topo_ids_from_apicid_epyc(cpu->apic_id, topo_info, &topo_ids);
> -
> + build_core_topology(cs->nr_cores, cpu->core_id, &topo);
> *eax = cpu->apic_id;
> /*
> * CPUID_Fn8000001E_EBX
> @@ -408,8 +496,12 @@ static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
> * 3 Core complex id
> * 1:0 Core id
> */
> - *ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.node_id << 3) |
> - (topo_ids.core_id);
> + if (cs->nr_threads - 1) {
> + *ebx = ((cs->nr_threads - 1) << 8) | (topo.node_id << 3) |
> + (topo.ccx_id << 2) | topo.core_id;
> + } else {
> + *ebx = (topo.node_id << 4) | (topo.ccx_id << 3) | topo.core_id;
> + }
> /*
> * CPUID_Fn8000001E_ECX
> * 31:11 Reserved
> @@ -418,8 +510,9 @@ static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
> * 2 Socket id
> * 1:0 Node id
> */
> - if (nodes <= 4) {
> - *ecx = ((nodes - 1) << 8) | (topo_ids.pkg_id << 2) | topo_ids.node_id;
> + if (topo.num_nodes <= 4) {
> + *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
> + topo.node_id;
> } else {
> /*
> * Node id fix up. Actual hardware supports up to 4 nodes. But with
> @@ -434,10 +527,10 @@ static void encode_topo_cpuid8000001e(X86CPUTopoInfo *topo_info, X86CPU *cpu,
> * number of nodes. find_last_bit returns last set bit(0 based). Left
> * shift(+1) the socket id to represent all the nodes.
> */
> - nodes -= 1;
> + nodes = topo.num_nodes - 1;
> shift = find_last_bit(&nodes, 8);
> - *ecx = (nodes << 8) | (topo_ids.pkg_id << (shift + 1)) |
> - topo_ids.node_id;
> + *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << (shift + 1)) |
> + topo.node_id;
> }
> *edx = 0;
> }
> @@ -5471,7 +5564,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> uint32_t signature[3];
> X86CPUTopoInfo topo_info;
>
> - topo_info.nodes_per_pkg = env->nr_nodes;
> topo_info.dies_per_pkg = env->nr_dies;
> topo_info.cores_per_die = cs->nr_cores;
> topo_info.threads_per_core = cs->nr_threads;
> @@ -5902,20 +5994,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> }
> switch (count) {
> case 0: /* L1 dcache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache,
> - &topo_info, eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, cs,
> + eax, ebx, ecx, edx);
> break;
> case 1: /* L1 icache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache,
> - &topo_info, eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, cs,
> + eax, ebx, ecx, edx);
> break;
> case 2: /* L2 cache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache,
> - &topo_info, eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, cs,
> + eax, ebx, ecx, edx);
> break;
> case 3: /* L3 cache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache,
> - &topo_info, eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, cs,
> + eax, ebx, ecx, edx);
> break;
> default: /* end of info */
> *eax = *ebx = *ecx = *edx = 0;
> @@ -5924,7 +6016,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> break;
> case 0x8000001E:
> assert(cpu->core_id <= 255);
> - encode_topo_cpuid8000001e(&topo_info, cpu, eax, ebx, ecx, edx);
> + encode_topo_cpuid8000001e(cs, cpu,
> + eax, ebx, ecx, edx);
> break;
> case 0xC0000000:
> *eax = env->cpuid_xlevel2;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 07/10] Revert "hw/386: Add EPYC mode topology decoding functions"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (5 preceding siblings ...)
2020-08-31 18:42 ` [PATCH v6 06/10] Revert "target/i386: Cleanup and use the EPYC mode topology functions" Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:15 ` Igor Mammedov
2020-08-31 18:42 ` [PATCH v6 08/10] Revert "hw/i386: Update structures to save the number of nodes per package" Babu Moger
` (3 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit 7568b205555a6405042f62c64af3268f4330aed5.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
include/hw/i386/topology.h | 100 --------------------------------------------
1 file changed, 100 deletions(-)
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 07239f95f4..b9593b9905 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -47,7 +47,6 @@ typedef uint32_t apic_id_t;
typedef struct X86CPUTopoIDs {
unsigned pkg_id;
- unsigned node_id;
unsigned die_id;
unsigned core_id;
unsigned smt_id;
@@ -89,11 +88,6 @@ static inline unsigned apicid_die_width(X86CPUTopoInfo *topo_info)
return apicid_bitwidth_for_count(topo_info->dies_per_pkg);
}
-/* Bit width of the node_id field per socket */
-static inline unsigned apicid_node_width_epyc(X86CPUTopoInfo *topo_info)
-{
- return apicid_bitwidth_for_count(MAX(topo_info->nodes_per_pkg, 1));
-}
/* Bit offset of the Core_ID field
*/
static inline unsigned apicid_core_offset(X86CPUTopoInfo *topo_info)
@@ -114,100 +108,6 @@ static inline unsigned apicid_pkg_offset(X86CPUTopoInfo *topo_info)
return apicid_die_offset(topo_info) + apicid_die_width(topo_info);
}
-#define NODE_ID_OFFSET 3 /* Minimum node_id offset if numa configured */
-
-/*
- * Bit offset of the node_id field
- *
- * Make sure nodes_per_pkg > 0 if numa configured else zero.
- */
-static inline unsigned apicid_node_offset_epyc(X86CPUTopoInfo *topo_info)
-{
- unsigned offset = apicid_die_offset(topo_info) +
- apicid_die_width(topo_info);
-
- if (topo_info->nodes_per_pkg) {
- return MAX(NODE_ID_OFFSET, offset);
- } else {
- return offset;
- }
-}
-
-/* Bit offset of the Pkg_ID (socket ID) field */
-static inline unsigned apicid_pkg_offset_epyc(X86CPUTopoInfo *topo_info)
-{
- return apicid_node_offset_epyc(topo_info) +
- apicid_node_width_epyc(topo_info);
-}
-
-/*
- * Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
- *
- * The caller must make sure core_id < nr_cores and smt_id < nr_threads.
- */
-static inline apic_id_t
-x86_apicid_from_topo_ids_epyc(X86CPUTopoInfo *topo_info,
- const X86CPUTopoIDs *topo_ids)
-{
- return (topo_ids->pkg_id << apicid_pkg_offset_epyc(topo_info)) |
- (topo_ids->node_id << apicid_node_offset_epyc(topo_info)) |
- (topo_ids->die_id << apicid_die_offset(topo_info)) |
- (topo_ids->core_id << apicid_core_offset(topo_info)) |
- topo_ids->smt_id;
-}
-
-static inline void x86_topo_ids_from_idx_epyc(X86CPUTopoInfo *topo_info,
- unsigned cpu_index,
- X86CPUTopoIDs *topo_ids)
-{
- unsigned nr_nodes = MAX(topo_info->nodes_per_pkg, 1);
- unsigned nr_dies = topo_info->dies_per_pkg;
- unsigned nr_cores = topo_info->cores_per_die;
- unsigned nr_threads = topo_info->threads_per_core;
- unsigned cores_per_node = DIV_ROUND_UP((nr_dies * nr_cores * nr_threads),
- nr_nodes);
-
- topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);
- topo_ids->node_id = (cpu_index / cores_per_node) % nr_nodes;
- topo_ids->die_id = cpu_index / (nr_cores * nr_threads) % nr_dies;
- topo_ids->core_id = cpu_index / nr_threads % nr_cores;
- topo_ids->smt_id = cpu_index % nr_threads;
-}
-
-/*
- * Calculate thread/core/package IDs for a specific topology,
- * based on APIC ID
- */
-static inline void x86_topo_ids_from_apicid_epyc(apic_id_t apicid,
- X86CPUTopoInfo *topo_info,
- X86CPUTopoIDs *topo_ids)
-{
- topo_ids->smt_id = apicid &
- ~(0xFFFFFFFFUL << apicid_smt_width(topo_info));
- topo_ids->core_id =
- (apicid >> apicid_core_offset(topo_info)) &
- ~(0xFFFFFFFFUL << apicid_core_width(topo_info));
- topo_ids->die_id =
- (apicid >> apicid_die_offset(topo_info)) &
- ~(0xFFFFFFFFUL << apicid_die_width(topo_info));
- topo_ids->node_id =
- (apicid >> apicid_node_offset_epyc(topo_info)) &
- ~(0xFFFFFFFFUL << apicid_node_width_epyc(topo_info));
- topo_ids->pkg_id = apicid >> apicid_pkg_offset_epyc(topo_info);
-}
-
-/*
- * Make APIC ID for the CPU 'cpu_index'
- *
- * 'cpu_index' is a sequential, contiguous ID for the CPU.
- */
-static inline apic_id_t x86_apicid_from_cpu_idx_epyc(X86CPUTopoInfo *topo_info,
- unsigned cpu_index)
-{
- X86CPUTopoIDs topo_ids;
- x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids);
- return x86_apicid_from_topo_ids_epyc(topo_info, &topo_ids);
-}
/* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
*
* The caller must make sure core_id < nr_cores and smt_id < nr_threads.
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 07/10] Revert "hw/386: Add EPYC mode topology decoding functions"
2020-08-31 18:42 ` [PATCH v6 07/10] Revert "hw/386: Add EPYC mode topology decoding functions" Babu Moger
@ 2020-09-01 12:15 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:15 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:48 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit 7568b205555a6405042f62c64af3268f4330aed5.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/i386/topology.h | 100 --------------------------------------------
> 1 file changed, 100 deletions(-)
>
> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> index 07239f95f4..b9593b9905 100644
> --- a/include/hw/i386/topology.h
> +++ b/include/hw/i386/topology.h
> @@ -47,7 +47,6 @@ typedef uint32_t apic_id_t;
>
> typedef struct X86CPUTopoIDs {
> unsigned pkg_id;
> - unsigned node_id;
> unsigned die_id;
> unsigned core_id;
> unsigned smt_id;
> @@ -89,11 +88,6 @@ static inline unsigned apicid_die_width(X86CPUTopoInfo *topo_info)
> return apicid_bitwidth_for_count(topo_info->dies_per_pkg);
> }
>
> -/* Bit width of the node_id field per socket */
> -static inline unsigned apicid_node_width_epyc(X86CPUTopoInfo *topo_info)
> -{
> - return apicid_bitwidth_for_count(MAX(topo_info->nodes_per_pkg, 1));
> -}
> /* Bit offset of the Core_ID field
> */
> static inline unsigned apicid_core_offset(X86CPUTopoInfo *topo_info)
> @@ -114,100 +108,6 @@ static inline unsigned apicid_pkg_offset(X86CPUTopoInfo *topo_info)
> return apicid_die_offset(topo_info) + apicid_die_width(topo_info);
> }
>
> -#define NODE_ID_OFFSET 3 /* Minimum node_id offset if numa configured */
> -
> -/*
> - * Bit offset of the node_id field
> - *
> - * Make sure nodes_per_pkg > 0 if numa configured else zero.
> - */
> -static inline unsigned apicid_node_offset_epyc(X86CPUTopoInfo *topo_info)
> -{
> - unsigned offset = apicid_die_offset(topo_info) +
> - apicid_die_width(topo_info);
> -
> - if (topo_info->nodes_per_pkg) {
> - return MAX(NODE_ID_OFFSET, offset);
> - } else {
> - return offset;
> - }
> -}
> -
> -/* Bit offset of the Pkg_ID (socket ID) field */
> -static inline unsigned apicid_pkg_offset_epyc(X86CPUTopoInfo *topo_info)
> -{
> - return apicid_node_offset_epyc(topo_info) +
> - apicid_node_width_epyc(topo_info);
> -}
> -
> -/*
> - * Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
> - *
> - * The caller must make sure core_id < nr_cores and smt_id < nr_threads.
> - */
> -static inline apic_id_t
> -x86_apicid_from_topo_ids_epyc(X86CPUTopoInfo *topo_info,
> - const X86CPUTopoIDs *topo_ids)
> -{
> - return (topo_ids->pkg_id << apicid_pkg_offset_epyc(topo_info)) |
> - (topo_ids->node_id << apicid_node_offset_epyc(topo_info)) |
> - (topo_ids->die_id << apicid_die_offset(topo_info)) |
> - (topo_ids->core_id << apicid_core_offset(topo_info)) |
> - topo_ids->smt_id;
> -}
> -
> -static inline void x86_topo_ids_from_idx_epyc(X86CPUTopoInfo *topo_info,
> - unsigned cpu_index,
> - X86CPUTopoIDs *topo_ids)
> -{
> - unsigned nr_nodes = MAX(topo_info->nodes_per_pkg, 1);
> - unsigned nr_dies = topo_info->dies_per_pkg;
> - unsigned nr_cores = topo_info->cores_per_die;
> - unsigned nr_threads = topo_info->threads_per_core;
> - unsigned cores_per_node = DIV_ROUND_UP((nr_dies * nr_cores * nr_threads),
> - nr_nodes);
> -
> - topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);
> - topo_ids->node_id = (cpu_index / cores_per_node) % nr_nodes;
> - topo_ids->die_id = cpu_index / (nr_cores * nr_threads) % nr_dies;
> - topo_ids->core_id = cpu_index / nr_threads % nr_cores;
> - topo_ids->smt_id = cpu_index % nr_threads;
> -}
> -
> -/*
> - * Calculate thread/core/package IDs for a specific topology,
> - * based on APIC ID
> - */
> -static inline void x86_topo_ids_from_apicid_epyc(apic_id_t apicid,
> - X86CPUTopoInfo *topo_info,
> - X86CPUTopoIDs *topo_ids)
> -{
> - topo_ids->smt_id = apicid &
> - ~(0xFFFFFFFFUL << apicid_smt_width(topo_info));
> - topo_ids->core_id =
> - (apicid >> apicid_core_offset(topo_info)) &
> - ~(0xFFFFFFFFUL << apicid_core_width(topo_info));
> - topo_ids->die_id =
> - (apicid >> apicid_die_offset(topo_info)) &
> - ~(0xFFFFFFFFUL << apicid_die_width(topo_info));
> - topo_ids->node_id =
> - (apicid >> apicid_node_offset_epyc(topo_info)) &
> - ~(0xFFFFFFFFUL << apicid_node_width_epyc(topo_info));
> - topo_ids->pkg_id = apicid >> apicid_pkg_offset_epyc(topo_info);
> -}
> -
> -/*
> - * Make APIC ID for the CPU 'cpu_index'
> - *
> - * 'cpu_index' is a sequential, contiguous ID for the CPU.
> - */
> -static inline apic_id_t x86_apicid_from_cpu_idx_epyc(X86CPUTopoInfo *topo_info,
> - unsigned cpu_index)
> -{
> - X86CPUTopoIDs topo_ids;
> - x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids);
> - return x86_apicid_from_topo_ids_epyc(topo_info, &topo_ids);
> -}
> /* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
> *
> * The caller must make sure core_id < nr_cores and smt_id < nr_threads.
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 08/10] Revert "hw/i386: Update structures to save the number of nodes per package"
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (6 preceding siblings ...)
2020-08-31 18:42 ` [PATCH v6 07/10] Revert "hw/386: Add EPYC mode topology decoding functions" Babu Moger
@ 2020-08-31 18:42 ` Babu Moger
2020-09-01 12:21 ` Igor Mammedov
2020-08-31 18:43 ` [PATCH v6 09/10] i386: Simplify CPUID_8000_001E for AMD Babu Moger
` (2 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:42 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
This reverts commit c24a41bb53c0854d22c96b30d57cfcaa543c409d.
Remove the EPYC specific apicid decoding and use the generic
default decoding.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
hw/i386/pc.c | 1 -
hw/i386/x86.c | 1 -
include/hw/i386/topology.h | 1 -
target/i386/cpu.c | 1 -
target/i386/cpu.h | 1 -
tests/test-x86-cpuid.c | 40 ++++++++++++++++++++--------------------
6 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0677d6a272..d11daacc23 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1501,7 +1501,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
init_topo_info(&topo_info, x86ms);
env->nr_dies = x86ms->smp_dies;
- env->nr_nodes = topo_info.nodes_per_pkg;
/*
* If APIC ID is not set,
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 727c4a0f1d..c1954db152 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -62,7 +62,6 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
{
MachineState *ms = MACHINE(x86ms);
- topo_info->nodes_per_pkg = ms->numa_state->num_nodes / ms->smp.sockets;
topo_info->dies_per_pkg = x86ms->smp_dies;
topo_info->cores_per_die = ms->smp.cores;
topo_info->threads_per_core = ms->smp.threads;
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index b9593b9905..81573f6cfd 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -53,7 +53,6 @@ typedef struct X86CPUTopoIDs {
} X86CPUTopoIDs;
typedef struct X86CPUTopoInfo {
- unsigned nodes_per_pkg;
unsigned dies_per_pkg;
unsigned cores_per_die;
unsigned threads_per_core;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 256bfa669f..ba4667b33c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7023,7 +7023,6 @@ static void x86_cpu_initfn(Object *obj)
FeatureWord w;
env->nr_dies = 1;
- env->nr_nodes = 1;
cpu_set_cpustate_pointers(cpu);
object_property_add(obj, "family", "int",
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5ff8ad8427..d3097be6a5 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1629,7 +1629,6 @@ typedef struct CPUX86State {
TPRAccess tpr_access_type;
unsigned nr_dies;
- unsigned nr_nodes;
} CPUX86State;
struct kvm_msrs;
diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c
index 049030a50e..bfabc0403a 100644
--- a/tests/test-x86-cpuid.c
+++ b/tests/test-x86-cpuid.c
@@ -31,12 +31,12 @@ static void test_topo_bits(void)
X86CPUTopoInfo topo_info = {0};
/* simple tests for 1 thread per core, 1 core per die, 1 die per package */
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 1};
+ topo_info = (X86CPUTopoInfo) {1, 1, 1};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 0);
g_assert_cmpuint(apicid_core_width(&topo_info), ==, 0);
g_assert_cmpuint(apicid_die_width(&topo_info), ==, 0);
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 1};
+ topo_info = (X86CPUTopoInfo) {1, 1, 1};
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0), ==, 0);
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1), ==, 1);
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2), ==, 2);
@@ -45,39 +45,39 @@ static void test_topo_bits(void)
/* Test field width calculation for multiple values
*/
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 2};
+ topo_info = (X86CPUTopoInfo) {1, 1, 2};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 1);
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 3};
+ topo_info = (X86CPUTopoInfo) {1, 1, 3};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 2);
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 4};
+ topo_info = (X86CPUTopoInfo) {1, 1, 4};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 2);
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 14};
+ topo_info = (X86CPUTopoInfo) {1, 1, 14};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 4);
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 15};
+ topo_info = (X86CPUTopoInfo) {1, 1, 15};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 4);
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 16};
+ topo_info = (X86CPUTopoInfo) {1, 1, 16};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 4);
- topo_info = (X86CPUTopoInfo) {0, 1, 1, 17};
+ topo_info = (X86CPUTopoInfo) {1, 1, 17};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 5);
- topo_info = (X86CPUTopoInfo) {0, 1, 30, 2};
+ topo_info = (X86CPUTopoInfo) {1, 30, 2};
g_assert_cmpuint(apicid_core_width(&topo_info), ==, 5);
- topo_info = (X86CPUTopoInfo) {0, 1, 31, 2};
+ topo_info = (X86CPUTopoInfo) {1, 31, 2};
g_assert_cmpuint(apicid_core_width(&topo_info), ==, 5);
- topo_info = (X86CPUTopoInfo) {0, 1, 32, 2};
+ topo_info = (X86CPUTopoInfo) {1, 32, 2};
g_assert_cmpuint(apicid_core_width(&topo_info), ==, 5);
- topo_info = (X86CPUTopoInfo) {0, 1, 33, 2};
+ topo_info = (X86CPUTopoInfo) {1, 33, 2};
g_assert_cmpuint(apicid_core_width(&topo_info), ==, 6);
- topo_info = (X86CPUTopoInfo) {0, 1, 30, 2};
+ topo_info = (X86CPUTopoInfo) {1, 30, 2};
g_assert_cmpuint(apicid_die_width(&topo_info), ==, 0);
- topo_info = (X86CPUTopoInfo) {0, 2, 30, 2};
+ topo_info = (X86CPUTopoInfo) {2, 30, 2};
g_assert_cmpuint(apicid_die_width(&topo_info), ==, 1);
- topo_info = (X86CPUTopoInfo) {0, 3, 30, 2};
+ topo_info = (X86CPUTopoInfo) {3, 30, 2};
g_assert_cmpuint(apicid_die_width(&topo_info), ==, 2);
- topo_info = (X86CPUTopoInfo) {0, 4, 30, 2};
+ topo_info = (X86CPUTopoInfo) {4, 30, 2};
g_assert_cmpuint(apicid_die_width(&topo_info), ==, 2);
/* build a weird topology and see if IDs are calculated correctly
@@ -85,18 +85,18 @@ static void test_topo_bits(void)
/* This will use 2 bits for thread ID and 3 bits for core ID
*/
- topo_info = (X86CPUTopoInfo) {0, 1, 6, 3};
+ topo_info = (X86CPUTopoInfo) {1, 6, 3};
g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 2);
g_assert_cmpuint(apicid_core_offset(&topo_info), ==, 2);
g_assert_cmpuint(apicid_die_offset(&topo_info), ==, 5);
g_assert_cmpuint(apicid_pkg_offset(&topo_info), ==, 5);
- topo_info = (X86CPUTopoInfo) {0, 1, 6, 3};
+ topo_info = (X86CPUTopoInfo) {1, 6, 3};
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0), ==, 0);
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1), ==, 1);
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2), ==, 2);
- topo_info = (X86CPUTopoInfo) {0, 1, 6, 3};
+ topo_info = (X86CPUTopoInfo) {1, 6, 3};
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0), ==,
(1 << 2) | 0);
g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1), ==,
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 08/10] Revert "hw/i386: Update structures to save the number of nodes per package"
2020-08-31 18:42 ` [PATCH v6 08/10] Revert "hw/i386: Update structures to save the number of nodes per package" Babu Moger
@ 2020-09-01 12:21 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 12:21 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:42:54 -0500
Babu Moger <babu.moger@amd.com> wrote:
> This reverts commit c24a41bb53c0854d22c96b30d57cfcaa543c409d.
>
> Remove the EPYC specific apicid decoding and use the generic
> default decoding.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/pc.c | 1 -
> hw/i386/x86.c | 1 -
> include/hw/i386/topology.h | 1 -
> target/i386/cpu.c | 1 -
> target/i386/cpu.h | 1 -
> tests/test-x86-cpuid.c | 40 ++++++++++++++++++++--------------------
> 6 files changed, 20 insertions(+), 25 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0677d6a272..d11daacc23 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1501,7 +1501,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
> init_topo_info(&topo_info, x86ms);
>
> env->nr_dies = x86ms->smp_dies;
> - env->nr_nodes = topo_info.nodes_per_pkg;
>
> /*
> * If APIC ID is not set,
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 727c4a0f1d..c1954db152 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -62,7 +62,6 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
> {
> MachineState *ms = MACHINE(x86ms);
>
> - topo_info->nodes_per_pkg = ms->numa_state->num_nodes / ms->smp.sockets;
> topo_info->dies_per_pkg = x86ms->smp_dies;
> topo_info->cores_per_die = ms->smp.cores;
> topo_info->threads_per_core = ms->smp.threads;
> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> index b9593b9905..81573f6cfd 100644
> --- a/include/hw/i386/topology.h
> +++ b/include/hw/i386/topology.h
> @@ -53,7 +53,6 @@ typedef struct X86CPUTopoIDs {
> } X86CPUTopoIDs;
>
> typedef struct X86CPUTopoInfo {
> - unsigned nodes_per_pkg;
> unsigned dies_per_pkg;
> unsigned cores_per_die;
> unsigned threads_per_core;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 256bfa669f..ba4667b33c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7023,7 +7023,6 @@ static void x86_cpu_initfn(Object *obj)
> FeatureWord w;
>
> env->nr_dies = 1;
> - env->nr_nodes = 1;
> cpu_set_cpustate_pointers(cpu);
>
> object_property_add(obj, "family", "int",
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 5ff8ad8427..d3097be6a5 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1629,7 +1629,6 @@ typedef struct CPUX86State {
> TPRAccess tpr_access_type;
>
> unsigned nr_dies;
> - unsigned nr_nodes;
> } CPUX86State;
>
> struct kvm_msrs;
> diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c
> index 049030a50e..bfabc0403a 100644
> --- a/tests/test-x86-cpuid.c
> +++ b/tests/test-x86-cpuid.c
> @@ -31,12 +31,12 @@ static void test_topo_bits(void)
> X86CPUTopoInfo topo_info = {0};
>
> /* simple tests for 1 thread per core, 1 core per die, 1 die per package */
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 1};
> + topo_info = (X86CPUTopoInfo) {1, 1, 1};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 0);
> g_assert_cmpuint(apicid_core_width(&topo_info), ==, 0);
> g_assert_cmpuint(apicid_die_width(&topo_info), ==, 0);
>
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 1};
> + topo_info = (X86CPUTopoInfo) {1, 1, 1};
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0), ==, 0);
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1), ==, 1);
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2), ==, 2);
> @@ -45,39 +45,39 @@ static void test_topo_bits(void)
>
> /* Test field width calculation for multiple values
> */
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 2};
> + topo_info = (X86CPUTopoInfo) {1, 1, 2};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 1);
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 3};
> + topo_info = (X86CPUTopoInfo) {1, 1, 3};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 2);
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 4};
> + topo_info = (X86CPUTopoInfo) {1, 1, 4};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 2);
>
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 14};
> + topo_info = (X86CPUTopoInfo) {1, 1, 14};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 4);
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 15};
> + topo_info = (X86CPUTopoInfo) {1, 1, 15};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 4);
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 16};
> + topo_info = (X86CPUTopoInfo) {1, 1, 16};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 4);
> - topo_info = (X86CPUTopoInfo) {0, 1, 1, 17};
> + topo_info = (X86CPUTopoInfo) {1, 1, 17};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 5);
>
>
> - topo_info = (X86CPUTopoInfo) {0, 1, 30, 2};
> + topo_info = (X86CPUTopoInfo) {1, 30, 2};
> g_assert_cmpuint(apicid_core_width(&topo_info), ==, 5);
> - topo_info = (X86CPUTopoInfo) {0, 1, 31, 2};
> + topo_info = (X86CPUTopoInfo) {1, 31, 2};
> g_assert_cmpuint(apicid_core_width(&topo_info), ==, 5);
> - topo_info = (X86CPUTopoInfo) {0, 1, 32, 2};
> + topo_info = (X86CPUTopoInfo) {1, 32, 2};
> g_assert_cmpuint(apicid_core_width(&topo_info), ==, 5);
> - topo_info = (X86CPUTopoInfo) {0, 1, 33, 2};
> + topo_info = (X86CPUTopoInfo) {1, 33, 2};
> g_assert_cmpuint(apicid_core_width(&topo_info), ==, 6);
>
> - topo_info = (X86CPUTopoInfo) {0, 1, 30, 2};
> + topo_info = (X86CPUTopoInfo) {1, 30, 2};
> g_assert_cmpuint(apicid_die_width(&topo_info), ==, 0);
> - topo_info = (X86CPUTopoInfo) {0, 2, 30, 2};
> + topo_info = (X86CPUTopoInfo) {2, 30, 2};
> g_assert_cmpuint(apicid_die_width(&topo_info), ==, 1);
> - topo_info = (X86CPUTopoInfo) {0, 3, 30, 2};
> + topo_info = (X86CPUTopoInfo) {3, 30, 2};
> g_assert_cmpuint(apicid_die_width(&topo_info), ==, 2);
> - topo_info = (X86CPUTopoInfo) {0, 4, 30, 2};
> + topo_info = (X86CPUTopoInfo) {4, 30, 2};
> g_assert_cmpuint(apicid_die_width(&topo_info), ==, 2);
>
> /* build a weird topology and see if IDs are calculated correctly
> @@ -85,18 +85,18 @@ static void test_topo_bits(void)
>
> /* This will use 2 bits for thread ID and 3 bits for core ID
> */
> - topo_info = (X86CPUTopoInfo) {0, 1, 6, 3};
> + topo_info = (X86CPUTopoInfo) {1, 6, 3};
> g_assert_cmpuint(apicid_smt_width(&topo_info), ==, 2);
> g_assert_cmpuint(apicid_core_offset(&topo_info), ==, 2);
> g_assert_cmpuint(apicid_die_offset(&topo_info), ==, 5);
> g_assert_cmpuint(apicid_pkg_offset(&topo_info), ==, 5);
>
> - topo_info = (X86CPUTopoInfo) {0, 1, 6, 3};
> + topo_info = (X86CPUTopoInfo) {1, 6, 3};
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0), ==, 0);
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1), ==, 1);
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2), ==, 2);
>
> - topo_info = (X86CPUTopoInfo) {0, 1, 6, 3};
> + topo_info = (X86CPUTopoInfo) {1, 6, 3};
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0), ==,
> (1 << 2) | 0);
> g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1), ==,
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 09/10] i386: Simplify CPUID_8000_001E for AMD
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (7 preceding siblings ...)
2020-08-31 18:42 ` [PATCH v6 08/10] Revert "hw/i386: Update structures to save the number of nodes per package" Babu Moger
@ 2020-08-31 18:43 ` Babu Moger
2020-09-01 11:52 ` Igor Mammedov
2020-08-31 18:43 ` [PATCH v6 10/10] " Babu Moger
2020-09-01 13:33 ` [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Eduardo Habkost
10 siblings, 1 reply; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:43 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
Remove all the hardcoded values and replace with generalized
fields.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ba4667b33c..d434c8545a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -395,9 +395,10 @@ static int cores_in_core_complex(int nr_cores)
}
/* Encode cache info for CPUID[8000001D] */
-static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
- uint32_t *eax, uint32_t *ebx,
- uint32_t *ecx, uint32_t *edx)
+static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
+ X86CPUTopoInfo *topo_info,
+ uint32_t *eax, uint32_t *ebx,
+ uint32_t *ecx, uint32_t *edx)
{
uint32_t l3_cores;
assert(cache->size == cache->line_size * cache->associativity *
@@ -408,10 +409,12 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
/* L3 is shared among multiple cores */
if (cache->level == 3) {
- l3_cores = cores_in_core_complex(cs->nr_cores);
- *eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
+ l3_cores = DIV_ROUND_UP((topo_info->cores_per_die *
+ topo_info->threads_per_core),
+ topo_info->dies_per_pkg);
+ *eax |= (l3_cores - 1) << 14;
} else {
- *eax |= ((cs->nr_threads - 1) << 14);
+ *eax |= ((topo_info->threads_per_core - 1) << 14);
}
assert(cache->line_size > 0);
@@ -5994,20 +5997,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
switch (count) {
case 0: /* L1 dcache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, cs,
- eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache,
+ &topo_info, eax, ebx, ecx, edx);
break;
case 1: /* L1 icache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, cs,
- eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache,
+ &topo_info, eax, ebx, ecx, edx);
break;
case 2: /* L2 cache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, cs,
- eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache,
+ &topo_info, eax, ebx, ecx, edx);
break;
case 3: /* L3 cache info */
- encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, cs,
- eax, ebx, ecx, edx);
+ encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache,
+ &topo_info, eax, ebx, ecx, edx);
break;
default: /* end of info */
*eax = *ebx = *ecx = *edx = 0;
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 09/10] i386: Simplify CPUID_8000_001E for AMD
2020-08-31 18:43 ` [PATCH v6 09/10] i386: Simplify CPUID_8000_001E for AMD Babu Moger
@ 2020-09-01 11:52 ` Igor Mammedov
2020-09-01 14:45 ` Babu Moger
0 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 11:52 UTC (permalink / raw)
To: Babu Moger; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On Mon, 31 Aug 2020 13:43:01 -0500
Babu Moger <babu.moger@amd.com> wrote:
> Remove all the hardcoded values and replace with generalized
> fields.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> target/i386/cpu.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ba4667b33c..d434c8545a 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -395,9 +395,10 @@ static int cores_in_core_complex(int nr_cores)
> }
>
> /* Encode cache info for CPUID[8000001D] */
> -static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
> - uint32_t *eax, uint32_t *ebx,
> - uint32_t *ecx, uint32_t *edx)
> +static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
> + X86CPUTopoInfo *topo_info,
> + uint32_t *eax, uint32_t *ebx,
> + uint32_t *ecx, uint32_t *edx)
> {
> uint32_t l3_cores;
> assert(cache->size == cache->line_size * cache->associativity *
> @@ -408,10 +409,12 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
>
> /* L3 is shared among multiple cores */
> if (cache->level == 3) {
> - l3_cores = cores_in_core_complex(cs->nr_cores);
> - *eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
> + l3_cores = DIV_ROUND_UP((topo_info->cores_per_die *
> + topo_info->threads_per_core),
> + topo_info->dies_per_pkg);
from spec:
"
NumSharingCache: number of '''logical processors''' sharing cache.
"
s/l3_cores/l3_vcpus|l3_threads/
Also why not use just:
val = topo_info->cores_per_die * topo_info->threads_per_core
> + *eax |= (l3_cores - 1) << 14;
> } else {
> - *eax |= ((cs->nr_threads - 1) << 14);
> + *eax |= ((topo_info->threads_per_core - 1) << 14);
> }
>
> assert(cache->line_size > 0);
> @@ -5994,20 +5997,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> }
> switch (count) {
> case 0: /* L1 dcache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, cs,
> - eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache,
> + &topo_info, eax, ebx, ecx, edx);
> break;
> case 1: /* L1 icache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, cs,
> - eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache,
> + &topo_info, eax, ebx, ecx, edx);
> break;
> case 2: /* L2 cache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, cs,
> - eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache,
> + &topo_info, eax, ebx, ecx, edx);
> break;
> case 3: /* L3 cache info */
> - encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, cs,
> - eax, ebx, ecx, edx);
> + encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache,
> + &topo_info, eax, ebx, ecx, edx);
> break;
> default: /* end of info */
> *eax = *ebx = *ecx = *edx = 0;
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 09/10] i386: Simplify CPUID_8000_001E for AMD
2020-09-01 11:52 ` Igor Mammedov
@ 2020-09-01 14:45 ` Babu Moger
0 siblings, 0 replies; 25+ messages in thread
From: Babu Moger @ 2020-09-01 14:45 UTC (permalink / raw)
To: Igor Mammedov; +Cc: ehabkost, mst, qemu-devel, pbonzini, rth
On 9/1/20 6:52 AM, Igor Mammedov wrote:
> On Mon, 31 Aug 2020 13:43:01 -0500
> Babu Moger <babu.moger@amd.com> wrote:
>
>> Remove all the hardcoded values and replace with generalized
>> fields.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> target/i386/cpu.c | 31 +++++++++++++++++--------------
>> 1 file changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index ba4667b33c..d434c8545a 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -395,9 +395,10 @@ static int cores_in_core_complex(int nr_cores)
>> }
>>
>> /* Encode cache info for CPUID[8000001D] */
>> -static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
>> - uint32_t *eax, uint32_t *ebx,
>> - uint32_t *ecx, uint32_t *edx)
>> +static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
>> + X86CPUTopoInfo *topo_info,
>> + uint32_t *eax, uint32_t *ebx,
>> + uint32_t *ecx, uint32_t *edx)
>> {
>> uint32_t l3_cores;
>> assert(cache->size == cache->line_size * cache->associativity *
>> @@ -408,10 +409,12 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
>>
>> /* L3 is shared among multiple cores */
>> if (cache->level == 3) {
>> - l3_cores = cores_in_core_complex(cs->nr_cores);
>> - *eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
>> + l3_cores = DIV_ROUND_UP((topo_info->cores_per_die *
>> + topo_info->threads_per_core),
>> + topo_info->dies_per_pkg);
>
> from spec:
> "
> NumSharingCache: number of '''logical processors''' sharing cache.
> "
>
> s/l3_cores/l3_vcpus|l3_threads/
>
Sure.
> Also why not use just:
>
> val = topo_info->cores_per_die * topo_info->threads_per_core
Yes. You are right. Will correct it. thanks
>
>
>
>> + *eax |= (l3_cores - 1) << 14;
>> } else {
>> - *eax |= ((cs->nr_threads - 1) << 14);
>> + *eax |= ((topo_info->threads_per_core - 1) << 14);
>> }
>>
>> assert(cache->line_size > 0);
>> @@ -5994,20 +5997,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>> }
>> switch (count) {
>> case 0: /* L1 dcache info */
>> - encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, cs,
>> - eax, ebx, ecx, edx);
>> + encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache,
>> + &topo_info, eax, ebx, ecx, edx);
>> break;
>> case 1: /* L1 icache info */
>> - encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, cs,
>> - eax, ebx, ecx, edx);
>> + encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache,
>> + &topo_info, eax, ebx, ecx, edx);
>> break;
>> case 2: /* L2 cache info */
>> - encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, cs,
>> - eax, ebx, ecx, edx);
>> + encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache,
>> + &topo_info, eax, ebx, ecx, edx);
>> break;
>> case 3: /* L3 cache info */
>> - encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, cs,
>> - eax, ebx, ecx, edx);
>> + encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache,
>> + &topo_info, eax, ebx, ecx, edx);
>> break;
>> default: /* end of info */
>> *eax = *ebx = *ecx = *edx = 0;
>>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 10/10] i386: Simplify CPUID_8000_001E for AMD
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (8 preceding siblings ...)
2020-08-31 18:43 ` [PATCH v6 09/10] i386: Simplify CPUID_8000_001E for AMD Babu Moger
@ 2020-08-31 18:43 ` Babu Moger
2020-09-01 11:58 ` Igor Mammedov
2020-09-18 21:38 ` Eduardo Habkost
2020-09-01 13:33 ` [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Eduardo Habkost
10 siblings, 2 replies; 25+ messages in thread
From: Babu Moger @ 2020-08-31 18:43 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, imammedo; +Cc: babu.moger, qemu-devel, mst
apic_id contains all the information required to build
CPUID_8000_001E. core_id and node_id is already part of
apic_id generated by x86_topo_ids_from_apicid.
Also remove the restriction on number bits on core_id and
node_id.
Remove all the hardcoded values and replace with generalized
fields.
Refer the Processor Programming Reference (PPR) documentation
available from the bugzilla Link below.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 195 ++++++++++++-----------------------------------------
1 file changed, 45 insertions(+), 150 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d434c8545a..ada9ec8f3a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -338,62 +338,6 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
}
}
-/*
- * Definitions used for building CPUID Leaf 0x8000001D and 0x8000001E
- * Please refer to the AMD64 Architecture Programmer’s Manual Volume 3.
- * Define the constants to build the cpu topology. Right now, TOPOEXT
- * feature is enabled only on EPYC. So, these constants are based on
- * EPYC supported configurations. We may need to handle the cases if
- * these values change in future.
- */
-/* Maximum core complexes in a node */
-#define MAX_CCX 2
-/* Maximum cores in a core complex */
-#define MAX_CORES_IN_CCX 4
-/* Maximum cores in a node */
-#define MAX_CORES_IN_NODE 8
-/* Maximum nodes in a socket */
-#define MAX_NODES_PER_SOCKET 4
-
-/*
- * Figure out the number of nodes required to build this config.
- * Max cores in a node is 8
- */
-static int nodes_in_socket(int nr_cores)
-{
- int nodes;
-
- nodes = DIV_ROUND_UP(nr_cores, MAX_CORES_IN_NODE);
-
- /* Hardware does not support config with 3 nodes, return 4 in that case */
- return (nodes == 3) ? 4 : nodes;
-}
-
-/*
- * Decide the number of cores in a core complex with the given nr_cores using
- * following set constants MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE and
- * MAX_NODES_PER_SOCKET. Maintain symmetry as much as possible
- * L3 cache is shared across all cores in a core complex. So, this will also
- * tell us how many cores are sharing the L3 cache.
- */
-static int cores_in_core_complex(int nr_cores)
-{
- int nodes;
-
- /* Check if we can fit all the cores in one core complex */
- if (nr_cores <= MAX_CORES_IN_CCX) {
- return nr_cores;
- }
- /* Get the number of nodes required to build this config */
- nodes = nodes_in_socket(nr_cores);
-
- /*
- * Divide the cores accros all the core complexes
- * Return rounded up value
- */
- return DIV_ROUND_UP(nr_cores, nodes * MAX_CCX);
-}
-
/* Encode cache info for CPUID[8000001D] */
static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
X86CPUTopoInfo *topo_info,
@@ -434,107 +378,58 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
(cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
}
-/* Data structure to hold the configuration info for a given core index */
-struct core_topology {
- /* core complex id of the current core index */
- int ccx_id;
- /*
- * Adjusted core index for this core in the topology
- * This can be 0,1,2,3 with max 4 cores in a core complex
- */
- int core_id;
- /* Node id for this core index */
- int node_id;
- /* Number of nodes in this config */
- int num_nodes;
-};
-
-/*
- * Build the configuration closely match the EPYC hardware. Using the EPYC
- * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE)
- * right now. This could change in future.
- * nr_cores : Total number of cores in the config
- * core_id : Core index of the current CPU
- * topo : Data structure to hold all the config info for this core index
- */
-static void build_core_topology(int nr_cores, int core_id,
- struct core_topology *topo)
-{
- int nodes, cores_in_ccx;
-
- /* First get the number of nodes required */
- nodes = nodes_in_socket(nr_cores);
-
- cores_in_ccx = cores_in_core_complex(nr_cores);
-
- topo->node_id = core_id / (cores_in_ccx * MAX_CCX);
- topo->ccx_id = (core_id % (cores_in_ccx * MAX_CCX)) / cores_in_ccx;
- topo->core_id = core_id % cores_in_ccx;
- topo->num_nodes = nodes;
-}
-
/* Encode cache info for CPUID[8000001E] */
-static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
- uint32_t *eax, uint32_t *ebx,
- uint32_t *ecx, uint32_t *edx)
+static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
+ uint32_t *eax, uint32_t *ebx,
+ uint32_t *ecx, uint32_t *edx)
{
- struct core_topology topo = {0};
- unsigned long nodes;
- int shift;
+ X86CPUTopoIDs topo_ids;
+
+ x86_topo_ids_from_apicid(cpu->apic_id, topo_info, &topo_ids);
- build_core_topology(cs->nr_cores, cpu->core_id, &topo);
*eax = cpu->apic_id;
+
/*
- * CPUID_Fn8000001E_EBX
- * 31:16 Reserved
- * 15:8 Threads per core (The number of threads per core is
- * Threads per core + 1)
- * 7:0 Core id (see bit decoding below)
- * SMT:
- * 4:3 node id
- * 2 Core complex id
- * 1:0 Core id
- * Non SMT:
- * 5:4 node id
- * 3 Core complex id
- * 1:0 Core id
+ * CPUID_Fn8000001E_EBX [Core Identifiers] (CoreId)
+ * Read-only. Reset: 0000_XXXXh.
+ * See Core::X86::Cpuid::ExtApicId.
+ * Core::X86::Cpuid::CoreId_lthree[1:0]_core[3:0]_thread[1:0];
+ * Bits Description
+ * 31:16 Reserved.
+ * 15:8 ThreadsPerCore: threads per core. Read-only. Reset: XXh.
+ * The number of threads per core is ThreadsPerCore+1.
+ * 7:0 CoreId: core ID. Read-only. Reset: XXh.
+ *
+ * NOTE: CoreId is already part of apic_id. Just use it. We can
+ * use all the 8 bits to represent the core_id here.
*/
- if (cs->nr_threads - 1) {
- *ebx = ((cs->nr_threads - 1) << 8) | (topo.node_id << 3) |
- (topo.ccx_id << 2) | topo.core_id;
- } else {
- *ebx = (topo.node_id << 4) | (topo.ccx_id << 3) | topo.core_id;
- }
+ *ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.core_id & 0xFF);
+
/*
- * CPUID_Fn8000001E_ECX
- * 31:11 Reserved
- * 10:8 Nodes per processor (Nodes per processor is number of nodes + 1)
- * 7:0 Node id (see bit decoding below)
- * 2 Socket id
- * 1:0 Node id
+ * CPUID_Fn8000001E_ECX [Node Identifiers] (NodeId)
+ * Read-only. Reset: 0000_0XXXh.
+ * Core::X86::Cpuid::NodeId_lthree[1:0]_core[3:0]_thread[1:0];
+ * Bits Description
+ * 31:11 Reserved.
+ * 10:8 NodesPerProcessor: Node per processor. Read-only. Reset: XXXb.
+ * ValidValues:
+ * Value Description
+ * 000b 1 node per processor.
+ * 001b 2 nodes per processor.
+ * 010b Reserved.
+ * 011b 4 nodes per processor.
+ * 111b-100b Reserved.
+ * 7:0 NodeId: Node ID. Read-only. Reset: XXh.
+ *
+ * NOTE: Hardware reserves 3 bits for number of nodes per processor.
+ * But users can create more nodes than the actual hardware can
+ * support. To genaralize we can use all the upper 8 bits for nodes.
+ * NodeId is combination of node and socket_id which is already decoded
+ * in apic_id. Just use it by shifting.
*/
- if (topo.num_nodes <= 4) {
- *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
- topo.node_id;
- } else {
- /*
- * Node id fix up. Actual hardware supports up to 4 nodes. But with
- * more than 32 cores, we may end up with more than 4 nodes.
- * Node id is a combination of socket id and node id. Only requirement
- * here is that this number should be unique accross the system.
- * Shift the socket id to accommodate more nodes. We dont expect both
- * socket id and node id to be big number at the same time. This is not
- * an ideal config but we need to to support it. Max nodes we can have
- * is 32 (255/8) with 8 cores per node and 255 max cores. We only need
- * 5 bits for nodes. Find the left most set bit to represent the total
- * number of nodes. find_last_bit returns last set bit(0 based). Left
- * shift(+1) the socket id to represent all the nodes.
- */
- nodes = topo.num_nodes - 1;
- shift = find_last_bit(&nodes, 8);
- *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << (shift + 1)) |
- topo.node_id;
- }
+ *ecx = ((topo_info->dies_per_pkg - 1) << 8) |
+ ((cpu->apic_id >> apicid_die_offset(topo_info)) & 0xFF);
+
*edx = 0;
}
@@ -6019,7 +5914,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
break;
case 0x8000001E:
assert(cpu->core_id <= 255);
- encode_topo_cpuid8000001e(cs, cpu,
+ encode_topo_cpuid8000001e(cpu, &topo_info,
eax, ebx, ecx, edx);
break;
case 0xC0000000:
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 10/10] i386: Simplify CPUID_8000_001E for AMD
2020-08-31 18:43 ` [PATCH v6 10/10] " Babu Moger
@ 2020-09-01 11:58 ` Igor Mammedov
2020-09-18 21:38 ` Eduardo Habkost
1 sibling, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2020-09-01 11:58 UTC (permalink / raw)
To: Babu Moger; +Cc: qemu-devel, pbonzini, mst, ehabkost, rth
On Mon, 31 Aug 2020 13:43:07 -0500
Babu Moger <babu.moger@amd.com> wrote:
> apic_id contains all the information required to build
> CPUID_8000_001E. core_id and node_id is already part of
> apic_id generated by x86_topo_ids_from_apicid.
>
> Also remove the restriction on number bits on core_id and
> node_id.
>
> Remove all the hardcoded values and replace with generalized
> fields.
>
> Refer the Processor Programming Reference (PPR) documentation
> available from the bugzilla Link below.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> target/i386/cpu.c | 195 ++++++++++++-----------------------------------------
> 1 file changed, 45 insertions(+), 150 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d434c8545a..ada9ec8f3a 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -338,62 +338,6 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
> }
> }
>
> -/*
> - * Definitions used for building CPUID Leaf 0x8000001D and 0x8000001E
> - * Please refer to the AMD64 Architecture Programmer’s Manual Volume 3.
> - * Define the constants to build the cpu topology. Right now, TOPOEXT
> - * feature is enabled only on EPYC. So, these constants are based on
> - * EPYC supported configurations. We may need to handle the cases if
> - * these values change in future.
> - */
> -/* Maximum core complexes in a node */
> -#define MAX_CCX 2
> -/* Maximum cores in a core complex */
> -#define MAX_CORES_IN_CCX 4
> -/* Maximum cores in a node */
> -#define MAX_CORES_IN_NODE 8
> -/* Maximum nodes in a socket */
> -#define MAX_NODES_PER_SOCKET 4
> -
> -/*
> - * Figure out the number of nodes required to build this config.
> - * Max cores in a node is 8
> - */
> -static int nodes_in_socket(int nr_cores)
> -{
> - int nodes;
> -
> - nodes = DIV_ROUND_UP(nr_cores, MAX_CORES_IN_NODE);
> -
> - /* Hardware does not support config with 3 nodes, return 4 in that case */
> - return (nodes == 3) ? 4 : nodes;
> -}
> -
> -/*
> - * Decide the number of cores in a core complex with the given nr_cores using
> - * following set constants MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE and
> - * MAX_NODES_PER_SOCKET. Maintain symmetry as much as possible
> - * L3 cache is shared across all cores in a core complex. So, this will also
> - * tell us how many cores are sharing the L3 cache.
> - */
> -static int cores_in_core_complex(int nr_cores)
> -{
> - int nodes;
> -
> - /* Check if we can fit all the cores in one core complex */
> - if (nr_cores <= MAX_CORES_IN_CCX) {
> - return nr_cores;
> - }
> - /* Get the number of nodes required to build this config */
> - nodes = nodes_in_socket(nr_cores);
> -
> - /*
> - * Divide the cores accros all the core complexes
> - * Return rounded up value
> - */
> - return DIV_ROUND_UP(nr_cores, nodes * MAX_CCX);
> -}
> -
> /* Encode cache info for CPUID[8000001D] */
> static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
> X86CPUTopoInfo *topo_info,
> @@ -434,107 +378,58 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
> (cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
> }
>
> -/* Data structure to hold the configuration info for a given core index */
> -struct core_topology {
> - /* core complex id of the current core index */
> - int ccx_id;
> - /*
> - * Adjusted core index for this core in the topology
> - * This can be 0,1,2,3 with max 4 cores in a core complex
> - */
> - int core_id;
> - /* Node id for this core index */
> - int node_id;
> - /* Number of nodes in this config */
> - int num_nodes;
> -};
> -
> -/*
> - * Build the configuration closely match the EPYC hardware. Using the EPYC
> - * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE)
> - * right now. This could change in future.
> - * nr_cores : Total number of cores in the config
> - * core_id : Core index of the current CPU
> - * topo : Data structure to hold all the config info for this core index
> - */
> -static void build_core_topology(int nr_cores, int core_id,
> - struct core_topology *topo)
> -{
> - int nodes, cores_in_ccx;
> -
> - /* First get the number of nodes required */
> - nodes = nodes_in_socket(nr_cores);
> -
> - cores_in_ccx = cores_in_core_complex(nr_cores);
> -
> - topo->node_id = core_id / (cores_in_ccx * MAX_CCX);
> - topo->ccx_id = (core_id % (cores_in_ccx * MAX_CCX)) / cores_in_ccx;
> - topo->core_id = core_id % cores_in_ccx;
> - topo->num_nodes = nodes;
> -}
> -
> /* Encode cache info for CPUID[8000001E] */
> -static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
> - uint32_t *eax, uint32_t *ebx,
> - uint32_t *ecx, uint32_t *edx)
> +static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
> + uint32_t *eax, uint32_t *ebx,
> + uint32_t *ecx, uint32_t *edx)
> {
> - struct core_topology topo = {0};
> - unsigned long nodes;
> - int shift;
> + X86CPUTopoIDs topo_ids;
> +
> + x86_topo_ids_from_apicid(cpu->apic_id, topo_info, &topo_ids);
>
> - build_core_topology(cs->nr_cores, cpu->core_id, &topo);
> *eax = cpu->apic_id;
> +
> /*
> - * CPUID_Fn8000001E_EBX
> - * 31:16 Reserved
> - * 15:8 Threads per core (The number of threads per core is
> - * Threads per core + 1)
> - * 7:0 Core id (see bit decoding below)
> - * SMT:
> - * 4:3 node id
> - * 2 Core complex id
> - * 1:0 Core id
> - * Non SMT:
> - * 5:4 node id
> - * 3 Core complex id
> - * 1:0 Core id
> + * CPUID_Fn8000001E_EBX [Core Identifiers] (CoreId)
> + * Read-only. Reset: 0000_XXXXh.
> + * See Core::X86::Cpuid::ExtApicId.
> + * Core::X86::Cpuid::CoreId_lthree[1:0]_core[3:0]_thread[1:0];
> + * Bits Description
> + * 31:16 Reserved.
> + * 15:8 ThreadsPerCore: threads per core. Read-only. Reset: XXh.
> + * The number of threads per core is ThreadsPerCore+1.
> + * 7:0 CoreId: core ID. Read-only. Reset: XXh.
> + *
> + * NOTE: CoreId is already part of apic_id. Just use it. We can
> + * use all the 8 bits to represent the core_id here.
> */
> - if (cs->nr_threads - 1) {
> - *ebx = ((cs->nr_threads - 1) << 8) | (topo.node_id << 3) |
> - (topo.ccx_id << 2) | topo.core_id;
> - } else {
> - *ebx = (topo.node_id << 4) | (topo.ccx_id << 3) | topo.core_id;
> - }
> + *ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.core_id & 0xFF);
> +
> /*
> - * CPUID_Fn8000001E_ECX
> - * 31:11 Reserved
> - * 10:8 Nodes per processor (Nodes per processor is number of nodes + 1)
> - * 7:0 Node id (see bit decoding below)
> - * 2 Socket id
> - * 1:0 Node id
> + * CPUID_Fn8000001E_ECX [Node Identifiers] (NodeId)
> + * Read-only. Reset: 0000_0XXXh.
> + * Core::X86::Cpuid::NodeId_lthree[1:0]_core[3:0]_thread[1:0];
> + * Bits Description
> + * 31:11 Reserved.
> + * 10:8 NodesPerProcessor: Node per processor. Read-only. Reset: XXXb.
> + * ValidValues:
> + * Value Description
> + * 000b 1 node per processor.
> + * 001b 2 nodes per processor.
> + * 010b Reserved.
> + * 011b 4 nodes per processor.
> + * 111b-100b Reserved.
> + * 7:0 NodeId: Node ID. Read-only. Reset: XXh.
> + *
> + * NOTE: Hardware reserves 3 bits for number of nodes per processor.
> + * But users can create more nodes than the actual hardware can
> + * support. To genaralize we can use all the upper 8 bits for nodes.
> + * NodeId is combination of node and socket_id which is already decoded
> + * in apic_id. Just use it by shifting.
> */
> - if (topo.num_nodes <= 4) {
> - *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
> - topo.node_id;
> - } else {
> - /*
> - * Node id fix up. Actual hardware supports up to 4 nodes. But with
> - * more than 32 cores, we may end up with more than 4 nodes.
> - * Node id is a combination of socket id and node id. Only requirement
> - * here is that this number should be unique accross the system.
> - * Shift the socket id to accommodate more nodes. We dont expect both
> - * socket id and node id to be big number at the same time. This is not
> - * an ideal config but we need to to support it. Max nodes we can have
> - * is 32 (255/8) with 8 cores per node and 255 max cores. We only need
> - * 5 bits for nodes. Find the left most set bit to represent the total
> - * number of nodes. find_last_bit returns last set bit(0 based). Left
> - * shift(+1) the socket id to represent all the nodes.
> - */
> - nodes = topo.num_nodes - 1;
> - shift = find_last_bit(&nodes, 8);
> - *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << (shift + 1)) |
> - topo.node_id;
> - }
> + *ecx = ((topo_info->dies_per_pkg - 1) << 8) |
> + ((cpu->apic_id >> apicid_die_offset(topo_info)) & 0xFF);
> +
> *edx = 0;
> }
>
> @@ -6019,7 +5914,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> break;
> case 0x8000001E:
> assert(cpu->core_id <= 255);
> - encode_topo_cpuid8000001e(cs, cpu,
> + encode_topo_cpuid8000001e(cpu, &topo_info,
> eax, ebx, ecx, edx);
> break;
> case 0xC0000000:
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 10/10] i386: Simplify CPUID_8000_001E for AMD
2020-08-31 18:43 ` [PATCH v6 10/10] " Babu Moger
2020-09-01 11:58 ` Igor Mammedov
@ 2020-09-18 21:38 ` Eduardo Habkost
2020-09-18 21:41 ` Babu Moger
1 sibling, 1 reply; 25+ messages in thread
From: Eduardo Habkost @ 2020-09-18 21:38 UTC (permalink / raw)
To: Babu Moger; +Cc: pbonzini, imammedo, mst, qemu-devel, rth
On Mon, Aug 31, 2020 at 01:43:07PM -0500, Babu Moger wrote:
> apic_id contains all the information required to build
> CPUID_8000_001E. core_id and node_id is already part of
> apic_id generated by x86_topo_ids_from_apicid.
>
> Also remove the restriction on number bits on core_id and
> node_id.
>
> Remove all the hardcoded values and replace with generalized
> fields.
>
> Refer the Processor Programming Reference (PPR) documentation
> available from the bugzilla Link below.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
[...]
> @@ -6019,7 +5914,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> break;
> case 0x8000001E:
> assert(cpu->core_id <= 255);
I assume we still want to remove this assert()? Do you plan to
redo and resubmit
Subject: [PATCH] target/i386: Remove core_id assert check in CPUID 0x8000001E
https://lore.kernel.org/qemu-devel/159257395689.52908.4409314503988289481.stgit@naples-babu.amd.com
?
> - encode_topo_cpuid8000001e(cs, cpu,
> + encode_topo_cpuid8000001e(cpu, &topo_info,
> eax, ebx, ecx, edx);
> break;
> case 0xC0000000:
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 10/10] i386: Simplify CPUID_8000_001E for AMD
2020-09-18 21:38 ` Eduardo Habkost
@ 2020-09-18 21:41 ` Babu Moger
0 siblings, 0 replies; 25+ messages in thread
From: Babu Moger @ 2020-09-18 21:41 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: pbonzini, imammedo, mst, qemu-devel, rth
On 9/18/20 4:38 PM, Eduardo Habkost wrote:
> On Mon, Aug 31, 2020 at 01:43:07PM -0500, Babu Moger wrote:
>> apic_id contains all the information required to build
>> CPUID_8000_001E. core_id and node_id is already part of
>> apic_id generated by x86_topo_ids_from_apicid.
>>
>> Also remove the restriction on number bits on core_id and
>> node_id.
>>
>> Remove all the hardcoded values and replace with generalized
>> fields.
>>
>> Refer the Processor Programming Reference (PPR) documentation
>> available from the bugzilla Link below.
>>
>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D206537&data=02%7C01%7Cbabu.moger%40amd.com%7C3d768d58412043311f8208d85c1b2432%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360619193685138&sdata=NEWhmXx4QbtPv9XVFOqw1i3AbSm74qjC%2FqZXU9E0BJ0%3D&reserved=0
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
> [...]
>> @@ -6019,7 +5914,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>> break;
>> case 0x8000001E:
>> assert(cpu->core_id <= 255);
>
> I assume we still want to remove this assert()? Do you plan to
> redo and resubmit
Sure. I will resubmit it.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode
2020-08-31 18:42 [PATCH v6 00/10] Remove EPYC mode apicid decode and use generic decode Babu Moger
` (9 preceding siblings ...)
2020-08-31 18:43 ` [PATCH v6 10/10] " Babu Moger
@ 2020-09-01 13:33 ` Eduardo Habkost
10 siblings, 0 replies; 25+ messages in thread
From: Eduardo Habkost @ 2020-09-01 13:33 UTC (permalink / raw)
To: Babu Moger; +Cc: mst, qemu-devel, imammedo, pbonzini, rth
On Mon, Aug 31, 2020 at 01:42:04PM -0500, Babu Moger wrote:
> To support some of the complex topology, we introduced EPYC mode apicid decode.
> But, EPYC mode decode is running into problems. Also it can become quite a
> maintenance problem in the future. So, it was decided to remove that code and
> use the generic decode which works for majority of the topology. Most of the
> SPECed configuration would work just fine. With some non-SPECed user inputs,
> it will create some sub-optimal configuration.
[...]
>
Thank you, I'm queueing patches 1-8 on machine-next.
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread