[kvmarm] [PATCH 09/17] ARM: KVM: Create specific per-target register tables.

[rusty.russell at linaro.org (Rusty Russell)]

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.