All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: "Fuad Tabba" <tabba@google.com>,
	kvmarm@lists.linux.dev, "Oliver Upton" <oupton@kernel.org>
Subject: Re: [PATCH v2 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2
Date: Fri, 19 Jun 2026 08:41:29 +0100	[thread overview]
Message-ID: <86bjd7rn86.wl-maz@kernel.org> (raw)
In-Reply-To: <20260619072121.395E71F000E9@smtp.kernel.org>

On Fri, 19 Jun 2026 08:21:20 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] flush_hyp_vgic_state() fails to copy vgic_vmcr from the host vCPU to the hyp vCPU, causing host-side VMCR updates to be silently lost and subsequently clobbered.
> 
> Pre-existing issues:
> - [High] flush_hyp_vcpu() unconditionally copies mdcr_el2 from the untrusted host vCPU, allowing the host to control debug trap configurations for protected VMs.
> --
> 
> commit 5eaa1440f7e2f022736f913bec34aaa0cbd53a5d
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Add primitives to flush/sync the VGIC state at EL2
> 
> pKVM performs its own world switch for protected VMs but has no primitives
> to move the per-vCPU VGIC state between the host and hypervisor vCPU
> contexts. Add flush_hyp_vgic_state() and sync_hyp_vgic_state() to handle
> the required copying.
> 
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index f3233ee343a39..2db56146ec493 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> [ ... ]
> > @@ -104,6 +104,45 @@ static void fpsimd_sve_sync(struct kvm_vcpu *vcpu)
> >  	*host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
> >  }
> >  
> > +static void flush_hyp_vgic_state(struct pkvm_hyp_vcpu *hyp_vcpu)
> > +{
> > +	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
> > +	struct vgic_v3_cpu_if *host_cpu_if, *hyp_cpu_if;
> > +	unsigned int used_lrs, i;
> > +
> > +	host_cpu_if	= &host_vcpu->arch.vgic_cpu.vgic_v3;
> > +	hyp_cpu_if	= &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
> > +
> > +	used_lrs	= host_cpu_if->used_lrs;
> > +	used_lrs	= min(used_lrs, hyp_gicv3_nr_lr);
> > +
> > +	hyp_cpu_if->vgic_hcr	= host_cpu_if->vgic_hcr;
> > +	/* Should be a one-off */
> 
> [Severity: High]
> Does flush_hyp_vgic_state() need to copy vgic_vmcr from the host vCPU to the
> hyp vCPU here?
> 
> For GICv2-on-GICv3 emulation, guest writes to GICC_VMCR and GICC_CTLR trap
> to the host (EL1) as MMIO.

GICC_VMCR doesn't exist, so the guest can't write to it. There is no
MMIO involved with pKVM, since it is GICv3 only. And the whole point
of having a VGIC is to avoid trapping on each access.

More importantly, VMCR is not allowed to change behind the guest's
back, because it contains the state that the *guest* has programmed
there.

Can someone *please* unplug Sashiko? I've had enough of it.

	M.

-- 
Without deviation from the norm, progress is not possible.

  reply	other threads:[~2026-06-19  7:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-19  7:07 [PATCH v2 0/8] KVM: arm64: Rework pKVM vCPU state synchronisation Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 1/8] KVM: arm64: Extract MPIDR computation into a shared header Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 3/8] KVM: arm64: Factor out reusable vCPU reset helpers Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 4/8] KVM: arm64: Move PSCI helper functions to a shared header Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 5/8] KVM: arm64: Add host and hypervisor vCPU lookup primitives Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch Fuad Tabba
2026-06-19  7:25   ` sashiko-bot
2026-06-19  7:38     ` Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2 Fuad Tabba
2026-06-19  7:21   ` sashiko-bot
2026-06-19  7:41     ` Marc Zyngier [this message]
2026-06-19  7:45       ` Fuad Tabba
2026-06-19  7:07 ` [PATCH v2 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests Fuad Tabba
2026-06-19  7:28   ` sashiko-bot
2026-06-19  7:54     ` Fuad Tabba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86bjd7rn86.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tabba@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.