On Fri, Aug 01, 2014 at 02:29:28PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
>
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 7170086..b04fb5d 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2107,10 +2107,36 @@ static const ARMCPRegInfo
> > v8_el3_no_el2_cp_reginfo[] = {
> > .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> > .access = PL2_RW,
> > .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > + { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> > + .type = ARM_CP_NO_MIGRATE,
> > + .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> > + .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
>
> Isn't this missing the .access specifier ?
Good catch, thanks.
>
> > REGINFO_SENTINEL
> > };
> >
> > +static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
> > value)
> > +{
> > + ARMCPU *cpu = arm_env_get_cpu(env);
> > + uint64_t valid_mask = HCR_MASK;
> > +
> > + if (!arm_feature(env, ARM_FEATURE_EL3)) {
> > + valid_mask &= ~HCR_HCD;
> > + }
>
> This is inconsistent. HCD isn't the only bit that's "RES0 if
> EL3 unimplemented"; TSC is as well, for instance.
> (In fact the RES0 definition means you don't actually have
> to mask this out unless it's more convenient to do so.)
I've added TSC. Couldn't see any others..
>
> > +
> > + /* Clear RES0 bits. */
> > + value &= valid_mask;
> > +
> > + if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_RW | HCR_PTW |
> > HCR_DC)) {
> > + tlb_flush(CPU(cpu), 1);
>
> Could maybe use a comment about why we need a TLB flush.
I'll add one. Actually, when thinking more about it, I'm not convinced that
we will need to flush when HCR_RW changes because the 32/64bit state is part
of the tbflags.
Cheers,
Edgar
