Re: [Qemu-devel] [PATCHv3] target-arm: Implement AArch64 OSLAR/OSLSR_EL1 sysregs

[Peter Maydell]

On 5 October 2015 at 22:36, Davorin Mista <address@hidden> wrote:
> Added oslar_write function to OSLAR_EL1 sysreg, using a status variable
> in ARMCPUState.cp15 struct (oslsr_el1). This variable is also linked
> to the newly added read-only OSLSR_EL1 register.
>
> Linux reads from this register during its suspend/resume procedure.
>
> Signed-off-by: Davorin Mista <address@hidden>
>
> ---
> Changed in v2:
> -switched from using dummy registers to an actual register implementation
> -implemented write function for OSLAR_EL1 sysreg
> -added state variable to ARMCPUState struct
>
> Changed in v3:
> -renamed variable to oslsr_el1 and moved to cp15
> -renamed write frunction to oslar_write
> -support both 32bit and 64bit ARM in oslar_write
> -moved resetvalue to the corresponding read-only register
> -removed "dummy" comments above registers
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 23 +++++++++++++++++++++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 5ea11a6..f9f9bfb 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -399,6 +399,7 @@ typedef struct CPUARMState {
>          uint64_t dbgwvr[16]; /* watchpoint value registers */
>          uint64_t dbgwcr[16]; /* watchpoint control registers */
>          uint64_t mdscr_el1;
> +        uint64_t oslsr_el1; /* OS Lock Status */
>          /* If the counter is enabled, this stores the last time the counter
>           * was reset. Otherwise it stores the counter value
>           */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 9d62c4c..ecc89c7 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3147,6 +3147,20 @@ static void dcc_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
>      putchar(value);
>  }
>
> +/* write to os_lock_status state variable */
> +static void oslar_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +    int oslock;
> +
> +    if (ri->state == ARM_CP_STATE_AA32) {
> +        oslock = (value == 0xC5ACCE55);
> +    } else {
> +        oslock = value & 1;
> +    }
> +
> +    env->cp15.oslsr_el1 = deposit32(env->cp15.oslsr_el1, 1, 1, oslock);
> +}
> +
>  static const ARMCPRegInfo debug_cp_reginfo[] = {
>      /* DBGDRAR, DBGDSAR: always RAZ since we don't implement memory mapped
>       * debug components. The AArch64 version of DBGDRAR is named MDRAR_EL1;
> @@ -3176,10 +3190,15 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>        .access = PL1_R,
>        .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1),
>        .resetfn = arm_cp_reset_ignore },
> -    /* We define a dummy WI OSLAR_EL1, because Linux writes to it. */
>      { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
>        .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
> -      .access = PL1_W, .type = ARM_CP_NOP },
> +      .access = PL1_W,
> +      .fieldoffset = offsetof(CPUARMState, cp15.oslsr_el1),

OSLAR doesn't want a fieldoffset.

Still missing .type = ARM_CP_NO_RAW. (As Alistair says, this does
exist in mainline -- please make sure you write and test your
patches against current git master, not any other tree.)

> +      .writefn = oslar_write },
> +    { .name = "OSLSR_EL1", .state = ARM_CP_STATE_BOTH,
> +      .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 4,
> +      .access = PL1_R, .resetvalue = 10,
> +      .fieldoffset = offsetof(CPUARMState, cp15.oslsr_el1) },

This part is OK.

>      /* Dummy OSDLR_EL1: 32-bit Linux will read this */
>      { .name = "OSDLR_EL1", .state = ARM_CP_STATE_BOTH,
>        .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 4,
> --
> 2.6.0

thanks
-- PMM