On Tue, Jun 20, 2017 at 11:07:34AM -0300, Eduardo Habkost wrote:
> On Tue, Jun 20, 2017 at 09:55:03PM +0800, Peter Xu wrote:
> > On Mon, Jun 19, 2017 at 01:14:03PM -0300, Eduardo Habkost wrote:
[...]
> > > This is where things get tricky and fragile: the translation from
> > > -cpu to global properties is done very late inside machine init
> > > today, but we should be able to do that much earlier, once we
> > > refactor the -cpu parsing code.
> > >
> > > Hence my suggestion is to not touch x86_cpu_change_kvm_default()
> > > and just move the other properties (everything in
> > > kvm_default_props except svm, x2apic, and kvm-pv-eoi) to a static
> > > AccelClass::global_props field.
> >
> > Yes it's fragile and complicated.
> >
> > How about this:
> >
> > I introduce AccelClass::global_props, only use it in Xen but nowhere
> > else? After all, what I really want to do is just let migration codes
> > start to use "-global" properties and compatibility fields. And if
> > there is still no good idea to ideally solve this x86 cpu property
> > issue, I would prefer to keep it (it'll also be simpler for me).
>
> Sounds good to me.
Thanks for the confirmation. Let me cook another simpler series then.
>
> >
> > Another thing worries me a bit is that I may make things more
> > confusing if I separate this list into two (then we'll have part of
> > the properties in accel code, and the rest ones still in cpu.c).
> >
> > (then I can also avoid using hard code in accel.c/kvm.c as well, which
> > is something I really want to stop from doing. Maybe there can be
> > some better idea, but I cannot really figure it out now...)
> >
> > I'll just hold here to see whether you like above idea before moving
> > on to further comments. Thanks,
>
>
> Agreed.
>
> When I suggested using accel-provided global properties to
> replace kvm_default_props, I forgot x86_cpu_change_kvm_default()
> existed, and it makes things much more complex.
>
> I really really want to make the existing x2apic/svm/kvm-pv-eoi
> compat stuff be based on static lists of properties. If we make
> them dynamically built at runtime, we still can't introspect them
> and it won't be worth the extra complexity.
>
> I believe we can still find a solution to represent the
> x2apic/svm/kvm-pv-eoi rules using static lists, but this
> shouldn't block the migration work you are doing.
One thing I think of is:
Let MachineClass::compat_props be MachCompatProp (rather than
GlobalProperty), then define it as:
struct MachCompatProp {
GlobalProperty prop;
bool (*prop_valid)();
};
(We may pass MigrationState *ms into prop_valid(), but for current
requirements we may not need that, since what we need is basically
tcg_enabled(), kvm_enabled(), or kvm_irqchip_is_split() checks)
Then this property will only be delivered to the global_props list
only if its prop_valid() check pass. Thanks,
--
Peter Xu
