Re: [Qemu-ppc] [PATCH v4 2/2] target/ppc: move POWER9 DD1 workaround to init_proc_POWER9()

[Greg Kurz]

On Wed, 5 Jul 2017 09:56:15 +0200
Thomas Huth <address@hidden> wrote:

> On 05.07.2017 08:36, David Gibson wrote:
> > On Tue, Jul 04, 2017 at 03:02:39PM +0200, Greg Kurz wrote:  
> >> On Tue, 4 Jul 2017 21:41:51 +1000
> >> David Gibson <address@hidden> wrote:
> >>  
> >>> On Tue, Jul 04, 2017 at 01:15:00PM +0200, Greg Kurz wrote:  
> >>>> On Tue,  4 Jul 2017 13:01:26 +0200
> >>>> Laurent Vivier <address@hidden> wrote:
> >>>>     
> >>>>> Commit 5f3066d ("target/ppc: Allow workarounds for POWER9 DD1")
> >>>>> disables compatibility mode for POWER9 DD1 to allow to
> >>>>> boot on POWER9 DD1 host with KVM.
> >>>>>
> >>>>> As the workaround has been added in kvmppc_host_cpu_class_init(),
> >>>>> it applies only on CPU created with "-cpu host".
> >>>>> As we want to be able to use also "-cpu POWER9" on a POWER9 DD1
> >>>>> host, this patch moves the workaround from kvmppc_host_cpu_class_init()
> >>>>> to init_proc_POWER9().
> >>>>>     
> >>>>
> >>>> As with ppc_cpu_initfn() in your previous version, init_proc_POWER9() is
> >>>> called for every CPU instance.. ie, all CPU will adjust the 
> >>>> @pcr_supported
> >>>> class attribute...    
> >>>
> >>> Ah.. yeah.. I didn't notice that before.  That's definitely not right.
> >>>  
> >>>> What about moving the workaround to ppc_POWER9_cpu_family_class_init()
> >>>> instead ? This would just require to expose mfpvr() in some header.    
> >>>
> >>> Yeah, as someone else pointed out using the host PVR is also
> >>> definitely not right (unless you're in a function specifically
> >>> connected to the host cpu class).
> >>>  
> >>
> >> I agree but the root issue is that we accept to pass -cpu POWER9 instead of
> >> -cpu host with -enable-kvm. And the host cpu class isn't involved in this
> >> case.  
> > 
> > Well.. it sort of is.  I believe the way we make this work (since
> > Thomas' cleanup) is that when KVM is active, we alter the alias for
> > the host cpu's family to point to the host cpu class, instead of
> > whatever specific version it usually points to.  
> 

Yeah, I saw that and I guess there's a problem: we alter the alias to
point to the class with the same PVR as the host CPU, not to the
host CPU class itself. The CPUs don't belong to TYPE_HOST_POWERPC_CPU
and aren't configured according to kvmppc_host_cpu_class_init().

> Right, it's the code at the and of the kvm_ppc_register_host_cpu_type()
> function. Maybe that function could also be a good spot to move the DD1
> workaround into (just a quick idea, I haven't checked whether it's
> feasible)?
> 

I have a patch to change the alias to point to TYPE_HOST_POWERPC_CPU. This
allows to use "-cpu POWER9" with KVM on a DD1 host.

Cheers,

--
Greg

>  Thomas
>

pgp0Kjmh4U0EQ.pgp
Description: OpenPGP digital signature