On Tue, Apr 30, 2013 at 07:04:23PM +0200, Igor Mammedov wrote: > On Fri, 26 Apr 2013 16:01:15 -0300 > Eduardo Habkost <ehabk...@redhat.com> wrote: > > > On Fri, Apr 26, 2013 at 07:41:10PM +0200, Igor Mammedov wrote: > > > On Fri, 26 Apr 2013 14:30:40 -0300 > > > Eduardo Habkost <ehabk...@redhat.com> wrote: > > > > > > > On Fri, Apr 26, 2013 at 05:39:15PM +0200, Igor Mammedov wrote: > > > > > On Fri, 26 Apr 2013 17:33:18 +0200 > > > > > Andreas Färber <afaer...@suse.de> wrote: > > > > > > > > > > > Am 26.04.2013 17:31, schrieb Eduardo Habkost: > > > > > > > On Fri, Apr 26, 2013 at 05:10:29PM +0200, Igor Mammedov wrote: > > > > > > >> On Thu, 25 Apr 2013 15:43:06 -0300 > > > > > > >> Eduardo Habkost <ehabk...@redhat.com> wrote: > > > > > > >> > > > > > > >>> The current code handling the CPUID 0xA leaf simply forwards > > > > > > >>> all data > > > > > > >>> from GET_SUPPORTED_CPUID directly to the guest, breaking > > > > > > >>> migration > > > > > > >>> between hosts with different number of PMU counters. > > > > > > >>> > > > > > > >>> This patch disables this behavior, except on older > > > > > > >>> machine-types (for > > > > > > >>> compatibility) and on the "host" CPU model. > > > > > > >> Please, make it static property and use compat properties. > > > > > > >> Result will be simpler and much less will have to be > > > > > > >> redone/discarded after > > > > > > >> converting to the rest to properties and sub-classes. > > > > > > > > > > > > > > I was going to say that static properties were too much work to > > > > > > > be done > > > > > > > in time for 1.5, but you are right: in this specific case adding a > > > > > > > static property for the cpuid_pmu_passthrough field looks very > > > > > > > easy. I > > > > > > > will give it a try. > > > > > > > > > > > > I am hoping to get as initial set (though not all) of the static > > > > > > properties still into 1.5. Using them to fix CPUID bugs can then be > > > > > > done > > > > > > during Hard Freeze. :) > > > > > patch "[PATCH 02/10] target-i386: cpu: convert existing dynamic > > > > > properties > > > > > into static properties" should be enough for using model,level compat > > > > > properties. > > > > > > > > ...except that we don't have X86CPU model subclasses yet, so we can't > > > > set model-specific compat properties using compat_props. > > > X86CPU could serve here > > > > How? > > > > If we need to set model=8 only on the "486" CPU model, how would we do > > that in the compat_props table without subclasses? > for model, it would be its possible with custom setter, might be preferred way > since it's local to cpu.c, and could be easily replaced with with one-liner > once > sub-classes are in tree.
A custom setter has to be set inside class_init, because instance_init is run _after_ global properties are set, but we still don't have subclasses so we can't have a different setter for "486" or a compat_props item that would affect only "486". How exactly would this solution look like? I can't see it. (My main question isn't about the implementation inside cpu.c, but how the machine-type code inside pc_piix.c would look like). > > > > > > > > > > > > > > Maybe we can make compat_props work for pmu-passthrough, but it may be > > > > not completely straightforward because I would like to keep it enabled > > > > by default on -cpu host. > > > Maybe for 'host' hack realize() to override default, will do and later we > > > can move it to host_subclass. > > > > Yes, it's doable. And I don't think we even need to hack methods, we > > just need a x86_def_t field that indicates that this is a model that > > overrides the default (and that field would be true only for "host"). > Extended x86_def_t might be harder to handle when switching to sub-classes, > although it's hard to say without comparing patches for both ways. I believe it's impossible to implement any behavior that will change depending on the CPU model (enabling PMU passthrough on "host", disabling on all other models) without extending x86_def_t. x86_def_t data is the only thing we get back from cpu_x86_find_by_name(). Would you prefer to change cpu_x86_find_by_name() to get a X86CPU* as argument, too? I was trying to avoid that, but it is doable. > > > > > But I still don't see how it would be possible to use compat_props for > > the 486 model=8 fix or for the n270 movbe fix. > > > It's impossible to implement "n270 movbe" fixup with compat props currently, > so 5/7 + 1/7 looks ok. We can drop them once features converted to static > properties > + sub-classes. True. But why this doesn't apply to 6/7 (486 model=8) as well? -- Eduardo
