On 31 October 2014 08:30, Peter Maydell <peter.mayd...@linaro.org> wrote:

> On 30 October 2014 21:28, Greg Bellows <greg.bell...@linaro.org> wrote:
> > From: Sergey Fedorov <s.fedo...@samsung.com>
> >
> > Added CP register defintions for SDER and SDER32_EL3 as well as
> cp15.sder for
> > register storage.
> >
> > Signed-off-by: Sergey Fedorov <s.fedo...@samsung.com>
> > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> >
> > ---
> >
> > v7 -> v8
> > - Added SDER32_EL3 register definition
> > - Changed sder name from c1_sder to sder
> > - Changed sder from uint32_t to uint64_t.
> > ---
> >  target-arm/cpu.h    | 1 +
> >  target-arm/helper.c | 8 ++++++++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 88e22fb..62cf48a 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -181,6 +181,7 @@ typedef struct CPUARMState {
> >          uint64_t c1_sys; /* System control register.  */
> >          uint64_t c1_coproc; /* Coprocessor access register.  */
> >          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.
> */
> > +        uint64_t sder; /* Secure debug enable register. */
> >          uint32_t nsacr; /* Non-secure access control register. */
> >          uint64_t ttbr0_el1; /* MMU translation table base 0. */
> >          uint64_t ttbr1_el1; /* MMU translation table base 1. */
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 3c12eb3..0be19f3 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2344,6 +2344,14 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
> >        .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
> >        .access = PL3_RW, .resetvalue = 0, .writefn = scr_write,
> >        .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3) },
> > +    { .name = "SDER32_EL3", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 1,
> > +      .access = PL3_RW, .resetvalue = 0,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.sder) },
> > +    { .name = "SDER",
> > +      .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 1,
>
> Can you keep the ordering in reginfo fields to
>  .cp [if needed], .opc0 [if needed], .opc1, .crn, .crm, .opc2
> please? I know we have some existing reginfo structs which don't
> follow that, but for new ones we're trying to follow the ordering
> used in the v8 ARM ARM (which in turn matches the order used
> in MRC/MCR instructions).
>
>
:-) I inherited the misordered definition, should have fixed when I added
SDER32_EL3.  Fixed in v9.


> > +      .access = PL3_RW, .resetvalue = 0,
> > +      .fieldoffset = offsetoflow32(CPUARMState, cp15.sder) },
> >      { .name = "NSACR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 =
> 2,
> >        .access = PL3_RW | PL1_R, .resetvalue = 0,
> >        .fieldoffset = offsetof(CPUARMState, cp15.nsacr) },
> > --
> > 1.8.3.2
> >
>
> Otherwise
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>
> thanks
> -- PMM
>

Reply via email to