On 06.01.23 17:37, Peter Maydell wrote:
On Mon, 19 Dec 2022 at 22:08, Alexander Graf <ag...@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 <ag...@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));
Good idea! Let me add that.
+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).
Ugh, thanks for catching that!
+} + +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.
I just validated, GICv3 EL1 registers trap to EL1 inside the guest: $ cat a.S .global start .global _main _main: start: mrs x0, ICC_AP0R0_EL1 mov x0, #0x1234 msr ICC_AP0R0_EL1, x0 mov x0, #0 ret $ gcc -nostdlib a.S $ gdb ./a.out (gdb) r Program received signal SIGILL, Illegal instruction. 0x00000000004000d4 in start () (gdb) x/i $pc => 0x4000d4 <start>: mrs x0, icc_ap0r0_el1 So no need to check ri->access :) Alex
