| Message ID | 20240919061128.769139-4-zhao1.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce Hybrid CPU Topology via Custom Topology Tree | expand |
On Thu, 19 Sep 2024 14:11:19 +0800 Zhao Liu <zhao1.liu@intel.com> wrote: > Custom topology will allow user to build CPU topology from CLI totally, > and this replaces machine's default CPU creation process (*_init_cpus() > in MachineClass.init()). > > For the machine's initialization, there may be CPU dependencies in the > remaining initialization after the CPU creation. > > To address such dependencies, create the CPU topology device (including > CPU devices) from the CLI earlier, so that the latter part of machine > initialization can be separated after qemu_add_cli_devices_early(). > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> Other than question of type of category from previous patch this looks fine to me. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> However, needs review from others more familiar with this code! > --- > system/vl.c | 55 +++++++++++++++++++++++++++++++++++------------------ > 1 file changed, 36 insertions(+), 19 deletions(-) > > diff --git a/system/vl.c b/system/vl.c > index c40364e2f091..8540454aa1c2 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -1211,8 +1211,9 @@ static int device_help_func(void *opaque, QemuOpts *opts, Error **errp) > static int device_init_func(void *opaque, QemuOpts *opts, Error **errp) > { > DeviceState *dev; > + long *category = opaque; > > - dev = qdev_device_add(opts, NULL, errp); > + dev = qdev_device_add(opts, category, errp); > if (!dev && *errp) { > error_report_err(*errp); > return -1; > @@ -2623,6 +2624,36 @@ static void qemu_init_displays(void) > } > } > > +static void qemu_add_devices(long *category) > +{ > + DeviceOption *opt; > + > + qemu_opts_foreach(qemu_find_opts("device"), > + device_init_func, category, &error_fatal); > + QTAILQ_FOREACH(opt, &device_opts, next) { > + DeviceState *dev; > + loc_push_restore(&opt->loc); > + /* > + * TODO Eventually we should call qmp_device_add() here to make sure it > + * behaves the same, but QMP still has to accept incorrectly typed > + * options until libvirt is fixed and we want to be strict on the CLI > + * from the start, so call qdev_device_add_from_qdict() directly for > + * now. > + */ > + dev = qdev_device_add_from_qdict(opt->opts, category, > + true, &error_fatal); > + object_unref(OBJECT(dev)); > + loc_pop(&opt->loc); > + } > +} > + > +static void qemu_add_cli_devices_early(void) > +{ > + long category = DEVICE_CATEGORY_CPU_DEF; > + > + qemu_add_devices(&category); > +} > + > static void qemu_init_board(void) > { > /* process plugin before CPUs are created, but once -smp has been parsed */ > @@ -2631,6 +2662,9 @@ static void qemu_init_board(void) > /* From here on we enter MACHINE_PHASE_INITIALIZED. */ > machine_run_board_init(current_machine, mem_path, &error_fatal); > > + /* Create CPU topology device if any. */ > + qemu_add_cli_devices_early(); > + > drive_check_orphaned(); > > realtime_init(); > @@ -2638,8 +2672,6 @@ static void qemu_init_board(void) > > static void qemu_create_cli_devices(void) > { > - DeviceOption *opt; > - > soundhw_init(); > > qemu_opts_foreach(qemu_find_opts("fw_cfg"), > @@ -2653,22 +2685,7 @@ static void qemu_create_cli_devices(void) > > /* init generic devices */ > rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE); > - qemu_opts_foreach(qemu_find_opts("device"), > - device_init_func, NULL, &error_fatal); > - QTAILQ_FOREACH(opt, &device_opts, next) { > - DeviceState *dev; > - loc_push_restore(&opt->loc); > - /* > - * TODO Eventually we should call qmp_device_add() here to make sure it > - * behaves the same, but QMP still has to accept incorrectly typed > - * options until libvirt is fixed and we want to be strict on the CLI > - * from the start, so call qdev_device_add_from_qdict() directly for > - * now. > - */ > - dev = qdev_device_add_from_qdict(opt->opts, NULL, true, &error_fatal); > - object_unref(OBJECT(dev)); > - loc_pop(&opt->loc); > - } > + qemu_add_devices(NULL); > rom_reset_order_override(); > } >
> + > +static void qemu_add_cli_devices_early(void) > +{ > + long category = DEVICE_CATEGORY_CPU_DEF; > + > + qemu_add_devices(&category); > +} > + > static void qemu_init_board(void) > { > /* process plugin before CPUs are created, but once -smp has been parsed */ > @@ -2631,6 +2662,9 @@ static void qemu_init_board(void) > /* From here on we enter MACHINE_PHASE_INITIALIZED. */ > machine_run_board_init(current_machine, mem_path, &error_fatal); > > + /* Create CPU topology device if any. */ > + qemu_add_cli_devices_early(); I wonder if this is too generic a name? There are various other things we might want to do early. Maybe qemu_add_cli_cpu_def() > + > drive_check_orphaned(); > > realtime_init(); > @@ -2638,8 +2672,6 @@ static void qemu_init_board(void) >
On Tue, Oct 08, 2024 at 10:55:45AM +0100, Jonathan Cameron wrote: > Date: Tue, 8 Oct 2024 10:55:45 +0100 > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Subject: Re: [RFC v2 03/12] system/vl: Create CPU topology devices from CLI > early > X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) > > > > + > > +static void qemu_add_cli_devices_early(void) > > +{ > > + long category = DEVICE_CATEGORY_CPU_DEF; > > + > > + qemu_add_devices(&category); > > +} > > + > > static void qemu_init_board(void) > > { > > /* process plugin before CPUs are created, but once -smp has been parsed */ > > @@ -2631,6 +2662,9 @@ static void qemu_init_board(void) > > /* From here on we enter MACHINE_PHASE_INITIALIZED. */ > > machine_run_board_init(current_machine, mem_path, &error_fatal); > > > > + /* Create CPU topology device if any. */ > > + qemu_add_cli_devices_early(); > I wonder if this is too generic a name? > > There are various other things we might want to do early. > Maybe qemu_add_cli_cpu_def() Sure, it makes sense.
On Tue, Oct 08, 2024 at 10:50:53AM +0100, Jonathan Cameron wrote: > Date: Tue, 8 Oct 2024 10:50:53 +0100 > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Subject: Re: [RFC v2 03/12] system/vl: Create CPU topology devices from CLI > early > X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) > > On Thu, 19 Sep 2024 14:11:19 +0800 > Zhao Liu <zhao1.liu@intel.com> wrote: > > > Custom topology will allow user to build CPU topology from CLI totally, > > and this replaces machine's default CPU creation process (*_init_cpus() > > in MachineClass.init()). > > > > For the machine's initialization, there may be CPU dependencies in the > > remaining initialization after the CPU creation. > > > > To address such dependencies, create the CPU topology device (including > > CPU devices) from the CLI earlier, so that the latter part of machine > > initialization can be separated after qemu_add_cli_devices_early(). > > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > Other than question of type of category from previous patch this looks > fine to me. > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > However, needs review from others more familiar with this code! Thanks! -Zhao
diff --git a/system/vl.c b/system/vl.c index c40364e2f091..8540454aa1c2 100644 --- a/system/vl.c +++ b/system/vl.c @@ -1211,8 +1211,9 @@ static int device_help_func(void *opaque, QemuOpts *opts, Error **errp) static int device_init_func(void *opaque, QemuOpts *opts, Error **errp) { DeviceState *dev; + long *category = opaque; - dev = qdev_device_add(opts, NULL, errp); + dev = qdev_device_add(opts, category, errp); if (!dev && *errp) { error_report_err(*errp); return -1; @@ -2623,6 +2624,36 @@ static void qemu_init_displays(void) } } +static void qemu_add_devices(long *category) +{ + DeviceOption *opt; + + qemu_opts_foreach(qemu_find_opts("device"), + device_init_func, category, &error_fatal); + QTAILQ_FOREACH(opt, &device_opts, next) { + DeviceState *dev; + loc_push_restore(&opt->loc); + /* + * TODO Eventually we should call qmp_device_add() here to make sure it + * behaves the same, but QMP still has to accept incorrectly typed + * options until libvirt is fixed and we want to be strict on the CLI + * from the start, so call qdev_device_add_from_qdict() directly for + * now. + */ + dev = qdev_device_add_from_qdict(opt->opts, category, + true, &error_fatal); + object_unref(OBJECT(dev)); + loc_pop(&opt->loc); + } +} + +static void qemu_add_cli_devices_early(void) +{ + long category = DEVICE_CATEGORY_CPU_DEF; + + qemu_add_devices(&category); +} + static void qemu_init_board(void) { /* process plugin before CPUs are created, but once -smp has been parsed */ @@ -2631,6 +2662,9 @@ static void qemu_init_board(void) /* From here on we enter MACHINE_PHASE_INITIALIZED. */ machine_run_board_init(current_machine, mem_path, &error_fatal); + /* Create CPU topology device if any. */ + qemu_add_cli_devices_early(); + drive_check_orphaned(); realtime_init(); @@ -2638,8 +2672,6 @@ static void qemu_init_board(void) static void qemu_create_cli_devices(void) { - DeviceOption *opt; - soundhw_init(); qemu_opts_foreach(qemu_find_opts("fw_cfg"), @@ -2653,22 +2685,7 @@ static void qemu_create_cli_devices(void) /* init generic devices */ rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE); - qemu_opts_foreach(qemu_find_opts("device"), - device_init_func, NULL, &error_fatal); - QTAILQ_FOREACH(opt, &device_opts, next) { - DeviceState *dev; - loc_push_restore(&opt->loc); - /* - * TODO Eventually we should call qmp_device_add() here to make sure it - * behaves the same, but QMP still has to accept incorrectly typed - * options until libvirt is fixed and we want to be strict on the CLI - * from the start, so call qdev_device_add_from_qdict() directly for - * now. - */ - dev = qdev_device_add_from_qdict(opt->opts, NULL, true, &error_fatal); - object_unref(OBJECT(dev)); - loc_pop(&opt->loc); - } + qemu_add_devices(NULL); rom_reset_order_override(); }
Custom topology will allow user to build CPU topology from CLI totally, and this replaces machine's default CPU creation process (*_init_cpus() in MachineClass.init()). For the machine's initialization, there may be CPU dependencies in the remaining initialization after the CPU creation. To address such dependencies, create the CPU topology device (including CPU devices) from the CLI earlier, so that the latter part of machine initialization can be separated after qemu_add_cli_devices_early(). Signed-off-by: Zhao Liu <zhao1.liu@intel.com> --- system/vl.c | 55 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 19 deletions(-)