1. Patchew
  2. QEMU
  3. [PATCH v5 00/31] Unified CPU type check
  4. View patch

[PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class

Gavin Shan posted 31 patches 1 year ago
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Beniamino Galvani <b.galvani@gmail.com>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Niek Linnenbank <nieklinnenbank@gmail.com>, Radoslaw Biernacki <rad@semihalf.com>, Leif Lindholm <quic_llindhol@quicinc.com>, Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Laurent Vivier <laurent@vivier.eu>, Vijai Kumar K <vijai@behindbytes.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Brian Cain <bcain@quicinc.com>, Song Gao <gaosong@loongson.cn>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Huacai Chen <chenhuacai@kernel.org>, Stafford Horne <shorne@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, Yoshinori Sato <ysato@users.sourceforge.jp>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
There is a newer version of this series
[PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class
Posted by Gavin Shan 1 year ago 
'ev67' CPU class will be returned to match everything, which makes
no sense as mentioned in the comments. Remove the logic to fall
back to 'ev67' CPU class to match everything.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 target/alpha/cpu.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 39cf841b3e..91fe8ae095 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -141,11 +141,8 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf(ALPHA_CPU_TYPE_NAME("%s"), cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-
-    /* TODO: remove match everything nonsense */
-    if (!oc || object_class_is_abstract(oc)) {
-        /* Default to ev67; no reason not to emulate insns by default. */
-        oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
+    if (!oc || !object_class_dynamic_cast(oc, TYPE_ALPHA_CPU)) {
+        return NULL;
     }
 
     return oc;
-- 
2.41.0
Re: [PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class
Posted by Philippe Mathieu-Daudé 10 months, 3 weeks ago 
On 15/11/23 00:55, Gavin Shan wrote:
> 'ev67' CPU class will be returned to match everything, which makes
> no sense as mentioned in the comments. Remove the logic to fall
> back to 'ev67' CPU class to match everything.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   target/alpha/cpu.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 39cf841b3e..91fe8ae095 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -141,11 +141,8 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
>       typename = g_strdup_printf(ALPHA_CPU_TYPE_NAME("%s"), cpu_model);
>       oc = object_class_by_name(typename);
>       g_free(typename);
> -
> -    /* TODO: remove match everything nonsense */
> -    if (!oc || object_class_is_abstract(oc)) {
> -        /* Default to ev67; no reason not to emulate insns by default. */
> -        oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
> +    if (!oc || !object_class_dynamic_cast(oc, TYPE_ALPHA_CPU)) {
> +        return NULL;
>       }

This breaks linux-user:

qemu-alpha: unable to find CPU model 'any'
Re: [PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class
Posted by Philippe Mathieu-Daudé 10 months, 3 weeks ago 
On 4/1/24 18:58, Philippe Mathieu-Daudé wrote:
> On 15/11/23 00:55, Gavin Shan wrote:
>> 'ev67' CPU class will be returned to match everything, which makes
>> no sense as mentioned in the comments. Remove the logic to fall
>> back to 'ev67' CPU class to match everything.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   target/alpha/cpu.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
>> index 39cf841b3e..91fe8ae095 100644
>> --- a/target/alpha/cpu.c
>> +++ b/target/alpha/cpu.c
>> @@ -141,11 +141,8 @@ static ObjectClass *alpha_cpu_class_by_name(const 
>> char *cpu_model)
>>       typename = g_strdup_printf(ALPHA_CPU_TYPE_NAME("%s"), cpu_model);
>>       oc = object_class_by_name(typename);
>>       g_free(typename);
>> -
>> -    /* TODO: remove match everything nonsense */
>> -    if (!oc || object_class_is_abstract(oc)) {
>> -        /* Default to ev67; no reason not to emulate insns by 
>> default. */
>> -        oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
>> +    if (!oc || !object_class_dynamic_cast(oc, TYPE_ALPHA_CPU)) {
>> +        return NULL;
>>       }
> 
> This breaks linux-user:
> 
> qemu-alpha: unable to find CPU model 'any'

Thread 1 "qemu-alpha" hit Breakpoint 1, alpha_cpu_class_by_name 
(cpu_model=0x5555557202a0 "any") at target/alpha/cpu.c:123
123	{
(gdb) bt
#0  alpha_cpu_class_by_name (cpu_model=0x5555557202a0 "any") at 
target/alpha/cpu.c:123
#1  0x0000555555583956 in cpu_class_by_name 
(typename=typename@entry=0x555555674af8 "alpha-cpu", 
cpu_model=0x5555557202a0 "any")
     at hw/core/cpu-common.c:156
#2  0x00005555555904a1 in parse_cpu_option (cpu_option=<optimized out>) 
at cpu-target.c:257
#3  0x00005555555822e9 in main (argc=2, argv=0x7fffffffe318, 
envp=<optimized out>) at linux-user/main.c:784

in main():

  781     if (cpu_model == NULL) {
  782         cpu_model = cpu_get_model(get_elf_eflags(execfd));
  783     }
  784     cpu_type = parse_cpu_option(cpu_model);

Having:

$ cat linux-user/alpha/target_elf.h
...
#ifndef ALPHA_TARGET_ELF_H
#define ALPHA_TARGET_ELF_H
static inline const char *cpu_get_model(uint32_t eflags)
{
     return "any";
}
#endif
Re: [PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class
Posted by Philippe Mathieu-Daudé 10 months, 3 weeks ago 
On 4/1/24 19:03, Philippe Mathieu-Daudé wrote:
> On 4/1/24 18:58, Philippe Mathieu-Daudé wrote:
>> On 15/11/23 00:55, Gavin Shan wrote:
>>> 'ev67' CPU class will be returned to match everything, which makes
>>> no sense as mentioned in the comments. Remove the logic to fall
>>> back to 'ev67' CPU class to match everything.
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>>   target/alpha/cpu.c | 7 ++-----
>>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
>>> index 39cf841b3e..91fe8ae095 100644
>>> --- a/target/alpha/cpu.c
>>> +++ b/target/alpha/cpu.c
>>> @@ -141,11 +141,8 @@ static ObjectClass 
>>> *alpha_cpu_class_by_name(const char *cpu_model)
>>>       typename = g_strdup_printf(ALPHA_CPU_TYPE_NAME("%s"), cpu_model);
>>>       oc = object_class_by_name(typename);
>>>       g_free(typename);
>>> -
>>> -    /* TODO: remove match everything nonsense */
>>> -    if (!oc || object_class_is_abstract(oc)) {
>>> -        /* Default to ev67; no reason not to emulate insns by 
>>> default. */
>>> -        oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
>>> +    if (!oc || !object_class_dynamic_cast(oc, TYPE_ALPHA_CPU)) {
>>> +        return NULL;
>>>       }
>>
>> This breaks linux-user:
>>
>> qemu-alpha: unable to find CPU model 'any'
> 
> Thread 1 "qemu-alpha" hit Breakpoint 1, alpha_cpu_class_by_name 
> (cpu_model=0x5555557202a0 "any") at target/alpha/cpu.c:123
> 123    {
> (gdb) bt
> #0  alpha_cpu_class_by_name (cpu_model=0x5555557202a0 "any") at 
> target/alpha/cpu.c:123
> #1  0x0000555555583956 in cpu_class_by_name 
> (typename=typename@entry=0x555555674af8 "alpha-cpu", 
> cpu_model=0x5555557202a0 "any")
>      at hw/core/cpu-common.c:156
> #2  0x00005555555904a1 in parse_cpu_option (cpu_option=<optimized out>) 
> at cpu-target.c:257
> #3  0x00005555555822e9 in main (argc=2, argv=0x7fffffffe318, 
> envp=<optimized out>) at linux-user/main.c:784
> 
> in main():
> 
>   781     if (cpu_model == NULL) {
>   782         cpu_model = cpu_get_model(get_elf_eflags(execfd));
>   783     }
>   784     cpu_type = parse_cpu_option(cpu_model);
> 
> Having:
> 
> $ cat linux-user/alpha/target_elf.h
> ...
> #ifndef ALPHA_TARGET_ELF_H
> #define ALPHA_TARGET_ELF_H
> static inline const char *cpu_get_model(uint32_t eflags)
> {
>      return "any";
> }
> #endif

Laurent, Richard, are you OK with this fix?

-- >8 --
diff --git a/linux-user/alpha/target_elf.h b/linux-user/alpha/target_elf.h
index 344e9f4d39..b77d638f6d 100644
--- a/linux-user/alpha/target_elf.h
+++ b/linux-user/alpha/target_elf.h
@@ -9,6 +9,6 @@
  #define ALPHA_TARGET_ELF_H
  static inline const char *cpu_get_model(uint32_t eflags)
  {
-    return "any";
+    return "ev67";
  }
  #endif
---
Re: [PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class
Posted by Richard Henderson 1 year ago 
On 11/14/23 15:55, Gavin Shan wrote:
> 'ev67' CPU class will be returned to match everything, which makes
> no sense as mentioned in the comments. Remove the logic to fall
> back to 'ev67' CPU class to match everything.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   target/alpha/cpu.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)

The subject is wrong -- ev67 cpu class is still present.
Better as

   target/alpha: Remove fallback to ev67 cpu class

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


r~

> 
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 39cf841b3e..91fe8ae095 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -141,11 +141,8 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
>       typename = g_strdup_printf(ALPHA_CPU_TYPE_NAME("%s"), cpu_model);
>       oc = object_class_by_name(typename);
>       g_free(typename);
> -
> -    /* TODO: remove match everything nonsense */
> -    if (!oc || object_class_is_abstract(oc)) {
> -        /* Default to ev67; no reason not to emulate insns by default. */
> -        oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
> +    if (!oc || !object_class_dynamic_cast(oc, TYPE_ALPHA_CPU)) {
> +        return NULL;
>       }
>   
>       return oc;
Re: [PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class
Posted by Philippe Mathieu-Daudé 1 year ago 
On 15/11/23 01:22, Richard Henderson wrote:
> On 11/14/23 15:55, Gavin Shan wrote:
>> 'ev67' CPU class will be returned to match everything, which makes
>> no sense as mentioned in the comments. Remove the logic to fall
>> back to 'ev67' CPU class to match everything.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   target/alpha/cpu.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> The subject is wrong -- ev67 cpu class is still present.
> Better as
> 
>    target/alpha: Remove fallback to ev67 cpu class
> 
> with that,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.