Re: [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1

On Thu, 6 Aug 2026 at 11:16, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Missing Context Synchronization Event (CSE) after writing to SYS_ICC_ICSR_EL1.

This runs on the guest-entry path, and the ERET into the guest is a
context synchronization event.

I really thought we've covered this in the review-prompts, I'll look
into this later...
/fuad

> --
>
> --- Patch [3]: [PATCH v1 3/4] KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1 ---
> commit 5b0cdbb37754741e9006b6abdf408b239ed6b887
> Author: Fuad Tabba <[email protected]>
>
> KVM: arm64: vgic: Do not access the GICv5 CPU interface from EL1
>
> This commit fixes an issue where the GICv5 CPU interface was incorrectly
> accessed from EL1 on systems without FEAT_GCIE_LEGACY. It restricts GICv5
> access to VHE, similar to the existing GICv3 handling.
>
> > diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> > index 74bace10a22ed..df58c0042ed63 100644
> > --- a/arch/arm64/kvm/vgic/vgic.c
> > +++ b/arch/arm64/kvm/vgic/vgic.c
> > @@ -1042,11 +1042,15 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
> >  static inline bool can_access_vgic_from_kernel(void)
> >  {
> >       /*
> > -      * GICv2 can always be accessed from the kernel because it is
> > -      * memory-mapped, and VHE systems can access GICv3 EL2 system
> > -      * registers.
> > +      * GICv3 and GICv5 drive the CPU interface through EL2 system
> > +      * registers, so only VHE reaches them from the kernel. GICv2 is
> > +      * memory-mapped and always reachable.
> >        */
> > -     return !static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif) || has_vhe();
> > +     if (kvm_vgic_global_state.type == VGIC_V5 ||
> > +         static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
> > +             return has_vhe();
> > +
> > +     return true;
> >  }
>
> While reviewing the GICv5 access paths related to this change, I noticed a
> related issue in the GICv5 context restore path:
>
> arch/arm64/kvm/hyp/vgic-v5-sr.c:__vgic_v5_restore_state() {
>     write_sysreg_s(cpu_if->vgic_icsr, SYS_ICC_ICSR_EL1);
> }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this write to
> SYS_ICC_ICSR_EL1 require a subsequent isb() instruction?
>
> According to the ARM64 KVM architectural rules, any write to an ICC_*_EL1
> register (excluding ICC_PMR_EL1) must be immediately followed by an isb()
> as the very next instruction.
>
> Because __vgic_v5_restore_state() writes to SYS_ICC_ICSR_EL1 and then
> returns, the caller will execute subsequent instructions without a Context
> Synchronization Event. Could this leave the CPU operating with an
> architecturally undefined pipeline state during the vCPU switch?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3