Re: Re: [RESEND PATCH v5 2/5] arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo

[Scott Branden]

Hi Bhupesh,

On 2020-02-23 10:25 p.m., Bhupesh Sharma wrote:
> Hi Amit,
>
> On Fri, Feb 21, 2020 at 2:36 PM Amit Kachhap <amit.kachhap@xxxxxxx> wrote:
> > Hi Bhupesh,
> >
> > On 1/13/20 5:44 PM, Bhupesh Sharma wrote:
> > > Hi James,
> > >
> > > On 01/11/2020 12:30 AM, Dave Anderson wrote:
> > > > ----- Original Message -----
> > > > > Hi Bhupesh,
> > > > >
> > > > > On 25/12/2019 19:01, Bhupesh Sharma wrote:
> > > > > > On 12/12/2019 04:02 PM, James Morse wrote:
> > > > > > > On 29/11/2019 19:59, Bhupesh Sharma wrote:
> > > > > > > > vabits_actual variable on arm64 indicates the actual VA space size,
> > > > > > > > and allows a single binary to support both 48-bit and 52-bit VA
> > > > > > > > spaces.
> > > > > > > >
> > > > > > > > If the ARMv8.2-LVA optional feature is present, and we are running
> > > > > > > > with a 64KB page size; then it is possible to use 52-bits of address
> > > > > > > > space for both userspace and kernel addresses. However, any kernel
> > > > > > > > binary that supports 52-bit must also be able to fall back to 48-bit
> > > > > > > > at early boot time if the hardware feature is not present.
> > > > > > > >
> > > > > > > > Since TCR_EL1.T1SZ indicates the size offset of the memory region
> > > > > > > > addressed by TTBR1_EL1 (and hence can be used for determining the
> > > > > > > > vabits_actual value) it makes more sense to export the same in
> > > > > > > > vmcoreinfo rather than vabits_actual variable, as the name of the
> > > > > > > > variable can change in future kernel versions, but the architectural
> > > > > > > > constructs like TCR_EL1.T1SZ can be used better to indicate intended
> > > > > > > > specific fields to user-space.
> > > > > > > >
> > > > > > > > User-space utilities like makedumpfile and crash-utility, need to
> > > > > > > > read/write this value from/to vmcoreinfo
> > > > > > > (write?)
> > > > > > Yes, also write so that the vmcoreinfo from an (crashing) arm64
> > > > > > system can
> > > > > > be used for
> > > > > > analysis of the root-cause of panic/crash on say an x86_64 host using
> > > > > > utilities like
> > > > > > crash-utility/gdb.
> > > > > I read this as as "User-space [...] needs to write to vmcoreinfo".
> > > That's correct. But for writing to vmcore dump in the kdump kernel, we
> > > need to read the symbols from the vmcoreinfo in the primary kernel.
> > >
> > > > > > > > for determining if a virtual address lies in the linear map range.
> > > > > > > I think this is a fragile example. The debugger shouldn't need to know
> > > > > > > this.
> > > > > > Well that the current user-space utility design, so I am not sure we
> > > > > > can
> > > > > > tweak that too much.
> > > > > >
> > > > > > > > The user-space computation for determining whether an address lies in
> > > > > > > > the linear map range is the same as we have in kernel-space:
> > > > > > > >
> > > > > > > >      #define __is_lm_address(addr)    (!(((u64)addr) &
> > > > > > > > BIT(vabits_actual -
> > > > > > > >      1)))
> > > > > > > This was changed with 14c127c957c1 ("arm64: mm: Flip kernel VA
> > > > > > > space"). If
> > > > > > > user-space
> > > > > > > tools rely on 'knowing' the kernel memory layout, they must have to
> > > > > > > constantly be fixed
> > > > > > > and updated. This is a poor argument for adding this to something that
> > > > > > > ends up as ABI.
> > > > > > See above. The user-space has to rely on some ABI/guaranteed
> > > > > > hardware-symbols which can be
> > > > > > used for 'determining' the kernel memory layout.
> > > > > I disagree. Everything and anything in the kernel will change. The
> > > > > ABI rules apply to
> > > > > stuff exposed via syscalls and kernel filesystems. It does not apply
> > > > > to kernel internals,
> > > > > like the memory layout we used yesterday. 14c127c957c1 is a case in
> > > > > point.
> > > > >
> > > > > A debugger trying to rely on this sort of thing would have to play
> > > > > catchup whenever it
> > > > > changes.
> > > > Exactly.  That's the whole point.
> > > >
> > > > The crash utility and makedumpfile are not in the same league as other
> > > > user-space tools.
> > > > They have always had to "play catchup" precisely because they depend
> > > > upon kernel internals,
> > > > which constantly change.
> > > I agree with you and DaveA here. Software user-space debuggers are
> > > dependent on kernel internals (which can change from time-to-time) and
> > > will have to play catch-up (which has been the case since the very start).
> > >
> > > Unfortunately we don't have any clear ABI for software debugging tools -
> > > may be something to look for in future.
> > >
> > > A case in point is gdb/kgdb, which still needs to run with KASLR
> > > turned-off (nokaslr) for debugging, as it confuses gdb which resolve
> > > kernel symbol address from symbol table of vmlinux. But we can
> > > work-around the same in makedumpfile/crash by reading the 'kaslr_offset'
> > > value. And I have several users telling me now they cannot use gdb on
> > > KASLR enabled kernel to debug panics, but can makedumpfile + crash
> > > combination to achieve the same.
> > >
> > > So, we should be looking to fix these utilities which are broken since
> > > the 52-bit changes for arm64. Accordingly, I will try to send the v6
> > > soon while incorporating the comments posted on the v5.
> > Any update on the next v6 version. Since this patch series is fixing the
> > current broken kdump so need this series to add some more fields in
> > vmcoreinfo for Pointer Authentication work.
> Sorry for the delay. I was caught up in some other urgent arm64
> user-space issues.
> I am preparing the v6 now and hopefully will be able to post it out
> for review later today.

Did v6 get sent out?

> Thanks,
> Bhupesh
Regards,
Scott