Re: [PULL 35/47] target/arm: Send interrupts on PMU counter overflow

[Peter Maydell]

On Wed, 1 Jul 2020 at 16:11, Aaron Lindsay <aaron@os.amperecomputing.com> wrote:
> Ouch - I'm sorry this slipped through the cracks in my inbox for so
> long.
>
> I assume you mean something like:
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index dc9c29f998..9b917f9425 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -2271,13 +2271,13 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>        .access = PL1_RW, .accessfn = access_tpm,
>        .type = ARM_CP_ALIAS | ARM_CP_IO,
>        .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
> -      .writefn = pmintenclr_write, },
> +      .writefn = pmintenclr_write, .raw_writefn = raw_write },
>      { .name = "PMINTENCLR_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 2,
>        .access = PL1_RW, .accessfn = access_tpm,
>        .type = ARM_CP_ALIAS | ARM_CP_IO,
>        .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
> -      .writefn = pmintenclr_write },
> +      .writefn = pmintenclr_write, .raw_writefn = raw_write },
>      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
>        .access = PL1_R,

For cases like this where you have a 'set' and a 'clr' register
that really are accessing the same state under the hood, the
way to do it is:
 * the 'set' register provides the raw_readfn/raw_writefn
   as raw_read/raw_write
 * the 'clr' register adds ARM_CP_NO_RAW to its .type flags,
   which means "don't do any raw accesses to this, it doesn't
   have any underlying state that's not already synced or
   migrated via some other mechanism".

> One thing I'm trying to figure out (talking non-KVM here) is whether
> skipping calling pmu_update_irq() can mean an interrupt would not be set
> when it should have been. It looks like the ARMCPRegInfo's for
> PMINTENSET already do `.raw_writefn = raw_write`, so I suppose at least
> we would be consistent with this change. But I can never remember - is
> it guaranteed that the raw functions are only ever called when the
> interrupt state would already be taken care of separately (i.e. when
> restoring a checkpoint)?

Yes, the raw writes only happen for migration or for when
we're syncing state from a KVM kernel, so the state of
the device at the other end of the irq line should already
be correct.

thanks
-- PMM