Re: [RFC PATCH 3/3] softmmu/vl: Add qemu_cpu_opts QemuOptsList
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
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
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