qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] hvf: arm: Add support for GICv3


From: Peter Maydell
Subject: Re: [PATCH] hvf: arm: Add support for GICv3
Date: Fri, 6 Jan 2023 16:37:48 +0000

On Mon, 19 Dec 2022 at 22:08, Alexander Graf <agraf@csgraf.de> wrote:
>
> We currently only support GICv2 emulation. To also support GICv3, we will
> need to pass a few system registers into their respective handler functions.
>
> This patch adds support for HVF to call into the TCG callbacks for GICv3
> system register handlers. This is safe because the GICv3 TCG code is generic
> as long as we limit ourselves to EL0 and EL1 - which are the only modes
> supported by HVF.
>
> To make sure nobody trips over that, we also annotate callbacks that don't
> work in HVF mode, such as EL state change hooks.
>
> With GICv3 support in place, we can run with more than 8 vCPUs.
>
> Signed-off-by: Alexander Graf <agraf@csgraf.de>
> ---
>  hw/intc/arm_gicv3_cpuif.c   |   8 +-
>  target/arm/hvf/hvf.c        | 151 ++++++++++++++++++++++++++++++++++++
>  target/arm/hvf/trace-events |   2 +
>  3 files changed, 160 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index b17b29288c..b4e387268c 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -21,6 +21,7 @@
>  #include "hw/irq.h"
>  #include "cpu.h"
>  #include "target/arm/cpregs.h"
> +#include "sysemu/tcg.h"
>
>  /*
>   * Special case return value from hppvi_index(); must be larger than
> @@ -2810,6 +2811,8 @@ void gicv3_init_cpuif(GICv3State *s)
>           * which case we'd get the wrong value.
>           * So instead we define the regs with no ri->opaque info, and
>           * get back to the GICv3CPUState from the CPUARMState.
> +         *
> +         * These CP regs callbacks can be called from either TCG or HVF code.
>           */
>          define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
>
> @@ -2905,6 +2908,9 @@ void gicv3_init_cpuif(GICv3State *s)
>                  define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr23_reginfo);
>              }
>          }
> -        arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);
> +        if (tcg_enabled()) {
> +            /* We can only trap EL changes with TCG for now */

We could expand this a bit:

 We can only trap EL changes with TCG. However the GIC interrupt
 state only changes on EL changes involving EL2 or EL3, so for
 the non-TCG case this is OK, as EL2 and EL3 can't exist.

and assert:
 assert(!arm_feature(&cpu->env, ARM_FEATURE_EL2));
 assert(!arm_feature(&cpu->env, ARM_FEATURE_EL3));

> +static uint32_t hvf_reg2cp_reg(uint32_t reg)
> +{
> +    return ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP,
> +                              (reg >> 10) & 0xf,
> +                              (reg >> 1) & 0xf,
> +                              (reg >> 20) & 0x3,
> +                              (reg >> 14) & 0x7,
> +                              (reg >> 17) & 0x7);

This file has #defines for these shift and mask constants
(SYSREG_OP0_SHIFT etc).

> +}
> +
> +static bool hvf_sysreg_read_cp(CPUState *cpu, uint32_t reg, uint64_t *val)
> +{
> +    ARMCPU *arm_cpu = ARM_CPU(cpu);
> +    CPUARMState *env = &arm_cpu->env;
> +    const ARMCPRegInfo *ri;
> +
> +    ri = get_arm_cp_reginfo(arm_cpu->cp_regs, hvf_reg2cp_reg(reg));
> +    if (ri) {
> +        if (ri->accessfn) {
> +            if (ri->accessfn(env, ri, true) != CP_ACCESS_OK) {
> +                return false;
> +            }
> +        }
> +        if (ri->type & ARM_CP_CONST) {
> +            *val = ri->resetvalue;
> +        } else if (ri->readfn) {
> +            *val = ri->readfn(env, ri);
> +        } else {
> +            *val = CPREG_FIELD64(env, ri);
> +        }
> +        trace_hvf_vgic_read(ri->name, *val);
> +        return true;
> +    }

Can we get here for attempts by EL0 to access EL1-only
sysregs, or does hvf send the exception to EL1 without
trapping out to us? If we can get here for EL0 accesses we
need to check against ri->access as well as ri->accessfn.

Otherwise looks OK.

thanks

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]