* [PATCH 0/3] arm64: make phys_to_virt() correct
@ 2021-12-10 3:07 Pingfan Liu
2021-12-10 3:07 ` [PATCH 1/3] arm64: make phys_offset signed Pingfan Liu
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Pingfan Liu @ 2021-12-10 3:07 UTC (permalink / raw)
To: kexec; +Cc: 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.
This series fixes them so vmcore-dmesg can work on 52-bits VA kernel
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 | 43 +++++++++++++++++++++++-------
kexec/arch/arm64/kexec-arm64.h | 4 ++-
util_lib/elf_info.c | 7 ++++-
util_lib/include/elf_info.h | 2 +-
5 files changed, 44 insertions(+), 35 deletions(-)
--
2.31.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] arm64: make phys_offset signed
2021-12-10 3:07 [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
@ 2021-12-10 3:07 ` Pingfan Liu
2021-12-10 3:07 ` [PATCH 2/3] arm64/crashdump: unify routine to get page_offset Pingfan Liu
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Pingfan Liu @ 2021-12-10 3:07 UTC (permalink / raw)
To: kexec; +Cc: 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>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] arm64/crashdump: unify routine to get page_offset
2021-12-10 3:07 [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
2021-12-10 3:07 ` [PATCH 1/3] arm64: make phys_offset signed Pingfan Liu
@ 2021-12-10 3:07 ` Pingfan Liu
2021-12-15 13:35 ` Philipp Rudo
2021-12-10 3:07 ` [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel Pingfan Liu
2021-12-15 1:43 ` [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
3 siblings, 1 reply; 12+ messages in thread
From: Pingfan Liu @ 2021-12-10 3:07 UTC (permalink / raw)
To: kexec; +Cc: 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>
---
kexec/arch/arm64/crashdump-arm64.c | 23 +----------------------
kexec/arch/arm64/kexec-arm64.c | 4 ++--
kexec/arch/arm64/kexec-arm64.h | 1 +
3 files changed, 4 insertions(+), 24 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..bd650e6 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;
@@ -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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel
2021-12-10 3:07 [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
2021-12-10 3:07 ` [PATCH 1/3] arm64: make phys_offset signed Pingfan Liu
2021-12-10 3:07 ` [PATCH 2/3] arm64/crashdump: unify routine to get page_offset Pingfan Liu
@ 2021-12-10 3:07 ` Pingfan Liu
2021-12-15 13:34 ` Philipp Rudo
[not found] ` <20211215130557.GB4314@vergenet.net>
2021-12-15 1:43 ` [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
3 siblings, 2 replies; 12+ messages in thread
From: Pingfan Liu @ 2021-12-10 3:07 UTC (permalink / raw)
To: kexec; +Cc: 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.
Signed-off-by: Pingfan Liu <piliu@redhat.com>
---
kexec/arch/arm64/kexec-arm64.c | 31 +++++++++++++++++++++++++++----
kexec/arch/arm64/kexec-arm64.h | 1 +
util_lib/elf_info.c | 5 +++++
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index bd650e6..ccc92db 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,15 @@ 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;
+
+ stext_sym_addr = get_kernel_sym("_stext");
if (stext_sym_addr == 0) {
fprintf(stderr, "Can't get the symbol of _stext.\n");
@@ -900,6 +908,7 @@ static int get_va_bits(void)
return -1;
}
+out:
dbgprintf("va_bits : %d\n", va_bits);
return 0;
@@ -917,14 +926,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 = (0xffffffffffffffffUL) << (va_bits - 1);
+ else
+ *page_offset = (0xffffffffffffffffUL) << 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 +959,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/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
index ed99d9d..d291705 100644
--- a/kexec/arch/arm64/kexec-arm64.h
+++ b/kexec/arch/arm64/kexec-arm64.h
@@ -66,6 +66,7 @@ struct arm64_mem {
#define arm64_mem_ngv UINT64_MAX
extern struct arm64_mem arm64_mem;
+extern void (*arch_scan_vmcoreinfo)(char *pos);
uint64_t get_phys_offset(void);
uint64_t get_vp_offset(void);
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;
}
--
2.31.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] arm64: make phys_to_virt() correct
2021-12-10 3:07 [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
` (2 preceding siblings ...)
2021-12-10 3:07 ` [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel Pingfan Liu
@ 2021-12-15 1:43 ` Pingfan Liu
3 siblings, 0 replies; 12+ messages in thread
From: Pingfan Liu @ 2021-12-15 1:43 UTC (permalink / raw)
To: kexec, AKASHI Takahiro, horms
+ AKASHI Takahiro
+ Simon Horman
Thank you guys for any suggestions.
And vmcore-dmesg just can not work for 52-bits VA arm64 now.
Thanks,
Pingfan
On Fri, Dec 10, 2021 at 11:07 AM 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.
>
> This series fixes them so vmcore-dmesg can work on 52-bits VA kernel
>
> 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 | 43 +++++++++++++++++++++++-------
> kexec/arch/arm64/kexec-arm64.h | 4 ++-
> util_lib/elf_info.c | 7 ++++-
> util_lib/include/elf_info.h | 2 +-
> 5 files changed, 44 insertions(+), 35 deletions(-)
>
> --
> 2.31.1
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel
2021-12-10 3:07 ` [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel Pingfan Liu
@ 2021-12-15 13:34 ` Philipp Rudo
2021-12-16 2:46 ` Pingfan Liu
[not found] ` <20211215130557.GB4314@vergenet.net>
1 sibling, 1 reply; 12+ messages in thread
From: Philipp Rudo @ 2021-12-15 13:34 UTC (permalink / raw)
To: Pingfan Liu; +Cc: kexec
Hi Pingfang,
On Fri, 10 Dec 2021 11:07:35 +0800
Pingfan Liu <piliu@redhat.com> wrote:
> 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.
>
> Signed-off-by: Pingfan Liu <piliu@redhat.com>
> ---
> kexec/arch/arm64/kexec-arm64.c | 31 +++++++++++++++++++++++++++----
> kexec/arch/arm64/kexec-arm64.h | 1 +
> util_lib/elf_info.c | 5 +++++
> 3 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index bd650e6..ccc92db 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,15 @@ 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;
> +
> + stext_sym_addr = get_kernel_sym("_stext");
>
> if (stext_sym_addr == 0) {
> fprintf(stderr, "Can't get the symbol of _stext.\n");
> @@ -900,6 +908,7 @@ static int get_va_bits(void)
> return -1;
> }
>
> +out:
> dbgprintf("va_bits : %d\n", va_bits);
>
> return 0;
> @@ -917,14 +926,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 = (0xffffffffffffffffUL) << (va_bits - 1);
> + else
> + *page_offset = (0xffffffffffffffffUL) << va_bits;
wouldn't it make sense to use ULONG_MAX here? At least for me it would
be much better readable.
> 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 +959,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/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
> index ed99d9d..d291705 100644
> --- a/kexec/arch/arm64/kexec-arm64.h
> +++ b/kexec/arch/arm64/kexec-arm64.h
> @@ -66,6 +66,7 @@ struct arm64_mem {
>
> #define arm64_mem_ngv UINT64_MAX
> extern struct arm64_mem arm64_mem;
> +extern void (*arch_scan_vmcoreinfo)(char *pos);
This definition isn't arm64 specific. I think this should go to
util_lib/include/elf_info.h.
Thanks
Philipp
>
> uint64_t get_phys_offset(void);
> uint64_t get_vp_offset(void);
> 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;
> }
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] arm64/crashdump: unify routine to get page_offset
2021-12-10 3:07 ` [PATCH 2/3] arm64/crashdump: unify routine to get page_offset Pingfan Liu
@ 2021-12-15 13:35 ` Philipp Rudo
2021-12-16 2:58 ` Pingfan Liu
0 siblings, 1 reply; 12+ messages in thread
From: Philipp Rudo @ 2021-12-15 13:35 UTC (permalink / raw)
To: Pingfan Liu; +Cc: kexec
Hi Pinfang,
On Fri, 10 Dec 2021 11:07:34 +0800
Pingfan Liu <piliu@redhat.com> 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>
> ---
> kexec/arch/arm64/crashdump-arm64.c | 23 +----------------------
> kexec/arch/arm64/kexec-arm64.c | 4 ++--
> kexec/arch/arm64/kexec-arm64.h | 1 +
> 3 files changed, 4 insertions(+), 24 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
[...]
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 7373fa3..bd650e6 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;
>
The new page_offset is never used in this patch. There's still the line
left with the global page_offset but that will cause a type miss-match
(unsigend long vs unsigned log *). In the next patch you do the update
(page_offset -> *page_offset) but in the meantime the code is broken.
Thanks
Philipp
> @@ -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)
> {
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel
[not found] ` <20211215130557.GB4314@vergenet.net>
@ 2021-12-16 1:59 ` Pingfan Liu
2021-12-16 11:02 ` Simon Horman
0 siblings, 1 reply; 12+ messages in thread
From: Pingfan Liu @ 2021-12-16 1:59 UTC (permalink / raw)
To: Signed-off-by@vergenet.net:Simon Horman; +Cc: kexec
On Wed, Dec 15, 2021 at 9:06 PM Signed-off-by@vergenet.net:Simon
Horman <horms@verge.net.au> wrote:
>
> On Fri, Dec 10, 2021 at 11:07:35AM +0800, Pingfan Liu wrote:
> > 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.
> >
> > Signed-off-by: Pingfan Liu <piliu@redhat.com>
> > ---
> > kexec/arch/arm64/kexec-arm64.c | 31 +++++++++++++++++++++++++++----
> > kexec/arch/arm64/kexec-arm64.h | 1 +
> > util_lib/elf_info.c | 5 +++++
> > 3 files changed, 33 insertions(+), 4 deletions(-)
> >
> > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > index bd650e6..ccc92db 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,15 @@ 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;
>
> If va_bits is exposed by the kernel then it will be used.
> Else we continue here. Are there actually cases (old kernels) where
> we expect to continue. Or could we get rid of the fallback code here?
>
va_bits is exposed by kernel commit 84c57dbd3c48 ("arm64: kernel:
arch_crash_save_vmcoreinfo() should depend on CONFIG_CRASH_CORE")
And the first kernel which contains VA_BITS is v4.19.
I have no idea about the need for old kernels. Maybe just keep the
compatible code and throw a warning to remind users?
> > +
> > + stext_sym_addr = get_kernel_sym("_stext");
> >
> > if (stext_sym_addr == 0) {
> > fprintf(stderr, "Can't get the symbol of _stext.\n");
> > @@ -900,6 +908,7 @@ static int get_va_bits(void)
> > return -1;
> > }
> >
> > +out:
> > dbgprintf("va_bits : %d\n", va_bits);
> >
> > return 0;
> > @@ -917,14 +926,27 @@ int get_page_offset(unsigned long *page_offset)
> > if (ret < 0)
> > return ret;
> >
>
> I'm confused about why there is both a (va_bits - 1)
> and va_bits case here.
>
It originates from the changes of memory layout on arm64.
And mostly it is contributed by kernel commit 14c127c957c1 ("arm64:
mm: Flip kernel VA space"),
where sees the changes:
-#define PAGE_OFFSET (UL(0xffffffffffffffff) - \
(UL(1) << (VA_BITS - 1)) + 1)
+#define PAGE_OFFSET (UL(0xffffffffffffffff) - \
+ (UL(1) << VA_BITS) + 1)
Thanks,
Pingfan
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel
2021-12-15 13:34 ` Philipp Rudo
@ 2021-12-16 2:46 ` Pingfan Liu
2021-12-16 3:05 ` Pingfan Liu
0 siblings, 1 reply; 12+ messages in thread
From: Pingfan Liu @ 2021-12-16 2:46 UTC (permalink / raw)
To: Philipp Rudo; +Cc: kexec
On Wed, Dec 15, 2021 at 9:35 PM Philipp Rudo <prudo@redhat.com> wrote:
>
> Hi Pingfang,
>
> On Fri, 10 Dec 2021 11:07:35 +0800
> Pingfan Liu <piliu@redhat.com> wrote:
>
> > 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.
> >
> > Signed-off-by: Pingfan Liu <piliu@redhat.com>
> > ---
> > kexec/arch/arm64/kexec-arm64.c | 31 +++++++++++++++++++++++++++----
> > kexec/arch/arm64/kexec-arm64.h | 1 +
> > util_lib/elf_info.c | 5 +++++
> > 3 files changed, 33 insertions(+), 4 deletions(-)
> >
> > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > index bd650e6..ccc92db 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,15 @@ 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;
> > +
> > + stext_sym_addr = get_kernel_sym("_stext");
> >
> > if (stext_sym_addr == 0) {
> > fprintf(stderr, "Can't get the symbol of _stext.\n");
> > @@ -900,6 +908,7 @@ static int get_va_bits(void)
> > return -1;
> > }
> >
> > +out:
> > dbgprintf("va_bits : %d\n", va_bits);
> >
> > return 0;
> > @@ -917,14 +926,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 = (0xffffffffffffffffUL) << (va_bits - 1);
> > + else
> > + *page_offset = (0xffffffffffffffffUL) << va_bits;
>
> wouldn't it make sense to use ULONG_MAX here? At least for me it would
> be much better readable.
>
Yes, I tend to agree and will update it in V2 (if there is no need to
compile it on 32-bits machine, which I consider as a rare case
nowadays.)
> > 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 +959,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/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
> > index ed99d9d..d291705 100644
> > --- a/kexec/arch/arm64/kexec-arm64.h
> > +++ b/kexec/arch/arm64/kexec-arm64.h
> > @@ -66,6 +66,7 @@ struct arm64_mem {
> >
> > #define arm64_mem_ngv UINT64_MAX
> > extern struct arm64_mem arm64_mem;
> > +extern void (*arch_scan_vmcoreinfo)(char *pos);
>
> This definition isn't arm64 specific. I think this should go to
> util_lib/include/elf_info.h.
>
Good suggestion and update it in V2.
Thanks,
Pingfan
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] arm64/crashdump: unify routine to get page_offset
2021-12-15 13:35 ` Philipp Rudo
@ 2021-12-16 2:58 ` Pingfan Liu
0 siblings, 0 replies; 12+ messages in thread
From: Pingfan Liu @ 2021-12-16 2:58 UTC (permalink / raw)
To: Philipp Rudo; +Cc: kexec
On Wed, Dec 15, 2021 at 9:35 PM Philipp Rudo <prudo@redhat.com> wrote:
>
> Hi Pinfang,
>
> On Fri, 10 Dec 2021 11:07:34 +0800
> Pingfan Liu <piliu@redhat.com> 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>
> > ---
> > kexec/arch/arm64/crashdump-arm64.c | 23 +----------------------
> > kexec/arch/arm64/kexec-arm64.c | 4 ++--
> > kexec/arch/arm64/kexec-arm64.h | 1 +
> > 3 files changed, 4 insertions(+), 24 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
>
> [...]
>
> > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > index 7373fa3..bd650e6 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;
> >
>
> The new page_offset is never used in this patch. There's still the line
> left with the global page_offset but that will cause a type miss-match
> (unsigend long vs unsigned log *). In the next patch you do the update
> (page_offset -> *page_offset) but in the meantime the code is broken.
>
I will fix it in V2.
Thanks,
Pingfan
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel
2021-12-16 2:46 ` Pingfan Liu
@ 2021-12-16 3:05 ` Pingfan Liu
0 siblings, 0 replies; 12+ messages in thread
From: Pingfan Liu @ 2021-12-16 3:05 UTC (permalink / raw)
To: Philipp Rudo; +Cc: kexec
On Thu, Dec 16, 2021 at 10:46 AM Pingfan Liu <piliu@redhat.com> wrote:
>
> On Wed, Dec 15, 2021 at 9:35 PM Philipp Rudo <prudo@redhat.com> wrote:
> >
> > Hi Pingfang,
> >
> > On Fri, 10 Dec 2021 11:07:35 +0800
> > Pingfan Liu <piliu@redhat.com> wrote:
> >
> > > 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.
> > >
> > > Signed-off-by: Pingfan Liu <piliu@redhat.com>
> > > ---
> > > kexec/arch/arm64/kexec-arm64.c | 31 +++++++++++++++++++++++++++----
> > > kexec/arch/arm64/kexec-arm64.h | 1 +
> > > util_lib/elf_info.c | 5 +++++
> > > 3 files changed, 33 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > > index bd650e6..ccc92db 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,15 @@ 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;
> > > +
> > > + stext_sym_addr = get_kernel_sym("_stext");
> > >
> > > if (stext_sym_addr == 0) {
> > > fprintf(stderr, "Can't get the symbol of _stext.\n");
> > > @@ -900,6 +908,7 @@ static int get_va_bits(void)
> > > return -1;
> > > }
> > >
> > > +out:
> > > dbgprintf("va_bits : %d\n", va_bits);
> > >
> > > return 0;
> > > @@ -917,14 +926,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 = (0xffffffffffffffffUL) << (va_bits - 1);
> > > + else
> > > + *page_offset = (0xffffffffffffffffUL) << va_bits;
> >
> > wouldn't it make sense to use ULONG_MAX here? At least for me it would
> > be much better readable.
> >
>
> Yes, I tend to agree and will update it in V2 (if there is no need to
> compile it on 32-bits machine, which I consider as a rare case
> nowadays.)
>
I think UINT64_MAX can free of this issue
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel
2021-12-16 1:59 ` Pingfan Liu
@ 2021-12-16 11:02 ` Simon Horman
0 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2021-12-16 11:02 UTC (permalink / raw)
To: Pingfan Liu; +Cc: kexec
On Thu, Dec 16, 2021 at 09:59:42AM +0800, Pingfan Liu wrote:
> On Wed, Dec 15, 2021 at 9:06 PM Signed-off-by@vergenet.net:Simon
> Horman <horms@verge.net.au> wrote:
> >
> > On Fri, Dec 10, 2021 at 11:07:35AM +0800, Pingfan Liu wrote:
> > > 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.
> > >
> > > Signed-off-by: Pingfan Liu <piliu@redhat.com>
> > > ---
> > > kexec/arch/arm64/kexec-arm64.c | 31 +++++++++++++++++++++++++++----
> > > kexec/arch/arm64/kexec-arm64.h | 1 +
> > > util_lib/elf_info.c | 5 +++++
> > > 3 files changed, 33 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > > index bd650e6..ccc92db 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,15 @@ 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;
> >
> > If va_bits is exposed by the kernel then it will be used.
> > Else we continue here. Are there actually cases (old kernels) where
> > we expect to continue. Or could we get rid of the fallback code here?
> >
>
> va_bits is exposed by kernel commit 84c57dbd3c48 ("arm64: kernel:
> arch_crash_save_vmcoreinfo() should depend on CONFIG_CRASH_CORE")
> And the first kernel which contains VA_BITS is v4.19.
> I have no idea about the need for old kernels. Maybe just keep the
> compatible code and throw a warning to remind users?
I suspect that kexec for ARM64 was present in v4.19, so we
should leave this part as-is.
> > > +
> > > + stext_sym_addr = get_kernel_sym("_stext");
> > >
> > > if (stext_sym_addr == 0) {
> > > fprintf(stderr, "Can't get the symbol of _stext.\n");
> > > @@ -900,6 +908,7 @@ static int get_va_bits(void)
> > > return -1;
> > > }
> > >
> > > +out:
> > > dbgprintf("va_bits : %d\n", va_bits);
> > >
> > > return 0;
> > > @@ -917,14 +926,27 @@ int get_page_offset(unsigned long *page_offset)
> > > if (ret < 0)
> > > return ret;
> > >
> >
> > I'm confused about why there is both a (va_bits - 1)
> > and va_bits case here.
> >
>
> It originates from the changes of memory layout on arm64.
> And mostly it is contributed by kernel commit 14c127c957c1 ("arm64:
> mm: Flip kernel VA space"),
> where sees the changes:
> -#define PAGE_OFFSET (UL(0xffffffffffffffff) - \
> (UL(1) << (VA_BITS - 1)) + 1)
> +#define PAGE_OFFSET (UL(0xffffffffffffffff) - \
> + (UL(1) << VA_BITS) + 1)
>
>
> Thanks,
>
> Pingfan
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-12-16 11:02 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-10 3:07 [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
2021-12-10 3:07 ` [PATCH 1/3] arm64: make phys_offset signed Pingfan Liu
2021-12-10 3:07 ` [PATCH 2/3] arm64/crashdump: unify routine to get page_offset Pingfan Liu
2021-12-15 13:35 ` Philipp Rudo
2021-12-16 2:58 ` Pingfan Liu
2021-12-10 3:07 ` [PATCH 3/3] arm64: read VA_BITS from kcore for 52-bits VA kernel Pingfan Liu
2021-12-15 13:34 ` Philipp Rudo
2021-12-16 2:46 ` Pingfan Liu
2021-12-16 3:05 ` Pingfan Liu
[not found] ` <20211215130557.GB4314@vergenet.net>
2021-12-16 1:59 ` Pingfan Liu
2021-12-16 11:02 ` Simon Horman
2021-12-15 1:43 ` [PATCH 0/3] arm64: make phys_to_virt() correct Pingfan Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.