[PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology test cases

[PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology test cases

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Hi list,

Currently, test-smp-parse lacks the following cases:
* The case to cover drawer and book parameters parsing in -smp.
* The case to cover the full topology (with total 7 levels) to ensure
  that the topology-related calculations are correct.
* The case to check smp_props.has_clusters of MachineClass.

Thus, add the above cases to improve test coverage.

In addition, people is trying to bump max_cpus to 4096 for PC machine
[1]. Without considering other changes, it's only a matter of time
before the maximum CPUs is raised. Therefore, aslo bump max_cpus to 4096
in -smp related test cases as a start.

[1]: https://lore.kernel.org/qemu-devel/20231208122611.32311-1-anisinha@redhat.com/

Regards,
Zhao

---
Zhao Liu (8):
  tests/unit/test-smp-parse.c: Use CPU number macros in invalid topology
    case
  tests/unit/test-smp-parse.c: Bump max_cpus to 4096
  tests/unit/test-smp-parse.c: Make test cases aware of the book/drawer
  tests/unit/test-smp-parse.c: Test "books" parameter in -smp
  tests/unit/test-smp-parse.c: Test "drawers" parameter in -smp
  tests/unit/test-smp-parse.c: Test "drawers" and "books" combination
    case
  tests/unit/test-smp-parse.c: Test the full 7-levels topology hierarchy
  tests/unit/test-smp-parse.c: Test smp_props.has_clusters

 tests/unit/test-smp-parse.c | 515 ++++++++++++++++++++++++++++++++++--
 1 file changed, 494 insertions(+), 21 deletions(-)

-- 
2.34.1

[PATCH 1/8] tests/unit/test-smp-parse.c: Use CPU number macros in invalid topology case

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Use MAX_CPUS/MIN_CPUS micros in invalid topology case. This gives us the
flexibility to change the maximum and minimum CPU limits.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 24972666a74d..20c663a006b3 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -323,15 +323,21 @@ static const struct SMPTestData data_generic_invalid[] = {
                         "sockets (2) * cores (4) * threads (2) "
                         "== maxcpus (16) < smp_cpus (18)",
     }, {
-        /* config: -smp 1
-         * should tweak the supported min CPUs to 2 for testing */
-        .config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0),
+        /*
+         * config: -smp 1
+         * The test machine should tweak the supported min CPUs to
+         * 2 (MIN_CPUS + 1) for testing.
+         */
+        .config = SMP_CONFIG_GENERIC(T, MIN_CPUS, F, 0, F, 0, F, 0, F, 0),
         .expect_error = "Invalid SMP CPUs 1. The min CPUs supported "
                         "by machine '" SMP_MACHINE_NAME "' is 2",
     }, {
-        /* config: -smp 512
-         * should tweak the supported max CPUs to 511 for testing */
-        .config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0),
+        /*
+         * config: -smp 512
+         * The test machine should tweak the supported max CPUs to
+         * 511 (MAX_CPUS - 1) for testing.
+         */
+        .config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0),
         .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
                         "by machine '" SMP_MACHINE_NAME "' is 511",
     },
@@ -575,8 +581,8 @@ static void machine_generic_invalid_class_init(ObjectClass *oc, void *data)
     MachineClass *mc = MACHINE_CLASS(oc);
 
     /* Force invalid min CPUs and max CPUs */
-    mc->min_cpus = 2;
-    mc->max_cpus = 511;
+    mc->min_cpus = MIN_CPUS + 1;
+    mc->max_cpus = MAX_CPUS - 1;
 }
 
 static void machine_with_dies_class_init(ObjectClass *oc, void *data)
-- 
2.34.1

[PATCH 2/8] tests/unit/test-smp-parse.c: Bump max_cpus to 4096

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

PC mahine is trying to support up to 4096 vCPUs [1], so it's necessary
to bump max_cpus in test-smp-parse to 4096 to cover the topological
needs of future machines.

[1]: https://lore.kernel.org/qemu-devel/20231208122611.32311-1-anisinha@redhat.com/

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 20c663a006b3..55ba13bf7d15 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -20,8 +20,8 @@
 #define T true
 #define F false
 
-#define MIN_CPUS 1   /* set the min CPUs supported by the machine as 1 */
-#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */
+#define MIN_CPUS 1    /* set the min CPUs supported by the machine as 1 */
+#define MAX_CPUS 4096 /* set the max CPUs supported by the machine as 4096 */
 
 #define SMP_MACHINE_NAME "TEST-SMP"
 
@@ -333,13 +333,13 @@ static const struct SMPTestData data_generic_invalid[] = {
                         "by machine '" SMP_MACHINE_NAME "' is 2",
     }, {
         /*
-         * config: -smp 512
+         * config: -smp 4096
          * The test machine should tweak the supported max CPUs to
-         * 511 (MAX_CPUS - 1) for testing.
+         * 4095 (MAX_CPUS - 1) for testing.
          */
-        .config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0),
-        .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
-                        "by machine '" SMP_MACHINE_NAME "' is 511",
+        .config = SMP_CONFIG_GENERIC(T, 4096, F, 0, F, 0, F, 0, F, 0),
+        .expect_error = "Invalid SMP CPUs 4096. The max CPUs supported "
+                        "by machine '" SMP_MACHINE_NAME "' is 4095",
     },
 };
 
-- 
2.34.1

[PATCH 3/8] tests/unit/test-smp-parse.c: Make test cases aware of the book/drawer

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Currently, -smp supports 2 more new levels: book and drawer.

It is necessary to consider the effects of book and drawer in the test
cases to ensure that the calculations are correct. This is also the
preparation to add new book and drawer test cases.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 55ba13bf7d15..a8eb3bbb35ed 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -384,6 +384,8 @@ static char *smp_config_to_string(const SMPConfiguration *config)
     return g_strdup_printf(
         "(SMPConfiguration) {\n"
         "    .has_cpus     = %5s, cpus     = %" PRId64 ",\n"
+        "    .has_drawers  = %5s, drawers  = %" PRId64 ",\n"
+        "    .has_books    = %5s, books    = %" PRId64 ",\n"
         "    .has_sockets  = %5s, sockets  = %" PRId64 ",\n"
         "    .has_dies     = %5s, dies     = %" PRId64 ",\n"
         "    .has_clusters = %5s, clusters = %" PRId64 ",\n"
@@ -392,6 +394,8 @@ static char *smp_config_to_string(const SMPConfiguration *config)
         "    .has_maxcpus  = %5s, maxcpus  = %" PRId64 ",\n"
         "}",
         config->has_cpus ? "true" : "false", config->cpus,
+        config->has_drawers ? "true" : "false", config->drawers,
+        config->has_books ? "true" : "false", config->books,
         config->has_sockets ? "true" : "false", config->sockets,
         config->has_dies ? "true" : "false", config->dies,
         config->has_clusters ? "true" : "false", config->clusters,
@@ -404,10 +408,10 @@ static char *smp_config_to_string(const SMPConfiguration *config)
 static unsigned int cpu_topology_get_threads_per_socket(const CpuTopology *topo)
 {
     /* Check the divisor to avoid invalid topology examples causing SIGFPE. */
-    if (!topo->sockets) {
+    if (!topo->drawers || !topo->books || !topo->sockets) {
         return 0;
     } else {
-        return topo->max_cpus / topo->sockets;
+        return topo->max_cpus / topo->drawers / topo->books / topo->sockets;
     }
 }
 
@@ -429,6 +433,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
     return g_strdup_printf(
         "(CpuTopology) {\n"
         "    .cpus               = %u,\n"
+        "    .drawers            = %u,\n"
+        "    .books              = %u,\n"
         "    .sockets            = %u,\n"
         "    .dies               = %u,\n"
         "    .clusters           = %u,\n"
@@ -438,7 +444,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
         "    .threads_per_socket = %u,\n"
         "    .cores_per_socket   = %u,\n"
         "}",
-        topo->cpus, topo->sockets, topo->dies, topo->clusters,
+        topo->cpus, topo->drawers, topo->books,
+        topo->sockets, topo->dies, topo->clusters,
         topo->cores, topo->threads, topo->max_cpus,
         threads_per_socket, cores_per_socket);
 }
@@ -473,6 +480,8 @@ static void check_parse(MachineState *ms, const SMPConfiguration *config,
     if (is_valid) {
         if ((err == NULL) &&
             (ms->smp.cpus == expect_topo->cpus) &&
+            (ms->smp.drawers == expect_topo->drawers) &&
+            (ms->smp.books == expect_topo->books) &&
             (ms->smp.sockets == expect_topo->sockets) &&
             (ms->smp.dies == expect_topo->dies) &&
             (ms->smp.clusters == expect_topo->clusters) &&
@@ -564,6 +573,16 @@ static void unsupported_params_init(const MachineClass *mc, SMPTestData *data)
         data->expect_prefer_sockets.clusters = 1;
         data->expect_prefer_cores.clusters = 1;
     }
+
+    if (!mc->smp_props.books_supported) {
+        data->expect_prefer_sockets.books = 1;
+        data->expect_prefer_cores.books = 1;
+    }
+
+    if (!mc->smp_props.drawers_supported) {
+        data->expect_prefer_sockets.drawers = 1;
+        data->expect_prefer_cores.drawers = 1;
+    }
 }
 
 static void machine_base_class_init(ObjectClass *oc, void *data)
-- 
2.34.1

[PATCH 4/8] tests/unit/test-smp-parse.c: Test "books" parameter in -smp

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Although book was introduced to -smp along with drawer by s390 machine,
as a general topology level in QEMU that may be reused by other arches
in the future, it is desirable to cover this parameter's parsing in a
separate case.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 105 ++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index a8eb3bbb35ed..31f3f713e851 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -75,6 +75,22 @@
             .has_maxcpus  = hf, .maxcpus  = f,                \
         }
 
+/*
+ * Currently a 5-level topology hierarchy is supported on s390 ccw machines
+ *  -drawers/books/sockets/cores/threads
+ */
+#define SMP_CONFIG_WITH_BOOKS_DRAWERS(ha, a, hb, b, hc, c, hd, \
+                                      d, he, e, hf, f, hg, g)  \
+        {                                                      \
+            .has_cpus     = ha, .cpus     = a,                 \
+            .has_drawers  = hb, .drawers  = b,                 \
+            .has_books    = hc, .books    = c,                 \
+            .has_sockets  = hd, .sockets  = d,                 \
+            .has_cores    = he, .cores    = e,                 \
+            .has_threads  = hf, .threads  = f,                 \
+            .has_maxcpus  = hg, .maxcpus  = g,                 \
+        }
+
 /**
  * @config - the given SMP configuration
  * @expect_prefer_sockets - the expected parsing result for the
@@ -308,6 +324,11 @@ static const struct SMPTestData data_generic_invalid[] = {
         /* config: -smp 2,clusters=2 */
         .config = SMP_CONFIG_WITH_CLUSTERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
         .expect_error = "clusters not supported by this machine's CPU topology",
+    }, {
+        /* config: -smp 2,books=2 */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F,
+                                                0, F, 0, F, 0, F, 0),
+        .expect_error = "books not supported by this machine's CPU topology",
     }, {
         /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
         .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),
@@ -379,6 +400,26 @@ static const struct SMPTestData data_with_clusters_invalid[] = {
     },
 };
 
+static const struct SMPTestData data_with_books_invalid[] = {
+    {
+        /* config: -smp 16,books=2,sockets=2,cores=4,threads=2,maxcpus=16 */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, F, 1, T, 2, T,
+                                                2, T, 4, T, 2, T, 16),
+        .expect_error = "Invalid CPU topology: "
+                        "product of the hierarchy must match maxcpus: "
+                        "books (2) * sockets (2) * cores (4) * threads (2) "
+                        "!= maxcpus (16)",
+    }, {
+        /* config: -smp 34,books=2,sockets=2,cores=4,threads=2,maxcpus=32 */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, F, 1, T, 2, T,
+                                                2, T, 4, T, 2, T, 32),
+        .expect_error = "Invalid CPU topology: "
+                        "maxcpus must be equal to or greater than smp: "
+                        "books (2) * sockets (2) * cores (4) * threads (2) "
+                        "== maxcpus (32) < smp_cpus (34)",
+    },
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
     return g_strdup_printf(
@@ -618,6 +659,13 @@ static void machine_with_clusters_class_init(ObjectClass *oc, void *data)
     mc->smp_props.clusters_supported = true;
 }
 
+static void machine_with_books_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->smp_props.books_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
     const char *machine_type = opaque;
@@ -761,6 +809,56 @@ static void test_with_clusters(const void *opaque)
     object_unref(obj);
 }
 
+static void test_with_books(const void *opaque)
+{
+    const char *machine_type = opaque;
+    Object *obj = object_new(machine_type);
+    MachineState *ms = MACHINE(obj);
+    MachineClass *mc = MACHINE_GET_CLASS(obj);
+    SMPTestData data = {};
+    unsigned int num_books = 2;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+        data = data_generic_valid[i];
+        unsupported_params_init(mc, &data);
+
+        /* when books parameter is omitted, it will be set as 1 */
+        data.expect_prefer_sockets.books = 1;
+        data.expect_prefer_cores.books = 1;
+
+        smp_parse_test(ms, &data, true);
+
+        /* when books parameter is specified */
+        data.config.has_books = true;
+        data.config.books = num_books;
+        if (data.config.has_cpus) {
+            data.config.cpus *= num_books;
+        }
+        if (data.config.has_maxcpus) {
+            data.config.maxcpus *= num_books;
+        }
+
+        data.expect_prefer_sockets.books = num_books;
+        data.expect_prefer_sockets.cpus *= num_books;
+        data.expect_prefer_sockets.max_cpus *= num_books;
+        data.expect_prefer_cores.books = num_books;
+        data.expect_prefer_cores.cpus *= num_books;
+        data.expect_prefer_cores.max_cpus *= num_books;
+
+        smp_parse_test(ms, &data, true);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(data_with_books_invalid); i++) {
+        data = data_with_books_invalid[i];
+        unsupported_params_init(mc, &data);
+
+        smp_parse_test(ms, &data, false);
+    }
+
+    object_unref(obj);
+}
+
 /* Type info of the tested machine */
 static const TypeInfo smp_machine_types[] = {
     {
@@ -785,6 +883,10 @@ static const TypeInfo smp_machine_types[] = {
         .name           = MACHINE_TYPE_NAME("smp-with-clusters"),
         .parent         = TYPE_MACHINE,
         .class_init     = machine_with_clusters_class_init,
+    }, {
+        .name           = MACHINE_TYPE_NAME("smp-with-books"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = machine_with_books_class_init,
     }
 };
 
@@ -808,6 +910,9 @@ int main(int argc, char *argv[])
     g_test_add_data_func("/test-smp-parse/with_clusters",
                          MACHINE_TYPE_NAME("smp-with-clusters"),
                          test_with_clusters);
+    g_test_add_data_func("/test-smp-parse/with_books",
+                         MACHINE_TYPE_NAME("smp-with-books"),
+                         test_with_books);
 
     g_test_run();
 
-- 
2.34.1

[PATCH 5/8] tests/unit/test-smp-parse.c: Test "drawers" parameter in -smp

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Although drawer was introduced to -smp along with book by s390 machine,
as a general topology level in QEMU that may be reused by other arches
in the future, it is desirable to cover this parameter's parsing in a
separate case.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 89 +++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 31f3f713e851..d59e220360d5 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -329,6 +329,11 @@ static const struct SMPTestData data_generic_invalid[] = {
         .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F,
                                                 0, F, 0, F, 0, F, 0),
         .expect_error = "books not supported by this machine's CPU topology",
+    }, {
+        /* config: -smp 2,drawers=2 */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, T, 2, F, 0, F,
+                                                0, F, 0, F, 0, F, 0),
+        .expect_error = "drawers not supported by this machine's CPU topology",
     }, {
         /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
         .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),
@@ -420,6 +425,26 @@ static const struct SMPTestData data_with_books_invalid[] = {
     },
 };
 
+static const struct SMPTestData data_with_drawers_invalid[] = {
+    {
+        /* config: -smp 16,drawers=2,sockets=2,cores=4,threads=2,maxcpus=16 */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, T, 2, F, 1, T,
+                                                2, T, 4, T, 2, T, 16),
+        .expect_error = "Invalid CPU topology: "
+                        "product of the hierarchy must match maxcpus: "
+                        "drawers (2) * sockets (2) * cores (4) * threads (2) "
+                        "!= maxcpus (16)",
+    }, {
+        /* config: -smp 34,drawers=2,sockets=2,cores=4,threads=2,maxcpus=32 */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, T, 2, F, 1, T,
+                                                2, T, 4, T, 2, T, 32),
+        .expect_error = "Invalid CPU topology: "
+                        "maxcpus must be equal to or greater than smp: "
+                        "drawers (2) * sockets (2) * cores (4) * threads (2) "
+                        "== maxcpus (32) < smp_cpus (34)",
+    },
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
     return g_strdup_printf(
@@ -666,6 +691,13 @@ static void machine_with_books_class_init(ObjectClass *oc, void *data)
     mc->smp_props.books_supported = true;
 }
 
+static void machine_with_drawers_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->smp_props.drawers_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
     const char *machine_type = opaque;
@@ -859,6 +891,56 @@ static void test_with_books(const void *opaque)
     object_unref(obj);
 }
 
+static void test_with_drawers(const void *opaque)
+{
+    const char *machine_type = opaque;
+    Object *obj = object_new(machine_type);
+    MachineState *ms = MACHINE(obj);
+    MachineClass *mc = MACHINE_GET_CLASS(obj);
+    SMPTestData data = {};
+    unsigned int num_drawers = 2;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+        data = data_generic_valid[i];
+        unsupported_params_init(mc, &data);
+
+        /* when drawers parameter is omitted, it will be set as 1 */
+        data.expect_prefer_sockets.drawers = 1;
+        data.expect_prefer_cores.drawers = 1;
+
+        smp_parse_test(ms, &data, true);
+
+        /* when drawers parameter is specified */
+        data.config.has_drawers = true;
+        data.config.drawers = num_drawers;
+        if (data.config.has_cpus) {
+            data.config.cpus *= num_drawers;
+        }
+        if (data.config.has_maxcpus) {
+            data.config.maxcpus *= num_drawers;
+        }
+
+        data.expect_prefer_sockets.drawers = num_drawers;
+        data.expect_prefer_sockets.cpus *= num_drawers;
+        data.expect_prefer_sockets.max_cpus *= num_drawers;
+        data.expect_prefer_cores.drawers = num_drawers;
+        data.expect_prefer_cores.cpus *= num_drawers;
+        data.expect_prefer_cores.max_cpus *= num_drawers;
+
+        smp_parse_test(ms, &data, true);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(data_with_drawers_invalid); i++) {
+        data = data_with_drawers_invalid[i];
+        unsupported_params_init(mc, &data);
+
+        smp_parse_test(ms, &data, false);
+    }
+
+    object_unref(obj);
+}
+
 /* Type info of the tested machine */
 static const TypeInfo smp_machine_types[] = {
     {
@@ -887,6 +969,10 @@ static const TypeInfo smp_machine_types[] = {
         .name           = MACHINE_TYPE_NAME("smp-with-books"),
         .parent         = TYPE_MACHINE,
         .class_init     = machine_with_books_class_init,
+    }, {
+        .name           = MACHINE_TYPE_NAME("smp-with-drawers"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = machine_with_drawers_class_init,
     }
 };
 
@@ -913,6 +999,9 @@ int main(int argc, char *argv[])
     g_test_add_data_func("/test-smp-parse/with_books",
                          MACHINE_TYPE_NAME("smp-with-books"),
                          test_with_books);
+    g_test_add_data_func("/test-smp-parse/with_drawers",
+                         MACHINE_TYPE_NAME("smp-with-drawers"),
+                         test_with_drawers);
 
     g_test_run();
 
-- 
2.34.1

[PATCH 6/8] tests/unit/test-smp-parse.c: Test "drawers" and "books" combination case

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Since s390 machine supports both "drawers" and "books" in -smp, add the
"drawers" and "books" combination test case to match the actual topology
usage scenario.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 103 ++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index d59e220360d5..a1ef297248cd 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -445,6 +445,33 @@ static const struct SMPTestData data_with_drawers_invalid[] = {
     },
 };
 
+static const struct SMPTestData data_with_drawers_books_invalid[] = {
+    {
+        /*
+         * config: -smp 200,drawers=2,books=2,sockets=2,cores=4,\
+         * threads=2,maxcpus=200
+         */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 200, T, 3, T, 5, T,
+                                                2, T, 4, T, 2, T, 200),
+        .expect_error = "Invalid CPU topology: "
+                        "product of the hierarchy must match maxcpus: "
+                        "drawers (3) * books (5) * sockets (2) * "
+                        "cores (4) * threads (2) != maxcpus (200)",
+    }, {
+        /*
+         * config: -smp 242,drawers=2,books=2,sockets=2,cores=4,\
+         * threads=2,maxcpus=240
+         */
+        .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 242, T, 3, T, 5, T,
+                                                2, T, 4, T, 2, T, 240),
+        .expect_error = "Invalid CPU topology: "
+                        "maxcpus must be equal to or greater than smp: "
+                        "drawers (3) * books (5) * sockets (2) * "
+                        "cores (4) * threads (2) "
+                        "== maxcpus (240) < smp_cpus (242)",
+    },
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
     return g_strdup_printf(
@@ -698,6 +725,14 @@ static void machine_with_drawers_class_init(ObjectClass *oc, void *data)
     mc->smp_props.drawers_supported = true;
 }
 
+static void machine_with_drawers_books_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->smp_props.drawers_supported = true;
+    mc->smp_props.books_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
     const char *machine_type = opaque;
@@ -941,6 +976,67 @@ static void test_with_drawers(const void *opaque)
     object_unref(obj);
 }
 
+static void test_with_drawers_books(const void *opaque)
+{
+    const char *machine_type = opaque;
+    Object *obj = object_new(machine_type);
+    MachineState *ms = MACHINE(obj);
+    MachineClass *mc = MACHINE_GET_CLASS(obj);
+    SMPTestData data = {};
+    unsigned int num_drawers = 5, num_books = 3;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+        data = data_generic_valid[i];
+        unsupported_params_init(mc, &data);
+
+        /*
+         * when drawers and books parameters are omitted, they will
+         * be both set as 1.
+         */
+        data.expect_prefer_sockets.drawers = 1;
+        data.expect_prefer_sockets.books = 1;
+        data.expect_prefer_cores.drawers = 1;
+        data.expect_prefer_cores.books = 1;
+
+        smp_parse_test(ms, &data, true);
+
+        /* when drawers and books parameters are both specified */
+        data.config.has_drawers = true;
+        data.config.drawers = num_drawers;
+        data.config.has_books = true;
+        data.config.books = num_books;
+
+        if (data.config.has_cpus) {
+            data.config.cpus *= num_drawers * num_books;
+        }
+        if (data.config.has_maxcpus) {
+            data.config.maxcpus *= num_drawers * num_books;
+        }
+
+        data.expect_prefer_sockets.drawers = num_drawers;
+        data.expect_prefer_sockets.books = num_books;
+        data.expect_prefer_sockets.cpus *= num_drawers * num_books;
+        data.expect_prefer_sockets.max_cpus *= num_drawers * num_books;
+
+        data.expect_prefer_cores.drawers = num_drawers;
+        data.expect_prefer_cores.books = num_books;
+        data.expect_prefer_cores.cpus *= num_drawers * num_books;
+        data.expect_prefer_cores.max_cpus *= num_drawers * num_books;
+
+        smp_parse_test(ms, &data, true);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(data_with_drawers_books_invalid); i++) {
+        data = data_with_drawers_books_invalid[i];
+        unsupported_params_init(mc, &data);
+
+        smp_parse_test(ms, &data, false);
+    }
+
+    object_unref(obj);
+}
+
 /* Type info of the tested machine */
 static const TypeInfo smp_machine_types[] = {
     {
@@ -973,6 +1069,10 @@ static const TypeInfo smp_machine_types[] = {
         .name           = MACHINE_TYPE_NAME("smp-with-drawers"),
         .parent         = TYPE_MACHINE,
         .class_init     = machine_with_drawers_class_init,
+    }, {
+        .name           = MACHINE_TYPE_NAME("smp-with-drawers-books"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = machine_with_drawers_books_class_init,
     }
 };
 
@@ -1002,6 +1102,9 @@ int main(int argc, char *argv[])
     g_test_add_data_func("/test-smp-parse/with_drawers",
                          MACHINE_TYPE_NAME("smp-with-drawers"),
                          test_with_drawers);
+    g_test_add_data_func("/test-smp-parse/with_drawers_books",
+                         MACHINE_TYPE_NAME("smp-with-drawers-books"),
+                         test_with_drawers_books);
 
     g_test_run();
 
-- 
2.34.1

[PATCH 7/8] tests/unit/test-smp-parse.c: Test the full 7-levels topology hierarchy

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Currently, -smp supports up to 7-levels topology hierarchy:
  -drawers/books/sockets/dies/clusters/cores/threads.

Though no machine supports all these 7 levels yet, these 7 levels have
the strict containment relationship and together form the generic CPU
topology representation of QEMU.

Also, note that the maxcpus is calculated by multiplying all 7 levels:

  maxcpus = drawers * books * sockets * dies * clusters *
            cores * threads.

To cover this code path, it is necessary to test the full topology case
(with all 7 levels). This also helps to avoid introducing new issues by
further expanding the CPU topology in the future.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 143 ++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index a1ef297248cd..1d1f10b4faf0 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -91,6 +91,24 @@
             .has_maxcpus  = hg, .maxcpus  = g,                 \
         }
 
+/*
+ * Currently QEMU supports up to a 7-level topology hierarchy, which is the
+ * QEMU's unified abstract representation of CPU topology.
+ *  -drawers/books/sockets/dies/clusters/cores/threads
+ */
+#define SMP_CONFIG_WITH_FULL_TOPO(a, b, c, d, e, f, g, h, i)    \
+        {                                                       \
+            .has_cpus     = true, .cpus     = a,                \
+            .has_drawers  = true, .drawers  = b,                \
+            .has_books    = true, .books    = c,                \
+            .has_sockets  = true, .sockets  = d,                \
+            .has_dies     = true, .dies     = e,                \
+            .has_clusters = true, .clusters = f,                \
+            .has_cores    = true, .cores    = g,                \
+            .has_threads  = true, .threads  = h,                \
+            .has_maxcpus  = true, .maxcpus  = i,                \
+        }
+
 /**
  * @config - the given SMP configuration
  * @expect_prefer_sockets - the expected parsing result for the
@@ -472,6 +490,40 @@ static const struct SMPTestData data_with_drawers_books_invalid[] = {
     },
 };
 
+static const struct SMPTestData data_full_topo_invalid[] = {
+    {
+        /*
+         * config: -smp 200,drawers=3,books=5,sockets=2,dies=4,\
+         *              clusters=2,cores=7,threads=2,maxcpus=200
+         */
+        .config = SMP_CONFIG_WITH_FULL_TOPO(200, 3, 5, 2, 4, 2, 7, 2, 200),
+        .expect_error = "Invalid CPU topology: "
+                        "product of the hierarchy must match maxcpus: "
+                        "drawers (3) * books (5) * sockets (2) * dies (4) * "
+                        "clusters (2) * cores (7) * threads (2) "
+                        "!= maxcpus (200)",
+    }, {
+        /*
+         * config: -smp 3361,drawers=3,books=5,sockets=2,dies=4,\
+         *              clusters=2,cores=7,threads=2,maxcpus=3360
+         */
+        .config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 2, 3360),
+        .expect_error = "Invalid CPU topology: "
+                        "maxcpus must be equal to or greater than smp: "
+                        "drawers (3) * books (5) * sockets (2) * dies (4) * "
+                        "clusters (2) * cores (7) * threads (2) "
+                        "== maxcpus (3360) < smp_cpus (3361)",
+    }, {
+        /*
+         * config: -smp 1,drawers=3,books=5,sockets=2,dies=4,\
+         *              clusters=2,cores=7,threads=3,maxcpus=5040
+         */
+        .config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 3, 5040),
+        .expect_error = "Invalid SMP CPUs 5040. The max CPUs supported "
+                        "by machine '" SMP_MACHINE_NAME "' is 4096",
+    },
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
     return g_strdup_printf(
@@ -733,6 +785,16 @@ static void machine_with_drawers_books_class_init(ObjectClass *oc, void *data)
     mc->smp_props.books_supported = true;
 }
 
+static void machine_full_topo_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->smp_props.drawers_supported = true;
+    mc->smp_props.books_supported = true;
+    mc->smp_props.dies_supported = true;
+    mc->smp_props.clusters_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
     const char *machine_type = opaque;
@@ -1037,6 +1099,80 @@ static void test_with_drawers_books(const void *opaque)
     object_unref(obj);
 }
 
+static void test_full_topo(const void *opaque)
+{
+    const char *machine_type = opaque;
+    Object *obj = object_new(machine_type);
+    MachineState *ms = MACHINE(obj);
+    MachineClass *mc = MACHINE_GET_CLASS(obj);
+    SMPTestData data = {};
+    unsigned int drawers = 5, books = 3, dies = 2, clusters = 7, multiplier;
+    int i;
+
+    multiplier = drawers * books * dies * clusters;
+    for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+        data = data_generic_valid[i];
+        unsupported_params_init(mc, &data);
+
+        /*
+         * when drawers, books, dies and clusters parameters are omitted,
+         * they will be set as 1.
+         */
+        data.expect_prefer_sockets.drawers = 1;
+        data.expect_prefer_sockets.books = 1;
+        data.expect_prefer_sockets.dies = 1;
+        data.expect_prefer_sockets.clusters = 1;
+        data.expect_prefer_cores.drawers = 1;
+        data.expect_prefer_cores.books = 1;
+        data.expect_prefer_cores.dies = 1;
+        data.expect_prefer_cores.clusters = 1;
+
+        smp_parse_test(ms, &data, true);
+
+        /* when drawers, books, dies and clusters parameters are specified. */
+        data.config.has_drawers = true;
+        data.config.drawers = drawers;
+        data.config.has_books = true;
+        data.config.books = books;
+        data.config.has_dies = true;
+        data.config.dies = dies;
+        data.config.has_clusters = true;
+        data.config.clusters = clusters;
+
+        if (data.config.has_cpus) {
+            data.config.cpus *= multiplier;
+        }
+        if (data.config.has_maxcpus) {
+            data.config.maxcpus *= multiplier;
+        }
+
+        data.expect_prefer_sockets.drawers = drawers;
+        data.expect_prefer_sockets.books = books;
+        data.expect_prefer_sockets.dies = dies;
+        data.expect_prefer_sockets.clusters = clusters;
+        data.expect_prefer_sockets.cpus *= multiplier;
+        data.expect_prefer_sockets.max_cpus *= multiplier;
+
+        data.expect_prefer_cores.drawers = drawers;
+        data.expect_prefer_cores.books = books;
+        data.expect_prefer_cores.dies = dies;
+        data.expect_prefer_cores.clusters = clusters;
+        data.expect_prefer_cores.cpus *= multiplier;
+        data.expect_prefer_cores.max_cpus *= multiplier;
+
+        smp_parse_test(ms, &data, true);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(data_full_topo_invalid); i++) {
+        data = data_full_topo_invalid[i];
+        unsupported_params_init(mc, &data);
+
+        smp_parse_test(ms, &data, false);
+    }
+
+    object_unref(obj);
+}
+
 /* Type info of the tested machine */
 static const TypeInfo smp_machine_types[] = {
     {
@@ -1073,6 +1209,10 @@ static const TypeInfo smp_machine_types[] = {
         .name           = MACHINE_TYPE_NAME("smp-with-drawers-books"),
         .parent         = TYPE_MACHINE,
         .class_init     = machine_with_drawers_books_class_init,
+    }, {
+        .name           = MACHINE_TYPE_NAME("smp-full-topo"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = machine_full_topo_class_init,
     }
 };
 
@@ -1105,6 +1245,9 @@ int main(int argc, char *argv[])
     g_test_add_data_func("/test-smp-parse/with_drawers_books",
                          MACHINE_TYPE_NAME("smp-with-drawers-books"),
                          test_with_drawers_books);
+    g_test_add_data_func("/test-smp-parse/full",
+                         MACHINE_TYPE_NAME("smp-full-topo"),
+                         test_full_topo);
 
     g_test_run();
 
-- 
2.34.1

[PATCH 8/8] tests/unit/test-smp-parse.c: Test smp_props.has_clusters

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

The smp_props.has_clusters in MachineClass is not a user configured
field, and it indicates if user specifies "clusters" in -smp.

After -smp parsing, other module could aware if the cluster level
is configured by user. This is used when the machine has only 1 cluster
since there's only 1 cluster by default.

Add the check to cover this field.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 tests/unit/test-smp-parse.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 1d1f10b4faf0..bc55af96f30d 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -573,7 +573,8 @@ static unsigned int cpu_topology_get_cores_per_socket(const CpuTopology *topo)
 
 static char *cpu_topology_to_string(const CpuTopology *topo,
                                     unsigned int threads_per_socket,
-                                    unsigned int cores_per_socket)
+                                    unsigned int cores_per_socket,
+                                    bool has_clusters)
 {
     return g_strdup_printf(
         "(CpuTopology) {\n"
@@ -588,17 +589,20 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
         "    .max_cpus           = %u,\n"
         "    .threads_per_socket = %u,\n"
         "    .cores_per_socket   = %u,\n"
+        "    .has_clusters       = %s,\n"
         "}",
         topo->cpus, topo->drawers, topo->books,
         topo->sockets, topo->dies, topo->clusters,
         topo->cores, topo->threads, topo->max_cpus,
-        threads_per_socket, cores_per_socket);
+        threads_per_socket, cores_per_socket,
+        has_clusters ? "true" : "false");
 }
 
 static void check_parse(MachineState *ms, const SMPConfiguration *config,
                         const CpuTopology *expect_topo, const char *expect_err,
                         bool is_valid)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
     g_autofree char *config_str = smp_config_to_string(config);
     g_autofree char *expect_topo_str = NULL, *output_topo_str = NULL;
     unsigned int expect_threads_per_socket, expect_cores_per_socket;
@@ -611,15 +615,18 @@ static void check_parse(MachineState *ms, const SMPConfiguration *config,
                         cpu_topology_get_cores_per_socket(expect_topo);
     expect_topo_str = cpu_topology_to_string(expect_topo,
                                              expect_threads_per_socket,
-                                             expect_cores_per_socket);
+                                             expect_cores_per_socket,
+                                             config->has_clusters);
 
     /* call the generic parser */
     machine_parse_smp_config(ms, config, &err);
 
     ms_threads_per_socket = machine_topo_get_threads_per_socket(ms);
     ms_cores_per_socket = machine_topo_get_cores_per_socket(ms);
-    output_topo_str = cpu_topology_to_string(&ms->smp, ms_threads_per_socket,
-                                             ms_cores_per_socket);
+    output_topo_str = cpu_topology_to_string(&ms->smp,
+                                             ms_threads_per_socket,
+                                             ms_cores_per_socket,
+                                             mc->smp_props.has_clusters);
 
     /* when the configuration is supposed to be valid */
     if (is_valid) {
@@ -634,7 +641,8 @@ static void check_parse(MachineState *ms, const SMPConfiguration *config,
             (ms->smp.threads == expect_topo->threads) &&
             (ms->smp.max_cpus == expect_topo->max_cpus) &&
             (ms_threads_per_socket == expect_threads_per_socket) &&
-            (ms_cores_per_socket == expect_cores_per_socket)) {
+            (ms_cores_per_socket == expect_cores_per_socket) &&
+            (mc->smp_props.has_clusters == config->has_clusters)) {
             return;
         }
 
-- 
2.34.1

Re: [PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology test cases

[Song, Xiaoling]

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

I tested the unit test cases with "make check" and result looks good.

Tested-by: Xiaoling Song <xiaoling.song@intel.com>

Regards,

Xiaoling

On 1/18/2024 10:48 PM, Zhao Liu wrote:
> From: Zhao Liu<zhao1.liu@intel.com>
>
> Hi list,
>
> Currently, test-smp-parse lacks the following cases:
> * The case to cover drawer and book parameters parsing in -smp.
> * The case to cover the full topology (with total 7 levels) to ensure
>    that the topology-related calculations are correct.
> * The case to check smp_props.has_clusters of MachineClass.
>
> Thus, add the above cases to improve test coverage.
>
> In addition, people is trying to bump max_cpus to 4096 for PC machine
> [1]. Without considering other changes, it's only a matter of time
> before the maximum CPUs is raised. Therefore, aslo bump max_cpus to 4096
> in -smp related test cases as a start.
>
> [1]:https://lore.kernel.org/qemu-devel/20231208122611.32311-1-anisinha@redhat.com/
>
> Regards,
> Zhao
>
> ---
> Zhao Liu (8):
>    tests/unit/test-smp-parse.c: Use CPU number macros in invalid topology
>      case
>    tests/unit/test-smp-parse.c: Bump max_cpus to 4096
>    tests/unit/test-smp-parse.c: Make test cases aware of the book/drawer
>    tests/unit/test-smp-parse.c: Test "books" parameter in -smp
>    tests/unit/test-smp-parse.c: Test "drawers" parameter in -smp
>    tests/unit/test-smp-parse.c: Test "drawers" and "books" combination
>      case
>    tests/unit/test-smp-parse.c: Test the full 7-levels topology hierarchy
>    tests/unit/test-smp-parse.c: Test smp_props.has_clusters
>
>   tests/unit/test-smp-parse.c | 515 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 494 insertions(+), 21 deletions(-)
>

[-- Attachment #2: Type: text/html, Size: 37916 bytes --]

Re: [PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology test cases

[Zhao Liu]

Hi Philippe,

A kindly ping.

Thanks,
Zhao

On Thu, Jan 18, 2024 at 10:48:49PM +0800, Zhao Liu wrote:
> Date: Thu, 18 Jan 2024 22:48:49 +0800
> From: Zhao Liu <zhao1.liu@linux.intel.com>
> Subject: [PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology
>  test cases
> X-Mailer: git-send-email 2.34.1
> 
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Hi list,
> 
> Currently, test-smp-parse lacks the following cases:
> * The case to cover drawer and book parameters parsing in -smp.
> * The case to cover the full topology (with total 7 levels) to ensure
>   that the topology-related calculations are correct.
> * The case to check smp_props.has_clusters of MachineClass.
> 
> Thus, add the above cases to improve test coverage.
> 
> In addition, people is trying to bump max_cpus to 4096 for PC machine
> [1]. Without considering other changes, it's only a matter of time
> before the maximum CPUs is raised. Therefore, aslo bump max_cpus to 4096
> in -smp related test cases as a start.
> 
> [1]: https://lore.kernel.org/qemu-devel/20231208122611.32311-1-anisinha@redhat.com/
> 
> Regards,
> Zhao
> 
> ---
> Zhao Liu (8):
>   tests/unit/test-smp-parse.c: Use CPU number macros in invalid topology
>     case
>   tests/unit/test-smp-parse.c: Bump max_cpus to 4096
>   tests/unit/test-smp-parse.c: Make test cases aware of the book/drawer
>   tests/unit/test-smp-parse.c: Test "books" parameter in -smp
>   tests/unit/test-smp-parse.c: Test "drawers" parameter in -smp
>   tests/unit/test-smp-parse.c: Test "drawers" and "books" combination
>     case
>   tests/unit/test-smp-parse.c: Test the full 7-levels topology hierarchy
>   tests/unit/test-smp-parse.c: Test smp_props.has_clusters
> 
>  tests/unit/test-smp-parse.c | 515 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 494 insertions(+), 21 deletions(-)
> 
> -- 
> 2.34.1
> 
> 

Re: [PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology test cases

[Zhao Liu]

Hi Philippe,

Could you pls help me review this series?

...Except you, I'm not sure the other machine core maintainers will
review this.

If this series is acceptable, next I can add module level test case,
cache topology test case and more clean up based on this.

Thanks,
Zhao

On Mon, Jan 29, 2024 at 05:38:24PM +0800, Zhao Liu wrote:
> Date: Mon, 29 Jan 2024 17:38:24 +0800
> From: Zhao Liu <zhao1.liu@linux.intel.com>
> Subject: Re: [PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology
>  test cases
> 
> Hi Philippe,
> 
> A kindly ping.
> 
> Thanks,
> Zhao
> 
> On Thu, Jan 18, 2024 at 10:48:49PM +0800, Zhao Liu wrote:
> > Date: Thu, 18 Jan 2024 22:48:49 +0800
> > From: Zhao Liu <zhao1.liu@linux.intel.com>
> > Subject: [PATCH 0/8] tests/unit/test-smp-parse.c: Add more CPU topology
> >  test cases
> > X-Mailer: git-send-email 2.34.1
> > 
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > Hi list,
> > 
> > Currently, test-smp-parse lacks the following cases:
> > * The case to cover drawer and book parameters parsing in -smp.
> > * The case to cover the full topology (with total 7 levels) to ensure
> >   that the topology-related calculations are correct.
> > * The case to check smp_props.has_clusters of MachineClass.
> > 
> > Thus, add the above cases to improve test coverage.
> > 
> > In addition, people is trying to bump max_cpus to 4096 for PC machine
> > [1]. Without considering other changes, it's only a matter of time
> > before the maximum CPUs is raised. Therefore, aslo bump max_cpus to 4096
> > in -smp related test cases as a start.
> > 
> > [1]: https://lore.kernel.org/qemu-devel/20231208122611.32311-1-anisinha@redhat.com/
> > 
> > Regards,
> > Zhao
> > 
> > ---
> > Zhao Liu (8):
> >   tests/unit/test-smp-parse.c: Use CPU number macros in invalid topology
> >     case
> >   tests/unit/test-smp-parse.c: Bump max_cpus to 4096
> >   tests/unit/test-smp-parse.c: Make test cases aware of the book/drawer
> >   tests/unit/test-smp-parse.c: Test "books" parameter in -smp
> >   tests/unit/test-smp-parse.c: Test "drawers" parameter in -smp
> >   tests/unit/test-smp-parse.c: Test "drawers" and "books" combination
> >     case
> >   tests/unit/test-smp-parse.c: Test the full 7-levels topology hierarchy
> >   tests/unit/test-smp-parse.c: Test smp_props.has_clusters
> > 
> >  tests/unit/test-smp-parse.c | 515 ++++++++++++++++++++++++++++++++++--
> >  1 file changed, 494 insertions(+), 21 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 
> > 
> 

Re: [PATCH 1/8] tests/unit/test-smp-parse.c: Use CPU number macros in invalid topology case

[Thomas Huth]

On 18/01/2024 15.48, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Use MAX_CPUS/MIN_CPUS micros in invalid topology case. This gives us the

s/micros/macros/

Apart from that typo, FWIW:
Reviewed-by: Thomas Huth <thuth@redhat.com>

> flexibility to change the maximum and minimum CPU limits.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   tests/unit/test-smp-parse.c | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
> index 24972666a74d..20c663a006b3 100644
> --- a/tests/unit/test-smp-parse.c
> +++ b/tests/unit/test-smp-parse.c
> @@ -323,15 +323,21 @@ static const struct SMPTestData data_generic_invalid[] = {
>                           "sockets (2) * cores (4) * threads (2) "
>                           "== maxcpus (16) < smp_cpus (18)",
>       }, {
> -        /* config: -smp 1
> -         * should tweak the supported min CPUs to 2 for testing */
> -        .config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0),
> +        /*
> +         * config: -smp 1
> +         * The test machine should tweak the supported min CPUs to
> +         * 2 (MIN_CPUS + 1) for testing.
> +         */
> +        .config = SMP_CONFIG_GENERIC(T, MIN_CPUS, F, 0, F, 0, F, 0, F, 0),
>           .expect_error = "Invalid SMP CPUs 1. The min CPUs supported "
>                           "by machine '" SMP_MACHINE_NAME "' is 2",
>       }, {
> -        /* config: -smp 512
> -         * should tweak the supported max CPUs to 511 for testing */
> -        .config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0),
> +        /*
> +         * config: -smp 512
> +         * The test machine should tweak the supported max CPUs to
> +         * 511 (MAX_CPUS - 1) for testing.
> +         */
> +        .config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0),
>           .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
>                           "by machine '" SMP_MACHINE_NAME "' is 511",
>       },
> @@ -575,8 +581,8 @@ static void machine_generic_invalid_class_init(ObjectClass *oc, void *data)
>       MachineClass *mc = MACHINE_CLASS(oc);
>   
>       /* Force invalid min CPUs and max CPUs */
> -    mc->min_cpus = 2;
> -    mc->max_cpus = 511;
> +    mc->min_cpus = MIN_CPUS + 1;
> +    mc->max_cpus = MAX_CPUS - 1;
>   }
>   
>   static void machine_with_dies_class_init(ObjectClass *oc, void *data)

Re: [PATCH 2/8] tests/unit/test-smp-parse.c: Bump max_cpus to 4096

[Thomas Huth]

On 18/01/2024 15.48, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> PC mahine is trying to support up to 4096 vCPUs [1], so it's necessary

s/mahine/machine/

> to bump max_cpus in test-smp-parse to 4096 to cover the topological
> needs of future machines.
> 
> [1]: https://lore.kernel.org/qemu-devel/20231208122611.32311-1-anisinha@redhat.com/

Is it ok for this test patch here to get included without that patch 
already, or shall this wait for the patch from Ani first?

  Thomas


> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   tests/unit/test-smp-parse.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
> index 20c663a006b3..55ba13bf7d15 100644
> --- a/tests/unit/test-smp-parse.c
> +++ b/tests/unit/test-smp-parse.c
> @@ -20,8 +20,8 @@
>   #define T true
>   #define F false
>   
> -#define MIN_CPUS 1   /* set the min CPUs supported by the machine as 1 */
> -#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */
> +#define MIN_CPUS 1    /* set the min CPUs supported by the machine as 1 */
> +#define MAX_CPUS 4096 /* set the max CPUs supported by the machine as 4096 */
>   
>   #define SMP_MACHINE_NAME "TEST-SMP"
>   
> @@ -333,13 +333,13 @@ static const struct SMPTestData data_generic_invalid[] = {
>                           "by machine '" SMP_MACHINE_NAME "' is 2",
>       }, {
>           /*
> -         * config: -smp 512
> +         * config: -smp 4096
>            * The test machine should tweak the supported max CPUs to
> -         * 511 (MAX_CPUS - 1) for testing.
> +         * 4095 (MAX_CPUS - 1) for testing.
>            */
> -        .config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0),
> -        .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
> -                        "by machine '" SMP_MACHINE_NAME "' is 511",
> +        .config = SMP_CONFIG_GENERIC(T, 4096, F, 0, F, 0, F, 0, F, 0),
> +        .expect_error = "Invalid SMP CPUs 4096. The max CPUs supported "
> +                        "by machine '" SMP_MACHINE_NAME "' is 4095",
>       },
>   };
>   

Re: [PATCH 3/8] tests/unit/test-smp-parse.c: Make test cases aware of the book/drawer

[Thomas Huth]

On 18/01/2024 15.48, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Currently, -smp supports 2 more new levels: book and drawer.
> 
> It is necessary to consider the effects of book and drawer in the test
> cases to ensure that the calculations are correct. This is also the
> preparation to add new book and drawer test cases.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   tests/unit/test-smp-parse.c | 25 ++++++++++++++++++++++---
>   1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
> index 55ba13bf7d15..a8eb3bbb35ed 100644
> --- a/tests/unit/test-smp-parse.c
> +++ b/tests/unit/test-smp-parse.c
> @@ -384,6 +384,8 @@ static char *smp_config_to_string(const SMPConfiguration *config)
>       return g_strdup_printf(
>           "(SMPConfiguration) {\n"
>           "    .has_cpus     = %5s, cpus     = %" PRId64 ",\n"
> +        "    .has_drawers  = %5s, drawers  = %" PRId64 ",\n"
> +        "    .has_books    = %5s, books    = %" PRId64 ",\n"
>           "    .has_sockets  = %5s, sockets  = %" PRId64 ",\n"
>           "    .has_dies     = %5s, dies     = %" PRId64 ",\n"
>           "    .has_clusters = %5s, clusters = %" PRId64 ",\n"
> @@ -392,6 +394,8 @@ static char *smp_config_to_string(const SMPConfiguration *config)
>           "    .has_maxcpus  = %5s, maxcpus  = %" PRId64 ",\n"
>           "}",
>           config->has_cpus ? "true" : "false", config->cpus,
> +        config->has_drawers ? "true" : "false", config->drawers,
> +        config->has_books ? "true" : "false", config->books,
>           config->has_sockets ? "true" : "false", config->sockets,
>           config->has_dies ? "true" : "false", config->dies,
>           config->has_clusters ? "true" : "false", config->clusters,
> @@ -404,10 +408,10 @@ static char *smp_config_to_string(const SMPConfiguration *config)
>   static unsigned int cpu_topology_get_threads_per_socket(const CpuTopology *topo)
>   {
>       /* Check the divisor to avoid invalid topology examples causing SIGFPE. */
> -    if (!topo->sockets) {
> +    if (!topo->drawers || !topo->books || !topo->sockets) {
>           return 0;
>       } else {
> -        return topo->max_cpus / topo->sockets;
> +        return topo->max_cpus / topo->drawers / topo->books / topo->sockets;
>       }
>   }
>   
> @@ -429,6 +433,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
>       return g_strdup_printf(
>           "(CpuTopology) {\n"
>           "    .cpus               = %u,\n"
> +        "    .drawers            = %u,\n"
> +        "    .books              = %u,\n"
>           "    .sockets            = %u,\n"
>           "    .dies               = %u,\n"
>           "    .clusters           = %u,\n"
> @@ -438,7 +444,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
>           "    .threads_per_socket = %u,\n"
>           "    .cores_per_socket   = %u,\n"
>           "}",
> -        topo->cpus, topo->sockets, topo->dies, topo->clusters,
> +        topo->cpus, topo->drawers, topo->books,
> +        topo->sockets, topo->dies, topo->clusters,
>           topo->cores, topo->threads, topo->max_cpus,
>           threads_per_socket, cores_per_socket);
>   }
> @@ -473,6 +480,8 @@ static void check_parse(MachineState *ms, const SMPConfiguration *config,
>       if (is_valid) {
>           if ((err == NULL) &&
>               (ms->smp.cpus == expect_topo->cpus) &&
> +            (ms->smp.drawers == expect_topo->drawers) &&
> +            (ms->smp.books == expect_topo->books) &&
>               (ms->smp.sockets == expect_topo->sockets) &&
>               (ms->smp.dies == expect_topo->dies) &&
>               (ms->smp.clusters == expect_topo->clusters) &&
> @@ -564,6 +573,16 @@ static void unsupported_params_init(const MachineClass *mc, SMPTestData *data)
>           data->expect_prefer_sockets.clusters = 1;
>           data->expect_prefer_cores.clusters = 1;
>       }
> +
> +    if (!mc->smp_props.books_supported) {
> +        data->expect_prefer_sockets.books = 1;
> +        data->expect_prefer_cores.books = 1;
> +    }
> +
> +    if (!mc->smp_props.drawers_supported) {
> +        data->expect_prefer_sockets.drawers = 1;
> +        data->expect_prefer_cores.drawers = 1;
> +    }
>   }
>   

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 4/8] tests/unit/test-smp-parse.c: Test "books" parameter in -smp

[Thomas Huth]

On 18/01/2024 15.48, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Although book was introduced to -smp along with drawer by s390 machine,
> as a general topology level in QEMU that may be reused by other arches
> in the future, it is desirable to cover this parameter's parsing in a
> separate case.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   tests/unit/test-smp-parse.c | 105 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 105 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 5/8] tests/unit/test-smp-parse.c: Test "drawers" parameter in -smp

[Thomas Huth]

On 18/01/2024 15.48, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Although drawer was introduced to -smp along with book by s390 machine,
> as a general topology level in QEMU that may be reused by other arches
> in the future, it is desirable to cover this parameter's parsing in a
> separate case.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   tests/unit/test-smp-parse.c | 89 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 89 insertions(+)


Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 2/8] tests/unit/test-smp-parse.c: Bump max_cpus to 4096

[Zhao Liu]

Hi Thomas,

On Fri, Mar 01, 2024 at 08:40:54AM +0100, Thomas Huth wrote:
> Date: Fri, 1 Mar 2024 08:40:54 +0100
> From: Thomas Huth <thuth@redhat.com>
> Subject: Re: [PATCH 2/8] tests/unit/test-smp-parse.c: Bump max_cpus to 4096
> 
> On 18/01/2024 15.48, Zhao Liu wrote:
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > PC mahine is trying to support up to 4096 vCPUs [1], so it's necessary
> 
> s/mahine/machine/
> 
> > to bump max_cpus in test-smp-parse to 4096 to cover the topological
> > needs of future machines.
> > 
> > [1]: https://lore.kernel.org/qemu-devel/20231208122611.32311-1-anisinha@redhat.com/
> 
> Is it ok for this test patch here to get included without that patch
> already, or shall this wait for the patch from Ani first?
>

This patch doesn't rely on Ani's, but given my typo and outdated links
(Ani already has v5), I can respin the series soon!

Thanks,
Zhao

Re: [PATCH 7/8] tests/unit/test-smp-parse.c: Test the full 7-levels topology hierarchy

[Thomas Huth]

On 18/01/2024 15.48, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Currently, -smp supports up to 7-levels topology hierarchy:
>    -drawers/books/sockets/dies/clusters/cores/threads.
> 
> Though no machine supports all these 7 levels yet, these 7 levels have
> the strict containment relationship and together form the generic CPU
> topology representation of QEMU.
> 
> Also, note that the maxcpus is calculated by multiplying all 7 levels:
> 
>    maxcpus = drawers * books * sockets * dies * clusters *
>              cores * threads.
> 
> To cover this code path, it is necessary to test the full topology case
> (with all 7 levels). This also helps to avoid introducing new issues by
> further expanding the CPU topology in the future.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   tests/unit/test-smp-parse.c | 143 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 143 insertions(+)

FWIW:
Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 8/8] tests/unit/test-smp-parse.c: Test smp_props.has_clusters

[Thomas Huth]

On 18/01/2024 15.48, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> The smp_props.has_clusters in MachineClass is not a user configured
> field, and it indicates if user specifies "clusters" in -smp.
> 
> After -smp parsing, other module could aware if the cluster level
> is configured by user. This is used when the machine has only 1 cluster
> since there's only 1 cluster by default.
> 
> Add the check to cover this field.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   tests/unit/test-smp-parse.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)

Acked-by: Thomas Huth <thuth@redhat.com>