diff mbox series

hvf: arm: Add support for GICv3

Message ID 20221219220808.26392-1-agraf@csgraf.de (mailing list archive)
State New, archived
Headers show
Series hvf: arm: Add support for GICv3 | expand

Commit Message

Alexander Graf Dec. 19, 2022, 10:08 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 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(-)

Comments

Zenghui Yu Dec. 20, 2022, 7:14 a.m. UTC | #1
On 2022/12/20 6:08, Alexander Graf wrote:
> We currently only support GICv2 emulation.

Before looking into it, I think it's worth finalizing the GIC version in
the hvf case - only v2 is allowed and fail early if user selects the
unsupported versions. Currently finalize_gic_version() does not deal
with hvf at all.
Alexander Graf Dec. 20, 2022, 7:37 a.m. UTC | #2
Hi Zenghui,

On 20.12.22 08:14, Zenghui Yu wrote:
> On 2022/12/20 6:08, Alexander Graf wrote:
>> We currently only support GICv2 emulation.
>
> Before looking into it, I think it's worth finalizing the GIC version in
> the hvf case - only v2 is allowed and fail early if user selects the
> unsupported versions. Currently finalize_gic_version() does not deal
> with hvf at all.


Currently finalize_gic_version() treats HVF the same as TCG, which is 
incorrect. However, with this patch applied, they happen to match.

I don't think it's worth changing the finalize_gic_version() 
implementation to reflect the gicv2 only state for HVF. Instead, let's 
rather get GICv3 support in and then add explicit handling for HVF on top.

Alex
Zenghui Yu Dec. 20, 2022, 2:13 p.m. UTC | #3
On 2022/12/20 15:37, Alexander Graf wrote:
> Hi Zenghui,
> 
> On 20.12.22 08:14, Zenghui Yu wrote:
>> On 2022/12/20 6:08, Alexander Graf wrote:
>>> We currently only support GICv2 emulation.
>>
>> Before looking into it, I think it's worth finalizing the GIC version in
>> the hvf case - only v2 is allowed and fail early if user selects the
>> unsupported versions. Currently finalize_gic_version() does not deal
>> with hvf at all.
> 
> 
> Currently finalize_gic_version() treats HVF the same as TCG, which is 
> incorrect. However, with this patch applied, they happen to match.
> 
> I don't think it's worth changing the finalize_gic_version() 
> implementation to reflect the gicv2 only state for HVF. Instead, let's 
> rather get GICv3 support in and then add explicit handling for HVF on top.

I'm fine with it. I was just a bit confused when the guest got stuck
with a (blindly selected) gic-version but *no* error_report like "Hey,
please do not try GIC version other than v2" was printed. Apparently I'm
not familiar with how things work on my M1. ;-)

I'll have a try with this patch. Thanks.
Peter Maydell Jan. 6, 2023, 4:37 p.m. UTC | #4
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
Alexander Graf Jan. 28, 2023, 10:32 p.m. UTC | #5
On 06.01.23 17:37, Peter Maydell wrote:
> 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));


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
Joelle van Dyne March 10, 2023, 4:55 a.m. UTC | #6
On Mon, Dec 19, 2022 at 2:08 PM 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>
> ---

Tested-by: Joelle van Dyne <j@getutm.app>
Peter Maydell March 10, 2023, 9:49 a.m. UTC | #7
On Fri, 10 Mar 2023 at 04:55, Joelle van Dyne <j@getutm.app> wrote:
>
> On Mon, Dec 19, 2022 at 2:08 PM 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>
> > ---
>
> Tested-by: Joelle van Dyne <j@getutm.app>

Just FYI, this patch is already upstream (commit a2260983c6553).

-- PMM
diff mbox series

Patch

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 */
+            arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);
+        }
     }
 }
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 060aa0ccf4..8ea4be5f30 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -80,6 +80,33 @@ 
 #define SYSREG_PMCCNTR_EL0    SYSREG(3, 3, 9, 13, 0)
 #define SYSREG_PMCCFILTR_EL0  SYSREG(3, 3, 14, 15, 7)
 
+#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)
 
 #define TMR_CTL_ENABLE  (1 << 0)
@@ -788,6 +815,43 @@  static bool is_id_sysreg(uint32_t reg)
            SYSREG_CRM(reg) < 8;
 }
 
+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 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;
+    }
+
+    return false;
+}
+
 static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
 {
     ARMCPU *arm_cpu = ARM_CPU(cpu);
@@ -839,6 +903,36 @@  static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
     case SYSREG_OSDLR_EL1:
         /* Dummy register */
         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:
+    case SYSREG_ICC_CTLR_EL1:
+        /* Call the TCG sysreg handler. This is only safe for GICv3 regs. */
+        if (!hvf_sysreg_read_cp(cpu, reg, &val)) {
+            hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
+        }
+        break;
     default:
         if (is_id_sysreg(reg)) {
             /* ID system registers read as RES0 */
@@ -944,6 +1038,33 @@  static void pmswinc_write(CPUARMState *env, uint64_t value)
     }
 }
 
+static bool 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->accessfn) {
+            if (ri->accessfn(env, ri, false) != CP_ACCESS_OK) {
+                return false;
+            }
+        }
+        if (ri->writefn) {
+            ri->writefn(env, ri, val);
+        } else {
+            CPREG_FIELD64(env, ri) = val;
+        }
+
+        trace_hvf_vgic_write(ri->name, val);
+        return true;
+    }
+
+    return false;
+}
+
 static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
 {
     ARMCPU *arm_cpu = ARM_CPU(cpu);
@@ -1021,6 +1142,36 @@  static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
     case SYSREG_OSDLR_EL1:
         /* Dummy register */
         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_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:
+        /* Call the TCG sysreg handler. This is only safe for GICv3 regs. */
+        if (!hvf_sysreg_write_cp(cpu, reg, val)) {
+            hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
+        }
+        break;
     default:
         cpu_synchronize_state(cpu);
         trace_hvf_unhandled_sysreg_write(env->pc, reg,
diff --git a/target/arm/hvf/trace-events b/target/arm/hvf/trace-events
index 820e8e0297..4fbbe4b45e 100644
--- a/target/arm/hvf/trace-events
+++ b/target/arm/hvf/trace-events
@@ -9,3 +9,5 @@  hvf_unknown_hvc(uint64_t x0) "unknown HVC! 0x%016"PRIx64
 hvf_unknown_smc(uint64_t x0) "unknown SMC! 0x%016"PRIx64
 hvf_exit(uint64_t syndrome, uint32_t ec, uint64_t pc) "exit: 0x%"PRIx64" [ec=0x%x pc=0x%"PRIx64"]"
 hvf_psci_call(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3, uint32_t cpuid) "PSCI Call x0=0x%016"PRIx64" x1=0x%016"PRIx64" x2=0x%016"PRIx64" x3=0x%016"PRIx64" cpu=0x%x"
+hvf_vgic_write(const char *name, uint64_t val) "vgic write to %s [val=0x%016"PRIx64"]"
+hvf_vgic_read(const char *name, uint64_t val) "vgic read from %s [val=0x%016"PRIx64"]"