diff mbox series

[v6,10/11] hvf: arm: Add support for GICv3

Message ID 20210120224444.71840-11-agraf@csgraf.de
State New
Headers show
Series hvf: Implement Apple Silicon Support | expand

Commit Message

Alexander Graf Jan. 20, 2021, 10:44 p.m. UTC
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 handling for all of the required system registers, so that
we can run with more than 8 vCPUs.

Signed-off-by: Alexander Graf <agraf@csgraf.de>
Acked-by: Roman Bolshakov <r.bolshakov@yadro.com>

---

v5 -> v6:

  - Adapt to new SYSREG() ordering
---
 target/arm/hvf/hvf.c | 141 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

Comments

Peter Maydell Jan. 28, 2021, 4:40 p.m. UTC | #1
On Wed, 20 Jan 2021 at 22:44, 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 handling for all of the required system registers, so that
> we can run with more than 8 vCPUs.
>
> Signed-off-by: Alexander Graf <agraf@csgraf.de>
> Acked-by: Roman Bolshakov <r.bolshakov@yadro.com>

So, how much of the GICv3 does Hypervisor.framework expect
userspace to implement ?

Currently we have two GICv3 implementations:
 * hw/intc/arm_gicv3_kvm.c -- which is the stub device that
   handles the KVM in-kernel GICv3
 * hw/intc/arm_gicv3.c -- which is the full-emulation device
   that assumes that it is working with a TCG CPU

Support for HVF GICv3 needs either another one of these or
some serious refactoring of the full-emulation device so that
it doesn't assume that the CPU it's dealing with is a TCG one.
(I suspect the right design is to bite the bullet and make the
implementation follow the hardware in having "the GIC device proper"
and "the GIC CPU interface" separate from each other and communicating
via an API approximately equivalent to the GIC Stream Protocol
as described in the GICv3 architecture specification; but that's
a painful refactor and there might be some other approach less
invasive but still reasonably clean.)

>  static uint64_t hvf_sysreg_read(CPUState *cpu, uint32_t reg)
>  {
>      ARMCPU *arm_cpu = ARM_CPU(cpu);
> @@ -431,6 +491,39 @@ static uint64_t hvf_sysreg_read(CPUState *cpu, uint32_t reg)
>      case SYSREG_PMCCNTR_EL0:
>          val = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>          break;
> +    case SYSREG_ICC_AP0R0_EL1:
> +    case SYSREG_ICC_AP0R1_EL1:
> +    case SYSREG_ICC_AP0R2_EL1:
> +    case SYSREG_ICC_AP0R3_EL1:
> +    case SYSREG_ICC_AP1R0_EL1:
> +    case SYSREG_ICC_AP1R1_EL1:
> +    case SYSREG_ICC_AP1R2_EL1:
> +    case SYSREG_ICC_AP1R3_EL1:
> +    case SYSREG_ICC_ASGI1R_EL1:
> +    case SYSREG_ICC_BPR0_EL1:
> +    case SYSREG_ICC_BPR1_EL1:
> +    case SYSREG_ICC_DIR_EL1:
> +    case SYSREG_ICC_EOIR0_EL1:
> +    case SYSREG_ICC_EOIR1_EL1:
> +    case SYSREG_ICC_HPPIR0_EL1:
> +    case SYSREG_ICC_HPPIR1_EL1:
> +    case SYSREG_ICC_IAR0_EL1:
> +    case SYSREG_ICC_IAR1_EL1:
> +    case SYSREG_ICC_IGRPEN0_EL1:
> +    case SYSREG_ICC_IGRPEN1_EL1:
> +    case SYSREG_ICC_PMR_EL1:
> +    case SYSREG_ICC_SGI0R_EL1:
> +    case SYSREG_ICC_SGI1R_EL1:
> +    case SYSREG_ICC_SRE_EL1:
> +        val = hvf_sysreg_read_cp(cpu, reg);
> +        break;
> +    case SYSREG_ICC_CTLR_EL1:
> +        val = hvf_sysreg_read_cp(cpu, reg);
> +
> +        /* AP0R registers above 0 don't trap, expose less PRIs to fit */
> +        val &= ~ICC_CTLR_EL1_PRIBITS_MASK;
> +        val |= 4 << ICC_CTLR_EL1_PRIBITS_SHIFT;
> +        break;

Pretty sure you don't want to be trying to squeeze even the
GICv3 cpuif implementation into this source file...

>      default:
>          DPRINTF("unhandled sysreg read %08x (op0=%d op1=%d op2=%d "
>                  "crn=%d crm=%d)", reg, (reg >> 20) & 0x3,
> @@ -442,6 +535,24 @@ static uint64_t hvf_sysreg_read(CPUState *cpu, uint32_t reg)
>      return val;
>  }
>
> +static void hvf_sysreg_write_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->writefn) {
> +            ri->writefn(env, ri, val);
> +        } else {
> +            CPREG_FIELD64(env, ri) = val;
> +        }
> +        DPRINTF("vgic write to %s [val=%016llx]", ri->name, val);
> +    }
> +}
> +
>  static void hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
>  {
>      ARMCPU *arm_cpu = ARM_CPU(cpu);
> @@ -449,6 +560,36 @@ static void hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
>      switch (reg) {
>      case SYSREG_CNTPCT_EL0:
>          break;
> +    case SYSREG_ICC_AP0R0_EL1:
> +    case SYSREG_ICC_AP0R1_EL1:
> +    case SYSREG_ICC_AP0R2_EL1:
> +    case SYSREG_ICC_AP0R3_EL1:
> +    case SYSREG_ICC_AP1R0_EL1:
> +    case SYSREG_ICC_AP1R1_EL1:
> +    case SYSREG_ICC_AP1R2_EL1:
> +    case SYSREG_ICC_AP1R3_EL1:
> +    case SYSREG_ICC_ASGI1R_EL1:
> +    case SYSREG_ICC_BPR0_EL1:
> +    case SYSREG_ICC_BPR1_EL1:
> +    case SYSREG_ICC_CTLR_EL1:
> +    case SYSREG_ICC_DIR_EL1:
> +    case SYSREG_ICC_HPPIR0_EL1:
> +    case SYSREG_ICC_HPPIR1_EL1:
> +    case SYSREG_ICC_IAR0_EL1:
> +    case SYSREG_ICC_IAR1_EL1:
> +    case SYSREG_ICC_IGRPEN0_EL1:
> +    case SYSREG_ICC_IGRPEN1_EL1:
> +    case SYSREG_ICC_PMR_EL1:
> +    case SYSREG_ICC_SGI0R_EL1:
> +    case SYSREG_ICC_SGI1R_EL1:
> +    case SYSREG_ICC_SRE_EL1:
> +        hvf_sysreg_write_cp(cpu, reg, val);
> +        break;
> +    case SYSREG_ICC_EOIR0_EL1:
> +    case SYSREG_ICC_EOIR1_EL1:
> +        hvf_sysreg_write_cp(cpu, reg, val);
> +        qemu_set_irq(arm_cpu->gt_timer_outputs[GTIMER_VIRT], 0);
> +        hv_vcpu_set_vtimer_mask(cpu->hvf->fd, false);

This definitely looks wrong. Not every interrupt is
a timer interrupt, and writing to EOIR in the GIC doesn't
squelch the underlying timer irq, that should happen somewhere
else.

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index f0850ab14a..98bd6712c0 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -22,6 +22,7 @@ 
 
 #include "exec/address-spaces.h"
 #include "hw/irq.h"
+#include "hw/intc/gicv3_internal.h"
 #include "qemu/main-loop.h"
 #include "sysemu/accel.h"
 #include "sysemu/cpus.h"
@@ -46,6 +47,33 @@ 
 #define SYSREG_CNTPCT_EL0     SYSREG(3, 3, 14, 0, 1)
 #define SYSREG_PMCCNTR_EL0    SYSREG(3, 3, 9, 13, 0)
 
+#define SYSREG_ICC_AP0R0_EL1     SYSREG(3, 0, 12, 8, 4)
+#define SYSREG_ICC_AP0R1_EL1     SYSREG(3, 0, 12, 8, 5)
+#define SYSREG_ICC_AP0R2_EL1     SYSREG(3, 0, 12, 8, 6)
+#define SYSREG_ICC_AP0R3_EL1     SYSREG(3, 0, 12, 8, 7)
+#define SYSREG_ICC_AP1R0_EL1     SYSREG(3, 0, 12, 9, 0)
+#define SYSREG_ICC_AP1R1_EL1     SYSREG(3, 0, 12, 9, 1)
+#define SYSREG_ICC_AP1R2_EL1     SYSREG(3, 0, 12, 9, 2)
+#define SYSREG_ICC_AP1R3_EL1     SYSREG(3, 0, 12, 9, 3)
+#define SYSREG_ICC_ASGI1R_EL1    SYSREG(3, 0, 12, 11, 6)
+#define SYSREG_ICC_BPR0_EL1      SYSREG(3, 0, 12, 8, 3)
+#define SYSREG_ICC_BPR1_EL1      SYSREG(3, 0, 12, 12, 3)
+#define SYSREG_ICC_CTLR_EL1      SYSREG(3, 0, 12, 12, 4)
+#define SYSREG_ICC_DIR_EL1       SYSREG(3, 0, 12, 11, 1)
+#define SYSREG_ICC_EOIR0_EL1     SYSREG(3, 0, 12, 8, 1)
+#define SYSREG_ICC_EOIR1_EL1     SYSREG(3, 0, 12, 12, 1)
+#define SYSREG_ICC_HPPIR0_EL1    SYSREG(3, 0, 12, 8, 2)
+#define SYSREG_ICC_HPPIR1_EL1    SYSREG(3, 0, 12, 12, 2)
+#define SYSREG_ICC_IAR0_EL1      SYSREG(3, 0, 12, 8, 0)
+#define SYSREG_ICC_IAR1_EL1      SYSREG(3, 0, 12, 12, 0)
+#define SYSREG_ICC_IGRPEN0_EL1   SYSREG(3, 0, 12, 12, 6)
+#define SYSREG_ICC_IGRPEN1_EL1   SYSREG(3, 0, 12, 12, 7)
+#define SYSREG_ICC_PMR_EL1       SYSREG(3, 0, 4, 6, 0)
+#define SYSREG_ICC_RPR_EL1       SYSREG(3, 0, 12, 11, 3)
+#define SYSREG_ICC_SGI0R_EL1     SYSREG(3, 0, 12, 11, 7)
+#define SYSREG_ICC_SGI1R_EL1     SYSREG(3, 0, 12, 11, 5)
+#define SYSREG_ICC_SRE_EL1       SYSREG(3, 0, 12, 12, 5)
+
 #define WFX_IS_WFE (1 << 0)
 
 struct hvf_reg_match {
@@ -418,6 +446,38 @@  void hvf_kick_vcpu_thread(CPUState *cpu)
     hv_vcpus_exit(&cpu->hvf->fd, 1);
 }
 
+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);
+}
+
+static uint64_t hvf_sysreg_read_cp(CPUState *cpu, uint32_t reg)
+{
+    ARMCPU *arm_cpu = ARM_CPU(cpu);
+    CPUARMState *env = &arm_cpu->env;
+    const ARMCPRegInfo *ri;
+    uint64_t val = 0;
+
+    ri = get_arm_cp_reginfo(arm_cpu->cp_regs, hvf_reg2cp_reg(reg));
+    if (ri) {
+        if (ri->type & ARM_CP_CONST) {
+            val = ri->resetvalue;
+        } else if (ri->readfn) {
+            val = ri->readfn(env, ri);
+        } else {
+            val = CPREG_FIELD64(env, ri);
+        }
+        DPRINTF("vgic read from %s [val=%016llx]", ri->name, val);
+    }
+
+    return val;
+}
+
 static uint64_t hvf_sysreg_read(CPUState *cpu, uint32_t reg)
 {
     ARMCPU *arm_cpu = ARM_CPU(cpu);
@@ -431,6 +491,39 @@  static uint64_t hvf_sysreg_read(CPUState *cpu, uint32_t reg)
     case SYSREG_PMCCNTR_EL0:
         val = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
         break;
+    case SYSREG_ICC_AP0R0_EL1:
+    case SYSREG_ICC_AP0R1_EL1:
+    case SYSREG_ICC_AP0R2_EL1:
+    case SYSREG_ICC_AP0R3_EL1:
+    case SYSREG_ICC_AP1R0_EL1:
+    case SYSREG_ICC_AP1R1_EL1:
+    case SYSREG_ICC_AP1R2_EL1:
+    case SYSREG_ICC_AP1R3_EL1:
+    case SYSREG_ICC_ASGI1R_EL1:
+    case SYSREG_ICC_BPR0_EL1:
+    case SYSREG_ICC_BPR1_EL1:
+    case SYSREG_ICC_DIR_EL1:
+    case SYSREG_ICC_EOIR0_EL1:
+    case SYSREG_ICC_EOIR1_EL1:
+    case SYSREG_ICC_HPPIR0_EL1:
+    case SYSREG_ICC_HPPIR1_EL1:
+    case SYSREG_ICC_IAR0_EL1:
+    case SYSREG_ICC_IAR1_EL1:
+    case SYSREG_ICC_IGRPEN0_EL1:
+    case SYSREG_ICC_IGRPEN1_EL1:
+    case SYSREG_ICC_PMR_EL1:
+    case SYSREG_ICC_SGI0R_EL1:
+    case SYSREG_ICC_SGI1R_EL1:
+    case SYSREG_ICC_SRE_EL1:
+        val = hvf_sysreg_read_cp(cpu, reg);
+        break;
+    case SYSREG_ICC_CTLR_EL1:
+        val = hvf_sysreg_read_cp(cpu, reg);
+
+        /* AP0R registers above 0 don't trap, expose less PRIs to fit */
+        val &= ~ICC_CTLR_EL1_PRIBITS_MASK;
+        val |= 4 << ICC_CTLR_EL1_PRIBITS_SHIFT;
+        break;
     default:
         DPRINTF("unhandled sysreg read %08x (op0=%d op1=%d op2=%d "
                 "crn=%d crm=%d)", reg, (reg >> 20) & 0x3,
@@ -442,6 +535,24 @@  static uint64_t hvf_sysreg_read(CPUState *cpu, uint32_t reg)
     return val;
 }
 
+static void hvf_sysreg_write_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->writefn) {
+            ri->writefn(env, ri, val);
+        } else {
+            CPREG_FIELD64(env, ri) = val;
+        }
+        DPRINTF("vgic write to %s [val=%016llx]", ri->name, val);
+    }
+}
+
 static void hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
 {
     ARMCPU *arm_cpu = ARM_CPU(cpu);
@@ -449,6 +560,36 @@  static void hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
     switch (reg) {
     case SYSREG_CNTPCT_EL0:
         break;
+    case SYSREG_ICC_AP0R0_EL1:
+    case SYSREG_ICC_AP0R1_EL1:
+    case SYSREG_ICC_AP0R2_EL1:
+    case SYSREG_ICC_AP0R3_EL1:
+    case SYSREG_ICC_AP1R0_EL1:
+    case SYSREG_ICC_AP1R1_EL1:
+    case SYSREG_ICC_AP1R2_EL1:
+    case SYSREG_ICC_AP1R3_EL1:
+    case SYSREG_ICC_ASGI1R_EL1:
+    case SYSREG_ICC_BPR0_EL1:
+    case SYSREG_ICC_BPR1_EL1:
+    case SYSREG_ICC_CTLR_EL1:
+    case SYSREG_ICC_DIR_EL1:
+    case SYSREG_ICC_HPPIR0_EL1:
+    case SYSREG_ICC_HPPIR1_EL1:
+    case SYSREG_ICC_IAR0_EL1:
+    case SYSREG_ICC_IAR1_EL1:
+    case SYSREG_ICC_IGRPEN0_EL1:
+    case SYSREG_ICC_IGRPEN1_EL1:
+    case SYSREG_ICC_PMR_EL1:
+    case SYSREG_ICC_SGI0R_EL1:
+    case SYSREG_ICC_SGI1R_EL1:
+    case SYSREG_ICC_SRE_EL1:
+        hvf_sysreg_write_cp(cpu, reg, val);
+        break;
+    case SYSREG_ICC_EOIR0_EL1:
+    case SYSREG_ICC_EOIR1_EL1:
+        hvf_sysreg_write_cp(cpu, reg, val);
+        qemu_set_irq(arm_cpu->gt_timer_outputs[GTIMER_VIRT], 0);
+        hv_vcpu_set_vtimer_mask(cpu->hvf->fd, false);
     default:
         DPRINTF("unhandled sysreg write %08x", reg);
         break;