Re: [Qemu-ppc] [QEMU-PPC] [PATCH 2/3] target/ppc: Don't require private l1d cache on POWER8 for cap_ppc_safe_cache

[Murilo Opsfelder Araujo]

On Fri, May 11, 2018 at 08:22:51PM +0200, Greg Kurz wrote:
> On Fri, 11 May 2018 12:21:11 -0300
> Murilo Opsfelder Araujo <address@hidden> wrote:
> 
> > On Fri, May 11, 2018 at 04:25:08PM +1000, Suraj Jitindar Singh wrote:
> > > For cap_ppc_safe_cache to be set to workaround, we require both a l1d
> > > cache flush instruction and private l1d cache.
> > > 
> > > On POWER8 don't require private l1d cache. This means a guest on a
> > > POWER8 machine can make use of the cache flush workarounds.
> > > 
> > > Signed-off-by: Suraj Jitindar Singh <address@hidden>
> > > ---
> > >  target/ppc/kvm.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > > index 2c0c34e125..7b33119b1a 100644
> > > --- a/target/ppc/kvm.c
> > > +++ b/target/ppc/kvm.c
> > > @@ -2414,9 +2414,17 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
> > >  
> > >  static int parse_cap_ppc_safe_cache(struct kvm_ppc_cpu_char c)
> > >  {
> > > +    PowerPCCPUClass *pcc = kvm_ppc_get_host_cpu_class();
> > > +    bool l1d_thread_priv_req = true;
> > > +
> > > +    if (pcc->pvr_match(pcc, CPU_POWERPC_POWER8_BASE)) {
> > > +        l1d_thread_priv_req = false;
> > > +    }
> > > +  
> > 
> > Hi, Suraj.
> > 
> > Why not use ppc_pvr_match_power8()?
> > 
> 
> ppc_pvr_match_power8() is the implementation of PowerPCCPUClass::pvr_match()
> for the POWER8 family, hence always returns true if passed 
> CPU_POWERPC_POWER8_*
> 
> static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
> {
>     if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8NVL_BASE) {
>         return true;
>     }
>     if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
>         return true;
>     }
>     if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
>         return true;
>     }
>     return false;
> }
> 
> It's the other way round, we want to guess that the host CPU belongs to the
> POWER8 family because only its pvr_match() returns true IIUC.

Hmm, I think I get it now. Thanks, Greg.

> 
> Suraj,
> 
> So this is ok for POWER8, POWER8E and POWER8NVL ? If so, it would be
> more appropriate to call pcc->pvr_match() for each pvr value IMHO.
> 
> > >      if (~c.behaviour & c.behaviour_mask & H_CPU_BEHAV_L1D_FLUSH_PR) {
> > >          return 2;
> > > -    } else if ((c.character & c.character_mask & 
> > > H_CPU_CHAR_L1D_THREAD_PRIV) &&
> > > +    } else if ((!l1d_thread_priv_req ||
> > > +                c.character & c.character_mask & 
> > > H_CPU_CHAR_L1D_THREAD_PRIV) &&
> > >                 (c.character & c.character_mask
> > >                  & (H_CPU_CHAR_L1D_FLUSH_ORI30 | 
> > > H_CPU_CHAR_L1D_FLUSH_TRIG2))) {
> > >          return 1;
> > > -- 
> > > 2.13.6
> > > 
> > >   
> > 
> 
> 

-- 
Murilo