[v1] softmmu: List CPU types again

[PATCH-for-7.0] softmmu: List CPU types again

Posted by Philippe Mathieu-Daudé 2 years, 1 month ago

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Commit e0220bb5b2 made cpus.c target-agnostic but didn't notice
the cpu_list() function is only defined in target-specific code
in "cpu.h". Extract list_cpus() from the generic cpus.c into a
new target-specific unit.

Fixes: e0220bb5b2 ("softmmu: Build target-agnostic objects once")
Reported-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/cpus.c      |  8 --------
 softmmu/cpus_list.c | 36 ++++++++++++++++++++++++++++++++++++
 softmmu/meson.build |  1 +
 3 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 softmmu/cpus_list.c

diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index e1d84c8ccb..7b75bb66d5 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -728,14 +728,6 @@ int vm_stop_force_state(RunState state)
     }
 }
 
-void list_cpus(const char *optarg)
-{
-    /* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list)
-    cpu_list();
-#endif
-}
-
 void qmp_memsave(int64_t addr, int64_t size, const char *filename,
                  bool has_cpu, int64_t cpu_index, Error **errp)
 {
diff --git a/softmmu/cpus_list.c b/softmmu/cpus_list.c
new file mode 100644
index 0000000000..59ef604423
--- /dev/null
+++ b/softmmu/cpus_list.c
@@ -0,0 +1,36 @@
+/*
+ * QEMU per target CPU list
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/cpus.h"
+#include "cpu.h"
+
+void list_cpus(const char *optarg)
+{
+    /* XXX: implement xxx_cpu_list for targets that still miss it */
+#if defined(cpu_list)
+    cpu_list();
+#endif
+}
+
diff --git a/softmmu/meson.build b/softmmu/meson.build
index 8138248661..1df0ad286a 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -1,5 +1,6 @@
 specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files(
   'arch_init.c',
+  'cpus_list.c',
   'ioport.c',
   'memory.c',
   'physmem.c',
-- 
2.34.1

Re: [PATCH-for-7.0] softmmu: List CPU types again

Posted by Max Filippov 2 years, 1 month ago

On Thu, Mar 10, 2022 at 3:55 AM Philippe Mathieu-Daudé
<philippe.mathieu.daude@gmail.com> wrote:
>
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Commit e0220bb5b2 made cpus.c target-agnostic but didn't notice
> the cpu_list() function is only defined in target-specific code
> in "cpu.h". Extract list_cpus() from the generic cpus.c into a
> new target-specific unit.
>
> Fixes: e0220bb5b2 ("softmmu: Build target-agnostic objects once")
> Reported-by: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  softmmu/cpus.c      |  8 --------
>  softmmu/cpus_list.c | 36 ++++++++++++++++++++++++++++++++++++
>  softmmu/meson.build |  1 +
>  3 files changed, 37 insertions(+), 8 deletions(-)
>  create mode 100644 softmmu/cpus_list.c

Tested-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

Re: [PATCH-for-7.0] softmmu: List CPU types again

Posted by Thomas Huth 2 years, 1 month ago

On 10/03/2022 12.55, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Commit e0220bb5b2 made cpus.c target-agnostic but didn't notice
> the cpu_list() function is only defined in target-specific code
> in "cpu.h". Extract list_cpus() from the generic cpus.c into a
> new target-specific unit.
> 
> Fixes: e0220bb5b2 ("softmmu: Build target-agnostic objects once")
> Reported-by: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   softmmu/cpus.c      |  8 --------
>   softmmu/cpus_list.c | 36 ++++++++++++++++++++++++++++++++++++
>   softmmu/meson.build |  1 +
>   3 files changed, 37 insertions(+), 8 deletions(-)
>   create mode 100644 softmmu/cpus_list.c
> 
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index e1d84c8ccb..7b75bb66d5 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -728,14 +728,6 @@ int vm_stop_force_state(RunState state)
>       }
>   }
>   
> -void list_cpus(const char *optarg)
> -{
> -    /* XXX: implement xxx_cpu_list for targets that still miss it */
> -#if defined(cpu_list)
> -    cpu_list();
> -#endif
> -}
> -
>   void qmp_memsave(int64_t addr, int64_t size, const char *filename,
>                    bool has_cpu, int64_t cpu_index, Error **errp)
>   {
> diff --git a/softmmu/cpus_list.c b/softmmu/cpus_list.c
> new file mode 100644
> index 0000000000..59ef604423
> --- /dev/null
> +++ b/softmmu/cpus_list.c

The other cpu*.c files in softmmu use a "-" instead of "_", so I'd also use 
a "-" for cpu-list.c.


> @@ -0,0 +1,36 @@
> +/*
> + * QEMU per target CPU list
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard

If you look at the history, Fabrice was hardly involved in this function, so 
I think it should be ok to drop this line in the new file.

> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/cpus.h"
> +#include "cpu.h"
> +
> +void list_cpus(const char *optarg)
> +{
> +    /* XXX: implement xxx_cpu_list for targets that still miss it */
> +#if defined(cpu_list)
> +    cpu_list();
> +#endif
> +}
> +

No empty new lines at the end of files, please.

  Thomas

Re: [PATCH-for-7.0] softmmu: List CPU types again

Posted by Paolo Bonzini 2 years, 1 month ago

On 3/10/22 13:23, Thomas Huth wrote:
> On 10/03/2022 12.55, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Commit e0220bb5b2 made cpus.c target-agnostic but didn't notice
>> the cpu_list() function is only defined in target-specific code
>> in "cpu.h". Extract list_cpus() from the generic cpus.c into a
>> new target-specific unit.

Let's just use cpu.c in the root, and move the prototype to 
include/exec/cpu-common.h.  The function can be used just fine by 
linux-user and bsd-user as well, it's not softmmu-specific.

Paolo

>> Fixes: e0220bb5b2 ("softmmu: Build target-agnostic objects once")
>> Reported-by: Max Filippov <jcmvbkbc@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   softmmu/cpus.c      |  8 --------
>>   softmmu/cpus_list.c | 36 ++++++++++++++++++++++++++++++++++++
>>   softmmu/meson.build |  1 +
>>   3 files changed, 37 insertions(+), 8 deletions(-)
>>   create mode 100644 softmmu/cpus_list.c
>>
>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>> index e1d84c8ccb..7b75bb66d5 100644
>> --- a/softmmu/cpus.c
>> +++ b/softmmu/cpus.c
>> @@ -728,14 +728,6 @@ int vm_stop_force_state(RunState state)
>>       }
>>   }
>> -void list_cpus(const char *optarg)
>> -{
>> -    /* XXX: implement xxx_cpu_list for targets that still miss it */
>> -#if defined(cpu_list)
>> -    cpu_list();
>> -#endif
>> -}
>> -
>>   void qmp_memsave(int64_t addr, int64_t size, const char *filename,
>>                    bool has_cpu, int64_t cpu_index, Error **errp)
>>   {
>> diff --git a/softmmu/cpus_list.c b/softmmu/cpus_list.c
>> new file mode 100644
>> index 0000000000..59ef604423
>> --- /dev/null
>> +++ b/softmmu/cpus_list.c
> 
> The other cpu*.c files in softmmu use a "-" instead of "_", so I'd also 
> use a "-" for cpu-list.c.
> 
> 
>> @@ -0,0 +1,36 @@
>> +/*
>> + * QEMU per target CPU list
>> + *
>> + * Copyright (c) 2003-2008 Fabrice Bellard
> 
> If you look at the history, Fabrice was hardly involved in this 
> function, so I think it should be ok to drop this line in the new file.
> 
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a copy
>> + * of this software and associated documentation files (the 
>> "Software"), to deal
>> + * in the Software without restriction, including without limitation 
>> the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, 
>> and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be 
>> included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "sysemu/cpus.h"
>> +#include "cpu.h"
>> +
>> +void list_cpus(const char *optarg)
>> +{
>> +    /* XXX: implement xxx_cpu_list for targets that still miss it */
>> +#if defined(cpu_list)
>> +    cpu_list();
>> +#endif
>> +}
>> +
> 
> No empty new lines at the end of files, please.
> 
>   Thomas
> 
>

Re: [PATCH-for-7.0] softmmu: List CPU types again

Posted by Philippe Mathieu-Daudé 2 years, 1 month ago

On 10/3/22 13:23, Thomas Huth wrote:
> On 10/03/2022 12.55, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Commit e0220bb5b2 made cpus.c target-agnostic but didn't notice
>> the cpu_list() function is only defined in target-specific code
>> in "cpu.h". Extract list_cpus() from the generic cpus.c into a
>> new target-specific unit.
>>
>> Fixes: e0220bb5b2 ("softmmu: Build target-agnostic objects once")
>> Reported-by: Max Filippov <jcmvbkbc@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   softmmu/cpus.c      |  8 --------
>>   softmmu/cpus_list.c | 36 ++++++++++++++++++++++++++++++++++++
>>   softmmu/meson.build |  1 +
>>   3 files changed, 37 insertions(+), 8 deletions(-)
>>   create mode 100644 softmmu/cpus_list.c
>>
>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>> index e1d84c8ccb..7b75bb66d5 100644
>> --- a/softmmu/cpus.c
>> +++ b/softmmu/cpus.c
>> @@ -728,14 +728,6 @@ int vm_stop_force_state(RunState state)
>>       }
>>   }
>> -void list_cpus(const char *optarg)
>> -{
>> -    /* XXX: implement xxx_cpu_list for targets that still miss it */
>> -#if defined(cpu_list)
>> -    cpu_list();
>> -#endif
>> -}
>> -
>>   void qmp_memsave(int64_t addr, int64_t size, const char *filename,
>>                    bool has_cpu, int64_t cpu_index, Error **errp)
>>   {
>> diff --git a/softmmu/cpus_list.c b/softmmu/cpus_list.c
>> new file mode 100644
>> index 0000000000..59ef604423
>> --- /dev/null
>> +++ b/softmmu/cpus_list.c
> 
> The other cpu*.c files in softmmu use a "-" instead of "_", so I'd also 
> use a "-" for cpu-list.c.

I guess I used '_' copy/pasting from arch_init.c:

  specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files(
    'arch_init.c',
+  'cpus_list.c',
    'ioport.c',

I don't mind changing, but it would be better to standardize that on
a project scope / level.