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;
MIME-Version: 1.0
References: <20220408200349.1529080-1-kaleshsingh@google.com>
 <20220408200349.1529080-7-kaleshsingh@google.com> <87tuaqae7h.wl-maz@kernel.org>
In-Reply-To: <87tuaqae7h.wl-maz@kernel.org>
From:   Kalesh Singh <kaleshsingh@google.com>
Date:   Mon, 18 Apr 2022 19:42:31 -0700
Message-ID: <CAC_TJvfZD0KAM9z3+GEpNDJxanePuq+xFgauaMyYabGhPPuvQA@mail.gmail.com>
Subject: Re: [PATCH v7 6/6] KVM: arm64: Symbolize the nVHE HYP addresses
To:     Marc Zyngier <maz@kernel.org>
Cc:     Will Deacon <will@kernel.org>, Quentin Perret <qperret@google.com>,
        Fuad Tabba <tabba@google.com>,
        Suren Baghdasaryan <surenb@google.com>,
        "Cc: Android Kernel" <kernel-team@android.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>,
        Andrew Walbran <qwandor@google.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        Andrew Jones <drjones@redhat.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Changbin Du <changbin.du@intel.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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 18, 2022 at 3:16 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Fri, 08 Apr 2022 21:03:29 +0100,
> Kalesh Singh <kaleshsingh@google.com> wrote:
> >
> > Reintroduce the __kvm_nvhe_ symbols in kallsyms, ignoring the local
> > symbols in this namespace. The local symbols are not informative and
> > can cause aliasing issues when symbolizing the addresses.
> >
> > With the necessary symbols now in kallsyms we can symbolize nVHE
> > addresses using the %p print format specifier:
> >
> > [   98.916444][  T426] kvm [426]: nVHE hyp panic at: [<ffffffc0096156fc=
>] __kvm_nvhe_overflow_stack+0x8/0x34!
> >
> > Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
> > Tested-by: Fuad Tabba <tabba@google.com>
> > Reviewed-by: Fuad Tabba <tabba@google.com>
> > ---
> >
> > Changes in v6:
> >   - Add Fuad's Reviewed-by and Tested-by tags.
> >
> > Changes in v2:
> >   - Fix printk warnings - %p expects (void *)
> >
> >
> >  arch/arm64/kvm/handle_exit.c | 13 +++++--------
> >  scripts/kallsyms.c           |  2 +-
> >  2 files changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.=
c
> > index 97fe14aab1a3..a377b871bf58 100644
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> > @@ -295,13 +295,8 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 =
esr, u64 spsr,
> >       u64 elr_in_kimg =3D __phys_to_kimg(elr_phys);
> >       u64 hyp_offset =3D elr_in_kimg - kaslr_offset() - elr_virt;
> >       u64 mode =3D spsr & PSR_MODE_MASK;
> > +     u64 panic_addr =3D elr_virt + hyp_offset;
> >
> > -     /*
> > -      * The nVHE hyp symbols are not included by kallsyms to avoid iss=
ues
> > -      * with aliasing. That means that the symbols cannot be printed w=
ith the
> > -      * "%pS" format specifier, so fall back to the vmlinux address if
> > -      * there's no better option.
> > -      */
> >       if (mode !=3D PSR_MODE_EL2t && mode !=3D PSR_MODE_EL2h) {
> >               kvm_err("Invalid host exception to nVHE hyp!\n");
> >       } else if (ESR_ELx_EC(esr) =3D=3D ESR_ELx_EC_BRK64 &&
> > @@ -321,9 +316,11 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 =
esr, u64 spsr,
> >               if (file)
> >                       kvm_err("nVHE hyp BUG at: %s:%u!\n", file, line);
> >               else
> > -                     kvm_err("nVHE hyp BUG at: %016llx!\n", elr_virt +=
 hyp_offset);
> > +                     kvm_err("nVHE hyp BUG at: [<%016llx>] %pB!\n", pa=
nic_addr,
> > +                                     (void *)panic_addr);
> >       } else {
> > -             kvm_err("nVHE hyp panic at: %016llx!\n", elr_virt + hyp_o=
ffset);
> > +             kvm_err("nVHE hyp panic at: [<%016llx>] %pB!\n", panic_ad=
dr,
> > +                             (void *)panic_addr);
> >       }
> >
> >       /*
> > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> > index 8caabddf817c..ad2c93640a92 100644
> > --- a/scripts/kallsyms.c
> > +++ b/scripts/kallsyms.c
> > @@ -111,7 +111,7 @@ static bool is_ignored_symbol(const char *name, cha=
r type)
> >               ".L",                   /* local labels, .LBB,.Ltmpxxx,.L=
__unnamed_xx,.LASANPC, etc. */
> >               "__crc_",               /* modversions */
> >               "__efistub_",           /* arm64 EFI stub namespace */
> > -             "__kvm_nvhe_",          /* arm64 non-VHE KVM namespace */
> > +             "__kvm_nvhe_$",         /* arm64 local symbols in non-VHE=
 KVM namespace */
> >               "__AArch64ADRPThunk_",  /* arm64 lld */
> >               "__ARMV5PILongThunk_",  /* arm lld */
> >               "__ARMV7PILongThunk_",
>
> If you are sanitising this, shouldn'tt you also apply the same rules
> as the rest of the kernel for non-'__-kvm_nvhe_' symbols? For example,
> I see a long list of .L* symbols:
>
> 0000000000000000 r __kvm_nvhe_.L144721
> 0000000000000090 r __kvm_nvhe_.L144721
> 00000000000000b4 r __kvm_nvhe_.L144721
> 00000000000004b0 r __kvm_nvhe_.L144721
> 000000000000051c r __kvm_nvhe_.L144721
> 0000000000000650 r __kvm_nvhe_.L144721
> 0000000000000694 r __kvm_nvhe_.L144721
> 0000000000000761 r __kvm_nvhe_.L144721
> 00000000000007a7 r __kvm_nvhe_.L144721
> 00000000000007c7 r __kvm_nvhe_.L144721
> 0000000000000814 r __kvm_nvhe_.L144721
> 0000000000000b08 r __kvm_nvhe_.L144721
> 0000000000003200 r __kvm_nvhe_.L144721
>
> (83 of them in total on a local build) that I think should also be
> trimmed.

Good catch. I=E2=80=99ll fix it in the next version along with your other
comments. Thanks for the reviews Mark.

-Kalesh

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