Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 19 Jul 2022 11:35:18 +0100
Message-ID: <87ilntz8yh.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Kalesh Singh <kaleshsingh@google.com>
Cc:     Mark Rutland <mark.rutland@arm.com>,
        Mark Brown <broonie@kernel.org>,
        "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>,
        Will Deacon <will@kernel.org>,
        Quentin Perret <qperret@google.com>,
        Fuad Tabba <tabba@google.com>,
        James Morse <james.morse@arm.com>,
        Alexandru Elisei <alexandru.elisei@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        andreyknvl@gmail.com, vincenzo.frascino@arm.com,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Kefeng Wang <wangkefeng.wang@huawei.com>,
        Marco Elver <elver@google.com>, Keir Fraser <keirf@google.com>,
        Zenghui Yu <yuzenghui@huawei.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        Oliver Upton <oupton@google.com>,
        "moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)" 
        <linux-arm-kernel@lists.infradead.org>,
        kvmarm <kvmarm@lists.cs.columbia.edu>,
        LKML <linux-kernel@vger.kernel.org>,
        "Cc: Android Kernel" <kernel-team@android.com>
Subject: Re: [PATCH v4 08/18] KVM: arm64: Add PROTECTED_NVHE_STACKTRACE Kconfig
In-Reply-To: <CAC_TJvcyb6xQhvuoSHpC6+LmeSAqbDk9Fnb=x-bB7UxxMek3EA@mail.gmail.com>
References: <20220715061027.1612149-1-kaleshsingh@google.com>
        <20220715061027.1612149-9-kaleshsingh@google.com>
        <87cze252q7.wl-maz@kernel.org>
        <CAC_TJvcyb6xQhvuoSHpC6+LmeSAqbDk9Fnb=x-bB7UxxMek3EA@mail.gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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.

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