Re: [Qemu-devel] [PATCH for-2.11 6/6] ppc: drop caching ObjectClass from PowerPCCPUAlias

[David Gibson]

On Fri, Aug 25, 2017 at 09:49:48AM +0200, Igor Mammedov wrote:
> On Fri, 25 Aug 2017 14:22:03 +1000
> David Gibson <address@hidden> wrote:
> 
> > On Thu, Aug 24, 2017 at 10:21:51AM +0200, Igor Mammedov wrote:
> > > Caching there practically doesn't give any benefits
> > > and that at slow path druring querying supported CPU list.
> > > But it introduces non conventional path of where from
> > > comes used CPU type name (kvm_ppc_register_host_cpu_type).
> > > 
> > > Taking in account that kvm_ppc_register_host_cpu_type()
> > > fixes up models the aliases point to, it's sufficient to
> > > make ppc_cpu_class_by_name() translate cpu alias to
> > > correct cpu type name.
> > > So drop PowerPCCPUAlias::oc field + ppc_cpu_class_by_alias()
> > > and let ppc_cpu_class_by_name() do conversion to cpu type name,
> > > which simplifies code a little bit saving ~20LOC and trouble
> > > wondering why ppc_cpu_class_by_alias() is necessary.
> > > 
> > > Signed-off-by: Igor Mammedov <address@hidden>  
> > 
> > Unfortunately, this will break things.  This isn't purely a cache, in
> > the case handled by kvm_ppc_register_host_cpu_type(), 'oc' is *not*
> > redundant with the name.  The name is based on the specific CPU
> > description, but the oc points to the information for the "host" cpu.
> > 
> > There may well be a better way to do this, but this isn't it.
> > 
> > The problem we're trying to solve here is that KVM HV basically only
> > works with -cpu host.  But for the benefit of libvirt (and others), we
> > want, say, -cpu POWER8 to also work if the host *is* a POWER8.  But we
> > *don't* want to require that the host be exactly the same model of
> > POWER8 as "POWER8" would be aliased to with TCG.
> 
> here is snippet from kvm_ppc_register_host_cpu_type()
> 
> 1:
>             ppc_cpu_aliases[i].model = g_strdup(object_class_get_name(oc));   
>    
>             suffix = strstr(ppc_cpu_aliases[i].model, "-"TYPE_POWERPC_CPU);   
>    
>             if (suffix) {                                                     
>    
>                 *suffix = 0;                                                  
>    
>             }
> 2:                                                        
>             ppc_cpu_aliases[i].oc = oc;
> 
> where model is fixed up (1) to the name that corresponds to
> exactly the same 'oc' that is cached into (2)
> 
> any follow up attempt to translate fixed up alias will end up resolving
> it into model => the same 'oc' that would be cached in ppc_cpu_aliases[i].oc
> 
> isn't it?


You're right, I misread the code, sorry.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature