Re: [PATCH v4 12/18] KVM: arm64: Save protected-nVHE (pKVM) hyp stacktrace

From: Marc Zyngier <maz@kernel.org>
To: Kalesh Singh <kaleshsingh@google.com>
Cc: mark.rutland@arm.com, broonie@kernel.org,
	madvenka@linux.microsoft.com, will@kernel.org,
	qperret@google.com, tabba@google.com, james.morse@arm.com,
	alexandru.elisei@arm.com, suzuki.poulose@arm.com,
	catalin.marinas@arm.com, andreyknvl@gmail.com,
	russell.king@oracle.com, vincenzo.frascino@arm.com,
	mhiramat@kernel.org, ast@kernel.org, drjones@redhat.com,
	wangkefeng.wang@huawei.com, elver@google.com, keirf@google.com,
	yuzenghui@huawei.com, ardb@kernel.org, oupton@google.com,
	linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org,
	android-mm@google.com, kernel-team@android.com
Subject: Re: [PATCH v4 12/18] KVM: arm64: Save protected-nVHE (pKVM) hyp stacktrace
Date: Mon, 18 Jul 2022 10:36:07 +0100	[thread overview]
Message-ID: <87tu7ezrso.wl-maz@kernel.org> (raw)
In-Reply-To: <20220715061027.1612149-13-kaleshsingh@google.com>

On Fri, 15 Jul 2022 07:10:21 +0100,
Kalesh Singh <kaleshsingh@google.com> wrote:
> 
> In protected nVHE mode, the host cannot access private owned hypervisor
> memory. Also the hypervisor aims to remains simple to reduce the attack
> surface and does not provide any printk support.
> 
> For the above reasons, the approach taken to provide hypervisor stacktraces
> in protected mode is:
>    1) Unwind and save the hyp stack addresses in EL2 to a shared buffer
>       with the host (done in this patch).
>    2) Delegate the dumping and symbolization of the addresses to the
>       host in EL1 (later patch in the series).
> 
> Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
> ---
>  arch/arm64/include/asm/stacktrace/nvhe.h | 18 ++++++
>  arch/arm64/kvm/hyp/nvhe/stacktrace.c     | 70 ++++++++++++++++++++++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/stacktrace/nvhe.h b/arch/arm64/include/asm/stacktrace/nvhe.h
> index 36cf7858ddd8..456a6ae08433 100644
> --- a/arch/arm64/include/asm/stacktrace/nvhe.h
> +++ b/arch/arm64/include/asm/stacktrace/nvhe.h
> @@ -21,6 +21,22 @@
>  
>  #include <asm/stacktrace/common.h>
>  
> +/**
> + * kvm_nvhe_unwind_init - Start an unwind from the given nVHE HYP fp and pc
> + *
> + * @fp : frame pointer at which to start the unwinding.
> + * @pc : program counter at which to start the unwinding.
> + */
> +static __always_inline void kvm_nvhe_unwind_init(struct unwind_state *state,
> +						 unsigned long fp,
> +						 unsigned long pc)
> +{
> +	unwind_init_common(state, NULL);

Huh. Be careful here. This function is only 'inline', which means it
may not be really inlined. We've had tons of similar issues like this
in the past, and although this will not break at runtime anymore, it
will definitely stop the kernel from linking.

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