[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 06/13] kvm: let kvm use AccelState.global_pro
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v3 06/13] kvm: let kvm use AccelState.global_props |
Date: |
Tue, 20 Jun 2017 21:55:03 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Mon, Jun 19, 2017 at 01:14:03PM -0300, Eduardo Habkost wrote:
> On Mon, Jun 19, 2017 at 08:49:41PM +0800, Peter Xu wrote:
> > Let KVM be the first user of the new AccelState.global_props field.
> > Basically kvm accel only contains accel props for TYPE_X86_CPUs but not
> > anything else yet.
> >
> > There will be a change on how these global properties are applied for
> > TYPE_X86_CPU devices. The general workflow of the global property stuff
> > for TYPE_X86_CPU devices can be simplied as following (this is a example
> > routine of KVM that contains both old/new workflow, similar thing apply
> > to TCG, but even simpler):
> >
> > - HW_COMPAT/type_init() magic played before even entering main() [1]
>
> What do you mean by this? HW_COMPAT_* is used only in
> MachineClass::compat_props[4], and type_init() magic is triggered
> by module_call_init(MODULE_INIT_QOM) in main().
Sorry. It should be the thing you mentioned.
>
> > - main() in vl.c
> > - configure_accelerator()
> > - AccelClass.init_machine() [2]
> > - kvm_init() (for KVM accel)
> > - register global properties
> > - accel_register_compat_props(): register accel compat props [3]
> > - machine_register_compat_props(): register machine compat
> > props (here we'll apply all the HW_COMPAT magic into
> > global_props) [4]
> > - machine init()
> > - cpu init() [5]
> > - ...
> >
> > Before this patch, the code setup TYPE_X86_CPU properties at [4] by
> > keeping the kvm_default_props list and apply them directly to the device
> > using x86_cpu_apply_props().
> >
> > After this patch, the code setup the same properties in the sequence of
> > [1]->[2]->[3][4]->[5]:
> >
> > - At [1] we setup machine global properties. Note: there will be
> > properties that with value==NULL but it's okay - when it was applied
> > to global list, it'll be used to remove an entry at step [4], it
> > works just like the old kvm_default_props, but we just unified it to
> > a single place, which is the global_props list.
> >
> > - At [2] we setup accel global properties.
>
> Why isn't AccelClass::global_props set up at class_init(), just
> like we do on MachineClass::compat_props?
(explained in other thread: there is a property we only need to
register when split irqchip is used)
>
> >
> > - At [3]/[4] we move machine/accel properties into global property
> > list. One thing to mention is that, we do [3] before [4], since we
> > have some cross-relation properties, e.g., property that is required
> > when both PC=2.1 && ACCEL=kvm happen. For now, all this kind of
> > properties are still in machine compat properties.
> >
> > - At [5] when TYPE_X86_CPU creates, it applies the global property from
> > the global property list, which is now a merged list of three: accel
> > property list, machine property list, and user specified "-global"
> > properties.
>
> On which category above would the x86_cpu_change_kvm_default()
> calls in pc_piix.c would be? We would need to ensure they
> override the globals registered by the accel code, but they must
> not override the user-provided global properties (including
> -global and -cpu options).
Oh I didn't realize this difference. x86_cpu_change_kvm_default() is
called in [5]. It indeed breaks this rule.
>
> 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).
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,
--
Peter Xu
[Qemu-devel] [PATCH v3 07/13] tcg: use AccelState.global_props, Peter Xu, 2017/06/19
[Qemu-devel] [PATCH v3 08/13] trace: add qdev_global_prop_apply, Peter Xu, 2017/06/19
[Qemu-devel] [PATCH v3 09/13] migration: let MigrationState be a qdev, Peter Xu, 2017/06/19
[Qemu-devel] [PATCH v3 10/13] migration: move global_state.optional out, Peter Xu, 2017/06/19
[Qemu-devel] [PATCH v3 11/13] migration: move only_migratable to MigrationState, Peter Xu, 2017/06/19