diff mbox series

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

Message ID 20240307174334.130407-2-thuth@redhat.com (mailing list archive)
State New, archived
Headers show
Series Sparc CPU naming and help text improvements | expand

Commit Message

Thomas Huth March 7, 2024, 5:43 p.m. UTC
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(-)

Comments

Richard Henderson March 7, 2024, 9:22 p.m. UTC | #1
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~
Thomas Huth March 8, 2024, 5:12 a.m. UTC | #2
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
Mark Cave-Ayland April 18, 2024, 8:03 p.m. UTC | #3
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.
diff mbox series

Patch

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;