| Message ID | 20220715061027.1612149-9-kaleshsingh@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM nVHE Hypervisor stack unwinder | expand |
[- Drew and android-mm, as both addresses bounce] On Fri, 15 Jul 2022 07:10:17 +0100, Kalesh Singh <kaleshsingh@google.com> wrote: > > This can be used to disable stacktrace for the protected KVM > nVHE hypervisor, in order to save on the associated memory usage. > > This option is disabled by default, since protected KVM is not widely > used on platforms other than Android currently. > > Signed-off-by: Kalesh Singh <kaleshsingh@google.com> > --- > arch/arm64/kvm/Kconfig | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig > index 8a5fbbf084df..1edab6f8a3b8 100644 > --- a/arch/arm64/kvm/Kconfig > +++ b/arch/arm64/kvm/Kconfig > @@ -46,6 +46,21 @@ menuconfig KVM > > If unsure, say N. > > +config PROTECTED_NVHE_STACKTRACE > + bool "Protected KVM hypervisor stacktraces" > + depends on KVM > + default n > + help > + Say Y here to enable pKVM hypervisor stacktraces on hyp_panic() > + > + If you are not using protected nVHE (pKVM), say N. > + > + If using protected nVHE mode, but cannot afford the associated > + memory cost (less than 0.75 page per CPU) of pKVM stacktraces, > + say N. > + > + If unsure, say N. > + Can we make this depend on NVHE_EL2_DEBUG instead? I'd like to keep the disclosing of EL2 information in protected mode a strict debug feature. > config NVHE_EL2_DEBUG > bool "Debug mode for non-VHE EL2 object" > depends on KVM Thanks, M.
On Sun, Jul 17, 2022 at 11:56 PM Marc Zyngier <maz@kernel.org> wrote: > > [- Drew and android-mm, as both addresses bounce] > > On Fri, 15 Jul 2022 07:10:17 +0100, > Kalesh Singh <kaleshsingh@google.com> wrote: > > > > This can be used to disable stacktrace for the protected KVM > > nVHE hypervisor, in order to save on the associated memory usage. > > > > This option is disabled by default, since protected KVM is not widely > > used on platforms other than Android currently. > > > > Signed-off-by: Kalesh Singh <kaleshsingh@google.com> > > --- > > arch/arm64/kvm/Kconfig | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig > > index 8a5fbbf084df..1edab6f8a3b8 100644 > > --- a/arch/arm64/kvm/Kconfig > > +++ b/arch/arm64/kvm/Kconfig > > @@ -46,6 +46,21 @@ menuconfig KVM > > > > If unsure, say N. > > > > +config PROTECTED_NVHE_STACKTRACE > > + bool "Protected KVM hypervisor stacktraces" > > + depends on KVM > > + default n > > + help > > + Say Y here to enable pKVM hypervisor stacktraces on hyp_panic() > > + > > + If you are not using protected nVHE (pKVM), say N. > > + > > + If using protected nVHE mode, but cannot afford the associated > > + memory cost (less than 0.75 page per CPU) of pKVM stacktraces, > > + say N. > > + > > + If unsure, say N. > > + > > Can we make this depend on NVHE_EL2_DEBUG instead? I'd like to keep > the disclosing of EL2 information in protected mode a strict debug > feature. Hi Marc, An earlier version was similar to what you propose. The unwinding depended on NVHE_EL2_DEBUG and all unwinding was done from EL1 with host stage 2 being disabled. The reason the design was changed is because Android expressed the need for pKVM hyp stacktraces in production environments. [1] [1] https://lore.kernel.org/all/CAC_TJveNRaDFcQGo9-eqKa3=1DnuVDs4U+ye795VcJ1ozVkMyg@mail.gmail.com/ --Kalesh > > > config NVHE_EL2_DEBUG > > bool "Debug mode for non-VHE EL2 object" > > depends on KVM > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
On Mon, 18 Jul 2022 18:03:30 +0100, Kalesh Singh <kaleshsingh@google.com> wrote: > > On Sun, Jul 17, 2022 at 11:56 PM Marc Zyngier <maz@kernel.org> wrote: > > > > [- Drew and android-mm, as both addresses bounce] > > > > On Fri, 15 Jul 2022 07:10:17 +0100, > > Kalesh Singh <kaleshsingh@google.com> wrote: > > > > > > This can be used to disable stacktrace for the protected KVM > > > nVHE hypervisor, in order to save on the associated memory usage. > > > > > > This option is disabled by default, since protected KVM is not widely > > > used on platforms other than Android currently. > > > > > > Signed-off-by: Kalesh Singh <kaleshsingh@google.com> > > > --- > > > arch/arm64/kvm/Kconfig | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig > > > index 8a5fbbf084df..1edab6f8a3b8 100644 > > > --- a/arch/arm64/kvm/Kconfig > > > +++ b/arch/arm64/kvm/Kconfig > > > @@ -46,6 +46,21 @@ menuconfig KVM > > > > > > If unsure, say N. > > > > > > +config PROTECTED_NVHE_STACKTRACE > > > + bool "Protected KVM hypervisor stacktraces" > > > + depends on KVM > > > + default n > > > + help > > > + Say Y here to enable pKVM hypervisor stacktraces on hyp_panic() > > > + > > > + If you are not using protected nVHE (pKVM), say N. > > > + > > > + If using protected nVHE mode, but cannot afford the associated > > > + memory cost (less than 0.75 page per CPU) of pKVM stacktraces, > > > + say N. > > > + > > > + If unsure, say N. > > > + > > > > Can we make this depend on NVHE_EL2_DEBUG instead? I'd like to keep > > the disclosing of EL2 information in protected mode a strict debug > > feature. > > Hi Marc, > > An earlier version was similar to what you propose. The unwinding > depended on NVHE_EL2_DEBUG and all unwinding was done from EL1 with > host stage 2 being disabled. The reason the design was changed is > because Android expressed the need for pKVM hyp stacktraces in > production environments. [1] I think that's an Android-specific requirement that doesn't apply to upstream. If Android wants to enable this in production (and potentially leak details of the hypervisor address space), that's Android's business, and they can carry a patch for that. Upstream shouldn't have to cater for such a thing. Thanks, M.
On Tue, Jul 19, 2022 at 3:35 AM Marc Zyngier <maz@kernel.org> wrote: > > On Mon, 18 Jul 2022 18:03:30 +0100, > Kalesh Singh <kaleshsingh@google.com> wrote: > > > > On Sun, Jul 17, 2022 at 11:56 PM Marc Zyngier <maz@kernel.org> wrote: > > > > > > [- Drew and android-mm, as both addresses bounce] > > > > > > On Fri, 15 Jul 2022 07:10:17 +0100, > > > Kalesh Singh <kaleshsingh@google.com> wrote: > > > > > > > > This can be used to disable stacktrace for the protected KVM > > > > nVHE hypervisor, in order to save on the associated memory usage. > > > > > > > > This option is disabled by default, since protected KVM is not widely > > > > used on platforms other than Android currently. > > > > > > > > Signed-off-by: Kalesh Singh <kaleshsingh@google.com> > > > > --- > > > > arch/arm64/kvm/Kconfig | 15 +++++++++++++++ > > > > 1 file changed, 15 insertions(+) > > > > > > > > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig > > > > index 8a5fbbf084df..1edab6f8a3b8 100644 > > > > --- a/arch/arm64/kvm/Kconfig > > > > +++ b/arch/arm64/kvm/Kconfig > > > > @@ -46,6 +46,21 @@ menuconfig KVM > > > > > > > > If unsure, say N. > > > > > > > > +config PROTECTED_NVHE_STACKTRACE > > > > + bool "Protected KVM hypervisor stacktraces" > > > > + depends on KVM > > > > + default n > > > > + help > > > > + Say Y here to enable pKVM hypervisor stacktraces on hyp_panic() > > > > + > > > > + If you are not using protected nVHE (pKVM), say N. > > > > + > > > > + If using protected nVHE mode, but cannot afford the associated > > > > + memory cost (less than 0.75 page per CPU) of pKVM stacktraces, > > > > + say N. > > > > + > > > > + If unsure, say N. > > > > + > > > > > > Can we make this depend on NVHE_EL2_DEBUG instead? I'd like to keep > > > the disclosing of EL2 information in protected mode a strict debug > > > feature. > > > > Hi Marc, > > > > An earlier version was similar to what you propose. The unwinding > > depended on NVHE_EL2_DEBUG and all unwinding was done from EL1 with > > host stage 2 being disabled. The reason the design was changed is > > because Android expressed the need for pKVM hyp stacktraces in > > production environments. [1] > > I think that's an Android-specific requirement that doesn't apply to > upstream. If Android wants to enable this in production (and > potentially leak details of the hypervisor address space), that's > Android's business, and they can carry a patch for that. Upstream > shouldn't have to cater for such a thing. Hi Marc, For android it's important to be able to debug issues from the field. But I agree no need to subject upstream to the same requirements. I'll guard this with the NVHE_EL2_DEBUG config in the next version. Thanks, Kalesh > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig index 8a5fbbf084df..1edab6f8a3b8 100644 --- a/arch/arm64/kvm/Kconfig +++ b/arch/arm64/kvm/Kconfig @@ -46,6 +46,21 @@ menuconfig KVM If unsure, say N. +config PROTECTED_NVHE_STACKTRACE + bool "Protected KVM hypervisor stacktraces" + depends on KVM + default n + help + Say Y here to enable pKVM hypervisor stacktraces on hyp_panic() + + If you are not using protected nVHE (pKVM), say N. + + If using protected nVHE mode, but cannot afford the associated + memory cost (less than 0.75 page per CPU) of pKVM stacktraces, + say N. + + If unsure, say N. + config NVHE_EL2_DEBUG bool "Debug mode for non-VHE EL2 object" depends on KVM
This can be used to disable stacktrace for the protected KVM nVHE hypervisor, in order to save on the associated memory usage. This option is disabled by default, since protected KVM is not widely used on platforms other than Android currently. Signed-off-by: Kalesh Singh <kaleshsingh@google.com> --- arch/arm64/kvm/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)