Re: [PATCH] KVM: arm64: nvhe: Disable profile optimization

[Denis Nikitin]

Adding a few more comments...

On Tue, Sep 20, 2022 at 5:08 PM Denis Nikitin <denik@xxxxxxxxxx> wrote:
>
> Hi Mark,
>
> Thank you for a quick response.
>
> On Tue, Sep 20, 2022 at 2:34 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> >
> > Hi Denis,
> >
> > On Tue, 20 Sep 2022 09:20:05 +0100,
> > Denis Nikitin <denik@xxxxxxxxxxxx> wrote:
> > >
> > > Kernel build with -fprofile-sample-use raises the following failure:
> > >
> > > error: arch/arm64/kvm/hyp/nvhe/kvm_nvhe.tmp.o: Unexpected SHT_REL
> > > section ".rel.llvm.call-graph-profile"
> >
> > How is this flag provided? I don't see any occurrence of it in the
> > kernel so far.
>
> On ChromeOS we build the kernel with sample profiles by adding
> -fprofile-sample-use=/path/to/gcov.profile to KCFLAGS.
>
> >
> > >
> > > SHT_REL is generated by the latest lld, see
> > > https://reviews.llvm.org/rGca3bdb57fa1ac98b711a735de048c12b5fdd8086.
> >
> > Is this part of a released toolchain? If so, can you spell out the
> > first version where this occurs?
>
> Yes, it was added in llvm-13. I will update the patch.
>
> >
> > > Disable profile optimization in kvm/nvhe to fix the build with
> > > AutoFDO.
> >
> > It'd be good to at least mention how AutoFDO and -fprofile-sample-use
> > relate to each other.
>
> Good point. AutoFDO is an example of sample profiles.
> It's not actually relevant for the bug. I will better remove it.
>
> >
> > >
> > > Signed-off-by: Denis Nikitin <denik@xxxxxxxxxxxx>
> > > ---
> > >  arch/arm64/kvm/hyp/nvhe/Makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> > > index b5c5119c7396..6a6188374a52 100644
> > > --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> > > +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> > > @@ -89,6 +89,9 @@ quiet_cmd_hypcopy = HYPCOPY $@
> > >  # Remove ftrace, Shadow Call Stack, and CFI CFLAGS.
> > >  # This is equivalent to the 'notrace', '__noscs', and '__nocfi' annotations.
> > >  KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS) $(CC_FLAGS_CFI), $(KBUILD_CFLAGS))
> > > +# Profile optimization creates SHT_REL section '.llvm.call-graph-profile' for
> > > +# the hot code. SHT_REL is currently not supported by the KVM tools.
> >
> > 'KVM tools' seems vague. Maybe call out the actual helper that
> > processes the relocations?
>
> Agreed.
>
> >
> > > +KBUILD_CFLAGS += $(call cc-option,-fno-profile-sample-use,-fno-profile-use)
> >
> > Why adding these options instead of filtering out the offending option
> > as it is done just above?
>
> That was actually the alternative solution and it worked as well.
> Let me double check if profile optimization doesn't mess up with other
> sections and if it doesn't I will remove the '.llvm.call-graph-profile'
> section instead.

When I remove the '.llvm.call-graph-profile' section the layout of other
sections slightly changes (offsets and sizes) compared to
`-fno-profile-sample-use`. But the list of sections remains the same.

>
> >
> > Also, is this the only place the kernel fails to compile? The EFI stub
> > does similar things AFAIR, and could potentially fail the same way.
>
> This was the only place in 5.15 where we tested it.
> Let me see if EFI has this section.

EFI code is not marked as hot in the profile.

Regarding "could potentially fail", I don't see any explicit manipulations
with code sections in EFI.
The hardcoded EFI stub entries should not be affected.

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

- Denis