[PATCH] KVM: arm64: Properly return allocated EL2 VA from hyp_alloc_private_va_range()

[PATCH] KVM: arm64: Properly return allocated EL2 VA from hyp_alloc_private_va_range()

[Marc Zyngier]

Marek reports that his RPi4 spits out a warning at boot time,
right at the point where the GICv2 virtual CPU interface gets
mapped.

Upon investigation, it seems that we never return the allocated
VA and use whatever was on the stack at this point. Yes, this
is good stuff, and Marek was pretty lucky that he ended-up with
a VA that intersected with something that was already mapped.

On my setup, this random value is plausible enough for the mapping
to take place. Who knows what happens...

Cc: Vincent Donnefort <vdonnefort@google.com>
Fixes: f156a7d13fc3 ("KVM: arm64: Remove size-order align in the nVHE hyp private VA range")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/79b0ad6e-0c2a-f777-d504-e40e8123d81d@samsung.com
---
 arch/arm64/kvm/mmu.c | 3 +++
 1 file changed, 3 insertions(+)

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;
 }
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] KVM: arm64: Properly return allocated EL2 VA from hyp_alloc_private_va_range()

[Philippe Mathieu-Daudé]

Hi Marc,

On 28/8/23 17:31, Marc Zyngier wrote:
> Marek reports that his RPi4 spits out a warning at boot time,
> right at the point where the GICv2 virtual CPU interface gets
> mapped.
> 
> Upon investigation, it seems that we never return the allocated
> VA and use whatever was on the stack at this point. Yes, this
> is good stuff, and Marek was pretty lucky that he ended-up with
> a VA that intersected with something that was already mapped.
> 
> On my setup, this random value is plausible enough for the mapping
> to take place. Who knows what happens...
> 
> Cc: Vincent Donnefort <vdonnefort@google.com>
> Fixes: f156a7d13fc3 ("KVM: arm64: Remove size-order align in the nVHE hyp private VA range")

I don't see your kvmarm-6.6 merged by Paolo, is it too late to squash
and send a new PR? Anyhow:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Regards,

Phil.

> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/r/79b0ad6e-0c2a-f777-d504-e40e8123d81d@samsung.com
> ---
>   arch/arm64/kvm/mmu.c | 3 +++
>   1 file changed, 3 insertions(+)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] KVM: arm64: Properly return allocated EL2 VA from hyp_alloc_private_va_range()

[Marc Zyngier]

Hi Philippe,

On Mon, 28 Aug 2023 18:00:54 +0100,
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> Hi Marc,
> 
> On 28/8/23 17:31, Marc Zyngier wrote:
> > Marek reports that his RPi4 spits out a warning at boot time,
> > right at the point where the GICv2 virtual CPU interface gets
> > mapped.
> > 
> > Upon investigation, it seems that we never return the allocated
> > VA and use whatever was on the stack at this point. Yes, this
> > is good stuff, and Marek was pretty lucky that he ended-up with
> > a VA that intersected with something that was already mapped.
> > 
> > On my setup, this random value is plausible enough for the mapping
> > to take place. Who knows what happens...
> > 
> > Cc: Vincent Donnefort <vdonnefort@google.com>
> > Fixes: f156a7d13fc3 ("KVM: arm64: Remove size-order align in the nVHE hyp private VA range")
> 
> I don't see your kvmarm-6.6 merged by Paolo, is it too late to squash
> and send a new PR?

In general, I keep the commits that are in -next stable (no squashing,
no rebasing), and only the merge commits that drag these commits onto
-next are throw-away (this allows me to rebuild the whole branch
without changing any of the commit SHAs).

So no, I won't send a new PR right now. However, I'll stick the patch
in -next and will aim to send Paolo another PR later this week or
early next week.

> Anyhow:
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] KVM: arm64: Properly return allocated EL2 VA from hyp_alloc_private_va_range()

[Philippe Mathieu-Daudé]

On 28/8/23 19:16, Marc Zyngier wrote:
> Hi Philippe,
> 
> On Mon, 28 Aug 2023 18:00:54 +0100,
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Hi Marc,
>>
>> On 28/8/23 17:31, Marc Zyngier wrote:
>>> Marek reports that his RPi4 spits out a warning at boot time,
>>> right at the point where the GICv2 virtual CPU interface gets
>>> mapped.
>>>
>>> Upon investigation, it seems that we never return the allocated
>>> VA and use whatever was on the stack at this point. Yes, this
>>> is good stuff, and Marek was pretty lucky that he ended-up with
>>> a VA that intersected with something that was already mapped.
>>>
>>> On my setup, this random value is plausible enough for the mapping
>>> to take place. Who knows what happens...
>>>
>>> Cc: Vincent Donnefort <vdonnefort@google.com>
>>> Fixes: f156a7d13fc3 ("KVM: arm64: Remove size-order align in the nVHE hyp private VA range")
>>
>> I don't see your kvmarm-6.6 merged by Paolo, is it too late to squash
>> and send a new PR?
> 
> In general, I keep the commits that are in -next stable (no squashing,
> no rebasing), and only the merge commits that drag these commits onto
> -next are throw-away (this allows me to rebuild the whole branch
> without changing any of the commit SHAs).
> 
> So no, I won't send a new PR right now. However, I'll stick the patch
> in -next and will aim to send Paolo another PR later this week or
> early next week.

Ah, I didn't know. Thanks for explaining :)

Phil.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] KVM: arm64: Properly return allocated EL2 VA from hyp_alloc_private_va_range()

[Vincent Donnefort]

On Mon, Aug 28, 2023 at 04:31:21PM +0100, Marc Zyngier wrote:
> Marek reports that his RPi4 spits out a warning at boot time,
> right at the point where the GICv2 virtual CPU interface gets
> mapped.
> 
> Upon investigation, it seems that we never return the allocated
> VA and use whatever was on the stack at this point. Yes, this
> is good stuff, and Marek was pretty lucky that he ended-up with
> a VA that intersected with something that was already mapped.
> 
> On my setup, this random value is plausible enough for the mapping
> to take place. Who knows what happens...
> 
> Cc: Vincent Donnefort <vdonnefort@google.com>
> Fixes: f156a7d13fc3 ("KVM: arm64: Remove size-order align in the nVHE hyp private VA range")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/r/79b0ad6e-0c2a-f777-d504-e40e8123d81d@samsung.com

Having a hard time reproducing the issue, but clearly that set is missing from
the original patch!

Sorry about that extra work.

> ---
>  arch/arm64/kvm/mmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 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;
>  }
>  
> -- 
> 2.34.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] KVM: arm64: Properly return allocated EL2 VA from hyp_alloc_private_va_range()

[Vincent Donnefort]

On Tue, Aug 29, 2023 at 09:03:05AM +0100, Vincent Donnefort wrote:
> On Mon, Aug 28, 2023 at 04:31:21PM +0100, Marc Zyngier wrote:
> > Marek reports that his RPi4 spits out a warning at boot time,
> > right at the point where the GICv2 virtual CPU interface gets
> > mapped.
> > 
> > Upon investigation, it seems that we never return the allocated
> > VA and use whatever was on the stack at this point. Yes, this
> > is good stuff, and Marek was pretty lucky that he ended-up with
> > a VA that intersected with something that was already mapped.
> > 
> > On my setup, this random value is plausible enough for the mapping
> > to take place. Who knows what happens...
> > 
> > Cc: Vincent Donnefort <vdonnefort@google.com>
> > Fixes: f156a7d13fc3 ("KVM: arm64: Remove size-order align in the nVHE hyp private VA range")
> > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Link: https://lore.kernel.org/r/79b0ad6e-0c2a-f777-d504-e40e8123d81d@samsung.com
> 
> Having a hard time reproducing the issue, but clearly that set is missing from
> the original patch!
> 
> Sorry about that extra work.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>

> 
> > ---
> >  arch/arm64/kvm/mmu.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > 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;
> >  }
> >  
> > -- 
> > 2.34.1
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel