[RFC PATCH 0/3] Add API for list cpu extensions

[RFC PATCH 0/3] Add API for list cpu extensions

[LIU Zhiwei]

Some times we want to know what is the really mean of one cpu option.
For example, in RISC-V, we usually specify a cpu in this way:
-cpu rv64,v=on

If we don't look into the source code, we can't get the ISA extensions
of this -cpu command line.

In this patch set, we add one list_cpu_props API for common cores. It
will output the enabled ISA extensions.

In the near future, I will also list all possible user configurable
options and all possible extensions for this cpu.

In order to reuse the options parse code, I also add a QemuOptsList
for cpu.


After this patch, we can output the extensions for cpu,
"""
 ./qemu-system-riscv64 -cpu rv64,help
    Enable extension:
            rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zba_zbb_zbc_zbs_sstc_svadu
"""

Notice currently this patch is only working for RISC-V system mode.

Thanks Andrew Jones for your suggestion!

Todo:
1) Output all possible user configurable options and all extensions.
2) Add support for RISC-V linux-user mode
3) Add support for other archs


LIU Zhiwei (3):
  cpu: Add new API cpu_type_by_name
  target/riscv: Add API list_cpu_props
  softmmu/vl: Add qemu_cpu_opts QemuOptsList

 cpu.c                     | 39 +++++++++++++++++++++++++++------------
 include/exec/cpu-common.h |  1 +
 include/hw/core/cpu.h     | 11 +++++++++++
 softmmu/vl.c              | 35 +++++++++++++++++++++++++++++++++++
 target/riscv/cpu.c        | 10 ++++++++++
 target/riscv/cpu.h        |  2 ++
 6 files changed, 86 insertions(+), 12 deletions(-)

-- 
2.17.1

[RFC PATCH 1/3] cpu: Add new API cpu_type_by_name

[LIU Zhiwei]

cpu_type_by_name is used to get the cpu type name from the command
line -cpu.

Currently it is only used by parse_cpu_option. In the next patch, it
will be used by other cpu query functions.

Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 cpu.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/cpu.c b/cpu.c
index 1c948d1161..e1a9239d0f 100644
--- a/cpu.c
+++ b/cpu.c
@@ -257,28 +257,35 @@ void cpu_exec_initfn(CPUState *cpu)
 #endif
 }
 
-const char *parse_cpu_option(const char *cpu_option)
+static const char *cpu_type_by_name(const char *cpu_model)
 {
     ObjectClass *oc;
-    CPUClass *cc;
-    gchar **model_pieces;
     const char *cpu_type;
 
-    model_pieces = g_strsplit(cpu_option, ",", 2);
-    if (!model_pieces[0]) {
-        error_report("-cpu option cannot be empty");
-        exit(1);
-    }
 
-    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
+    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, cpu_model);
     if (oc == NULL) {
-        error_report("unable to find CPU model '%s'", model_pieces[0]);
-        g_strfreev(model_pieces);
+        error_report("unable to find CPU model '%s'", cpu_model);
         exit(EXIT_FAILURE);
     }
 
     cpu_type = object_class_get_name(oc);
-    cc = CPU_CLASS(oc);
+    return cpu_type;
+}
+
+const char *parse_cpu_option(const char *cpu_option)
+{
+    const char *cpu_type;
+    CPUClass *cc;
+    gchar **model_pieces;
+
+    model_pieces = g_strsplit(cpu_option, ",", 2);
+    if (!model_pieces[0]) {
+        error_report("-cpu option cannot be empty");
+        exit(1);
+    }
+    cpu_type = cpu_type_by_name(model_pieces[0]);
+    cc = CPU_CLASS(object_class_by_name(cpu_type));
     cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
     g_strfreev(model_pieces);
     return cpu_type;
-- 
2.17.1

[RFC PATCH 2/3] target/riscv: Add API list_cpu_props

[LIU Zhiwei]

This API used for output current configuration for one specified CPU.
Currently only RISC-V frontend implements this API.

Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 cpu.c                     |  8 ++++++++
 include/exec/cpu-common.h |  1 +
 target/riscv/cpu.c        | 10 ++++++++++
 target/riscv/cpu.h        |  2 ++
 4 files changed, 21 insertions(+)

diff --git a/cpu.c b/cpu.c
index e1a9239d0f..03a313cd72 100644
--- a/cpu.c
+++ b/cpu.c
@@ -299,6 +299,14 @@ void list_cpus(void)
 #endif
 }
 
+void list_cpu_props(CPUState *cs)
+{
+    /* XXX: implement xxx_cpu_list_props for targets that still miss it */
+#if defined(cpu_list_props)
+    cpu_list_props(cs);
+#endif
+}
+
 #if defined(CONFIG_USER_ONLY)
 void tb_invalidate_phys_addr(hwaddr addr)
 {
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 87dc9a752c..b3160d9218 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -166,5 +166,6 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
 
 /* vl.c */
 void list_cpus(void);
+void list_cpu_props(CPUState *);
 
 #endif /* CPU_COMMON_H */
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6b93b04453..3ea18de06f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2226,6 +2226,16 @@ void riscv_cpu_list(void)
     g_slist_free(list);
 }
 
+void riscv_cpu_list_props(CPUState *cs)
+{
+    char *enabled_isa;
+
+    enabled_isa = riscv_isa_string(RISCV_CPU(cs));
+    qemu_printf("Enable extension:\n");
+    qemu_printf("\t%s\n", enabled_isa);
+    /* TODO: output all user configurable options and all possible extensions */
+}
+
 #define DEFINE_CPU(type_name, initfn)      \
     {                                      \
         .name = type_name,                 \
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6ea22e0eea..af1d47605b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -443,9 +443,11 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                         bool probe, uintptr_t retaddr);
 char *riscv_isa_string(RISCVCPU *cpu);
 void riscv_cpu_list(void);
+void riscv_cpu_list_props(CPUState *cs);
 void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp);
 
 #define cpu_list riscv_cpu_list
+#define cpu_list_props riscv_cpu_list_props
 #define cpu_mmu_index riscv_cpu_mmu_index
 
 #ifndef CONFIG_USER_ONLY
-- 
2.17.1

[RFC PATCH 3/3] softmmu/vl: Add qemu_cpu_opts QemuOptsList

[LIU Zhiwei]

This make the cpu works the similar way like the -device option.

For device option,
"""
./qemu-system-riscv64 -device e1000,help
e1000 options:
  acpi-index=<uint32>    -  (default: 0)
  addr=<int32>           - Slot and optional function number, example: 06.0 or 06 (default: -1)
  autonegotiation=<bool> - on/off (default: true)
  bootindex=<int32>
  extra_mac_registers=<bool> - on/off (default: true)
  failover_pair_id=<str>
"""

After this patch, the cpu can output its configurations,
"""
./qemu-system-riscv64 -cpu rv64,help
Enable extension:
	rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zba_zbb_zbc_zbs_sstc_svadu
"""

Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 cpu.c                 |  2 +-
 include/hw/core/cpu.h | 11 +++++++++++
 softmmu/vl.c          | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/cpu.c b/cpu.c
index 03a313cd72..712bd02684 100644
--- a/cpu.c
+++ b/cpu.c
@@ -257,7 +257,7 @@ void cpu_exec_initfn(CPUState *cpu)
 #endif
 }
 
-static const char *cpu_type_by_name(const char *cpu_model)
+const char *cpu_type_by_name(const char *cpu_model)
 {
     ObjectClass *oc;
     const char *cpu_type;
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index fdcbe87352..49d41afdfa 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -657,6 +657,17 @@ CPUState *cpu_create(const char *typename);
  */
 const char *parse_cpu_option(const char *cpu_option);
 
+/**
+ * cpu_type_by_name:
+ * @cpu_model: The -cpu command line model name.
+ *
+ * Looks up type name by the -cpu command line model name
+ *
+ * Returns: type name of CPU or prints error and terminates process
+ *          if an error occurred.
+ */
+const char *cpu_type_by_name(const char *cpu_model);
+
 /**
  * cpu_has_work:
  * @cpu: The vCPU to check.
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..bc30f3954d 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -218,6 +218,15 @@ static struct {
     { .driver = "virtio-vga-gl",        .flag = &default_vga       },
 };
 
+static QemuOptsList qemu_cpu_opts = {
+    .name = "cpu",
+    .implied_opt_name = "cpu_model",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
+    .desc = {
+        { /* end of list */ }
+    },
+};
+
 static QemuOptsList qemu_rtc_opts = {
     .name = "rtc",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_opts.head),
@@ -1140,6 +1149,21 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+static int cpu_help_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+    const char *cpu_model, *cpu_type;
+    cpu_model = qemu_opt_get(opts, "cpu_model");
+    if (!cpu_model) {
+        return 1;
+    }
+    if (!qemu_opt_has_help_opt(opts)) {
+        return 0;
+    }
+    cpu_type = cpu_type_by_name(cpu_model);
+    list_cpu_props((CPUState *)object_new(cpu_type));
+    return 1;
+}
+
 static int device_help_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     return qdev_device_help(opts);
@@ -2467,6 +2491,11 @@ static void qemu_process_help_options(void)
         exit(0);
     }
 
+    if (qemu_opts_foreach(qemu_find_opts("cpu"),
+                          cpu_help_func, NULL, NULL)) {
+        exit(0);
+    }
+
     if (qemu_opts_foreach(qemu_find_opts("device"),
                           device_help_func, NULL, NULL)) {
         exit(0);
@@ -2680,6 +2709,7 @@ void qemu_init(int argc, char **argv)
     qemu_add_drive_opts(&bdrv_runtime_opts);
     qemu_add_opts(&qemu_chardev_opts);
     qemu_add_opts(&qemu_device_opts);
+    qemu_add_opts(&qemu_cpu_opts);
     qemu_add_opts(&qemu_netdev_opts);
     qemu_add_opts(&qemu_nic_opts);
     qemu_add_opts(&qemu_net_opts);
@@ -2756,6 +2786,11 @@ void qemu_init(int argc, char **argv)
             case QEMU_OPTION_cpu:
                 /* hw initialization will check this */
                 cpu_option = optarg;
+                opts = qemu_opts_parse_noisily(qemu_find_opts("cpu"),
+                                               optarg, true);
+                if (!opts) {
+                    exit(1);
+                }
                 break;
             case QEMU_OPTION_hda:
             case QEMU_OPTION_hdb:
-- 
2.17.1

Re: [RFC PATCH 2/3] target/riscv: Add API list_cpu_props

[Daniel Henrique Barboza]

On 8/25/23 09:16, LIU Zhiwei wrote:
> This API used for output current configuration for one specified CPU.
> Currently only RISC-V frontend implements this API.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>   cpu.c                     |  8 ++++++++
>   include/exec/cpu-common.h |  1 +
>   target/riscv/cpu.c        | 10 ++++++++++
>   target/riscv/cpu.h        |  2 ++
>   4 files changed, 21 insertions(+)
> 
> diff --git a/cpu.c b/cpu.c
> index e1a9239d0f..03a313cd72 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -299,6 +299,14 @@ void list_cpus(void)
>   #endif
>   }
>   
> +void list_cpu_props(CPUState *cs)
> +{
> +    /* XXX: implement xxx_cpu_list_props for targets that still miss it */
> +#if defined(cpu_list_props)
> +    cpu_list_props(cs);
> +#endif
> +}
> +
>   #if defined(CONFIG_USER_ONLY)
>   void tb_invalidate_phys_addr(hwaddr addr)
>   {
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 87dc9a752c..b3160d9218 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -166,5 +166,6 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>   
>   /* vl.c */
>   void list_cpus(void);
> +void list_cpu_props(CPUState *);
>   
>   #endif /* CPU_COMMON_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6b93b04453..3ea18de06f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2226,6 +2226,16 @@ void riscv_cpu_list(void)
>       g_slist_free(list);
>   }
>   
> +void riscv_cpu_list_props(CPUState *cs)
> +{
> +    char *enabled_isa;
> +
> +    enabled_isa = riscv_isa_string(RISCV_CPU(cs));
> +    qemu_printf("Enable extension:\n");

I suggest "Enabled extensions". LGTM otherwise.

Daniel

> +    qemu_printf("\t%s\n", enabled_isa);
> +    /* TODO: output all user configurable options and all possible extensions */
> +}
> +
>   #define DEFINE_CPU(type_name, initfn)      \
>       {                                      \
>           .name = type_name,                 \
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 6ea22e0eea..af1d47605b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -443,9 +443,11 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>                           bool probe, uintptr_t retaddr);
>   char *riscv_isa_string(RISCVCPU *cpu);
>   void riscv_cpu_list(void);
> +void riscv_cpu_list_props(CPUState *cs);
>   void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp);
>   
>   #define cpu_list riscv_cpu_list
> +#define cpu_list_props riscv_cpu_list_props
>   #define cpu_mmu_index riscv_cpu_mmu_index
>   
>   #ifndef CONFIG_USER_ONLY

Re: [RFC PATCH 0/3] Add API for list cpu extensions

[Daniel Henrique Barboza]

Hi Zhiwei! I have two observations:

- this API doesn't play well with KVM as is. In a KVM environment, asking for the
enabled extensions of the 'host' CPU returns:

$ ./mnt/qemu/bin/qemu-system-riscv64 -cpu host,help
Enable extension:
	rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintntl_zihintpause_zawrs_zfa_zba_zbb_zbc_zbs_sstc_svadu

This is the same set of extensions enabled in the 'rv64' CPU for TCG. This is
happening because they're sharing the same code that creates properties.

If I apply these patches on top of the "split TCG/KVM accelerators from cpu.c" I
sent earlier, this happens:

$ ./mnt/qemu/bin/qemu-system-riscv64 -cpu host,help
Enable extension:
	rv64

For TCG only CPUs (vendor CPUs) the API works even on a KVM host, regardless of
applying on top of riscv-to-apply.next or those accel patches:

$ ./mnt/qemu/bin/qemu-system-riscv64 -cpu veyron-v1,help
Enable extension:
	rv64ch_zicbom_zicboz_zicsr_zifencei_zba_zbb_zbc_zbs_smaia_smstateen_ssaia_sscofpmf_sstc_svinval_svnapot_svpbmt_xventanacondops

It seems to me that 'cpu help' doesn't engage the KVM driver accel_init() function.
If we decide to go ahead with this API we'll need to either figure out if accel-specific
initialization is possible. If not, we should declare that this API works only for TCG.


- I think the presence of the 'cpu help' API limits the command line parsing altogether,
making cheeky things like this possible:


(disabling extensions in the cmd line and asking the extensions)
$ ./build/qemu-system-riscv64 -cpu veyron-v1,icbom=false,icboz=false,help
Enable extension:
	rv64ch_zicbom_zicboz_zicsr_zifencei_zba_zbb_zbc_zbs_smaia_smstateen_ssaia_sscofpmf_sstc_svinval_svnapot_svpbmt_xventanacondops


(silly option ignored)
$ ./build/qemu-system-riscv64 -cpu veyron-v1,lalala=true,help
Enable extension:
	rv64ch_zicbom_zicboz_zicsr_zifencei_zba_zbb_zbc_zbs_smaia_smstateen_ssaia_sscofpmf_sstc_svinval_svnapot_svpbmt_xventanacondops


This is not a gamebreaker but something to keep in mind when using this API. Thanks,


Daniel



On 8/25/23 09:16, LIU Zhiwei wrote:
> Some times we want to know what is the really mean of one cpu option.
> For example, in RISC-V, we usually specify a cpu in this way:
> -cpu rv64,v=on
> 
> If we don't look into the source code, we can't get the ISA extensions
> of this -cpu command line.
> 
> In this patch set, we add one list_cpu_props API for common cores. It
> will output the enabled ISA extensions.
> 
> In the near future, I will also list all possible user configurable
> options and all possible extensions for this cpu.
> 
> In order to reuse the options parse code, I also add a QemuOptsList
> for cpu.
> 
> 
> After this patch, we can output the extensions for cpu,
> """
>   ./qemu-system-riscv64 -cpu rv64,help
>      Enable extension:
>              rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zba_zbb_zbc_zbs_sstc_svadu
> """
> 
> Notice currently this patch is only working for RISC-V system mode.
> 
> Thanks Andrew Jones for your suggestion!
> 
> Todo:
> 1) Output all possible user configurable options and all extensions.
> 2) Add support for RISC-V linux-user mode
> 3) Add support for other archs
> 
> 
> LIU Zhiwei (3):
>    cpu: Add new API cpu_type_by_name
>    target/riscv: Add API list_cpu_props
>    softmmu/vl: Add qemu_cpu_opts QemuOptsList
> 
>   cpu.c                     | 39 +++++++++++++++++++++++++++------------
>   include/exec/cpu-common.h |  1 +
>   include/hw/core/cpu.h     | 11 +++++++++++
>   softmmu/vl.c              | 35 +++++++++++++++++++++++++++++++++++
>   target/riscv/cpu.c        | 10 ++++++++++
>   target/riscv/cpu.h        |  2 ++
>   6 files changed, 86 insertions(+), 12 deletions(-)
> 

Re: [RFC PATCH 3/3] softmmu/vl: Add qemu_cpu_opts QemuOptsList

[Andrew Jones]

On Fri, Aug 25, 2023 at 08:16:51PM +0800, LIU Zhiwei wrote:
> This make the cpu works the similar way like the -device option.
> 
> For device option,
> """
> ./qemu-system-riscv64 -device e1000,help
> e1000 options:
>   acpi-index=<uint32>    -  (default: 0)
>   addr=<int32>           - Slot and optional function number, example: 06.0 or 06 (default: -1)
>   autonegotiation=<bool> - on/off (default: true)
>   bootindex=<int32>
>   extra_mac_registers=<bool> - on/off (default: true)
>   failover_pair_id=<str>
> """
> 
> After this patch, the cpu can output its configurations,
> """
> ./qemu-system-riscv64 -cpu rv64,help
> Enable extension:
> 	rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zba_zbb_zbc_zbs_sstc_svadu
> """

I recommend we make it more similar to -device and list the properties
(not just extensions). Besides a listing being easier to read than the
isa string format, listing properties would also output, e.g.

 cbom_blocksize=<uint16>    -  (default: 64)

which would also be helpful.

Thanks,
drew

> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>  cpu.c                 |  2 +-
>  include/hw/core/cpu.h | 11 +++++++++++
>  softmmu/vl.c          | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/cpu.c b/cpu.c
> index 03a313cd72..712bd02684 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -257,7 +257,7 @@ void cpu_exec_initfn(CPUState *cpu)
>  #endif
>  }
>  
> -static const char *cpu_type_by_name(const char *cpu_model)
> +const char *cpu_type_by_name(const char *cpu_model)
>  {
>      ObjectClass *oc;
>      const char *cpu_type;
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index fdcbe87352..49d41afdfa 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -657,6 +657,17 @@ CPUState *cpu_create(const char *typename);
>   */
>  const char *parse_cpu_option(const char *cpu_option);
>  
> +/**
> + * cpu_type_by_name:
> + * @cpu_model: The -cpu command line model name.
> + *
> + * Looks up type name by the -cpu command line model name
> + *
> + * Returns: type name of CPU or prints error and terminates process
> + *          if an error occurred.
> + */
> +const char *cpu_type_by_name(const char *cpu_model);
> +
>  /**
>   * cpu_has_work:
>   * @cpu: The vCPU to check.
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index b0b96f67fa..bc30f3954d 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -218,6 +218,15 @@ static struct {
>      { .driver = "virtio-vga-gl",        .flag = &default_vga       },
>  };
>  
> +static QemuOptsList qemu_cpu_opts = {
> +    .name = "cpu",
> +    .implied_opt_name = "cpu_model",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
> +    .desc = {
> +        { /* end of list */ }
> +    },
> +};
> +
>  static QemuOptsList qemu_rtc_opts = {
>      .name = "rtc",
>      .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_opts.head),
> @@ -1140,6 +1149,21 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
>      return 0;
>  }
>  
> +static int cpu_help_func(void *opaque, QemuOpts *opts, Error **errp)
> +{
> +    const char *cpu_model, *cpu_type;
> +    cpu_model = qemu_opt_get(opts, "cpu_model");
> +    if (!cpu_model) {
> +        return 1;
> +    }
> +    if (!qemu_opt_has_help_opt(opts)) {
> +        return 0;
> +    }
> +    cpu_type = cpu_type_by_name(cpu_model);
> +    list_cpu_props((CPUState *)object_new(cpu_type));
> +    return 1;
> +}
> +
>  static int device_help_func(void *opaque, QemuOpts *opts, Error **errp)
>  {
>      return qdev_device_help(opts);
> @@ -2467,6 +2491,11 @@ static void qemu_process_help_options(void)
>          exit(0);
>      }
>  
> +    if (qemu_opts_foreach(qemu_find_opts("cpu"),
> +                          cpu_help_func, NULL, NULL)) {
> +        exit(0);
> +    }
> +
>      if (qemu_opts_foreach(qemu_find_opts("device"),
>                            device_help_func, NULL, NULL)) {
>          exit(0);
> @@ -2680,6 +2709,7 @@ void qemu_init(int argc, char **argv)
>      qemu_add_drive_opts(&bdrv_runtime_opts);
>      qemu_add_opts(&qemu_chardev_opts);
>      qemu_add_opts(&qemu_device_opts);
> +    qemu_add_opts(&qemu_cpu_opts);
>      qemu_add_opts(&qemu_netdev_opts);
>      qemu_add_opts(&qemu_nic_opts);
>      qemu_add_opts(&qemu_net_opts);
> @@ -2756,6 +2786,11 @@ void qemu_init(int argc, char **argv)
>              case QEMU_OPTION_cpu:
>                  /* hw initialization will check this */
>                  cpu_option = optarg;
> +                opts = qemu_opts_parse_noisily(qemu_find_opts("cpu"),
> +                                               optarg, true);
> +                if (!opts) {
> +                    exit(1);
> +                }
>                  break;
>              case QEMU_OPTION_hda:
>              case QEMU_OPTION_hdb:
> -- 
> 2.17.1
> 

Re: [RFC PATCH 3/3] softmmu/vl: Add qemu_cpu_opts QemuOptsList

[LIU Zhiwei]

Hi Drew

On 2023/8/25 23:58, Andrew Jones wrote:
> On Fri, Aug 25, 2023 at 08:16:51PM +0800, LIU Zhiwei wrote:
>> This make the cpu works the similar way like the -device option.
>>
>> For device option,
>> """
>> ./qemu-system-riscv64 -device e1000,help
>> e1000 options:
>>    acpi-index=<uint32>    -  (default: 0)
>>    addr=<int32>           - Slot and optional function number, example: 06.0 or 06 (default: -1)
>>    autonegotiation=<bool> - on/off (default: true)
>>    bootindex=<int32>
>>    extra_mac_registers=<bool> - on/off (default: true)
>>    failover_pair_id=<str>
>> """
>>
>> After this patch, the cpu can output its configurations,
>> """
>> ./qemu-system-riscv64 -cpu rv64,help
>> Enable extension:
>> 	rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zba_zbb_zbc_zbs_sstc_svadu
>> """
> I recommend we make it more similar to -device and list the properties
> (not just extensions). Besides a listing being easier to read than the
> isa string format, listing properties would also output, e.g.
>
>   cbom_blocksize=<uint16>    -  (default: 64)
>
> which would also be helpful.

I agree that we should add more outputs in cpu_list_props to aid the 
understanding of the isa string output. And let users know what they 
should explicitly added the -cpu command line.

I will refer to the -device option output. However, The -device option 
is not enough for cpu model.

"""

qemu-system-riscv64 -device rv64-riscv-cpu,zba=false,help

rv64-riscv-cpu options:
   Zawrs=<bool>           -  (default: true)
   Zfa=<bool>             -  (default: true)
   Zfh=<bool>             -  (default: false)
   Zfhmin=<bool>          -  (default: false)
   Zicsr=<bool>           -  (default: true)
   Zifencei=<bool>        -  (default: true)
   Zihintpause=<bool>     -  (default: true)
   Zve32f=<bool>          -  (default: false)
   Zve64d=<bool>          -  (default: false)
   Zve64f=<bool>          -  (default: false)
   a=<bool>               - Atomic instructions
   c=<bool>               - Compressed instructions
   cbom_blocksize=<uint16> -  (default: 64)
   cboz_blocksize=<uint16> -  (default: 64)
   d=<bool>               - Double-precision float point

   ...

  unnamed-gpio-in[0]=<child<irq>>
   unnamed-gpio-in[10]=<child<irq>>
   unnamed-gpio-in[11]=<child<irq>>
   unnamed-gpio-in[12]=<child<irq>>
   unnamed-gpio-in[13]=<child<irq>>
   unnamed-gpio-in[14]=<child<irq>>

...

memory=<link<memory-region>>

...

start-powered-off=<bool>

...

   v=<bool>               - Vector operations
   vext_spec=<str>

...

   zba=<bool>             -  (default: true)
   zbb=<bool>             -  (default: true)
   zbc=<bool>             -  (default: true)
   zbkb=<bool>            -  (default: false)

...

"""

1) IMHO, unnamed-gpio-in and start-powered-off exposing to users is 
meaningless.

2) Option like v and vext_spec doesn't output the defalut value.

3) The zba=false  in command line can't reflect  in the output.

That is the reason  why I want to add a new API cpu_list_props.

Thanks,
Zhwei

>
> Thanks,
> drew
>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>>   cpu.c                 |  2 +-
>>   include/hw/core/cpu.h | 11 +++++++++++
>>   softmmu/vl.c          | 35 +++++++++++++++++++++++++++++++++++
>>   3 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/cpu.c b/cpu.c
>> index 03a313cd72..712bd02684 100644
>> --- a/cpu.c
>> +++ b/cpu.c
>> @@ -257,7 +257,7 @@ void cpu_exec_initfn(CPUState *cpu)
>>   #endif
>>   }
>>   
>> -static const char *cpu_type_by_name(const char *cpu_model)
>> +const char *cpu_type_by_name(const char *cpu_model)
>>   {
>>       ObjectClass *oc;
>>       const char *cpu_type;
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index fdcbe87352..49d41afdfa 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -657,6 +657,17 @@ CPUState *cpu_create(const char *typename);
>>    */
>>   const char *parse_cpu_option(const char *cpu_option);
>>   
>> +/**
>> + * cpu_type_by_name:
>> + * @cpu_model: The -cpu command line model name.
>> + *
>> + * Looks up type name by the -cpu command line model name
>> + *
>> + * Returns: type name of CPU or prints error and terminates process
>> + *          if an error occurred.
>> + */
>> +const char *cpu_type_by_name(const char *cpu_model);
>> +
>>   /**
>>    * cpu_has_work:
>>    * @cpu: The vCPU to check.
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index b0b96f67fa..bc30f3954d 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -218,6 +218,15 @@ static struct {
>>       { .driver = "virtio-vga-gl",        .flag = &default_vga       },
>>   };
>>   
>> +static QemuOptsList qemu_cpu_opts = {
>> +    .name = "cpu",
>> +    .implied_opt_name = "cpu_model",
>> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
>> +    .desc = {
>> +        { /* end of list */ }
>> +    },
>> +};
>> +
>>   static QemuOptsList qemu_rtc_opts = {
>>       .name = "rtc",
>>       .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_opts.head),
>> @@ -1140,6 +1149,21 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
>>       return 0;
>>   }
>>   
>> +static int cpu_help_func(void *opaque, QemuOpts *opts, Error **errp)
>> +{
>> +    const char *cpu_model, *cpu_type;
>> +    cpu_model = qemu_opt_get(opts, "cpu_model");
>> +    if (!cpu_model) {
>> +        return 1;
>> +    }
>> +    if (!qemu_opt_has_help_opt(opts)) {
>> +        return 0;
>> +    }
>> +    cpu_type = cpu_type_by_name(cpu_model);
>> +    list_cpu_props((CPUState *)object_new(cpu_type));
>> +    return 1;
>> +}
>> +
>>   static int device_help_func(void *opaque, QemuOpts *opts, Error **errp)
>>   {
>>       return qdev_device_help(opts);
>> @@ -2467,6 +2491,11 @@ static void qemu_process_help_options(void)
>>           exit(0);
>>       }
>>   
>> +    if (qemu_opts_foreach(qemu_find_opts("cpu"),
>> +                          cpu_help_func, NULL, NULL)) {
>> +        exit(0);
>> +    }
>> +
>>       if (qemu_opts_foreach(qemu_find_opts("device"),
>>                             device_help_func, NULL, NULL)) {
>>           exit(0);
>> @@ -2680,6 +2709,7 @@ void qemu_init(int argc, char **argv)
>>       qemu_add_drive_opts(&bdrv_runtime_opts);
>>       qemu_add_opts(&qemu_chardev_opts);
>>       qemu_add_opts(&qemu_device_opts);
>> +    qemu_add_opts(&qemu_cpu_opts);
>>       qemu_add_opts(&qemu_netdev_opts);
>>       qemu_add_opts(&qemu_nic_opts);
>>       qemu_add_opts(&qemu_net_opts);
>> @@ -2756,6 +2786,11 @@ void qemu_init(int argc, char **argv)
>>               case QEMU_OPTION_cpu:
>>                   /* hw initialization will check this */
>>                   cpu_option = optarg;
>> +                opts = qemu_opts_parse_noisily(qemu_find_opts("cpu"),
>> +                                               optarg, true);
>> +                if (!opts) {
>> +                    exit(1);
>> +                }
>>                   break;
>>               case QEMU_OPTION_hda:
>>               case QEMU_OPTION_hdb:
>> -- 
>> 2.17.1
>>

Re: [RFC PATCH 2/3] target/riscv: Add API list_cpu_props

[LIU Zhiwei]

On 2023/8/25 21:46, Daniel Henrique Barboza wrote:
>
>
> On 8/25/23 09:16, LIU Zhiwei wrote:
>> This API used for output current configuration for one specified CPU.
>> Currently only RISC-V frontend implements this API.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>>   cpu.c                     |  8 ++++++++
>>   include/exec/cpu-common.h |  1 +
>>   target/riscv/cpu.c        | 10 ++++++++++
>>   target/riscv/cpu.h        |  2 ++
>>   4 files changed, 21 insertions(+)
>>
>> diff --git a/cpu.c b/cpu.c
>> index e1a9239d0f..03a313cd72 100644
>> --- a/cpu.c
>> +++ b/cpu.c
>> @@ -299,6 +299,14 @@ void list_cpus(void)
>>   #endif
>>   }
>>   +void list_cpu_props(CPUState *cs)
>> +{
>> +    /* XXX: implement xxx_cpu_list_props for targets that still miss 
>> it */
>> +#if defined(cpu_list_props)
>> +    cpu_list_props(cs);
>> +#endif
>> +}
>> +
>>   #if defined(CONFIG_USER_ONLY)
>>   void tb_invalidate_phys_addr(hwaddr addr)
>>   {
>> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 87dc9a752c..b3160d9218 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -166,5 +166,6 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>>     /* vl.c */
>>   void list_cpus(void);
>> +void list_cpu_props(CPUState *);
>>     #endif /* CPU_COMMON_H */
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 6b93b04453..3ea18de06f 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -2226,6 +2226,16 @@ void riscv_cpu_list(void)
>>       g_slist_free(list);
>>   }
>>   +void riscv_cpu_list_props(CPUState *cs)
>> +{
>> +    char *enabled_isa;
>> +
>> +    enabled_isa = riscv_isa_string(RISCV_CPU(cs));
>> +    qemu_printf("Enable extension:\n");
>
> I suggest "Enabled extensions". LGTM otherwise.

Fixed, thanks.

Zhiwei

>
> Daniel
>
>> +    qemu_printf("\t%s\n", enabled_isa);
>> +    /* TODO: output all user configurable options and all possible 
>> extensions */
>> +}
>> +
>>   #define DEFINE_CPU(type_name, initfn)      \
>>       {                                      \
>>           .name = type_name,                 \
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 6ea22e0eea..af1d47605b 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -443,9 +443,11 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr 
>> address, int size,
>>                           bool probe, uintptr_t retaddr);
>>   char *riscv_isa_string(RISCVCPU *cpu);
>>   void riscv_cpu_list(void);
>> +void riscv_cpu_list_props(CPUState *cs);
>>   void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp);
>>     #define cpu_list riscv_cpu_list
>> +#define cpu_list_props riscv_cpu_list_props
>>   #define cpu_mmu_index riscv_cpu_mmu_index
>>     #ifndef CONFIG_USER_ONLY

Re: [RFC PATCH 1/3] cpu: Add new API cpu_type_by_name

[Philippe Mathieu-Daudé]

On 25/8/23 14:16, LIU Zhiwei wrote:
> cpu_type_by_name is used to get the cpu type name from the command
> line -cpu.
> 
> Currently it is only used by parse_cpu_option. In the next patch, it
> will be used by other cpu query functions.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>   cpu.c | 31 +++++++++++++++++++------------
>   1 file changed, 19 insertions(+), 12 deletions(-)

Alternative patch subject:
"cpu: Extract cpu_type_by_name() from parse_cpu_option()"

Otherwise,

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