Re: [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support

[Pavel Fedin]

 Hello!

> -----Original Message-----
> From: Shlomo Pongratz [mailto:address@hidden
> Sent: Tuesday, October 20, 2015 8:22 PM
> To: address@hidden
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden; Shlomo Pongratz
> Subject: [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
> 
> From: Shlomo Pongratz <address@hidden>
> 
> Add system instructions used by the Linux (kernel) GICv3
> device driver
> 
> Signed-off-by: Shlomo Pongratz <address@hidden>
> ---
>  target-arm/cpu-qom.h |   1 +
>  target-arm/cpu.h     |  12 ++++++
>  target-arm/cpu64.c   | 118 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 25fb1ce..6a50433 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -220,6 +220,7 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr 
> addr);
> 
>  int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> +void aarch64_registers_with_opaque_set(Object *obj, void *opaque);
> 
>  /* Callback functions for the generic timer's timers. */
>  void arm_gt_ptimer_cb(void *opaque);
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 3daa7f5..d561313 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1034,6 +1034,18 @@ void armv7m_nvic_set_pending(void *opaque, int irq);
>  int armv7m_nvic_acknowledge_irq(void *opaque);
>  void armv7m_nvic_complete_irq(void *opaque, int irq);
> 
> +void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value);
> +uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex,
> +                              MemTxAttrs attrs);
> +void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq,
> +                              MemTxAttrs attrs);
> +uint64_t armv8_gicv3_get_priority_mask(void *opaque, int cpuindex);
> +void armv8_gicv3_set_priority_mask(void *opaque, int cpuindex, uint32_t 
> mask);
> +uint64_t armv8_gicv3_get_sre(void *opaque);
> +void armv8_gicv3_set_sre(void *opaque, uint64_t sre);
> +uint64_t armv8_gicv3_get_igrpen1(void *opaque, int cpuindex);
> +void armv8_gicv3_set_igrpen1(void *opaque, int cpuindex, uint64_t igrpen1);
> +
>  /* Interface for defining coprocessor registers.
>   * Registers are defined in tables of arm_cp_reginfo structs
>   * which are passed to define_arm_cp_regs().
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 63c8b1c..4224779 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -45,6 +45,115 @@ static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, 
> const ARMCPRegInfo
> *ri)
>  }
>  #endif
> 
> +#ifndef CONFIG_USER_ONLY
> +static void sgi_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    armv8_gicv3_set_sgi(ri->opaque, cpu->cpu_index, value);
> +}
> +
> +static uint64_t iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    MemTxAttrs attrs;;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
> +    value = armv8_gicv3_acknowledge_irq(ri->opaque, cpu->cpu_index, attrs);
> +    return value;
> +}
> +
> +static void sre_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +    armv8_gicv3_set_sre(ri->opaque, value);
> +}
> +
> +static uint64_t sre_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    value = armv8_gicv3_get_sre(ri->opaque);
> +    return value;
> +}
> +
> +static void eoir_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +    MemTxAttrs attrs;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
> +    armv8_gicv3_complete_irq(ri->opaque, cpu->cpu_index, value, attrs);
> +}
> +
> +static uint64_t pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    value = armv8_gicv3_get_priority_mask(ri->opaque, cpu->cpu_index);
> +    return value;
> +}
> +
> +static void pmr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    armv8_gicv3_set_priority_mask(ri->opaque, cpu->cpu_index, value);
> +}
> +
> +static uint64_t igrpen1_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    value = armv8_gicv3_get_igrpen1(ri->opaque, cpu->cpu_index);
> +    return value;
> +}
> +
> +static void igrpen1_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    armv8_gicv3_set_igrpen1(ri->opaque, cpu->cpu_index, value);
> +}
> +#endif
> +
> +static const ARMCPRegInfo cortex_a57_a53_cp_with_opaque_reginfo[] = {
> +    { .name = "EIOR1_EL1", .state = ARM_CP_STATE_AA64,
> +#ifndef CONFIG_USER_ONLY
> +      .writefn = eoir_write,
> +#endif
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 1,
> +      .access = PL1_W, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "IAR1_EL1", .state = ARM_CP_STATE_AA64,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = iar_read,
> +#endif
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 0,
> +      .access = PL1_R, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "SGI1R_EL1", .state = ARM_CP_STATE_AA64,
> +#ifndef CONFIG_USER_ONLY
> +      .writefn = sgi_write,
> +#endif
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 5,
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "PMR_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 6, .opc2 = 0,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = pmr_read, .writefn = pmr_write,
> +#endif
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "CTLR_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4,
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "SRE_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 5, .resetvalue = 0,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = sre_read, .writefn = sre_write,
> +#endif
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "IGRPEN1_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 7,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = igrpen1_read, .writefn = igrpen1_write,
> +#endif
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    REGINFO_SENTINEL
> +};
> +

 One more note on this table, which i previously forgot. We tried to run EFI 
here, and it crashes with your emulation because when
setting up GICv3 it also pokes BPR1_EL1 register. You should implement it.

>  static const ARMCPRegInfo cortex_a57_a53_cp_reginfo[] = {
>  #ifndef CONFIG_USER_ONLY
>      { .name = "L2CTLR_EL1", .state = ARM_CP_STATE_AA64,
> @@ -258,6 +367,15 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool 
> value, Error **errp)
>      }
>  }
> 
> +void aarch64_registers_with_opaque_set(Object *obj, void *opaque)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    define_arm_cp_regs_with_opaque(cpu,
> +                                   cortex_a57_a53_cp_with_opaque_reginfo,
> +                                   opaque);
> +}
> +
>  static void aarch64_cpu_initfn(Object *obj)
>  {
>      object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64,
> --
> 1.9.1

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia