| Message ID | 159362466828.36204.14044362989991188460.stgit@naples-babu.amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix couple of issues with AMD topology | expand |
On Wed, 01 Jul 2020 12:31:08 -0500 Babu Moger <babu.moger@amd.com> wrote: > Build apic_id from CpuInstanceProperties if numa configured. > Use the node_id from user provided numa information. This > will avoid conflicts between numa information and apic_id > generated. > > Re-arranged the code little bit to make sure CpuInstanceProperties > is initialized before calling. > > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > hw/i386/pc.c | 6 +++++- > hw/i386/x86.c | 19 +++++++++++++------ > include/hw/i386/topology.h | 14 +++++++++++--- > include/hw/i386/x86.h | 6 ++++-- > tests/test-x86-cpuid.c | 39 ++++++++++++++++++++------------------- > 5 files changed, 53 insertions(+), 31 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d103b8c0ab..e613b2299f 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -800,13 +800,17 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts) > void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp) > { > X86MachineState *x86ms = X86_MACHINE(ms); > - int64_t apic_id = x86_cpu_apic_id_from_index(x86ms, id); > + CpuInstanceProperties props; > + int64_t apic_id; > Error *local_err = NULL; > > if (id < 0) { > error_setg(errp, "Invalid CPU id: %" PRIi64, id); > return; > } > + props = ms->possible_cpus->cpus[id].props; > + > + apic_id = x86_cpu_apic_id_from_index(x86ms, id, props); > > if (apic_id >= ACPI_CPU_HOTPLUG_ID_LIMIT) { > error_setg(errp, "Unable to add CPU: %" PRIi64 > diff --git a/hw/i386/x86.c b/hw/i386/x86.c > index 34229b45c7..7554416ae0 100644 > --- a/hw/i386/x86.c > +++ b/hw/i386/x86.c > @@ -93,7 +93,8 @@ static void x86_set_epyc_topo_handlers(MachineState *machine) > * all CPUs up to max_cpus. > */ > uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms, > - unsigned int cpu_index) > + unsigned int cpu_index, > + CpuInstanceProperties props) > { > X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms); > X86CPUTopoInfo topo_info; > @@ -102,7 +103,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 = x86ms->apicid_from_cpu_idx(&topo_info, cpu_index, props); > if (x86mc->compat_apic_id_mode) { > if (cpu_index != correct_id && !warned && !qtest_enabled()) { > error_report("APIC IDs set in compatibility mode, " > @@ -136,6 +137,8 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) > const CPUArchIdList *possible_cpus; > MachineState *ms = MACHINE(x86ms); > MachineClass *mc = MACHINE_GET_CLASS(x86ms); > + CpuInstanceProperties props; > + > > /* Check for apicid encoding */ > if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) { > @@ -144,6 +147,8 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) > > x86_cpu_set_default_version(default_cpu_version); > > + possible_cpus = mc->possible_cpu_arch_ids(ms); > + > /* > * Calculates the limit to CPU APIC ID values > * > @@ -152,13 +157,15 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) > * > * This is used for FW_CFG_MAX_CPUS. See comments on fw_cfg_arch_create(). > */ > - 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); > + props = ms->possible_cpus->cpus[ms->smp.max_cpus - 1].props; > > + x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms, > + ms->smp.max_cpus - 1, > + props) + 1; > for (i = 0; i < ms->possible_cpus->len; i++) { > + props = ms->possible_cpus->cpus[i].props; > ms->possible_cpus->cpus[i].arch_id = > - x86_cpu_apic_id_from_index(x86ms, i); > + x86_cpu_apic_id_from_index(x86ms, i, props); > } > > for (i = 0; i < ms->smp.cpus; i++) { > diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h > index 7cb21e9c82..a800fc905f 100644 > --- a/include/hw/i386/topology.h > +++ b/include/hw/i386/topology.h > @@ -221,10 +221,17 @@ static inline void x86_init_topo_ids(X86CPUTopoInfo *topo_info, > * '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) > + unsigned cpu_index, > + CpuInstanceProperties props) > { > X86CPUTopoIDs topo_ids; > - x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids); > + > + if (props.has_node_id) { > + x86_init_topo_ids(topo_info, props, &topo_ids); > + } else { > + x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids); why this branch is needed? > + } > + > 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 > @@ -280,7 +287,8 @@ static inline void x86_topo_ids_from_apicid(apic_id_t apicid, > * 'cpu_index' is a sequential, contiguous ID for the CPU. > */ > static inline apic_id_t x86_apicid_from_cpu_idx(X86CPUTopoInfo *topo_info, > - unsigned cpu_index) > + unsigned cpu_index, > + CpuInstanceProperties props) > { > X86CPUTopoIDs topo_ids; > x86_topo_ids_from_idx(topo_info, cpu_index, &topo_ids); > diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h > index b79f24e285..3109f39554 100644 > --- a/include/hw/i386/x86.h > +++ b/include/hw/i386/x86.h > @@ -65,7 +65,8 @@ typedef struct { > > /* Apic id specific handlers */ > uint32_t (*apicid_from_cpu_idx)(X86CPUTopoInfo *topo_info, > - unsigned cpu_index); > + unsigned cpu_index, > + CpuInstanceProperties props); > 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, > @@ -93,7 +94,8 @@ typedef struct { > void init_topo_info(X86CPUTopoInfo *topo_info, const X86MachineState *x86ms); > > uint32_t x86_cpu_apic_id_from_index(X86MachineState *pcms, > - unsigned int cpu_index); > + unsigned int cpu_index, > + CpuInstanceProperties props); > > void x86_cpu_new(X86MachineState *pcms, int64_t apic_id, Error **errp); > void x86_cpus_init(X86MachineState *pcms, int default_cpu_version); > diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c > index 049030a50e..a1308e214b 100644 > --- a/tests/test-x86-cpuid.c > +++ b/tests/test-x86-cpuid.c > @@ -29,6 +29,7 @@ > static void test_topo_bits(void) > { > X86CPUTopoInfo topo_info = {0}; > + CpuInstanceProperties props = {0}; > > /* simple tests for 1 thread per core, 1 core per die, 1 die per package */ > topo_info = (X86CPUTopoInfo) {0, 1, 1, 1}; > @@ -37,10 +38,10 @@ static void test_topo_bits(void) > g_assert_cmpuint(apicid_die_width(&topo_info), ==, 0); > > topo_info = (X86CPUTopoInfo) {0, 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); > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 3), ==, 3); > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0, props), ==, 0); > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1, props), ==, 1); > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2, props), ==, 2); > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 3, props), ==, 3); > > > /* Test field width calculation for multiple values > @@ -92,38 +93,38 @@ static void test_topo_bits(void) > g_assert_cmpuint(apicid_pkg_offset(&topo_info), ==, 5); > > topo_info = (X86CPUTopoInfo) {0, 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); > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0, props), ==, 0); > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1, props), ==, 1); > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2, props), ==, 2); > > topo_info = (X86CPUTopoInfo) {0, 1, 6, 3}; > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0, props), ==, > (1 << 2) | 0); > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1, props), ==, > (1 << 2) | 1); > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 2), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 2, props), ==, > (1 << 2) | 2); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 0), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 0, props), ==, > (2 << 2) | 0); > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 1), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 1, props), ==, > (2 << 2) | 1); > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 2), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 2, props), ==, > (2 << 2) | 2); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 0), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 0, props), ==, > (5 << 2) | 0); > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 1), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 1, props), ==, > (5 << 2) | 1); > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 2), ==, > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 2, props), ==, > (5 << 2) | 2); > > g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, > - 1 * 6 * 3 + 0 * 3 + 0), ==, (1 << 5)); > + 1 * 6 * 3 + 0 * 3 + 0, props), ==, (1 << 5)); > g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, > - 1 * 6 * 3 + 1 * 3 + 1), ==, (1 << 5) | (1 << 2) | 1); > + 1 * 6 * 3 + 1 * 3 + 1, props), ==, (1 << 5) | (1 << 2) | 1); > g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, > - 3 * 6 * 3 + 5 * 3 + 2), ==, (3 << 5) | (5 << 2) | 2); > + 3 * 6 * 3 + 5 * 3 + 2, props), ==, (3 << 5) | (5 << 2) | 2); > } > > int main(int argc, char **argv) >
> -----Original Message----- > From: Igor Mammedov <imammedo@redhat.com> > Sent: Monday, July 13, 2020 4:15 AM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com; qemu- > devel@nongnu.org > Subject: Re: [PATCH v2 2/3] hw/i386: Build apic_id from CpuInstanceProperties > > On Wed, 01 Jul 2020 12:31:08 -0500 > Babu Moger <babu.moger@amd.com> wrote: > > > Build apic_id from CpuInstanceProperties if numa configured. > > Use the node_id from user provided numa information. This will avoid > > conflicts between numa information and apic_id generated. > > > > Re-arranged the code little bit to make sure CpuInstanceProperties is > > initialized before calling. > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > --- > > hw/i386/pc.c | 6 +++++- > > hw/i386/x86.c | 19 +++++++++++++------ > > include/hw/i386/topology.h | 14 +++++++++++--- > > include/hw/i386/x86.h | 6 ++++-- > > tests/test-x86-cpuid.c | 39 ++++++++++++++++++++------------------- > > 5 files changed, 53 insertions(+), 31 deletions(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d103b8c0ab..e613b2299f > > 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -800,13 +800,17 @@ void pc_smp_parse(MachineState *ms, QemuOpts > > *opts) void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error > > **errp) { > > X86MachineState *x86ms = X86_MACHINE(ms); > > - int64_t apic_id = x86_cpu_apic_id_from_index(x86ms, id); > > + CpuInstanceProperties props; > > + int64_t apic_id; > > Error *local_err = NULL; > > > > if (id < 0) { > > error_setg(errp, "Invalid CPU id: %" PRIi64, id); > > return; > > } > > + props = ms->possible_cpus->cpus[id].props; > > + > > + apic_id = x86_cpu_apic_id_from_index(x86ms, id, props); > > > > if (apic_id >= ACPI_CPU_HOTPLUG_ID_LIMIT) { > > error_setg(errp, "Unable to add CPU: %" PRIi64 diff --git > > a/hw/i386/x86.c b/hw/i386/x86.c index 34229b45c7..7554416ae0 100644 > > --- a/hw/i386/x86.c > > +++ b/hw/i386/x86.c > > @@ -93,7 +93,8 @@ static void x86_set_epyc_topo_handlers(MachineState > *machine) > > * all CPUs up to max_cpus. > > */ > > uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms, > > - unsigned int cpu_index) > > + unsigned int cpu_index, > > + CpuInstanceProperties props) > > { > > X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms); > > X86CPUTopoInfo topo_info; > > @@ -102,7 +103,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 = x86ms->apicid_from_cpu_idx(&topo_info, cpu_index, > > + props); > > if (x86mc->compat_apic_id_mode) { > > if (cpu_index != correct_id && !warned && !qtest_enabled()) { > > error_report("APIC IDs set in compatibility mode, " > > @@ -136,6 +137,8 @@ void x86_cpus_init(X86MachineState *x86ms, int > default_cpu_version) > > const CPUArchIdList *possible_cpus; > > MachineState *ms = MACHINE(x86ms); > > MachineClass *mc = MACHINE_GET_CLASS(x86ms); > > + CpuInstanceProperties props; > > + > > > > /* Check for apicid encoding */ > > if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) { @@ -144,6 > > +147,8 @@ void x86_cpus_init(X86MachineState *x86ms, int > > default_cpu_version) > > > > x86_cpu_set_default_version(default_cpu_version); > > > > + possible_cpus = mc->possible_cpu_arch_ids(ms); > > + > > /* > > * Calculates the limit to CPU APIC ID values > > * > > @@ -152,13 +157,15 @@ void x86_cpus_init(X86MachineState *x86ms, int > default_cpu_version) > > * > > * This is used for FW_CFG_MAX_CPUS. See comments on > fw_cfg_arch_create(). > > */ > > - 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); > > + props = ms->possible_cpus->cpus[ms->smp.max_cpus - 1].props; > > > > + x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms, > > + ms->smp.max_cpus - 1, > > + props) + 1; > > for (i = 0; i < ms->possible_cpus->len; i++) { > > + props = ms->possible_cpus->cpus[i].props; > > ms->possible_cpus->cpus[i].arch_id = > > - x86_cpu_apic_id_from_index(x86ms, i); > > + x86_cpu_apic_id_from_index(x86ms, i, props); > > } > > > > for (i = 0; i < ms->smp.cpus; i++) { diff --git > > a/include/hw/i386/topology.h b/include/hw/i386/topology.h index > > 7cb21e9c82..a800fc905f 100644 > > --- a/include/hw/i386/topology.h > > +++ b/include/hw/i386/topology.h > > @@ -221,10 +221,17 @@ static inline void > x86_init_topo_ids(X86CPUTopoInfo *topo_info, > > * '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) > > + unsigned cpu_index, > > + > > + CpuInstanceProperties props) > > { > > X86CPUTopoIDs topo_ids; > > - x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids); > > + > > + if (props.has_node_id) { > > + x86_init_topo_ids(topo_info, props, &topo_ids); > > + } else { > > + x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids); > why this branch is needed? I have added it to make sure to handle the case if this function is called without initializing the props. I have to check if that case can happen. > > > + } > > + > > 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 @@ > > -280,7 +287,8 @@ static inline void x86_topo_ids_from_apicid(apic_id_t > apicid, > > * 'cpu_index' is a sequential, contiguous ID for the CPU. > > */ > > static inline apic_id_t x86_apicid_from_cpu_idx(X86CPUTopoInfo *topo_info, > > - unsigned cpu_index) > > + unsigned cpu_index, > > + CpuInstanceProperties > > + props) > > { > > X86CPUTopoIDs topo_ids; > > x86_topo_ids_from_idx(topo_info, cpu_index, &topo_ids); diff > > --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index > > b79f24e285..3109f39554 100644 > > --- a/include/hw/i386/x86.h > > +++ b/include/hw/i386/x86.h > > @@ -65,7 +65,8 @@ typedef struct { > > > > /* Apic id specific handlers */ > > uint32_t (*apicid_from_cpu_idx)(X86CPUTopoInfo *topo_info, > > - unsigned cpu_index); > > + unsigned cpu_index, > > + CpuInstanceProperties props); > > 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, @@ > > -93,7 +94,8 @@ typedef struct { void init_topo_info(X86CPUTopoInfo > > *topo_info, const X86MachineState *x86ms); > > > > uint32_t x86_cpu_apic_id_from_index(X86MachineState *pcms, > > - unsigned int cpu_index); > > + unsigned int cpu_index, > > + CpuInstanceProperties props); > > > > void x86_cpu_new(X86MachineState *pcms, int64_t apic_id, Error > > **errp); void x86_cpus_init(X86MachineState *pcms, int > > default_cpu_version); diff --git a/tests/test-x86-cpuid.c > > b/tests/test-x86-cpuid.c index 049030a50e..a1308e214b 100644 > > --- a/tests/test-x86-cpuid.c > > +++ b/tests/test-x86-cpuid.c > > @@ -29,6 +29,7 @@ > > static void test_topo_bits(void) > > { > > X86CPUTopoInfo topo_info = {0}; > > + CpuInstanceProperties props = {0}; > > > > /* simple tests for 1 thread per core, 1 core per die, 1 die per package */ > > topo_info = (X86CPUTopoInfo) {0, 1, 1, 1}; @@ -37,10 +38,10 @@ > > static void test_topo_bits(void) > > g_assert_cmpuint(apicid_die_width(&topo_info), ==, 0); > > > > topo_info = (X86CPUTopoInfo) {0, 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); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 3), ==, 3); > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0, props), ==, 0); > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1, props), ==, 1); > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2, props), ==, 2); > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 3, props), > > + ==, 3); > > > > > > /* Test field width calculation for multiple values @@ -92,38 > > +93,38 @@ static void test_topo_bits(void) > > g_assert_cmpuint(apicid_pkg_offset(&topo_info), ==, 5); > > > > topo_info = (X86CPUTopoInfo) {0, 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); > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0, props), ==, 0); > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1, props), ==, 1); > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2, props), > > + ==, 2); > > > > topo_info = (X86CPUTopoInfo) {0, 1, 6, 3}; > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0, > > + props), ==, > > (1 << 2) | 0); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1, > > + props), ==, > > (1 << 2) | 1); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 2), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 2, > > + props), ==, > > (1 << 2) | 2); > > > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 0), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 0, > > + props), ==, > > (2 << 2) | 0); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 1), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 1, > > + props), ==, > > (2 << 2) | 1); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 2), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 2, > > + props), ==, > > (2 << 2) | 2); > > > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 0), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 0, > > + props), ==, > > (5 << 2) | 0); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 1), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 1, > > + props), ==, > > (5 << 2) | 1); > > - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 2), ==, > > + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 2, > > + props), ==, > > (5 << 2) | 2); > > > > g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, > > - 1 * 6 * 3 + 0 * 3 + 0), ==, (1 << 5)); > > + 1 * 6 * 3 + 0 * 3 + 0, props), ==, (1 << 5)); > > g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, > > - 1 * 6 * 3 + 1 * 3 + 1), ==, (1 << 5) | (1 << 2) | 1); > > + 1 * 6 * 3 + 1 * 3 + 1, props), ==, (1 << 5) | (1 > > + << 2) | 1); > > g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, > > - 3 * 6 * 3 + 5 * 3 + 2), ==, (3 << 5) | (5 << 2) | 2); > > + 3 * 6 * 3 + 5 * 3 + 2, props), ==, (3 << 5) | (5 > > + << 2) | 2); > > } > > > > int main(int argc, char **argv) > >
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d103b8c0ab..e613b2299f 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -800,13 +800,17 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts) void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp) { X86MachineState *x86ms = X86_MACHINE(ms); - int64_t apic_id = x86_cpu_apic_id_from_index(x86ms, id); + CpuInstanceProperties props; + int64_t apic_id; Error *local_err = NULL; if (id < 0) { error_setg(errp, "Invalid CPU id: %" PRIi64, id); return; } + props = ms->possible_cpus->cpus[id].props; + + apic_id = x86_cpu_apic_id_from_index(x86ms, id, props); if (apic_id >= ACPI_CPU_HOTPLUG_ID_LIMIT) { error_setg(errp, "Unable to add CPU: %" PRIi64 diff --git a/hw/i386/x86.c b/hw/i386/x86.c index 34229b45c7..7554416ae0 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -93,7 +93,8 @@ static void x86_set_epyc_topo_handlers(MachineState *machine) * all CPUs up to max_cpus. */ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms, - unsigned int cpu_index) + unsigned int cpu_index, + CpuInstanceProperties props) { X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms); X86CPUTopoInfo topo_info; @@ -102,7 +103,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 = x86ms->apicid_from_cpu_idx(&topo_info, cpu_index, props); if (x86mc->compat_apic_id_mode) { if (cpu_index != correct_id && !warned && !qtest_enabled()) { error_report("APIC IDs set in compatibility mode, " @@ -136,6 +137,8 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) const CPUArchIdList *possible_cpus; MachineState *ms = MACHINE(x86ms); MachineClass *mc = MACHINE_GET_CLASS(x86ms); + CpuInstanceProperties props; + /* Check for apicid encoding */ if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) { @@ -144,6 +147,8 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) x86_cpu_set_default_version(default_cpu_version); + possible_cpus = mc->possible_cpu_arch_ids(ms); + /* * Calculates the limit to CPU APIC ID values * @@ -152,13 +157,15 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) * * This is used for FW_CFG_MAX_CPUS. See comments on fw_cfg_arch_create(). */ - 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); + props = ms->possible_cpus->cpus[ms->smp.max_cpus - 1].props; + x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms, + ms->smp.max_cpus - 1, + props) + 1; for (i = 0; i < ms->possible_cpus->len; i++) { + props = ms->possible_cpus->cpus[i].props; ms->possible_cpus->cpus[i].arch_id = - x86_cpu_apic_id_from_index(x86ms, i); + x86_cpu_apic_id_from_index(x86ms, i, props); } for (i = 0; i < ms->smp.cpus; i++) { diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h index 7cb21e9c82..a800fc905f 100644 --- a/include/hw/i386/topology.h +++ b/include/hw/i386/topology.h @@ -221,10 +221,17 @@ static inline void x86_init_topo_ids(X86CPUTopoInfo *topo_info, * '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) + unsigned cpu_index, + CpuInstanceProperties props) { X86CPUTopoIDs topo_ids; - x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids); + + if (props.has_node_id) { + x86_init_topo_ids(topo_info, props, &topo_ids); + } else { + 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 @@ -280,7 +287,8 @@ static inline void x86_topo_ids_from_apicid(apic_id_t apicid, * 'cpu_index' is a sequential, contiguous ID for the CPU. */ static inline apic_id_t x86_apicid_from_cpu_idx(X86CPUTopoInfo *topo_info, - unsigned cpu_index) + unsigned cpu_index, + CpuInstanceProperties props) { X86CPUTopoIDs topo_ids; x86_topo_ids_from_idx(topo_info, cpu_index, &topo_ids); diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index b79f24e285..3109f39554 100644 --- a/include/hw/i386/x86.h +++ b/include/hw/i386/x86.h @@ -65,7 +65,8 @@ typedef struct { /* Apic id specific handlers */ uint32_t (*apicid_from_cpu_idx)(X86CPUTopoInfo *topo_info, - unsigned cpu_index); + unsigned cpu_index, + CpuInstanceProperties props); 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, @@ -93,7 +94,8 @@ typedef struct { void init_topo_info(X86CPUTopoInfo *topo_info, const X86MachineState *x86ms); uint32_t x86_cpu_apic_id_from_index(X86MachineState *pcms, - unsigned int cpu_index); + unsigned int cpu_index, + CpuInstanceProperties props); void x86_cpu_new(X86MachineState *pcms, int64_t apic_id, Error **errp); void x86_cpus_init(X86MachineState *pcms, int default_cpu_version); diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c index 049030a50e..a1308e214b 100644 --- a/tests/test-x86-cpuid.c +++ b/tests/test-x86-cpuid.c @@ -29,6 +29,7 @@ static void test_topo_bits(void) { X86CPUTopoInfo topo_info = {0}; + CpuInstanceProperties props = {0}; /* simple tests for 1 thread per core, 1 core per die, 1 die per package */ topo_info = (X86CPUTopoInfo) {0, 1, 1, 1}; @@ -37,10 +38,10 @@ static void test_topo_bits(void) g_assert_cmpuint(apicid_die_width(&topo_info), ==, 0); topo_info = (X86CPUTopoInfo) {0, 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); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 3), ==, 3); + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0, props), ==, 0); + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1, props), ==, 1); + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2, props), ==, 2); + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 3, props), ==, 3); /* Test field width calculation for multiple values @@ -92,38 +93,38 @@ static void test_topo_bits(void) g_assert_cmpuint(apicid_pkg_offset(&topo_info), ==, 5); topo_info = (X86CPUTopoInfo) {0, 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); + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 0, props), ==, 0); + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1, props), ==, 1); + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2, props), ==, 2); topo_info = (X86CPUTopoInfo) {0, 1, 6, 3}; - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 0, props), ==, (1 << 2) | 0); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 1, props), ==, (1 << 2) | 1); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 2), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 1 * 3 + 2, props), ==, (1 << 2) | 2); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 0), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 0, props), ==, (2 << 2) | 0); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 1), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 1, props), ==, (2 << 2) | 1); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 2), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 2 * 3 + 2, props), ==, (2 << 2) | 2); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 0), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 0, props), ==, (5 << 2) | 0); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 1), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 1, props), ==, (5 << 2) | 1); - g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 2), ==, + g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, 5 * 3 + 2, props), ==, (5 << 2) | 2); g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, - 1 * 6 * 3 + 0 * 3 + 0), ==, (1 << 5)); + 1 * 6 * 3 + 0 * 3 + 0, props), ==, (1 << 5)); g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, - 1 * 6 * 3 + 1 * 3 + 1), ==, (1 << 5) | (1 << 2) | 1); + 1 * 6 * 3 + 1 * 3 + 1, props), ==, (1 << 5) | (1 << 2) | 1); g_assert_cmpuint(x86_apicid_from_cpu_idx(&topo_info, - 3 * 6 * 3 + 5 * 3 + 2), ==, (3 << 5) | (5 << 2) | 2); + 3 * 6 * 3 + 5 * 3 + 2, props), ==, (3 << 5) | (5 << 2) | 2); } int main(int argc, char **argv)
Build apic_id from CpuInstanceProperties if numa configured. Use the node_id from user provided numa information. This will avoid conflicts between numa information and apic_id generated. Re-arranged the code little bit to make sure CpuInstanceProperties is initialized before calling. Signed-off-by: Babu Moger <babu.moger@amd.com> --- hw/i386/pc.c | 6 +++++- hw/i386/x86.c | 19 +++++++++++++------ include/hw/i386/topology.h | 14 +++++++++++--- include/hw/i386/x86.h | 6 ++++-- tests/test-x86-cpuid.c | 39 ++++++++++++++++++++------------------- 5 files changed, 53 insertions(+), 31 deletions(-)