[PATCH 0/5] Sparc CPU naming and help text improvements

[PATCH 0/5] Sparc CPU naming and help text improvements

[Thomas Huth]

The Sparc CPU naming and the corresponding help text is somewhat
confusing for the users. We should avoid spaces in the Names and
provide clear information to the users what can be passed to the
"-cpu" option.
While we're at it, also remove the "+" from two of the CPU names
since this character is now not allowed in device names anymore
(and was worked around with an ugly hack in qom/object.c so far).

Thomas Huth (5):
  target/sparc/cpu: Rename the CPU models with a "+" in their names
  target/sparc/cpu: Avoid spaces by default in the CPU names
  target/sparc/cpu: Improve the CPU help text
  docs/system/target-sparc: Improve the Sparc documentation
  docs/about: Deprecate the old "UltraSparc" CPU names that contain a
    "+"

 docs/about/deprecated.rst    |  9 +++++
 docs/system/target-sparc.rst |  8 +++-
 qom/object.c                 |  8 ----
 target/sparc/cpu.c           | 71 +++++++++++++++++++++---------------
 4 files changed, 56 insertions(+), 40 deletions(-)

-- 
2.44.0

[PATCH 1/5] target/sparc/cpu: Rename the CPU models with a "+" in their names

[Thomas Huth]

Commit b447378e12 ("qom/object: Limit type names to alphanumerical ...")
cut down the amount of allowed characters for QOM types to a saner set.
The "+" character was not meant to be included in this set, so we had
to add a hack there to still allow the legacy names of POWER and Sparc64
CPUs. However, instead of putting such a hack in the common QOM code,
there is a much better place to do this: The sparc_cpu_class_by_name()
function which is used to look up the names of all Sparc CPUs.
Thus let's finally get rid of the "+" in the Sparc CPU names, and provide
backward compatibility for the old names via some simple checks in the
sparc_cpu_class_by_name() function.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 qom/object.c       |  8 --------
 target/sparc/cpu.c | 14 ++++++++++++--
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index d4a001cf41..759e194972 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -158,14 +158,6 @@ static bool type_name_is_valid(const char *name)
                         "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
                         "0123456789-_.");
 
-    /* Allow some legacy names with '+' in it for compatibility reasons */
-    if (name[plen] == '+') {
-        if (plen >= 17 && g_str_has_prefix(name, "Sun-UltraSparc-I")) {
-            /* Allow "Sun-UltraSparc-IV+" and "Sun-UltraSparc-IIIi+" */
-            return true;
-        }
-    }
-
     return plen == slen;
 }
 
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 313ebc4c11..651e49bfeb 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -316,7 +316,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Sun UltraSparc IV+",
+        .name = "Sun UltraSparc IVp",
         .iu_version = ((0x3eULL << 48) | (0x19ULL << 32) | (0x22ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -325,7 +325,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES | CPU_FEATURE_CMT,
     },
     {
-        .name = "Sun UltraSparc IIIi+",
+        .name = "Sun UltraSparc IIIip",
         .iu_version = ((0x3eULL << 48) | (0x22ULL << 32) | (0ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_3,
@@ -767,6 +767,16 @@ static ObjectClass *sparc_cpu_class_by_name(const char *cpu_model)
     char *typename;
 
     typename = sparc_cpu_type_name(cpu_model);
+
+    /* Fix up legacy names with '+' in it */
+    if (g_str_equal(typename, SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IV+"))) {
+        g_free(typename);
+        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IVp"));
+    } else if (g_str_equal(typename, SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIi+"))) {
+        g_free(typename);
+        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIip"));
+    }
+
     oc = object_class_by_name(typename);
     g_free(typename);
     return oc;
-- 
2.44.0

[PATCH 2/5] target/sparc/cpu: Avoid spaces by default in the CPU names

[Thomas Huth]

The output of "-cpu help" is currently rather confusing to the users:
It is not clear which part of the output defines the CPU names since
the CPU names contain white spaces (which we later have to convert
into dashes internally) For example:

Sparc TI UltraSparc II IU 0017001120000000 FPU 00000000 MMU 00000000 NWINS 8

At a first glance, should the name for -cpu be "Sparc TI Ultrasparc II"
or "TI UltraSparc II IU" here? Both would be wrong, the right guess is
"TI UltraSparc II" only. Let's start cleaning up this mess by using
dashes instead of white spaces for the CPU names, like we're doing it
internally later (and like we're doing it in most other targets of QEMU).
Note that it is still possible to pass the CPU names with spaces to the
"-cpu" option, since sparc_cpu_type_name() still translates those to "-".

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/2141
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/sparc/cpu.c | 56 +++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 651e49bfeb..ae30cded22 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -208,7 +208,7 @@ void cpu_sparc_set_id(CPUSPARCState *env, unsigned int cpu)
 static const sparc_def_t sparc_defs[] = {
 #ifdef TARGET_SPARC64
     {
-        .name = "Fujitsu Sparc64",
+        .name = "Fujitsu-Sparc64",
         .iu_version = ((0x04ULL << 48) | (0x02ULL << 32) | (0ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -217,7 +217,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Fujitsu Sparc64 III",
+        .name = "Fujitsu-Sparc64-III",
         .iu_version = ((0x04ULL << 48) | (0x03ULL << 32) | (0ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -226,7 +226,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Fujitsu Sparc64 IV",
+        .name = "Fujitsu-Sparc64-IV",
         .iu_version = ((0x04ULL << 48) | (0x04ULL << 32) | (0ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -235,7 +235,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Fujitsu Sparc64 V",
+        .name = "Fujitsu-Sparc64-V",
         .iu_version = ((0x04ULL << 48) | (0x05ULL << 32) | (0x51ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -244,7 +244,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI UltraSparc I",
+        .name = "TI-UltraSparc-I",
         .iu_version = ((0x17ULL << 48) | (0x10ULL << 32) | (0x40ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -253,7 +253,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI UltraSparc II",
+        .name = "TI-UltraSparc-II",
         .iu_version = ((0x17ULL << 48) | (0x11ULL << 32) | (0x20ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -262,7 +262,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI UltraSparc IIi",
+        .name = "TI-UltraSparc-IIi",
         .iu_version = ((0x17ULL << 48) | (0x12ULL << 32) | (0x91ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -271,7 +271,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI UltraSparc IIe",
+        .name = "TI-UltraSparc-IIe",
         .iu_version = ((0x17ULL << 48) | (0x13ULL << 32) | (0x14ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -280,7 +280,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Sun UltraSparc III",
+        .name = "Sun-UltraSparc-III",
         .iu_version = ((0x3eULL << 48) | (0x14ULL << 32) | (0x34ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -289,7 +289,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Sun UltraSparc III Cu",
+        .name = "Sun-UltraSparc-III-Cu",
         .iu_version = ((0x3eULL << 48) | (0x15ULL << 32) | (0x41ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_3,
@@ -298,7 +298,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Sun UltraSparc IIIi",
+        .name = "Sun-UltraSparc-IIIi",
         .iu_version = ((0x3eULL << 48) | (0x16ULL << 32) | (0x34ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -307,7 +307,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Sun UltraSparc IV",
+        .name = "Sun-UltraSparc-IV",
         .iu_version = ((0x3eULL << 48) | (0x18ULL << 32) | (0x31ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_4,
@@ -316,7 +316,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Sun UltraSparc IVp",
+        .name = "Sun-UltraSparc-IVp",
         .iu_version = ((0x3eULL << 48) | (0x19ULL << 32) | (0x22ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -325,7 +325,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES | CPU_FEATURE_CMT,
     },
     {
-        .name = "Sun UltraSparc IIIip",
+        .name = "Sun-UltraSparc-IIIip",
         .iu_version = ((0x3eULL << 48) | (0x22ULL << 32) | (0ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_3,
@@ -334,7 +334,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Sun UltraSparc T1",
+        .name = "Sun-UltraSparc-T1",
         /* defined in sparc_ifu_fdp.v and ctu.h */
         .iu_version = ((0x3eULL << 48) | (0x23ULL << 32) | (0x02ULL << 24)),
         .fpu_version = 0x00000000,
@@ -345,7 +345,7 @@ static const sparc_def_t sparc_defs[] = {
         | CPU_FEATURE_GL,
     },
     {
-        .name = "Sun UltraSparc T2",
+        .name = "Sun-UltraSparc-T2",
         /* defined in tlu_asi_ctl.v and n2_revid_cust.v */
         .iu_version = ((0x3eULL << 48) | (0x24ULL << 32) | (0x02ULL << 24)),
         .fpu_version = 0x00000000,
@@ -356,7 +356,7 @@ static const sparc_def_t sparc_defs[] = {
         | CPU_FEATURE_GL,
     },
     {
-        .name = "NEC UltraSparc I",
+        .name = "NEC-UltraSparc-I",
         .iu_version = ((0x22ULL << 48) | (0x10ULL << 32) | (0x40ULL << 24)),
         .fpu_version = 0x00000000,
         .mmu_version = mmu_us_12,
@@ -366,7 +366,7 @@ static const sparc_def_t sparc_defs[] = {
     },
 #else
     {
-        .name = "Fujitsu MB86904",
+        .name = "Fujitsu-MB86904",
         .iu_version = 0x04 << 24, /* Impl 0, ver 4 */
         .fpu_version = 4 << FSR_VER_SHIFT, /* FPU version 4 (Meiko) */
         .mmu_version = 0x04 << 24, /* Impl 0, ver 4 */
@@ -379,7 +379,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "Fujitsu MB86907",
+        .name = "Fujitsu-MB86907",
         .iu_version = 0x05 << 24, /* Impl 0, ver 5 */
         .fpu_version = 4 << FSR_VER_SHIFT, /* FPU version 4 (Meiko) */
         .mmu_version = 0x05 << 24, /* Impl 0, ver 5 */
@@ -392,7 +392,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI MicroSparc I",
+        .name = "TI-MicroSparc-I",
         .iu_version = 0x41000000,
         .fpu_version = 4 << FSR_VER_SHIFT,
         .mmu_version = 0x41000000,
@@ -405,7 +405,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_FEATURE_MUL | CPU_FEATURE_DIV,
     },
     {
-        .name = "TI MicroSparc II",
+        .name = "TI-MicroSparc-II",
         .iu_version = 0x42000000,
         .fpu_version = 4 << FSR_VER_SHIFT,
         .mmu_version = 0x02000000,
@@ -418,7 +418,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI MicroSparc IIep",
+        .name = "TI-MicroSparc-IIep",
         .iu_version = 0x42000000,
         .fpu_version = 4 << FSR_VER_SHIFT,
         .mmu_version = 0x04000000,
@@ -431,7 +431,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI SuperSparc 40", /* STP1020NPGA */
+        .name = "TI-SuperSparc-40", /* STP1020NPGA */
         .iu_version = 0x41000000, /* SuperSPARC 2.x */
         .fpu_version = 0 << FSR_VER_SHIFT,
         .mmu_version = 0x00000800, /* SuperSPARC 2.x, no MXCC */
@@ -444,7 +444,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI SuperSparc 50", /* STP1020PGA */
+        .name = "TI-SuperSparc-50", /* STP1020PGA */
         .iu_version = 0x40000000, /* SuperSPARC 3.x */
         .fpu_version = 0 << FSR_VER_SHIFT,
         .mmu_version = 0x01000800, /* SuperSPARC 3.x, no MXCC */
@@ -457,7 +457,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI SuperSparc 51",
+        .name = "TI-SuperSparc-51",
         .iu_version = 0x40000000, /* SuperSPARC 3.x */
         .fpu_version = 0 << FSR_VER_SHIFT,
         .mmu_version = 0x01000000, /* SuperSPARC 3.x, MXCC */
@@ -471,7 +471,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI SuperSparc 60", /* STP1020APGA */
+        .name = "TI-SuperSparc-60", /* STP1020APGA */
         .iu_version = 0x40000000, /* SuperSPARC 3.x */
         .fpu_version = 0 << FSR_VER_SHIFT,
         .mmu_version = 0x01000800, /* SuperSPARC 3.x, no MXCC */
@@ -484,7 +484,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI SuperSparc 61",
+        .name = "TI-SuperSparc-61",
         .iu_version = 0x44000000, /* SuperSPARC 3.x */
         .fpu_version = 0 << FSR_VER_SHIFT,
         .mmu_version = 0x01000000, /* SuperSPARC 3.x, MXCC */
@@ -498,7 +498,7 @@ static const sparc_def_t sparc_defs[] = {
         .features = CPU_DEFAULT_FEATURES,
     },
     {
-        .name = "TI SuperSparc II",
+        .name = "TI-SuperSparc-II",
         .iu_version = 0x40000000, /* SuperSPARC II 1.x */
         .fpu_version = 0 << FSR_VER_SHIFT,
         .mmu_version = 0x08000000, /* SuperSPARC II 1.x, MXCC */
-- 
2.44.0

[PATCH 3/5] target/sparc/cpu: Improve the CPU help text

[Thomas Huth]

Remove the unnecessary "Sparc" at the beginning of the line and
put the chip information into parentheses so that it is clearer
which part of the line have to be passed to "-cpu" to specify a
different CPU.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2141
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/sparc/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index ae30cded22..6650248ffe 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -576,9 +576,10 @@ void sparc_cpu_list(void)
 {
     unsigned int i;
 
+    qemu_printf("Available CPU types:\n");
     for (i = 0; i < ARRAY_SIZE(sparc_defs); i++) {
-        qemu_printf("Sparc %16s IU " TARGET_FMT_lx
-                    " FPU %08x MMU %08x NWINS %d ",
+        qemu_printf(" %-20s (IU " TARGET_FMT_lx
+                    " FPU %08x MMU %08x NWINS %d) ",
                     sparc_defs[i].name,
                     sparc_defs[i].iu_version,
                     sparc_defs[i].fpu_version,
-- 
2.44.0

[PATCH 4/5] docs/system/target-sparc: Improve the Sparc documentation

[Thomas Huth]

Add some words about how to enable or disable boolean features,
and remove the note about a Linux kernel being available on the
QEMU website (they have been removed long ago already).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/system/target-sparc.rst | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/docs/system/target-sparc.rst b/docs/system/target-sparc.rst
index 9ec8c90c14..9f418b9d3e 100644
--- a/docs/system/target-sparc.rst
+++ b/docs/system/target-sparc.rst
@@ -27,6 +27,11 @@ architecture machines:
 The emulation is somewhat complete. SMP up to 16 CPUs is supported, but
 Linux limits the number of usable CPUs to 4.
 
+The list of available CPUs can be viewed by starting QEMU with ``-cpu help``.
+Optional boolean features can be added with a "+" in front of the feature name,
+or disabled with a "-" in front of the name, for example
+``-cpu TI-SuperSparc-II,+float128``.
+
 QEMU emulates the following sun4m peripherals:
 
 -  IOMMU
@@ -55,8 +60,7 @@ OpenBIOS is a free (GPL v2) portable firmware implementation. The goal
 is to implement a 100% IEEE 1275-1994 (referred to as Open Firmware)
 compliant firmware.
 
-A sample Linux 2.6 series kernel and ram disk image are available on the
-QEMU web site. There are still issues with NetBSD and OpenBSD, but most
+There are still issues with NetBSD and OpenBSD, but most
 kernel versions work. Please note that currently older Solaris kernels
 don't work probably due to interface issues between OpenBIOS and
 Solaris.
-- 
2.44.0

[PATCH 5/5] docs/about: Deprecate the old "UltraSparc" CPU names that contain a "+"

[Thomas Huth]

For consistency we should drop the names with a "+" in it in the
long run.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 8565644da6..7058341f8f 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -202,6 +202,15 @@ in the QEMU object model anymore. ``power5+``, ``power5+_v2.1``,
 an alias, but for consistency these will get removed in a future
 release, too. Use ``power5p_v2.1`` and ``power7p_v2.1`` instead.
 
+``Sun-UltraSparc-IIIi+`` and ``Sun-UltraSparc-IV+`` CPU names (since 9.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The character "+" in device (and thus also CPU) names is not allowed
+in the QEMU object model anymore. ``Sun-UltraSparc-IIIi+`` and
+``Sun-UltraSparc-IV+`` are currently still supported via a workaround,
+but for consistency these will get removed in a future release, too.
+Use ``Sun-UltraSparc-IIIip`` and ``Sun-UltraSparc-IVp`` instead.
+
 CRIS CPU architecture (since 9.0)
 '''''''''''''''''''''''''''''''''
 
-- 
2.44.0

Re: [PATCH 3/5] target/sparc/cpu: Improve the CPU help text

[Philippe Mathieu-Daudé]

On 7/3/24 18:43, Thomas Huth wrote:
> Remove the unnecessary "Sparc" at the beginning of the line and
> put the chip information into parentheses so that it is clearer
> which part of the line have to be passed to "-cpu" to specify a
> different CPU.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2141
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   target/sparc/cpu.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 1/5] target/sparc/cpu: Rename the CPU models with a "+" in their names

[Richard Henderson]

On 3/7/24 07:43, Thomas Huth wrote:
> +    /* Fix up legacy names with '+' in it */
> +    if (g_str_equal(typename, SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IV+"))) {
> +        g_free(typename);
> +        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IVp"));
> +    } else if (g_str_equal(typename, SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIi+"))) {
> +        g_free(typename);
> +        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIip"));
> +    }
> +

Legacy names don't include dashes.


r~

Re: [PATCH 2/5] target/sparc/cpu: Avoid spaces by default in the CPU names

[Richard Henderson]

On 3/7/24 07:43, Thomas Huth wrote:
> The output of "-cpu help" is currently rather confusing to the users:
> It is not clear which part of the output defines the CPU names since
> the CPU names contain white spaces (which we later have to convert
> into dashes internally) For example:
> 
> Sparc TI UltraSparc II IU 0017001120000000 FPU 00000000 MMU 00000000 NWINS 8
> 
> At a first glance, should the name for -cpu be "Sparc TI Ultrasparc II"
> or "TI UltraSparc II IU" here? Both would be wrong, the right guess is
> "TI UltraSparc II" only. Let's start cleaning up this mess by using
> dashes instead of white spaces for the CPU names, like we're doing it
> internally later (and like we're doing it in most other targets of QEMU).
> Note that it is still possible to pass the CPU names with spaces to the
> "-cpu" option, since sparc_cpu_type_name() still translates those to "-".
> 
> Buglink:https://gitlab.com/qemu-project/qemu/-/issues/2141
> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   target/sparc/cpu.c | 56 +++++++++++++++++++++++-----------------------
>   1 file changed, 28 insertions(+), 28 deletions(-)

I think the names are still a bit too long, and the case sensitivity is a titch annoying. 
But it's still an improvement, and I don't want to bike-shed this too much.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 3/5] target/sparc/cpu: Improve the CPU help text

[Richard Henderson]

On 3/7/24 07:43, Thomas Huth wrote:
> Remove the unnecessary "Sparc" at the beginning of the line and
> put the chip information into parentheses so that it is clearer
> which part of the line have to be passed to "-cpu" to specify a
> different CPU.
> 
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/2141
> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   target/sparc/cpu.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 1/5] target/sparc/cpu: Rename the CPU models with a "+" in their names

[Thomas Huth]

On 07/03/2024 22.22, Richard Henderson wrote:
> On 3/7/24 07:43, Thomas Huth wrote:
>> +    /* Fix up legacy names with '+' in it */
>> +    if (g_str_equal(typename, SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IV+"))) {
>> +        g_free(typename);
>> +        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IVp"));
>> +    } else if (g_str_equal(typename, 
>> SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIi+"))) {
>> +        g_free(typename);
>> +        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIip"));
>> +    }
>> +
> 
> Legacy names don't include dashes.

This check is done after sparc_cpu_type_name() has been called, which 
transforms the spaces into dashes, so we need the dashes here.
(otherwise I'd need to check for all "valid" combinations, like 
"Sun-UltraSparc-IV+", "Sun UltraSparc-IV+", "Sun-UltraSparc IV+" and "Sun 
UltraSparc IV+").

  Thomas

Re: [PATCH 0/5] Sparc CPU naming and help text improvements

[Thomas Huth]

On 07/03/2024 18.43, Thomas Huth wrote:
> The Sparc CPU naming and the corresponding help text is somewhat
> confusing for the users. We should avoid spaces in the Names and
> provide clear information to the users what can be passed to the
> "-cpu" option.
> While we're at it, also remove the "+" from two of the CPU names
> since this character is now not allowed in device names anymore
> (and was worked around with an ugly hack in qom/object.c so far).
> 
> Thomas Huth (5):
>    target/sparc/cpu: Rename the CPU models with a "+" in their names
>    target/sparc/cpu: Avoid spaces by default in the CPU names
>    target/sparc/cpu: Improve the CPU help text
>    docs/system/target-sparc: Improve the Sparc documentation
>    docs/about: Deprecate the old "UltraSparc" CPU names that contain a
>      "+"

Ping!

Mark, Aryom, could you please comment on this patch series, too?

Thanks,
  Thomas

Re: [PATCH 1/5] target/sparc/cpu: Rename the CPU models with a "+" in their names

[Mark Cave-Ayland]

On 07/03/2024 17:43, Thomas Huth wrote:

> Commit b447378e12 ("qom/object: Limit type names to alphanumerical ...")
> cut down the amount of allowed characters for QOM types to a saner set.
> The "+" character was not meant to be included in this set, so we had
> to add a hack there to still allow the legacy names of POWER and Sparc64
> CPUs. However, instead of putting such a hack in the common QOM code,
> there is a much better place to do this: The sparc_cpu_class_by_name()
> function which is used to look up the names of all Sparc CPUs.
> Thus let's finally get rid of the "+" in the Sparc CPU names, and provide
> backward compatibility for the old names via some simple checks in the
> sparc_cpu_class_by_name() function.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   qom/object.c       |  8 --------
>   target/sparc/cpu.c | 14 ++++++++++++--
>   2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index d4a001cf41..759e194972 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -158,14 +158,6 @@ static bool type_name_is_valid(const char *name)
>                           "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
>                           "0123456789-_.");
>   
> -    /* Allow some legacy names with '+' in it for compatibility reasons */
> -    if (name[plen] == '+') {
> -        if (plen >= 17 && g_str_has_prefix(name, "Sun-UltraSparc-I")) {
> -            /* Allow "Sun-UltraSparc-IV+" and "Sun-UltraSparc-IIIi+" */
> -            return true;
> -        }
> -    }
> -
>       return plen == slen;
>   }
>   
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 313ebc4c11..651e49bfeb 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -316,7 +316,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Sun UltraSparc IV+",
> +        .name = "Sun UltraSparc IVp",
>           .iu_version = ((0x3eULL << 48) | (0x19ULL << 32) | (0x22ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -325,7 +325,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES | CPU_FEATURE_CMT,
>       },
>       {
> -        .name = "Sun UltraSparc IIIi+",
> +        .name = "Sun UltraSparc IIIip",
>           .iu_version = ((0x3eULL << 48) | (0x22ULL << 32) | (0ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_3,
> @@ -767,6 +767,16 @@ static ObjectClass *sparc_cpu_class_by_name(const char *cpu_model)
>       char *typename;
>   
>       typename = sparc_cpu_type_name(cpu_model);
> +
> +    /* Fix up legacy names with '+' in it */
> +    if (g_str_equal(typename, SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IV+"))) {
> +        g_free(typename);
> +        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IVp"));
> +    } else if (g_str_equal(typename, SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIi+"))) {
> +        g_free(typename);
> +        typename = g_strdup(SPARC_CPU_TYPE_NAME("Sun-UltraSparc-IIIip"));
> +    }
> +
>       oc = object_class_by_name(typename);
>       g_free(typename);
>       return oc;

I've seen some references in Sun documentation to processors in the form "UltraSparc 
IIIi plus" so I'd be inclined to use that form for the new type names e.g. 
"UltraSparc-IIIi-plus".

Otherwise looks good to me, thanks for having a look at this!

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH 2/5] target/sparc/cpu: Avoid spaces by default in the CPU names

[Mark Cave-Ayland]

On 07/03/2024 17:43, Thomas Huth wrote:

> The output of "-cpu help" is currently rather confusing to the users:
> It is not clear which part of the output defines the CPU names since
> the CPU names contain white spaces (which we later have to convert
> into dashes internally) For example:
> 
> Sparc TI UltraSparc II IU 0017001120000000 FPU 00000000 MMU 00000000 NWINS 8
> 
> At a first glance, should the name for -cpu be "Sparc TI Ultrasparc II"
> or "TI UltraSparc II IU" here? Both would be wrong, the right guess is
> "TI UltraSparc II" only. Let's start cleaning up this mess by using
> dashes instead of white spaces for the CPU names, like we're doing it
> internally later (and like we're doing it in most other targets of QEMU).
> Note that it is still possible to pass the CPU names with spaces to the
> "-cpu" option, since sparc_cpu_type_name() still translates those to "-".
> 
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/2141
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   target/sparc/cpu.c | 56 +++++++++++++++++++++++-----------------------
>   1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 651e49bfeb..ae30cded22 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -208,7 +208,7 @@ void cpu_sparc_set_id(CPUSPARCState *env, unsigned int cpu)
>   static const sparc_def_t sparc_defs[] = {
>   #ifdef TARGET_SPARC64
>       {
> -        .name = "Fujitsu Sparc64",
> +        .name = "Fujitsu-Sparc64",
>           .iu_version = ((0x04ULL << 48) | (0x02ULL << 32) | (0ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -217,7 +217,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Fujitsu Sparc64 III",
> +        .name = "Fujitsu-Sparc64-III",
>           .iu_version = ((0x04ULL << 48) | (0x03ULL << 32) | (0ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -226,7 +226,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Fujitsu Sparc64 IV",
> +        .name = "Fujitsu-Sparc64-IV",
>           .iu_version = ((0x04ULL << 48) | (0x04ULL << 32) | (0ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -235,7 +235,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Fujitsu Sparc64 V",
> +        .name = "Fujitsu-Sparc64-V",
>           .iu_version = ((0x04ULL << 48) | (0x05ULL << 32) | (0x51ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -244,7 +244,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI UltraSparc I",
> +        .name = "TI-UltraSparc-I",
>           .iu_version = ((0x17ULL << 48) | (0x10ULL << 32) | (0x40ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -253,7 +253,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI UltraSparc II",
> +        .name = "TI-UltraSparc-II",
>           .iu_version = ((0x17ULL << 48) | (0x11ULL << 32) | (0x20ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -262,7 +262,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI UltraSparc IIi",
> +        .name = "TI-UltraSparc-IIi",
>           .iu_version = ((0x17ULL << 48) | (0x12ULL << 32) | (0x91ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -271,7 +271,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI UltraSparc IIe",
> +        .name = "TI-UltraSparc-IIe",
>           .iu_version = ((0x17ULL << 48) | (0x13ULL << 32) | (0x14ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -280,7 +280,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Sun UltraSparc III",
> +        .name = "Sun-UltraSparc-III",
>           .iu_version = ((0x3eULL << 48) | (0x14ULL << 32) | (0x34ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -289,7 +289,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Sun UltraSparc III Cu",
> +        .name = "Sun-UltraSparc-III-Cu",
>           .iu_version = ((0x3eULL << 48) | (0x15ULL << 32) | (0x41ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_3,
> @@ -298,7 +298,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Sun UltraSparc IIIi",
> +        .name = "Sun-UltraSparc-IIIi",
>           .iu_version = ((0x3eULL << 48) | (0x16ULL << 32) | (0x34ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -307,7 +307,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Sun UltraSparc IV",
> +        .name = "Sun-UltraSparc-IV",
>           .iu_version = ((0x3eULL << 48) | (0x18ULL << 32) | (0x31ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_4,
> @@ -316,7 +316,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Sun UltraSparc IVp",
> +        .name = "Sun-UltraSparc-IVp",
>           .iu_version = ((0x3eULL << 48) | (0x19ULL << 32) | (0x22ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -325,7 +325,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES | CPU_FEATURE_CMT,
>       },
>       {
> -        .name = "Sun UltraSparc IIIip",
> +        .name = "Sun-UltraSparc-IIIip",
>           .iu_version = ((0x3eULL << 48) | (0x22ULL << 32) | (0ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_3,
> @@ -334,7 +334,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Sun UltraSparc T1",
> +        .name = "Sun-UltraSparc-T1",
>           /* defined in sparc_ifu_fdp.v and ctu.h */
>           .iu_version = ((0x3eULL << 48) | (0x23ULL << 32) | (0x02ULL << 24)),
>           .fpu_version = 0x00000000,
> @@ -345,7 +345,7 @@ static const sparc_def_t sparc_defs[] = {
>           | CPU_FEATURE_GL,
>       },
>       {
> -        .name = "Sun UltraSparc T2",
> +        .name = "Sun-UltraSparc-T2",
>           /* defined in tlu_asi_ctl.v and n2_revid_cust.v */
>           .iu_version = ((0x3eULL << 48) | (0x24ULL << 32) | (0x02ULL << 24)),
>           .fpu_version = 0x00000000,
> @@ -356,7 +356,7 @@ static const sparc_def_t sparc_defs[] = {
>           | CPU_FEATURE_GL,
>       },
>       {
> -        .name = "NEC UltraSparc I",
> +        .name = "NEC-UltraSparc-I",
>           .iu_version = ((0x22ULL << 48) | (0x10ULL << 32) | (0x40ULL << 24)),
>           .fpu_version = 0x00000000,
>           .mmu_version = mmu_us_12,
> @@ -366,7 +366,7 @@ static const sparc_def_t sparc_defs[] = {
>       },
>   #else
>       {
> -        .name = "Fujitsu MB86904",
> +        .name = "Fujitsu-MB86904",
>           .iu_version = 0x04 << 24, /* Impl 0, ver 4 */
>           .fpu_version = 4 << FSR_VER_SHIFT, /* FPU version 4 (Meiko) */
>           .mmu_version = 0x04 << 24, /* Impl 0, ver 4 */
> @@ -379,7 +379,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "Fujitsu MB86907",
> +        .name = "Fujitsu-MB86907",
>           .iu_version = 0x05 << 24, /* Impl 0, ver 5 */
>           .fpu_version = 4 << FSR_VER_SHIFT, /* FPU version 4 (Meiko) */
>           .mmu_version = 0x05 << 24, /* Impl 0, ver 5 */
> @@ -392,7 +392,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI MicroSparc I",
> +        .name = "TI-MicroSparc-I",
>           .iu_version = 0x41000000,
>           .fpu_version = 4 << FSR_VER_SHIFT,
>           .mmu_version = 0x41000000,
> @@ -405,7 +405,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_FEATURE_MUL | CPU_FEATURE_DIV,
>       },
>       {
> -        .name = "TI MicroSparc II",
> +        .name = "TI-MicroSparc-II",
>           .iu_version = 0x42000000,
>           .fpu_version = 4 << FSR_VER_SHIFT,
>           .mmu_version = 0x02000000,
> @@ -418,7 +418,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI MicroSparc IIep",
> +        .name = "TI-MicroSparc-IIep",
>           .iu_version = 0x42000000,
>           .fpu_version = 4 << FSR_VER_SHIFT,
>           .mmu_version = 0x04000000,
> @@ -431,7 +431,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI SuperSparc 40", /* STP1020NPGA */
> +        .name = "TI-SuperSparc-40", /* STP1020NPGA */
>           .iu_version = 0x41000000, /* SuperSPARC 2.x */
>           .fpu_version = 0 << FSR_VER_SHIFT,
>           .mmu_version = 0x00000800, /* SuperSPARC 2.x, no MXCC */
> @@ -444,7 +444,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI SuperSparc 50", /* STP1020PGA */
> +        .name = "TI-SuperSparc-50", /* STP1020PGA */
>           .iu_version = 0x40000000, /* SuperSPARC 3.x */
>           .fpu_version = 0 << FSR_VER_SHIFT,
>           .mmu_version = 0x01000800, /* SuperSPARC 3.x, no MXCC */
> @@ -457,7 +457,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI SuperSparc 51",
> +        .name = "TI-SuperSparc-51",
>           .iu_version = 0x40000000, /* SuperSPARC 3.x */
>           .fpu_version = 0 << FSR_VER_SHIFT,
>           .mmu_version = 0x01000000, /* SuperSPARC 3.x, MXCC */
> @@ -471,7 +471,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI SuperSparc 60", /* STP1020APGA */
> +        .name = "TI-SuperSparc-60", /* STP1020APGA */
>           .iu_version = 0x40000000, /* SuperSPARC 3.x */
>           .fpu_version = 0 << FSR_VER_SHIFT,
>           .mmu_version = 0x01000800, /* SuperSPARC 3.x, no MXCC */
> @@ -484,7 +484,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI SuperSparc 61",
> +        .name = "TI-SuperSparc-61",
>           .iu_version = 0x44000000, /* SuperSPARC 3.x */
>           .fpu_version = 0 << FSR_VER_SHIFT,
>           .mmu_version = 0x01000000, /* SuperSPARC 3.x, MXCC */
> @@ -498,7 +498,7 @@ static const sparc_def_t sparc_defs[] = {
>           .features = CPU_DEFAULT_FEATURES,
>       },
>       {
> -        .name = "TI SuperSparc II",
> +        .name = "TI-SuperSparc-II",
>           .iu_version = 0x40000000, /* SuperSPARC II 1.x */
>           .fpu_version = 0 << FSR_VER_SHIFT,
>           .mmu_version = 0x08000000, /* SuperSPARC II 1.x, MXCC */

Thanks Thomas, this looks much better!

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH 0/5] Sparc CPU naming and help text improvements

[Mark Cave-Ayland]

On 15/04/2024 08:26, Thomas Huth wrote:

> On 07/03/2024 18.43, Thomas Huth wrote:
>> The Sparc CPU naming and the corresponding help text is somewhat
>> confusing for the users. We should avoid spaces in the Names and
>> provide clear information to the users what can be passed to the
>> "-cpu" option.
>> While we're at it, also remove the "+" from two of the CPU names
>> since this character is now not allowed in device names anymore
>> (and was worked around with an ugly hack in qom/object.c so far).
>>
>> Thomas Huth (5):
>>    target/sparc/cpu: Rename the CPU models with a "+" in their names
>>    target/sparc/cpu: Avoid spaces by default in the CPU names
>>    target/sparc/cpu: Improve the CPU help text
>>    docs/system/target-sparc: Improve the Sparc documentation
>>    docs/about: Deprecate the old "UltraSparc" CPU names that contain a
>>      "+"
> 
> Ping!
> 
> Mark, Aryom, could you please comment on this patch series, too?
> 
> Thanks,
>   Thomas

Done! I didn't realise that it was only patches 1 and 2 that were still outstanding 
until I tested the series, so I've replied to those separately.


ATB,

Mark.

Re: [PATCH 4/5] docs/system/target-sparc: Improve the Sparc documentation

[Mark Cave-Ayland]

On 07/03/2024 17:43, Thomas Huth wrote:

> Add some words about how to enable or disable boolean features,
> and remove the note about a Linux kernel being available on the
> QEMU website (they have been removed long ago already).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   docs/system/target-sparc.rst | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/system/target-sparc.rst b/docs/system/target-sparc.rst
> index 9ec8c90c14..9f418b9d3e 100644
> --- a/docs/system/target-sparc.rst
> +++ b/docs/system/target-sparc.rst
> @@ -27,6 +27,11 @@ architecture machines:
>   The emulation is somewhat complete. SMP up to 16 CPUs is supported, but
>   Linux limits the number of usable CPUs to 4.
>   
> +The list of available CPUs can be viewed by starting QEMU with ``-cpu help``.
> +Optional boolean features can be added with a "+" in front of the feature name,
> +or disabled with a "-" in front of the name, for example
> +``-cpu TI-SuperSparc-II,+float128``.
> +
>   QEMU emulates the following sun4m peripherals:
>   
>   -  IOMMU
> @@ -55,8 +60,7 @@ OpenBIOS is a free (GPL v2) portable firmware implementation. The goal
>   is to implement a 100% IEEE 1275-1994 (referred to as Open Firmware)
>   compliant firmware.
>   
> -A sample Linux 2.6 series kernel and ram disk image are available on the
> -QEMU web site. There are still issues with NetBSD and OpenBSD, but most
> +There are still issues with NetBSD and OpenBSD, but most
>   kernel versions work. Please note that currently older Solaris kernels
>   don't work probably due to interface issues between OpenBIOS and
>   Solaris.

Just curious as to what current issues exist with NetBSD and OpenBSD? At least both 
my NetBSD and OpenBSD test images survive a casual boot test here with latest git.


ATB,

Mark.

Re: [PATCH 5/5] docs/about: Deprecate the old "UltraSparc" CPU names that contain a "+"

[Mark Cave-Ayland]

On 07/03/2024 17:43, Thomas Huth wrote:

> For consistency we should drop the names with a "+" in it in the
> long run.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   docs/about/deprecated.rst | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 8565644da6..7058341f8f 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -202,6 +202,15 @@ in the QEMU object model anymore. ``power5+``, ``power5+_v2.1``,
>   an alias, but for consistency these will get removed in a future
>   release, too. Use ``power5p_v2.1`` and ``power7p_v2.1`` instead.
>   
> +``Sun-UltraSparc-IIIi+`` and ``Sun-UltraSparc-IV+`` CPU names (since 9.0)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The character "+" in device (and thus also CPU) names is not allowed
> +in the QEMU object model anymore. ``Sun-UltraSparc-IIIi+`` and
> +``Sun-UltraSparc-IV+`` are currently still supported via a workaround,
> +but for consistency these will get removed in a future release, too.
> +Use ``Sun-UltraSparc-IIIip`` and ``Sun-UltraSparc-IVp`` instead.
> +
>   CRIS CPU architecture (since 9.0)
>   '''''''''''''''''''''''''''''''''

See my previous comment about the CPU names, otherwise:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH 0/5] Sparc CPU naming and help text improvements

[Mark Cave-Ayland]

On 18/04/2024 21:08, Mark Cave-Ayland wrote:

> On 15/04/2024 08:26, Thomas Huth wrote:
> 
>> On 07/03/2024 18.43, Thomas Huth wrote:
>>> The Sparc CPU naming and the corresponding help text is somewhat
>>> confusing for the users. We should avoid spaces in the Names and
>>> provide clear information to the users what can be passed to the
>>> "-cpu" option.
>>> While we're at it, also remove the "+" from two of the CPU names
>>> since this character is now not allowed in device names anymore
>>> (and was worked around with an ugly hack in qom/object.c so far).
>>>
>>> Thomas Huth (5):
>>>    target/sparc/cpu: Rename the CPU models with a "+" in their names
>>>    target/sparc/cpu: Avoid spaces by default in the CPU names
>>>    target/sparc/cpu: Improve the CPU help text
>>>    docs/system/target-sparc: Improve the Sparc documentation
>>>    docs/about: Deprecate the old "UltraSparc" CPU names that contain a
>>>      "+"
>>
>> Ping!
>>
>> Mark, Aryom, could you please comment on this patch series, too?
>>
>> Thanks,
>>   Thomas
> 
> Done! I didn't realise that it was only patches 1 and 2 that were still outstanding 
> until I tested the series, so I've replied to those separately.

Oops actually that's not quite right: looks like my git-am failed on patch 3 because 
it was already applied, but the remaining 2 patches are still relevant. I've just 
replied to those too.


ATB,

Mark.

Re: [PATCH 4/5] docs/system/target-sparc: Improve the Sparc documentation

[Thomas Huth]

On 18/04/2024 22.27, Mark Cave-Ayland wrote:
> On 07/03/2024 17:43, Thomas Huth wrote:
> 
>> Add some words about how to enable or disable boolean features,
>> and remove the note about a Linux kernel being available on the
>> QEMU website (they have been removed long ago already).
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   docs/system/target-sparc.rst | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/system/target-sparc.rst b/docs/system/target-sparc.rst
>> index 9ec8c90c14..9f418b9d3e 100644
>> --- a/docs/system/target-sparc.rst
>> +++ b/docs/system/target-sparc.rst
>> @@ -27,6 +27,11 @@ architecture machines:
>>   The emulation is somewhat complete. SMP up to 16 CPUs is supported, but
>>   Linux limits the number of usable CPUs to 4.
>> +The list of available CPUs can be viewed by starting QEMU with ``-cpu 
>> help``.
>> +Optional boolean features can be added with a "+" in front of the feature 
>> name,
>> +or disabled with a "-" in front of the name, for example
>> +``-cpu TI-SuperSparc-II,+float128``.
>> +
>>   QEMU emulates the following sun4m peripherals:
>>   -  IOMMU
>> @@ -55,8 +60,7 @@ OpenBIOS is a free (GPL v2) portable firmware 
>> implementation. The goal
>>   is to implement a 100% IEEE 1275-1994 (referred to as Open Firmware)
>>   compliant firmware.
>> -A sample Linux 2.6 series kernel and ram disk image are available on the
>> -QEMU web site. There are still issues with NetBSD and OpenBSD, but most
>> +There are still issues with NetBSD and OpenBSD, but most
>>   kernel versions work. Please note that currently older Solaris kernels
>>   don't work probably due to interface issues between OpenBIOS and
>>   Solaris.
> 
> Just curious as to what current issues exist with NetBSD and OpenBSD? At 
> least both my NetBSD and OpenBSD test images survive a casual boot test here 
> with latest git.

Maybe it's also about a long fixed bug ... shall I remove that sentence 
while I'm at it?

  Thomas

Re: [PATCH 4/5] docs/system/target-sparc: Improve the Sparc documentation

[Mark Cave-Ayland]

On 19/04/2024 05:59, Thomas Huth wrote:

> On 18/04/2024 22.27, Mark Cave-Ayland wrote:
>> On 07/03/2024 17:43, Thomas Huth wrote:
>>
>>> Add some words about how to enable or disable boolean features,
>>> and remove the note about a Linux kernel being available on the
>>> QEMU website (they have been removed long ago already).
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   docs/system/target-sparc.rst | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/system/target-sparc.rst b/docs/system/target-sparc.rst
>>> index 9ec8c90c14..9f418b9d3e 100644
>>> --- a/docs/system/target-sparc.rst
>>> +++ b/docs/system/target-sparc.rst
>>> @@ -27,6 +27,11 @@ architecture machines:
>>>   The emulation is somewhat complete. SMP up to 16 CPUs is supported, but
>>>   Linux limits the number of usable CPUs to 4.
>>> +The list of available CPUs can be viewed by starting QEMU with ``-cpu help``.
>>> +Optional boolean features can be added with a "+" in front of the feature name,
>>> +or disabled with a "-" in front of the name, for example
>>> +``-cpu TI-SuperSparc-II,+float128``.
>>> +
>>>   QEMU emulates the following sun4m peripherals:
>>>   -  IOMMU
>>> @@ -55,8 +60,7 @@ OpenBIOS is a free (GPL v2) portable firmware implementation. 
>>> The goal
>>>   is to implement a 100% IEEE 1275-1994 (referred to as Open Firmware)
>>>   compliant firmware.
>>> -A sample Linux 2.6 series kernel and ram disk image are available on the
>>> -QEMU web site. There are still issues with NetBSD and OpenBSD, but most
>>> +There are still issues with NetBSD and OpenBSD, but most
>>>   kernel versions work. Please note that currently older Solaris kernels
>>>   don't work probably due to interface issues between OpenBIOS and
>>>   Solaris.
>>
>> Just curious as to what current issues exist with NetBSD and OpenBSD? At least both 
>> my NetBSD and OpenBSD test images survive a casual boot test here with latest git.
> 
> Maybe it's also about a long fixed bug ... shall I remove that sentence while I'm at it?

Yeah, I don't believe that has been true about NetBSD/OpenBSD for quite some time :) 
Please do keep the wording about Solaris since I get asked that often (and it's a 
task that will be quite difficult and time-consuming to do).


ATB,

Mark.

Re: [PATCH 4/5] docs/system/target-sparc: Improve the Sparc documentation

[Brad Smith]

On 2024-04-18 4:27 p.m., Mark Cave-Ayland wrote:
> On 07/03/2024 17:43, Thomas Huth wrote:
>
>> Add some words about how to enable or disable boolean features,
>> and remove the note about a Linux kernel being available on the
>> QEMU website (they have been removed long ago already).
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   docs/system/target-sparc.rst | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/system/target-sparc.rst b/docs/system/target-sparc.rst
>> index 9ec8c90c14..9f418b9d3e 100644
>> --- a/docs/system/target-sparc.rst
>> +++ b/docs/system/target-sparc.rst
>> @@ -27,6 +27,11 @@ architecture machines:
>>   The emulation is somewhat complete. SMP up to 16 CPUs is supported, 
>> but
>>   Linux limits the number of usable CPUs to 4.
>>   +The list of available CPUs can be viewed by starting QEMU with 
>> ``-cpu help``.
>> +Optional boolean features can be added with a "+" in front of the 
>> feature name,
>> +or disabled with a "-" in front of the name, for example
>> +``-cpu TI-SuperSparc-II,+float128``.
>> +
>>   QEMU emulates the following sun4m peripherals:
>>     -  IOMMU
>> @@ -55,8 +60,7 @@ OpenBIOS is a free (GPL v2) portable firmware 
>> implementation. The goal
>>   is to implement a 100% IEEE 1275-1994 (referred to as Open Firmware)
>>   compliant firmware.
>>   -A sample Linux 2.6 series kernel and ram disk image are available 
>> on the
>> -QEMU web site. There are still issues with NetBSD and OpenBSD, but most
>> +There are still issues with NetBSD and OpenBSD, but most
>>   kernel versions work. Please note that currently older Solaris kernels
>>   don't work probably due to interface issues between OpenBIOS and
>>   Solaris.
>
> Just curious as to what current issues exist with NetBSD and OpenBSD? 
> At least both my NetBSD and OpenBSD test images survive a casual boot 
> test here with latest git.

I was just trying OpenBSD/sparc64 with 8.2 recently and found hme(4) does
not work. I tried with the NE2k driver as I remember adding the driver 
to the
OpenBSD kernel before an hme driver existed and it sort of worked, but there
were still issues.

I'll re-test with 9 now and see what happens.

Re: [PATCH 4/5] docs/system/target-sparc: Improve the Sparc documentation

[Mark Cave-Ayland]

On 20/04/2024 00:14, Brad Smith wrote:

> On 2024-04-18 4:27 p.m., Mark Cave-Ayland wrote:
>> On 07/03/2024 17:43, Thomas Huth wrote:
>>
>>> Add some words about how to enable or disable boolean features,
>>> and remove the note about a Linux kernel being available on the
>>> QEMU website (they have been removed long ago already).
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   docs/system/target-sparc.rst | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/system/target-sparc.rst b/docs/system/target-sparc.rst
>>> index 9ec8c90c14..9f418b9d3e 100644
>>> --- a/docs/system/target-sparc.rst
>>> +++ b/docs/system/target-sparc.rst
>>> @@ -27,6 +27,11 @@ architecture machines:
>>>   The emulation is somewhat complete. SMP up to 16 CPUs is supported, but
>>>   Linux limits the number of usable CPUs to 4.
>>>   +The list of available CPUs can be viewed by starting QEMU with ``-cpu help``.
>>> +Optional boolean features can be added with a "+" in front of the feature name,
>>> +or disabled with a "-" in front of the name, for example
>>> +``-cpu TI-SuperSparc-II,+float128``.
>>> +
>>>   QEMU emulates the following sun4m peripherals:
>>>     -  IOMMU
>>> @@ -55,8 +60,7 @@ OpenBIOS is a free (GPL v2) portable firmware implementation. 
>>> The goal
>>>   is to implement a 100% IEEE 1275-1994 (referred to as Open Firmware)
>>>   compliant firmware.
>>>   -A sample Linux 2.6 series kernel and ram disk image are available on the
>>> -QEMU web site. There are still issues with NetBSD and OpenBSD, but most
>>> +There are still issues with NetBSD and OpenBSD, but most
>>>   kernel versions work. Please note that currently older Solaris kernels
>>>   don't work probably due to interface issues between OpenBIOS and
>>>   Solaris.
>>
>> Just curious as to what current issues exist with NetBSD and OpenBSD? At least both 
>> my NetBSD and OpenBSD test images survive a casual boot test here with latest git.
> 
> I was just trying OpenBSD/sparc64 with 8.2 recently and found hme(4) does
> not work. I tried with the NE2k driver as I remember adding the driver to the
> OpenBSD kernel before an hme driver existed and it sort of worked, but there
> were still issues.
> 
> I'll re-test with 9 now and see what happens.

Thanks for the update: my local tests for SPARC changes are boot tests, so it's 
entirely possible I could miss an issue with the hme device.

Feel free to open a GitLab issue with the relevant information and I'll take a look 
as time allows.


ATB,

Mark.