On Mon, Jun 04, 2018 at 06:06:04PM +0930, Joel Stanley wrote:
> On 4 June 2018 at 17:34, David Gibson <da...@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
signature.asc
Description: PGP signature
