Re: [PATCH v2 14/17] KVM: arm64: Reject host access to protected VM private state

[Marc Zyngier]

On Mon, 07 Sep 2026 07:59:59 +0100,
Fuad Tabba <fuad.tabba@xxxxxxxxx> wrote:
> 
> A protected vCPU's register and debug state is no longer exposed to
> the host. Host ioctls that would reach that state now fail rather
> than operate on a copy that is not the guest's:
> 
>   - KVM_GET_ONE_REG and KVM_SET_ONE_REG return -EPERM once the vCPU has
>     run: the copy then holds reset values plus what the exit handlers
>     marshal out. Pre-run access still builds the guest's boot state.
>   - KVM_ARM_VCPU_INIT returns -EPERM once the vCPU has run: it would
>     reset the host copy alone and rewrite mp_state, which EL2 reads
>     only at hyp vCPU creation, so a vCPU the guest powered off would
>     come back RUNNABLE.
>   - KVM_SET_VCPU_EVENTS rejects external-abort injection with -EPERM;
>     SError injection is forwarded and stays permitted.
>   - KVM_SET_GUEST_DEBUG returns -EPERM: a protected guest's debug state
>     is hypervisor-owned.
> 
> Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> ---
>  arch/arm64/kvm/arm.c   |  7 +++++++
>  arch/arm64/kvm/guest.c | 19 +++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 5c2587da815ab..37a260b139795 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1750,6 +1750,13 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
>  	bool power_off = false;
>  	int ret;
>  
> +	/*
> +	 * Re-init would reset the host copy alone and rewrite mp_state
> +	 * after EL2 has read it. Pre-run init stays allowed.
> +	 */
> +	if (vcpu_is_protected(vcpu) && vcpu_has_run_once(vcpu))
> +		return -EPERM;
> +
>  	/*
>  	 * Treat the power-off vCPU feature as ephemeral. Clear the bit to avoid
>  	 * reflecting it in the finalized feature set, thus limiting its scope
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index ab659795d4b2f..abab580a659d4 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -701,6 +701,10 @@ int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
>  
>  int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  {
> +	/* Once the vCPU has run, the host copy is not the guest's state. */
> +	if (vcpu_is_protected(vcpu) && vcpu_has_run_once(vcpu))
> +		return -EPERM;
> +
>  	/* We currently use nothing arch-specific in upper 32 bits */
>  	if ((reg->id & ~KVM_REG_SIZE_MASK) >> 32 != KVM_REG_ARM64 >> 32)
>  		return -EINVAL;
> @@ -718,6 +722,10 @@ int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  
>  int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  {
> +	/* Writes build the boot state; once run, EL2 owns the registers. */
> +	if (vcpu_is_protected(vcpu) && vcpu_has_run_once(vcpu))
> +		return -EPERM;
> +

Why can't these be moved to the ioctl callback? Something like:

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index eaf583b771931..0ffd435bf6904 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1856,6 +1856,19 @@ static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
 	return __kvm_arm_vcpu_set_events(vcpu, events);
 }
 
+static long pkvm_filter_vcpu_ioctl(struct kvm_vcpu *vcpu, unsigned int ioctl)
+{
+	switch (ioctl) {
+	case KVM_ARM_VCPU_INIT:
+	case KVM_SET_ONE_REG:
+	case KVM_GET_ONE_REG:
+		if (vcpu_is_protected(vcpu) && vcpu_has_run_once(vcpu))
+			return -EPERM;
+	}
+
+	return 0;
+}
+
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
 {
@@ -1864,6 +1877,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 	struct kvm_device_attr attr;
 	long r;
 
+	r = pvkm_filter_vcpu_ioctl(vcpu, ioctl);
+	if (r)
+		return r;
+
 	switch (ioctl) {
 	case KVM_ARM_VCPU_INIT: {
 		struct kvm_vcpu_init init;

>  	/* We currently use nothing arch-specific in upper 32 bits */
>  	if ((reg->id & ~KVM_REG_SIZE_MASK) >> 32 != KVM_REG_ARM64 >> 32)
>  		return -EINVAL;
> @@ -786,6 +794,13 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
>  	u64 esr = events->exception.serror_esr;
>  	int ret = 0;
>  
> +	/*
> +	 * EL2 injects an external abort only to complete a forwarded abort.
> +	 * SError injection is forwarded.
> +	 */
> +	if (vcpu_is_protected(vcpu) && ext_dabt_pending)
> +		return -EPERM;
> +

Where is KVM_CAP_ARM_INJECT_EXT_DABT rejected?

Thanks,

	M.

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