Hi Peter!
This is a jumbo all-in-one response:
On Thu, 12 Jul 2012 14:59:16 +0100, Peter Maydell <peter.maydell at linaro.org> wrote:
> On 12 July 2012 06:54, Rusty Russell <rusty.russell at linaro.org> wrote:
> > +
> > +/* A15-specific CP15 registers. */
> > +static const struct coproc_reg cp15_cortex_a15_regs[] = {
> > + /*
> > + * ACTRL access:
>
> "ACTLR". (These comments are verging on the edge of "add one to i" territory
> given that the data structure is now nearly self-documenting...)
In fact this patch just moved the erroneous comment. The documentation
patch then reduces it to one line:
/* ACTLR: trapped by HCR.TAC bit. */
> > These registers are defined to be WO, meaning reads are unpredicable.
>
> "unpredictable".
>
> > Unless some actual guest breaks, it's most friendly to fall though and
>
> "through".
Thanks, fixed.
> > + *
> > + * Therefore we tell the guest we have 0 counters. Unfortunately, we
> > + * must always support PMCCNTR (the cycle counter): we just WI/RAZ for
>
> "RAZ/WI" is the term preferred in the ARM ARM.
Indeed, fixed.
> > functions. We also rename the table from 'struct coproc_emulate
> > coproc_emulate' to 'struct coproc_reg coproc_regs', to reflect the more
> > generic role.
>
> This is not quite the rename the patch performs:
Well spotted. Here's the new description:
This means we fold the read and write functionality into the accessor
functions. We rename struct coproc_emulate to struct coproc_reg
to reflect the more generic role, and the coproc_emulate[] table to
the more precise 'cp15_regs[]'.
> > This means we generate the value at reset time. ACTLR already had a spot
> > in the cp15 array (unused), by L2CTLR needs a new one.
>
> "but".
Thanks, fixed.
And re-pushed.
What do you want to do about the API breakage? Should we just insist on
a lockstep qemu/kernel update, or some temporary bandaid in qemu? The
latter should be possible, but is it worth the hassle?
Cheers,
Rusty.
