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 <pet...@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.
--
Eduardo
