Re: [PATCH v5 6/7] KVM: arm64: Don't advertise eager page splitting under pKVM

[Fuad Tabba]

Hi Bradley,

On Tue, 21 Jul 2026 at 12:19, Bradley Morgan <include@xxxxxxxxx> wrote:
>
> On July 17, 2026 2:03:16 PM GMT+01:00, Fuad Tabba <fuad.tabba@xxxxxxxxx>
> wrote:
> >Under pKVM the stage-2 walker resolves to pkvm_pgtable_stage2_split(), a
> >WARN_ON_ONCE(1) stub, yet KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE is still
> >enabled and reported for non-protected guests: the capability check keys
> >on the per-VM protected state while the walker dispatch keys on the
> >host-global mode. Enabling the cap and then dirty-logging the guest
> >reaches the stub, splatting a userspace-reachable WARN.
> >
> >Reject the capability, and stop reporting a chunk size and the
> >supported block sizes, for every VM once pKVM is enabled, keyed on the
> >host-global mode like the split dispatch. Gating only protected VMs
> >would leave the non-protected guests that reach the stub still able to
> >enable it. Userspace decides whether eager splitting is available from
> >the block-size bitmap (QEMU falls back to no eager splitting when it
> >reads 0), so leaving it advertised steers an explicit request into the
> >enable failure instead of the fallback.
> >
> >Fixes: e912efed485a ("KVM: arm64: Introduce the EL1 pKVM MMU")
> >Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
>
> Do you think CC stable would be good? I do but sometimes I'm
> embarrassingly wrong

I'd lean against it here. The "Fixes" commit has been in mainline
since v6.14, the symptom is a WARN rather than a crash or data
corruption.

>
> >---
> > arch/arm64/include/asm/kvm_pkvm.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/arch/arm64/include/asm/kvm_pkvm.h b/arch/arm64/include/asm/kvm_pkvm.h
> >index 57afb07d6b13..beea00e693a0 100644
> >--- a/arch/arm64/include/asm/kvm_pkvm.h
> >+++ b/arch/arm64/include/asm/kvm_pkvm.h
> >@@ -45,6 +45,9 @@ static inline bool kvm_pkvm_ext_allowed(struct kvm *kvm, long ext)
> >               return true;
> >       case KVM_CAP_ARM_MTE:
> >               return false;
> >+      case KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE:
> >+      case KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES:
> >+              return false;
>
>
> Nit: KVM_CAP_ARM_MTE right above already returns false, fold the two
> new cases into that and drop the duplicate return. And please put a small
> comment saying eager splitting is unimplemented under pKVM (WARN
> stub), so nobody reads the list later and "fixes" it back, here's a
> example, feel free to bikeshed

I'd prefer to keep the separate return. The grouping is deliberate:
MTE and eager splitting are unrelated features, and the two eager
splitting caps belong together. The function is self-explanatory from
its name and header comment, so I don't think an inline comment is
needed.

Thanks for the reviews!
/fuad

>
>
>         case KVM_CAP_ARM_MTE:
>         /* eager splitting is unimplemented in pKVM, please don't refix this */
>         case KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE:
>         case KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES:
>                 return false;
>         default:
>                 return !kvm || !kvm_vm_is_protected(kvm);
>         }
>
>
>
>
> Killing the reporting too is right imo. Qemu reads a 0 block size
> bitmap and quietly falls back, an explicit enable eating EINVAL is the
> worse failure mode. Good.
>
> if u choose not to, it's ok, please add
>
> Reviewed-by: Bradley Morgan
> <include@xxxxxxxxx>
>
>
> >       default:
> >               return !kvm || !kvm_vm_is_protected(kvm);
> >       }
> >
>
> Thanks!