Re: [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr.

[Mark Rutland <mark.rutland@arm.com>]

Hi,

[Roping in the arm64 maintainers]

On Tue, Oct 08, 2019 at 01:54:34PM -0400, Chris von Recklinghausen wrote:
> With the reshuffling of kernel VA space to support 52 bits, the
> kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they are
> based on VA_START, but VA_START no longer points to the base of the kernel
> virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has
> default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on
> PAGE_OFFSET, so simply remove the arm64 definitions of them.
> 
> Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space")

I think this patch is correct, but it's missing a Signed-off-by. Can you
please provide that here?

As James said in another reply, the commit message should describe the
intended semantic, what broke, and how this patch fixes the issue. As
VA_START got renamed (and no longer exists), it's also a bit unclear.

How about:

| arm64: fix kcore macros 52-bit va fallout
|
| We export the entire kernel address space (i.e. the whole of the TTBR1
| address range) via /proc/kcore. The kc_vaddr_to_offset() and
| kc_offset_to_vaddr() macros are intended to convert between a kernel
| virtual address and its offset relative to the start of the TTBR1
| address space.
|
| Prior to commit:
|
|   14c127c957c1c607 ("arm64: mm: Flip kernel VA space")
|
| ... the offset was calculated relative to VA_START, which at the time
| was the start of the TTBR1 address space. At this time, PAGE_OFFSET
| pointed to the high half of the TTBR1 address space where arm64's
| linear map lived.
|
| That commit swapped the position of VA_START and PAGE_OFFSET, but
| failed to update kc_vaddr_to_offset() or kc_offset_to_vaddr(), so
| since then the two macros behave incorrectly.
|
| Note that VA_START was subsequently renamed to PAGE_END in commit:
|
|   77ad4ce69321abbe ("arm64: memory: rename VA_START to PAGE_END")
|
| As the generic implementations of the two macros calculate the offset
| relative to PAGE_OFFSET (which is now the start of the TTBR1 address
| space), we can delete the arm64 implementation and use those.
|
| Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space")

With that wording (and your Signed-off-by):

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> ---
>  arch/arm64/include/asm/pgtable.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 7576df00eb50..8330810f699e 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct vm_area_struct *vma,
>  
>  #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0)
>  
> -#define kc_vaddr_to_offset(v)	((v) & ~PAGE_END)
> -#define kc_offset_to_vaddr(o)	((o) | PAGE_END)
> -
>  #ifdef CONFIG_ARM64_PA_BITS_52
>  #define phys_to_ttbr(addr)	(((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52)
>  #else
> -- 
> 2.18.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel