Re: [PATCH v2] KVM: arm64: Remove size-order align in the nVHE hyp private VA range

From: Marc Zyngier <maz@kernel.org>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Vincent Donnefort <vdonnefort@google.com>,
	oliver.upton@linux.dev, kvmarm@lists.linux.dev,
	qperret@google.com, smostafa@google.com, kaleshsingh@google.com,
	linux-arm-kernel@lists.infradead.org, kernel-team@android.com,
	will@kernel.org
Subject: Re: [PATCH v2] KVM: arm64: Remove size-order align in the nVHE hyp private VA range
Date: Mon, 28 Aug 2023 14:19:30 +0100	[thread overview]
Message-ID: <861qfnfgnx.wl-maz@kernel.org> (raw)
In-Reply-To: <79b0ad6e-0c2a-f777-d504-e40e8123d81d@samsung.com>

On Mon, 28 Aug 2023 13:16:51 +0100,
Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> On 11.08.2023 13:20, Vincent Donnefort wrote:
> > commit f922c13e778d ("KVM: arm64: Introduce
> > pkvm_alloc_private_va_range()") and commit 92abe0f81e13 ("KVM: arm64:
> > Introduce hyp_alloc_private_va_range()") added an alignment for the
> > start address of any allocation into the nVHE hypervisor private VA
> > range.
> >
> > This alignment (order of the size of the allocation) intends to enable
> > efficient stack verification (if the PAGE_SHIFT bit is zero, the stack
> > pointer is on the guard page and a stack overflow occurred).
> >
> > But this is only necessary for stack allocation and can waste a lot of
> > VA space. So instead make stack-specific functions, handling the guard
> > page requirements, while other users (e.g.  fixmap) will only get page
> > alignment.
> >
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> 
> This change, merged to linux-next as f156a7d13fc3 ("KVM: arm64: Remove 
> size-order align in the nVHE hyp private VA range"), introduced the 
> following regression on Raspberry Pi4b board. Here is a log observed on 
> linux next-20230828 release:
> 
> --->8---
> 
> kvm [1]: IPA Size Limit: 44 bits
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at arch/arm64/kvm/hyp/pgtable.c:453 
> hyp_map_walker+0xb0/0x120
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-next-20230828 #13889
> Hardware name: Raspberry Pi 4 Model B (DT)
> pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : hyp_map_walker+0xb0/0x120
> lr : hyp_map_walker+0x30/0x120
> ...
> Call trace:
>   hyp_map_walker+0xb0/0x120
>   kvm_pgtable_visitor_cb.isra.0+0x38/0x5c
>   __kvm_pgtable_walk+0x1dc/0x32c
>   __kvm_pgtable_walk+0xd0/0x32c
>   __kvm_pgtable_walk+0xd0/0x32c
>   __kvm_pgtable_walk+0xd0/0x32c
>   kvm_pgtable_walk+0xd0/0x224
>   kvm_pgtable_hyp_map+0x8c/0xf0
>   __create_hyp_mappings+0x98/0xc0
>   __create_hyp_private_mapping+0xf0/0x14c
>   create_hyp_io_mappings+0x8c/0x118
>   vgic_v2_probe+0xbc/0x1ec
>   kvm_vgic_hyp_init+0xcc/0x210
>   kvm_arm_init+0x6a4/0x1040

Ah, wonderful. Can you give the snippet below a go?

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 11c1d786c506..50be51cc40cc 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -652,6 +652,9 @@ int hyp_alloc_private_va_range(size_t size, unsigned long *haddr)
 
 	mutex_unlock(&kvm_hyp_pgd_mutex);
 
+	if (!ret)
+		*haddr = base;
+
 	return ret;
 }
 

Thanks,

	M.

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