Re: [Qemu-devel] [PATCH v2] target/ppc: Allow privileged access to SPR_PCR

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On Mon, Jun 04, 2018 at 06:06:04PM +0930, Joel Stanley wrote:
> On 4 June 2018 at 17:34, David Gibson <david@gibson.dropbear.id.au> wrote:
> > On Thu, May 31, 2018 at 10:34:29PM +0930, Joel Stanley wrote:
> >> @@ -98,6 +99,15 @@ void helper_store_ptcr(CPUPPCState *env, target_ulong val)
> >>          tlb_flush(CPU(cpu));
> >>      }
> >>  }
> >> +
> >> +void helper_store_pcr(CPUPPCState *env, target_ulong value)
> >> +{
> >> +    if (value != 0) {
> >> +        error_report("Invalid PCR value 0x"TARGET_FMT_lx, value);
> >> +        return;
> >> +    }
> >
> > I don't see a lot of point to this check.  For now the only user just
> > clears the PCR, but why limit yourself to that explicitly?
> 
> As we're not emulating the features of the register when the value is
> non-zero, I thought it would make make sense to do something.

Ah.. I see what you mean.

Ok, the check makes sense in that case - but it needs a comment saying
it should go away once we actually put the proper PCR checks in the
various bits of TCG that should have them.

> 
> I'll drop it in v3.
> 
> >
> >> +    env->spr[SPR_PCR] = value;
> >
> > You definitely should be masking against pcr_mask too.
> 
> Okay. Thanks for the review.
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]