Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Igor Mammedov]

On Fri, 26 Apr 2013 16:01:15 -0300
Eduardo Habkost <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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.

> 
> > 
> > > 
> > > 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.

> 
> 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.