| Message ID | 1497876588-947-5-git-send-email-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 19, 2017 at 08:49:39PM +0800, Peter Xu wrote: > Introduce this new field for the accelerator instances so that each > specific accelerator in the future can register its own global > properties to be used further by the system. It works just like how the > old machine compatible properties do, but only tailored for > accelerators. > > Use the newly exported register_compat_prop() to pass the accelerator > global properties to the global_props list. > > Signed-off-by: Peter Xu <peterx@redhat.com> As noted on other patches, I believe this should be AccelClass::global_props (initialized statically in class_init) instead of AccelState::global_props (built at runtime). Otherwise, I don't see the benefit of the new field.
On Mon, Jun 19, 2017 at 01:17:39PM -0300, Eduardo Habkost wrote: > On Mon, Jun 19, 2017 at 08:49:39PM +0800, Peter Xu wrote: > > Introduce this new field for the accelerator instances so that each > > specific accelerator in the future can register its own global > > properties to be used further by the system. It works just like how the > > old machine compatible properties do, but only tailored for > > accelerators. > > > > Use the newly exported register_compat_prop() to pass the accelerator > > global properties to the global_props list. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > As noted on other patches, I believe this should be > AccelClass::global_props (initialized statically in class_init) > instead of AccelState::global_props (built at runtime). > Otherwise, I don't see the benefit of the new field. The reason is that there is property that can only be defined after user specified all the parameters (when kvm irqchip is enabled). See: static void kvm_register_accel_props(KVMState *kvm) { AccelState *accel = ACCEL(kvm); AccelPropValue *entry; for (entry = x86_kvm_default_props; entry->prop; entry++) { accel_register_x86_cpu_props(accel, entry->prop, entry->value); } if (!kvm_irqchip_in_kernel()) { accel_register_x86_cpu_props(accel, "x2apic", "off"); } } So the list is not really static when class init. Thanks,
On Tue, Jun 20, 2017 at 09:20:36PM +0800, Peter Xu wrote: > On Mon, Jun 19, 2017 at 01:17:39PM -0300, Eduardo Habkost wrote: > > On Mon, Jun 19, 2017 at 08:49:39PM +0800, Peter Xu wrote: > > > Introduce this new field for the accelerator instances so that each > > > specific accelerator in the future can register its own global > > > properties to be used further by the system. It works just like how the > > > old machine compatible properties do, but only tailored for > > > accelerators. > > > > > > Use the newly exported register_compat_prop() to pass the accelerator > > > global properties to the global_props list. > > > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > > > As noted on other patches, I believe this should be > > AccelClass::global_props (initialized statically in class_init) > > instead of AccelState::global_props (built at runtime). > > Otherwise, I don't see the benefit of the new field. > > The reason is that there is property that can only be defined after > user specified all the parameters (when kvm irqchip is enabled). See: > > static void kvm_register_accel_props(KVMState *kvm) > { > AccelState *accel = ACCEL(kvm); > AccelPropValue *entry; > > for (entry = x86_kvm_default_props; entry->prop; entry++) { > accel_register_x86_cpu_props(accel, entry->prop, entry->value); > } > > if (!kvm_irqchip_in_kernel()) { > accel_register_x86_cpu_props(accel, "x2apic", "off"); > } > } > > So the list is not really static when class init. Thanks, I suggest leaving x2apic for later, and address the ones that are static first. I'm not sure a dynamic list on AccelState is the right solution for x2apic. I still wish to find a way to represent the x2apic/svm/kvm-pv-eoi rules using static data.
diff --git a/accel.c b/accel.c index 62edd29..f747d65 100644 --- a/accel.c +++ b/accel.c @@ -130,6 +130,18 @@ void configure_accelerator(MachineState *ms) } } +static void accel_prop_pass_to_global(gpointer data, gpointer user_data) +{ + GlobalProperty *prop = data; + + register_compat_prop(prop->driver, prop->property, + prop->value, false); +} + +void accel_register_compat_props(AccelState *accel) +{ + g_list_foreach(accel->global_props, accel_prop_pass_to_global, NULL); +} static void tcg_accel_class_init(ObjectClass *oc, void *data) { diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h index 15944c1..83bb60e 100644 --- a/include/sysemu/accel.h +++ b/include/sysemu/accel.h @@ -28,6 +28,15 @@ typedef struct AccelState { /*< private >*/ Object parent_obj; + + /*< public >*/ + /* + * Global properties that would be applied when specific + * accelerator is chosen. It works just like + * MachineState.compat_props but it's for accelerators not + * machines. + */ + GList *global_props; } AccelState; typedef struct AccelClass { @@ -57,5 +66,7 @@ typedef struct AccelClass { extern int tcg_tb_size; void configure_accelerator(MachineState *ms); +/* Register accelerator specific global properties */ +void accel_register_compat_props(AccelState *accel); #endif diff --git a/vl.c b/vl.c index c58ffc0..d3f5594 100644 --- a/vl.c +++ b/vl.c @@ -4553,6 +4553,7 @@ int main(int argc, char **argv, char **envp) exit (i == 1 ? 1 : 0); } + accel_register_compat_props(current_machine->accelerator); machine_register_compat_props(current_machine); qemu_opts_foreach(qemu_find_opts("global"),
Introduce this new field for the accelerator instances so that each specific accelerator in the future can register its own global properties to be used further by the system. It works just like how the old machine compatible properties do, but only tailored for accelerators. Use the newly exported register_compat_prop() to pass the accelerator global properties to the global_props list. Signed-off-by: Peter Xu <peterx@redhat.com> --- accel.c | 12 ++++++++++++ include/sysemu/accel.h | 11 +++++++++++ vl.c | 1 + 3 files changed, 24 insertions(+)