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

2023-08-27 Thread 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=-  (default: 0)
   addr=   - Slot and optional function number, example: 06.0 or 
06 (default: -1)
   autonegotiation= - on/off (default: true)
   bootindex=
   extra_mac_registers= - on/off (default: true)
   failover_pair_id=
"""

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=-  (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=   -  (default: true)
  Zfa= -  (default: true)
  Zfh= -  (default: false)
  Zfhmin=  -  (default: false)
  Zicsr=   -  (default: true)
  Zifencei=    -  (default: true)
  Zihintpause= -  (default: true)
  Zve32f=  -  (default: false)
  Zve64d=  -  (default: false)
  Zve64f=  -  (default: false)
  a=   - Atomic instructions
  c=   - Compressed instructions
  cbom_blocksize= -  (default: 64)
  cboz_blocksize= -  (default: 64)
  d=   - Double-precision float point

  ...

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

...

memory=>

...

start-powered-off=

...

  v=   - Vector operations
  vext_spec=

...

  zba= -  (default: true)
  zbb= -  (default: true)
  zbc= -  (default: true)
  zbkb=    -  (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 
---
  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 = _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 

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

2023-08-25 Thread 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=-  (default: 0)
>   addr=   - Slot and optional function number, example: 06.0 
> or 06 (default: -1)
>   autonegotiation= - on/off (default: true)
>   bootindex=
>   extra_mac_registers= - on/off (default: true)
>   failover_pair_id=
> """
> 
> 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=-  (default: 64)

which would also be helpful.

Thanks,
drew

> 
> Signed-off-by: LIU Zhiwei 
> ---
>  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 = _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(_runtime_opts);
>  qemu_add_opts(_chardev_opts);
>  qemu_add_opts(_device_opts);
> +qemu_add_opts(_cpu_opts);
>  qemu_add_opts(_netdev_opts);
>  qemu_add_opts(_nic_opts);
>  qemu_add_opts(_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
> 



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

2023-08-25 Thread 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=-  (default: 0)
  addr=   - Slot and optional function number, example: 06.0 or 
06 (default: -1)
  autonegotiation= - on/off (default: true)
  bootindex=
  extra_mac_registers= - on/off (default: true)
  failover_pair_id=
"""

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 
---
 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 = _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(_runtime_opts);
 qemu_add_opts(_chardev_opts);
 qemu_add_opts(_device_opts);
+qemu_add_opts(_cpu_opts);
 qemu_add_opts(_netdev_opts);
 qemu_add_opts(_nic_opts);
 qemu_add_opts(_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