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

[Bradley Morgan]

On July 21, 2026 1:24:04 PM GMT+01:00, Fuad Tabba <fuad.tabba@xxxxxxxxx>
wrote:
>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.

Don't worry, I mean. It could be back ported to ACK, right? Or anything
against it?

If you want I'll have to sign up for the gerrit and I'll do the backports


>>
>> >---
>> > 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.

hmm. ok fair enough, this comment opinion is just my opinion anyway, not a
blocker or anything.


>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!
>

Thanks!