Re: [PATCH v4 11/17] KVM: arm64: Filter out non-kernel addresses in kern_hyp_va
Vincent Donnefort <[email protected]>
| Newsgroups | org.infradead.lists.linux-arm-kernel,dev.linux.lists.kvmarm |
|---|---|
| Message-ID | <[email protected]> |
On Tue, Aug 18, 2026 at 03:45:23PM +0100, Fuad Tabba wrote: > Hi Vincent, > > > On Fri, 31 Jul 2026 at 15:36, Vincent Donnefort <[email protected]> wrote: > > > > kern_hyp_va() is idempotent for the hypervisor linear space. This is > > handy for nVHE hypervisor callers handling kvm_vcpu or kvm_arch > > pointers. Those pointers can originate from the hypervisor space (when > > protected mode is enabled, we don't trust the kernel and the hypervisor > > uses its own copy) or from the kernel space (we do trust the kernel in > > "non-protected" nVHE). > > > > This idempotence does not hold for addresses within the hypervisor > > private range, like the ones you get from the pKVM heap allocator > > (hyp_alloc()). To resolve this, filter out non-kernel addresses based on > > PAGE_OFFSET. > > > > Leave the assembly version untouched as it has no current users. > > > > Reviewed-by: Fuad Tabba <[email protected]> > > Tested-by: Fuad Tabba <[email protected]> > > Signed-off-by: Vincent Donnefort <[email protected]> > > > > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h > > index 6eae7e7e2a68..d60e5f2de10c 100644 > > --- a/arch/arm64/include/asm/kvm_mmu.h > > +++ b/arch/arm64/include/asm/kvm_mmu.h > > @@ -126,6 +126,9 @@ static __always_inline unsigned long __kern_hyp_va(unsigned long v) > > * replace the instructions with `nop`s. > > */ > > #ifndef __KVM_VHE_HYPERVISOR__ > > + if (!is_ttbr1_addr(v)) > > + return v; > > + > > I gave this a tag earlier and only came back to the cost side now, sorry. > > The check is unconditional, so plain nVHE pays it on every > kern_hyp_va(), including the one in __kvm_vcpu_run(), and it has no > private range to protect. > > Could it be gated on is_protected_kvm_enabled(), the same way patch 17 > gates hyp_trace_buffer_alloc_bpages()? > > Cheers, > /fuad Sure, that's really a micro optimisation though, I don't think that is_ttbr1_addr() is expensive. -- Vincent > > > > asm volatile(ALTERNATIVE_CB("and %0, %0, #1\n" /* mask with va_mask */ > > "ror %0, %0, #1\n" /* rotate to the first tag bit */ > > "add %0, %0, #0\n" /* insert the low 12 bits of the tag */ > > -- > > 2.55.0.508.g3f0d502094-goog > >