Re: [Qemu-devel] [PATCH 09/17] ppc: SPURR & PURR are HV writeable and privileged

[David Gibson]

On Mon, Mar 14, 2016 at 08:37:59PM +0100, Thomas Huth wrote:
> On 14.03.2016 17:56, Cédric Le Goater wrote:
> > From: Benjamin Herrenschmidt <address@hidden>
> > 
> > Those are HV writeable, so we provide a dummy write. We eventually need
> > to provide a better emulation but for now this will get us going.
> > 
> > We also make them non-user readable as per the architecture.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> > ---
> >  target-ppc/translate_init.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> > index 7a399b97bc6f..10f67136b609 100644
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -293,6 +293,13 @@ static void spr_read_purr (DisasContext *ctx, int 
> > gprn, int sprn)
> >  {
> >      gen_helper_load_purr(cpu_gpr[gprn], cpu_env);
> >  }
> > +
> > +__attribute__ ((unused))
> > +static void spr_write_purr(DisasContext *ctx, int gprn, int sprn)
> > +{
> > +    /* Temporary placeholder */
> > +}
> 
> What's the "__attribute__ ((unused))" needed here for? The function is
> referenced below, so it should be "used"?
> Or is this simply about handling the CONFIG_USER_ONLY case? Then I think
> it would be nicer to change the #ifdef in front of it to include
> "!defined(CONFIG_USER_ONLY)", too.

Ah, yes.  I second Thomas' comment.

> 
>  Thomas
> 
> >  #endif
> >  #endif
> >  
> > @@ -7828,14 +7835,16 @@ static void gen_spr_book3s_purr(CPUPPCState *env)
> >  {
> >  #if !defined(CONFIG_USER_ONLY)
> >      /* PURR & SPURR: Hack - treat these as aliases for the TB for now */
> > -    spr_register_kvm(env, SPR_PURR,   "PURR",
> > -                     &spr_read_purr, SPR_NOACCESS,
> > -                     &spr_read_purr, SPR_NOACCESS,
> > -                     KVM_REG_PPC_PURR, 0x00000000);
> > -    spr_register_kvm(env, SPR_SPURR,   "SPURR",
> > -                     &spr_read_purr, SPR_NOACCESS,
> > -                     &spr_read_purr, SPR_NOACCESS,
> > -                     KVM_REG_PPC_SPURR, 0x00000000);
> > +    spr_register_kvm_hv(env, SPR_PURR,   "PURR",
> > +                        SPR_NOACCESS, SPR_NOACCESS,
> > +                        &spr_read_purr, SPR_NOACCESS,
> > +                        &spr_read_purr, &spr_write_purr,
> > +                        KVM_REG_PPC_PURR, 0x00000000);
> > +    spr_register_kvm_hv(env, SPR_SPURR,   "SPURR",
> > +                        SPR_NOACCESS, SPR_NOACCESS,
> > +                        &spr_read_purr, SPR_NOACCESS,
> > +                        &spr_read_purr, &spr_write_purr,
> > +                        KVM_REG_PPC_SPURR, 0x00000000);
> >  #endif
> >  }
> >  
> > 
> 

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