[PATCHv3 0/4] arm64: make phys_to_virt() correct

[PATCHv3 0/4] arm64: make phys_to_virt() correct

[Pingfan Liu]

Currently phys_to_virt() does not work well on 52-bits VA arm64 kernel.
One issue is contributed by phys_offset not signed.
The other is contributed by wrong page_offset.

v2 -> v3:
Discussed with Kairui off-list, 48-bits VA kernel can not handle flipped
mm yet. So introducing [4/4], which judges whether the kernel is with
flipped mm layout and adopt different formula accordingly.

As for [1-3/4], they are the same as [1-3/3] in V2.

v1 -> v2
Fix broken patch [2/3] in v1
Move arch_scan_vmcoreinfo declaration to util_lib/include/elf_info.h
Using UINT64_MAX instread of 0xffffffffffffffff


Cc: Kairui Song <kasong@tencent.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Philipp Rudo <prudo@redhat.com>
To: kexec@lists.infradead.org
*** BLURB HERE ***

Kairui Song (1):
  arm64: fix PAGE_OFFSET calc for flipped mm

Pingfan Liu (3):
  arm64: make phys_offset signed
  arm64/crashdump: unify routine to get page_offset
  arm64: read VA_BITS from kcore for 52-bits VA kernel

 kexec/arch/arm64/crashdump-arm64.c | 23 +-----------
 kexec/arch/arm64/kexec-arm64.c     | 60 ++++++++++++++++++++++++------
 kexec/arch/arm64/kexec-arm64.h     |  3 +-
 util_lib/elf_info.c                |  7 +++-
 util_lib/include/elf_info.h        |  3 +-
 5 files changed, 60 insertions(+), 36 deletions(-)

-- 
2.31.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv3 1/4] arm64: make phys_offset signed

[Pingfan Liu]

After kernel commit 7bc1a0f9e176 ("arm64: mm: use single quantity to
represent the PA to VA translation"), phys_offset can be negative if
running 52-bits kernel on 48-bits hardware.

So changing phys_offset from unsigned to signed.

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Kairui Song <kasong@tencent.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Philipp Rudo <prudo@redhat.com>
To: kexec@lists.infradead.org
---
 kexec/arch/arm64/kexec-arm64.c | 8 ++++----
 kexec/arch/arm64/kexec-arm64.h | 2 +-
 util_lib/elf_info.c            | 2 +-
 util_lib/include/elf_info.h    | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 6f572ed..7373fa3 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -859,7 +859,7 @@ void add_segment(struct kexec_info *info, const void *buf, size_t bufsz,
 	add_segment_phys_virt(info, buf, bufsz, base, memsz, 1);
 }
 
-static inline void set_phys_offset(uint64_t v, char *set_method)
+static inline void set_phys_offset(long v, char *set_method)
 {
 	if (arm64_mem.phys_offset == arm64_mem_ngv
 		|| v < arm64_mem.phys_offset) {
@@ -928,7 +928,7 @@ static int get_page_offset(void)
  * from VMCOREINFO note inside 'kcore'.
  */
 
-static int get_phys_offset_from_vmcoreinfo_pt_note(unsigned long *phys_offset)
+static int get_phys_offset_from_vmcoreinfo_pt_note(long *phys_offset)
 {
 	int fd, ret = 0;
 
@@ -948,7 +948,7 @@ static int get_phys_offset_from_vmcoreinfo_pt_note(unsigned long *phys_offset)
  * from PT_LOADs inside 'kcore'.
  */
 
-int get_phys_base_from_pt_load(unsigned long *phys_offset)
+int get_phys_base_from_pt_load(long *phys_offset)
 {
 	int i, fd, ret;
 	unsigned long long phys_start;
@@ -997,7 +997,7 @@ static bool to_be_excluded(char *str)
 int get_memory_ranges(struct memory_range **range, int *ranges,
 	unsigned long kexec_flags)
 {
-	unsigned long phys_offset = UINT64_MAX;
+	long phys_offset = (long)UINT64_MAX;
 	FILE *fp;
 	const char *iomem = proc_iomem();
 	char line[MAX_LINE], *str;
diff --git a/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
index ed447ac..1844b67 100644
--- a/kexec/arch/arm64/kexec-arm64.h
+++ b/kexec/arch/arm64/kexec-arm64.h
@@ -58,7 +58,7 @@ extern off_t initrd_size;
  */
 
 struct arm64_mem {
-	uint64_t phys_offset;
+	long phys_offset;
 	uint64_t text_offset;
 	uint64_t image_size;
 	uint64_t vp_offset;
diff --git a/util_lib/elf_info.c b/util_lib/elf_info.c
index 51d8b92..5574c7f 100644
--- a/util_lib/elf_info.c
+++ b/util_lib/elf_info.c
@@ -1236,7 +1236,7 @@ int read_elf(int fd)
 	return 0;
 }
 
-int read_phys_offset_elf_kcore(int fd, unsigned long *phys_off)
+int read_phys_offset_elf_kcore(int fd, long *phys_off)
 {
 	int ret;
 
diff --git a/util_lib/include/elf_info.h b/util_lib/include/elf_info.h
index 4bc9279..f550d86 100644
--- a/util_lib/include/elf_info.h
+++ b/util_lib/include/elf_info.h
@@ -28,7 +28,7 @@ int get_pt_load(int idx,
 	unsigned long long *phys_end,
 	unsigned long long *virt_start,
 	unsigned long long *virt_end);
-int read_phys_offset_elf_kcore(int fd, unsigned long *phys_off);
+int read_phys_offset_elf_kcore(int fd, long *phys_off);
 int read_elf(int fd);
 void dump_dmesg(int fd, void (*handler)(char*, unsigned int));
 
-- 
2.31.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv3 2/4] arm64/crashdump: unify routine to get page_offset

[Pingfan Liu]

There are two funcs to get page_offset:
  get_kernel_page_offset()
  get_page_offset()

Since get_kernel_page_offset() does not observe the kernel formula, and
remove it. Unify them in order to introduce 52-bits VA kernel more
easily in the coming patch.

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Kairui Song <kasong@tencent.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Philipp Rudo <prudo@redhat.com>
To: kexec@lists.infradead.org
---
 kexec/arch/arm64/crashdump-arm64.c | 23 +----------------------
 kexec/arch/arm64/kexec-arm64.c     |  8 ++++----
 kexec/arch/arm64/kexec-arm64.h     |  1 +
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/kexec/arch/arm64/crashdump-arm64.c b/kexec/arch/arm64/crashdump-arm64.c
index a02019a..0a8d44c 100644
--- a/kexec/arch/arm64/crashdump-arm64.c
+++ b/kexec/arch/arm64/crashdump-arm64.c
@@ -46,27 +46,6 @@ static struct crash_elf_info elf_info = {
 	.machine	= EM_AARCH64,
 };
 
-/*
- * Note: The returned value is correct only if !CONFIG_RANDOMIZE_BASE.
- */
-static uint64_t get_kernel_page_offset(void)
-{
-	int i;
-
-	if (elf_info.kern_vaddr_start == UINT64_MAX)
-		return UINT64_MAX;
-
-	/* Current max virtual memory range is 48-bits. */
-	for (i = 48; i > 0; i--)
-		if (!(elf_info.kern_vaddr_start & (1UL << i)))
-			break;
-
-	if (i <= 0)
-		return UINT64_MAX;
-	else
-		return UINT64_MAX << i;
-}
-
 /*
  * iomem_range_callback() - callback called for each iomem region
  * @data: not used
@@ -198,7 +177,7 @@ int load_crashdump_segments(struct kexec_info *info)
 	if (err)
 		return EFAILED;
 
-	elf_info.page_offset = get_kernel_page_offset();
+	get_page_offset(&elf_info.page_offset);
 	dbgprintf("%s: page_offset:   %016llx\n", __func__,
 			elf_info.page_offset);
 
diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 7373fa3..037e51c 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -909,7 +909,7 @@ static int get_va_bits(void)
  * get_page_offset - Helper for getting PAGE_OFFSET
  */
 
-static int get_page_offset(void)
+int get_page_offset(unsigned long *page_offset)
 {
 	int ret;
 
@@ -917,8 +917,8 @@ static int get_page_offset(void)
 	if (ret < 0)
 		return ret;
 
-	page_offset = (0xffffffffffffffffUL) << (va_bits - 1);
-	dbgprintf("page_offset : %lx\n", page_offset);
+	*page_offset = (0xffffffffffffffffUL) << (va_bits - 1);
+	dbgprintf("page_offset : %lx\n", *page_offset);
 
 	return 0;
 }
@@ -954,7 +954,7 @@ int get_phys_base_from_pt_load(long *phys_offset)
 	unsigned long long phys_start;
 	unsigned long long virt_start;
 
-	ret = get_page_offset();
+	ret = get_page_offset(&page_offset);
 	if (ret < 0)
 		return ret;
 
diff --git a/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
index 1844b67..ed99d9d 100644
--- a/kexec/arch/arm64/kexec-arm64.h
+++ b/kexec/arch/arm64/kexec-arm64.h
@@ -69,6 +69,7 @@ extern struct arm64_mem arm64_mem;
 
 uint64_t get_phys_offset(void);
 uint64_t get_vp_offset(void);
+int get_page_offset(unsigned long *offset);
 
 static inline void reset_vp_offset(void)
 {
-- 
2.31.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv3 3/4] arm64: read VA_BITS from kcore for 52-bits VA kernel

[Pingfan Liu]

phys_to_virt() calculates virtual address. As a important factor,
page_offset is excepted to be accurate.

Since arm64 kernel exposes va_bits through vmcore, using it.

And for 52-bits kernel, due to flipped mm layout, the PAGE_OFFSET
formula should be:
  UNIT64_MAX << va_bits
instead of
  UNIT64_MAX << (va_bits - 1)

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Kairui Song <kasong@tencent.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Philipp Rudo <prudo@redhat.com>
To: kexec@lists.infradead.org
---
 kexec/arch/arm64/kexec-arm64.c | 34 ++++++++++++++++++++++++++++++----
 util_lib/elf_info.c            |  5 +++++
 util_lib/include/elf_info.h    |  1 +
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 037e51c..86aadf0 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -54,7 +54,7 @@
 static bool try_read_phys_offset_from_kcore = false;
 
 /* Machine specific details. */
-static int va_bits;
+static int va_bits = -1;
 static unsigned long page_offset;
 
 /* Global varables the core kexec routines expect. */
@@ -876,7 +876,18 @@ static inline void set_phys_offset(long v, char *set_method)
 
 static int get_va_bits(void)
 {
-	unsigned long long stext_sym_addr = get_kernel_sym("_stext");
+	unsigned long long stext_sym_addr;
+
+	/*
+	 * if already got from kcore
+	 */
+	if (va_bits != -1)
+		goto out;
+
+
+	/* For kernel older than v4.19 */
+	fprintf(stderr, "Warning, can't get the VA_BITS from kcore\n");
+	stext_sym_addr = get_kernel_sym("_stext");
 
 	if (stext_sym_addr == 0) {
 		fprintf(stderr, "Can't get the symbol of _stext.\n");
@@ -900,6 +911,7 @@ static int get_va_bits(void)
 		return -1;
 	}
 
+out:
 	dbgprintf("va_bits : %d\n", va_bits);
 
 	return 0;
@@ -917,14 +929,27 @@ int get_page_offset(unsigned long *page_offset)
 	if (ret < 0)
 		return ret;
 
-	*page_offset = (0xffffffffffffffffUL) << (va_bits - 1);
+	if (va_bits < 52)
+		*page_offset = UINT64_MAX << (va_bits - 1);
+	else
+		*page_offset = UINT64_MAX << va_bits;
+
 	dbgprintf("page_offset : %lx\n", *page_offset);
 
 	return 0;
 }
 
+static void arm64_scan_vmcoreinfo(char *pos)
+{
+	const char *str;
+
+	str = "NUMBER(VA_BITS)=";
+	if (memcmp(str, pos, strlen(str)) == 0)
+		va_bits = strtoul(pos + strlen(str), NULL, 10);
+}
+
 /**
- * get_phys_offset_from_vmcoreinfo_pt_note - Helper for getting PHYS_OFFSET
+ * get_phys_offset_from_vmcoreinfo_pt_note - Helper for getting PHYS_OFFSET (and va_bits)
  * from VMCOREINFO note inside 'kcore'.
  */
 
@@ -937,6 +962,7 @@ static int get_phys_offset_from_vmcoreinfo_pt_note(long *phys_offset)
 		return EFAILED;
 	}
 
+	arch_scan_vmcoreinfo = arm64_scan_vmcoreinfo;
 	ret = read_phys_offset_elf_kcore(fd, phys_offset);
 
 	close(fd);
diff --git a/util_lib/elf_info.c b/util_lib/elf_info.c
index 5574c7f..d252eff 100644
--- a/util_lib/elf_info.c
+++ b/util_lib/elf_info.c
@@ -310,6 +310,8 @@ int get_pt_load(int idx,
 
 #define NOT_FOUND_LONG_VALUE		(-1)
 
+void (*arch_scan_vmcoreinfo)(char *pos);
+
 void scan_vmcoreinfo(char *start, size_t size)
 {
 	char *last = start + size - 1;
@@ -551,6 +553,9 @@ void scan_vmcoreinfo(char *start, size_t size)
 			}
 		}
 
+		if (arch_scan_vmcoreinfo != NULL)
+			(*arch_scan_vmcoreinfo)(pos);
+
 		if (last_line)
 			break;
 	}
diff --git a/util_lib/include/elf_info.h b/util_lib/include/elf_info.h
index f550d86..fdf4c3d 100644
--- a/util_lib/include/elf_info.h
+++ b/util_lib/include/elf_info.h
@@ -31,5 +31,6 @@ int get_pt_load(int idx,
 int read_phys_offset_elf_kcore(int fd, long *phys_off);
 int read_elf(int fd);
 void dump_dmesg(int fd, void (*handler)(char*, unsigned int));
+extern void (*arch_scan_vmcoreinfo)(char *pos);
 
 #endif /* ELF_INFO_H */
-- 
2.31.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv3 4/4] arm64: fix PAGE_OFFSET calc for flipped mm

[Pingfan Liu]

From: Kairui Song <kasong@tencent.com>

Since kernel commit 14c127c957c1 ('arm64: mm: Flip kernel VA space'),
the memory layout on arm64 have changed, and kexec-tools can no longer
get the the right PAGE_OFFSET based on _text symbol.

Prior to that, the kimage (_text) lays above PAGE_END with this layout:
0               -> VA_START                 : Usespace
VA_START        -> VA_START + 256M          : BPF JIT, Modules
VA_START + 256M -> PAGE_OFFSET - (~GB misc) : Vmalloc (KERNEL _text HERE)
PAGE_OFFSET     -> ...                      : * Linear map *

And here we have:
VA_START    = -1UL << VA_BITS
PAGE_OFFSET = -1UL << (VA_BITS - 1)
_text < -1UL << (VA_BITS - 1)

Kernel image lays somewhere between VA_START and PAGE_OFFSET, so we just
calc VA_BITS by getting the highest unset bit of _text symbol address,
and shift one less bit of VA_BITS to get page offset. This works as long
as KASLR don't put kernel in a too high location (which is commented inline).

And after that commit, kernel layout have changed:
0               -> PAGE_OFFSET              : Userspace
PAGE_OFFSET     -> PAGE_END                 : * Linear map *
PAGE_END        -> PAGE_END + 128M          : bpf jit region
PAGE_END + 128M -> PAGE_END + 256MB         : modules
PAGE_END + 256M -> ...                      : vmalloc (KERNEL _text HERE)

Here we have:
PAGE_OFFSET = -1UL << VA_BITS
PAGE_END    = -1UL << (VA_BITS - 1)
_text > -1UL << (VA_BITS - 1)

Kernel image now lays above PAGE_END, so we have to shift one more bit to
get the VA_BITS, and shift the exact VA_BITS for PAGE_OFFSET.

We can simply check if "_text > -1UL << (VA_BITS - 1)" is true to judge
which layout is being used and shift the page offset occordingly.

Signed-off-by: Kairui Song <kasong@tencent.com>
(rebased and stripped by Pingfan )
Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Philipp Rudo <prudo@redhat.com>
To: kexec@lists.infradead.org
---
 kexec/arch/arm64/kexec-arm64.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 86aadf0..2e2fa8e 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -923,13 +923,25 @@ out:
 
 int get_page_offset(unsigned long *page_offset)
 {
+	unsigned long long text_sym_addr, kernel_va_mid;
 	int ret;
 
+	text_sym_addr = get_kernel_sym("_text");
+	if (text_sym_addr == 0) {
+		fprintf(stderr, "Can't get the symbol of _text to calculate page_offset.\n");
+		return -1;
+	}
+
 	ret = get_va_bits();
 	if (ret < 0)
 		return ret;
 
-	if (va_bits < 52)
+	/* Since kernel 5.4, kernel image is put above
+	 * UINT64_MAX << (va_bits - 1)
+	 */
+	kernel_va_mid = UINT64_MAX << (va_bits - 1);
+	/* older kernel */
+	if (text_sym_addr < kernel_va_mid)
 		*page_offset = UINT64_MAX << (va_bits - 1);
 	else
 		*page_offset = UINT64_MAX << va_bits;
-- 
2.31.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv3 0/4] arm64: make phys_to_virt() correct

[Pingfan Liu]

Gently ping, any comments or suggestions?

Thanks

On Tue, Dec 28, 2021 at 9:27 PM Pingfan Liu <piliu@redhat.com> wrote:
>
> Currently phys_to_virt() does not work well on 52-bits VA arm64 kernel.
> One issue is contributed by phys_offset not signed.
> The other is contributed by wrong page_offset.
>
> v2 -> v3:
> Discussed with Kairui off-list, 48-bits VA kernel can not handle flipped
> mm yet. So introducing [4/4], which judges whether the kernel is with
> flipped mm layout and adopt different formula accordingly.
>
> As for [1-3/4], they are the same as [1-3/3] in V2.
>
> v1 -> v2
> Fix broken patch [2/3] in v1
> Move arch_scan_vmcoreinfo declaration to util_lib/include/elf_info.h
> Using UINT64_MAX instread of 0xffffffffffffffff
>
>
> Cc: Kairui Song <kasong@tencent.com>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Philipp Rudo <prudo@redhat.com>
> To: kexec at lists.infradead.org
> *** BLURB HERE ***
>
> Kairui Song (1):
>   arm64: fix PAGE_OFFSET calc for flipped mm
>
> Pingfan Liu (3):
>   arm64: make phys_offset signed
>   arm64/crashdump: unify routine to get page_offset
>   arm64: read VA_BITS from kcore for 52-bits VA kernel
>
>  kexec/arch/arm64/crashdump-arm64.c | 23 +-----------
>  kexec/arch/arm64/kexec-arm64.c     | 60 ++++++++++++++++++++++++------
>  kexec/arch/arm64/kexec-arm64.h     |  3 +-
>  util_lib/elf_info.c                |  7 +++-
>  util_lib/include/elf_info.h        |  3 +-
>  5 files changed, 60 insertions(+), 36 deletions(-)
>
> --
> 2.31.1
>

[PATCHv3 1/4] arm64: make phys_offset signed

[Simon Horman]

On Tue, Dec 28, 2021 at 09:26:56PM +0800, Pingfan Liu wrote:
> After kernel commit 7bc1a0f9e176 ("arm64: mm: use single quantity to
> represent the PA to VA translation"), phys_offset can be negative if
> running 52-bits kernel on 48-bits hardware.
> 
> So changing phys_offset from unsigned to signed.
> 
> Signed-off-by: Pingfan Liu <piliu@redhat.com>
> Cc: Kairui Song <kasong@tencent.com>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Philipp Rudo <prudo@redhat.com>
> To: kexec at lists.infradead.org
> ---
>  kexec/arch/arm64/kexec-arm64.c | 8 ++++----
>  kexec/arch/arm64/kexec-arm64.h | 2 +-
>  util_lib/elf_info.c            | 2 +-
>  util_lib/include/elf_info.h    | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 6f572ed..7373fa3 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -859,7 +859,7 @@ void add_segment(struct kexec_info *info, const void *buf, size_t bufsz,
>  	add_segment_phys_virt(info, buf, bufsz, base, memsz, 1);
>  }
>  
> -static inline void set_phys_offset(uint64_t v, char *set_method)
> +static inline void set_phys_offset(long v, char *set_method)

Perhaps it would be more in keeping with the current implementation
to use int64_t rather than long ?

>  {
>  	if (arm64_mem.phys_offset == arm64_mem_ngv
>  		|| v < arm64_mem.phys_offset) {
> @@ -928,7 +928,7 @@ static int get_page_offset(void)
>   * from VMCOREINFO note inside 'kcore'.
>   */
>  
> -static int get_phys_offset_from_vmcoreinfo_pt_note(unsigned long *phys_offset)
> +static int get_phys_offset_from_vmcoreinfo_pt_note(long *phys_offset)
>  {
>  	int fd, ret = 0;
>  
> @@ -948,7 +948,7 @@ static int get_phys_offset_from_vmcoreinfo_pt_note(unsigned long *phys_offset)
>   * from PT_LOADs inside 'kcore'.
>   */
>  
> -int get_phys_base_from_pt_load(unsigned long *phys_offset)
> +int get_phys_base_from_pt_load(long *phys_offset)
>  {
>  	int i, fd, ret;
>  	unsigned long long phys_start;
> @@ -997,7 +997,7 @@ static bool to_be_excluded(char *str)
>  int get_memory_ranges(struct memory_range **range, int *ranges,
>  	unsigned long kexec_flags)
>  {
> -	unsigned long phys_offset = UINT64_MAX;
> +	long phys_offset = (long)UINT64_MAX;

Effectively this sets phys_offset to -1.
Is that ok / intended ? If so, perhaps it's clearer to just use -1.

Regardless, can clean up other uses of long with UINT64_MAX introduced
by this patch?

I see the following:

make
...
kexec/arch/arm64/kexec-arm64.c: In function ?get_memory_ranges?:
kexec/arch/arm64/kexec-arm64.c:1022:20: warning: comparison between signed and u
nsigned integer expressions [-Wsign-compare]
    if (phys_offset != UINT64_MAX)
...
kexec/arch/arm64/kexec-arm64.c:1034:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (phys_offset != UINT64_MAX)


>  	FILE *fp;
>  	const char *iomem = proc_iomem();
>  	char line[MAX_LINE], *str;
> diff --git a/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
> index ed447ac..1844b67 100644
> --- a/kexec/arch/arm64/kexec-arm64.h
> +++ b/kexec/arch/arm64/kexec-arm64.h
> @@ -58,7 +58,7 @@ extern off_t initrd_size;
>   */
>  
>  struct arm64_mem {
> -	uint64_t phys_offset;
> +	long phys_offset;
>  	uint64_t text_offset;
>  	uint64_t image_size;
>  	uint64_t vp_offset;
> diff --git a/util_lib/elf_info.c b/util_lib/elf_info.c
> index 51d8b92..5574c7f 100644
> --- a/util_lib/elf_info.c
> +++ b/util_lib/elf_info.c
> @@ -1236,7 +1236,7 @@ int read_elf(int fd)
>  	return 0;
>  }
>  
> -int read_phys_offset_elf_kcore(int fd, unsigned long *phys_off)
> +int read_phys_offset_elf_kcore(int fd, long *phys_off)
>  {
>  	int ret;
>  
> diff --git a/util_lib/include/elf_info.h b/util_lib/include/elf_info.h
> index 4bc9279..f550d86 100644
> --- a/util_lib/include/elf_info.h
> +++ b/util_lib/include/elf_info.h
> @@ -28,7 +28,7 @@ int get_pt_load(int idx,
>  	unsigned long long *phys_end,
>  	unsigned long long *virt_start,
>  	unsigned long long *virt_end);
> -int read_phys_offset_elf_kcore(int fd, unsigned long *phys_off);
> +int read_phys_offset_elf_kcore(int fd, long *phys_off);
>  int read_elf(int fd);
>  void dump_dmesg(int fd, void (*handler)(char*, unsigned int));
>  
> -- 
> 2.31.1
> 

[PATCHv3 2/4] arm64/crashdump: unify routine to get page_offset

[Simon Horman]

On Tue, Dec 28, 2021 at 09:26:57PM +0800, Pingfan Liu wrote:
> There are two funcs to get page_offset:
>   get_kernel_page_offset()
>   get_page_offset()
> 
> Since get_kernel_page_offset() does not observe the kernel formula, and
> remove it. Unify them in order to introduce 52-bits VA kernel more
> easily in the coming patch.
> 
> Signed-off-by: Pingfan Liu <piliu@redhat.com>
> Cc: Kairui Song <kasong@tencent.com>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Philipp Rudo <prudo@redhat.com>
> To: kexec at lists.infradead.org
> ---
>  kexec/arch/arm64/crashdump-arm64.c | 23 +----------------------
>  kexec/arch/arm64/kexec-arm64.c     |  8 ++++----
>  kexec/arch/arm64/kexec-arm64.h     |  1 +
>  3 files changed, 6 insertions(+), 26 deletions(-)
> 
> diff --git a/kexec/arch/arm64/crashdump-arm64.c b/kexec/arch/arm64/crashdump-arm64.c
> index a02019a..0a8d44c 100644
> --- a/kexec/arch/arm64/crashdump-arm64.c
> +++ b/kexec/arch/arm64/crashdump-arm64.c
> @@ -46,27 +46,6 @@ static struct crash_elf_info elf_info = {
>  	.machine	= EM_AARCH64,
>  };
>  
> -/*
> - * Note: The returned value is correct only if !CONFIG_RANDOMIZE_BASE.
> - */
> -static uint64_t get_kernel_page_offset(void)
> -{
> -	int i;
> -
> -	if (elf_info.kern_vaddr_start == UINT64_MAX)
> -		return UINT64_MAX;
> -
> -	/* Current max virtual memory range is 48-bits. */
> -	for (i = 48; i > 0; i--)
> -		if (!(elf_info.kern_vaddr_start & (1UL << i)))
> -			break;
> -
> -	if (i <= 0)
> -		return UINT64_MAX;
> -	else
> -		return UINT64_MAX << i;
> -}
> -
>  /*
>   * iomem_range_callback() - callback called for each iomem region
>   * @data: not used
> @@ -198,7 +177,7 @@ int load_crashdump_segments(struct kexec_info *info)
>  	if (err)
>  		return EFAILED;
>  
> -	elf_info.page_offset = get_kernel_page_offset();
> +	get_page_offset(&elf_info.page_offset);
>  	dbgprintf("%s: page_offset:   %016llx\n", __func__,
>  			elf_info.page_offset);
>  
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 7373fa3..037e51c 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -909,7 +909,7 @@ static int get_va_bits(void)
>   * get_page_offset - Helper for getting PAGE_OFFSET
>   */
>  
> -static int get_page_offset(void)
> +int get_page_offset(unsigned long *page_offset)
>  {
>  	int ret;
>  
> @@ -917,8 +917,8 @@ static int get_page_offset(void)
>  	if (ret < 0)
>  		return ret;
>  
> -	page_offset = (0xffffffffffffffffUL) << (va_bits - 1);
> -	dbgprintf("page_offset : %lx\n", page_offset);
> +	*page_offset = (0xffffffffffffffffUL) << (va_bits - 1);

As this line is being updated, perhaps we could also make use of
UINT64_MAX or ULONG_MAX here.

> +	dbgprintf("page_offset : %lx\n", *page_offset);
>  
>  	return 0;
>  }
> @@ -954,7 +954,7 @@ int get_phys_base_from_pt_load(long *phys_offset)
>  	unsigned long long phys_start;
>  	unsigned long long virt_start;
>  
> -	ret = get_page_offset();
> +	ret = get_page_offset(&page_offset);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
> index 1844b67..ed99d9d 100644
> --- a/kexec/arch/arm64/kexec-arm64.h
> +++ b/kexec/arch/arm64/kexec-arm64.h
> @@ -69,6 +69,7 @@ extern struct arm64_mem arm64_mem;
>  
>  uint64_t get_phys_offset(void);
>  uint64_t get_vp_offset(void);
> +int get_page_offset(unsigned long *offset);
>  
>  static inline void reset_vp_offset(void)
>  {
> -- 
> 2.31.1
> 

[PATCHv3 0/4] arm64: make phys_to_virt() correct

[Simon Horman]

Hi Pingfan,

On Tue, Dec 28, 2021 at 09:26:55PM +0800, Pingfan Liu wrote:
> Currently phys_to_virt() does not work well on 52-bits VA arm64 kernel.
> One issue is contributed by phys_offset not signed.
> The other is contributed by wrong page_offset.
> 
> v2 -> v3:
> Discussed with Kairui off-list, 48-bits VA kernel can not handle flipped
> mm yet. So introducing [4/4], which judges whether the kernel is with
> flipped mm layout and adopt different formula accordingly.
> 
> As for [1-3/4], they are the same as [1-3/3] in V2.
> 
> v1 -> v2
> Fix broken patch [2/3] in v1
> Move arch_scan_vmcoreinfo declaration to util_lib/include/elf_info.h
> Using UINT64_MAX instread of 0xffffffffffffffff

Thanks for the updated patches and sorry for the delay in reviewing them.

I responded with some minor nits on patches 1/4 and 2/4.
The other patches 3/4 and 4/4 look good to me.

Modulo any feedback from others I'd be happy to move forwards
with applying this series once my nits have been addressed
(via email or fresh patches).

Thanks again,
Simon

[PATCHv3 0/4] arm64: make phys_to_virt() correct

[Pingfan Liu]

On Fri, Jan 14, 2022 at 10:48 PM Simon Horman <horms@verge.net.au> wrote:
>
> Hi Pingfan,
>
> On Tue, Dec 28, 2021 at 09:26:55PM +0800, Pingfan Liu wrote:
> > Currently phys_to_virt() does not work well on 52-bits VA arm64 kernel.
> > One issue is contributed by phys_offset not signed.
> > The other is contributed by wrong page_offset.
> >
> > v2 -> v3:
> > Discussed with Kairui off-list, 48-bits VA kernel can not handle flipped
> > mm yet. So introducing [4/4], which judges whether the kernel is with
> > flipped mm layout and adopt different formula accordingly.
> >
> > As for [1-3/4], they are the same as [1-3/3] in V2.
> >
> > v1 -> v2
> > Fix broken patch [2/3] in v1
> > Move arch_scan_vmcoreinfo declaration to util_lib/include/elf_info.h
> > Using UINT64_MAX instread of 0xffffffffffffffff
>
> Thanks for the updated patches and sorry for the delay in reviewing them.
>
> I responded with some minor nits on patches 1/4 and 2/4.
> The other patches 3/4 and 4/4 look good to me.
>
Thank you for kindly reviewing.

I will update V4

Best Regards,

Pingfan
> Modulo any feedback from others I'd be happy to move forwards
> with applying this series once my nits have been addressed
> (via email or fresh patches).
>
> Thanks again,
> Simon
>