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
>