Message-ID: <974f3601-25f8-f4e6-43a8-ff4275e9c174@arm.com>
Date:   Fri, 21 Feb 2020 14:36:05 +0530
From:   Amit Kachhap <amit.kachhap@....com>
To:     Bhupesh Sharma <bhsharma@...hat.com>,
        Dave Anderson <anderson@...hat.com>,
        James Morse <james.morse@....com>
Cc:     Mark Rutland <mark.rutland@....com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        linux-doc@...r.kernel.org, Will Deacon <will@...nel.org>,
        x86@...nel.org, kexec@...ts.infradead.org,
        linux-kernel@...r.kernel.org,
        Kazuhito Hagio <k-hagio@...jp.nec.com>,
        Catalin Marinas <catalin.marinas@....com>,
        bhupesh linux <bhupesh.linux@...il.com>,
        linuxppc-dev@...ts.ozlabs.org,
        linux-arm-kernel@...ts.infradead.org,
        Steve Capper <steve.capper@....com>
Subject: Re: [RESEND PATCH v5 2/5] arm64/crash_core: Export TCR_EL1.T1SZ in
 vmcoreinfo

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.

Thanks,
Amit Daniel
> 
> Thanks,
> Bhupesh
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Powered by blists - more mailing lists