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
