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 <th...@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-ayl...@ilande.co.uk>


ATB,

Mark.