Discussion:
[PATCH 0/2] makedumpfile: Adapt code to get value of phys_base
Baoquan He
2016-11-08 08:55:17 UTC
Permalink
This patchset is on top of Pratyush's patchset:
**x86_64: Fix page_offset for randomized base enabled**

Because exporting virtual address of phys_base is really helpless for Crash
and Makedumpfile, so has posted a kernel patch to export value of phys_base
instead. Patch 2/1 is used to adapt code for that.

Patch 2/2 is a clean up patch, it's based on Pratyush's patchset. MODULES_VADDR
is used to judge if a virtual address in inside kernel text region or direct
mapping region. And MODULES_VADDR is decided by KERNEL_IMAGE_SIZE. With Pratyush's
above patchset, KERNEL_IMAGE_SIZE is not needed. Remove it here. But Crash needs
it to check if kaslr is enabled. It still need be exported.

Baoquan He (2):
makedumpfile: Adapt code to get value of phys_base
makedumpfile: Clean up unused KERNEL_IMAGE_SIZE

arch/x86_64.c | 7 +++++--
makedumpfile.c | 16 +++-------------
makedumpfile.h | 5 +----
3 files changed, 9 insertions(+), 19 deletions(-)
--
2.5.5
Baoquan He
2016-11-08 08:55:19 UTC
Permalink
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.

Signed-off-by: Baoquan He <***@redhat.com>
---
makedumpfile.c | 11 +----------
makedumpfile.h | 3 ---
2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index a3f711e..1c95306 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1994,14 +1994,7 @@ get_value_for_old_linux(void)
NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) =
PAGE_BUDDY_MAPCOUNT_VALUE_v2_6_39_to_latest_version;
}
-#ifdef __x86_64__
- if (NUMBER(KERNEL_IMAGE_SIZE) == NOT_FOUND_NUMBER) {
- if (info->kernel_version < KERNEL_VERSION(2, 6, 26))
- NUMBER(KERNEL_IMAGE_SIZE) = KERNEL_IMAGE_SIZE_ORIG;
- else
- NUMBER(KERNEL_IMAGE_SIZE) = KERNEL_IMAGE_SIZE_2_6_26;
- }
-#endif
+
if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) {
if (info->kernel_version >= KERNEL_VERSION(2, 6, 27))
SIZE(pageflags) =
@@ -2258,7 +2251,6 @@ write_vmcoreinfo_data(void)
WRITE_NUMBER("PG_hwpoison", PG_hwpoison);

WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
- WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
WRITE_NUMBER("phys_base", phys_base);

WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
@@ -2606,7 +2598,6 @@ read_vmcoreinfo(void)
READ_SRCFILE("pud_t", pud_t);

READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
- READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
READ_NUMBER("phys_base", phys_base);

READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
diff --git a/makedumpfile.h b/makedumpfile.h
index 422b6be..b627e32 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -573,8 +573,6 @@ int get_va_bits_arm64(void);
#define VMEMMAP_END_2_6_31 (0xffffeaffffffffff) /* 2.6.31, or later */

#define __START_KERNEL_map (0xffffffff80000000)
-#define KERNEL_IMAGE_SIZE_ORIG (0x0000000008000000) /* 2.6.25, or former */
-#define KERNEL_IMAGE_SIZE_2_6_26 (0x0000000020000000) /* 2.6.26, or later */
#define KVBASE PAGE_OFFSET
#define _SECTION_SIZE_BITS (27)
#define _MAX_PHYSMEM_BITS_ORIG (40)
@@ -1713,7 +1711,6 @@ struct number_table {
long PG_hwpoison;

long PAGE_BUDDY_MAPCOUNT_VALUE;
- long KERNEL_IMAGE_SIZE;
long SECTION_SIZE_BITS;
long MAX_PHYSMEM_BITS;
long HUGETLB_PAGE_DTOR;
--
2.5.5
Dave Anderson
2016-11-08 14:26:03 UTC
Permalink
----- Original Message -----
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,

I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?

Dave
Post by Baoquan He
---
makedumpfile.c | 11 +----------
makedumpfile.h | 3 ---
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index a3f711e..1c95306 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1994,14 +1994,7 @@ get_value_for_old_linux(void)
NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) =
PAGE_BUDDY_MAPCOUNT_VALUE_v2_6_39_to_latest_version;
}
-#ifdef __x86_64__
- if (NUMBER(KERNEL_IMAGE_SIZE) == NOT_FOUND_NUMBER) {
- if (info->kernel_version < KERNEL_VERSION(2, 6, 26))
- NUMBER(KERNEL_IMAGE_SIZE) = KERNEL_IMAGE_SIZE_ORIG;
- else
- NUMBER(KERNEL_IMAGE_SIZE) = KERNEL_IMAGE_SIZE_2_6_26;
- }
-#endif
+
if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) {
if (info->kernel_version >= KERNEL_VERSION(2, 6, 27))
SIZE(pageflags) =
@@ -2258,7 +2251,6 @@ write_vmcoreinfo_data(void)
WRITE_NUMBER("PG_hwpoison", PG_hwpoison);
WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
- WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
WRITE_NUMBER("phys_base", phys_base);
WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
@@ -2606,7 +2598,6 @@ read_vmcoreinfo(void)
READ_SRCFILE("pud_t", pud_t);
READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
- READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
READ_NUMBER("phys_base", phys_base);
READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
diff --git a/makedumpfile.h b/makedumpfile.h
index 422b6be..b627e32 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -573,8 +573,6 @@ int get_va_bits_arm64(void);
#define VMEMMAP_END_2_6_31 (0xffffeaffffffffff) /* 2.6.31, or later */
#define __START_KERNEL_map (0xffffffff80000000)
-#define KERNEL_IMAGE_SIZE_ORIG (0x0000000008000000) /* 2.6.25, or former */
-#define KERNEL_IMAGE_SIZE_2_6_26 (0x0000000020000000) /* 2.6.26, or later
*/
#define KVBASE PAGE_OFFSET
#define _SECTION_SIZE_BITS (27)
#define _MAX_PHYSMEM_BITS_ORIG (40)
@@ -1713,7 +1711,6 @@ struct number_table {
long PG_hwpoison;
long PAGE_BUDDY_MAPCOUNT_VALUE;
- long KERNEL_IMAGE_SIZE;
long SECTION_SIZE_BITS;
long MAX_PHYSMEM_BITS;
long HUGETLB_PAGE_DTOR;
--
2.5.5
Baoquan He
2016-11-09 00:16:50 UTC
Permalink
Post by Dave Anderson
----- Original Message -----
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,
I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?
Aha, seems makedumpfile will get the vmcoreinfo offset and size, then
write the whole vmcoreinfo block into dumped vmcore header. But if
specify '-g' for makedumpfile to only generate a vmcoreinfo file, it
won't contain KERNEL_IMAGE_SIZE.

makedumpfile -g vmcoreinfo -x vmlinux

So I am not sure if you care about vmcoreinfo file, but you are right, I
should not remove the vmcoreinfo reading and writing data.

Thanks for pointing it out, will send v2 post.

Thanks
Baoquan
Atsushi Kumagai
2016-11-10 01:15:40 UTC
Permalink
Hello Baoquan,
Post by Baoquan He
Post by Dave Anderson
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,
I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?
Aha, seems makedumpfile will get the vmcoreinfo offset and size, then
write the whole vmcoreinfo block into dumped vmcore header. But if
specify '-g' for makedumpfile to only generate a vmcoreinfo file, it
won't contain KERNEL_IMAGE_SIZE.
makedumpfile -g vmcoreinfo -x vmlinux
So I am not sure if you care about vmcoreinfo file, but you are right, I
should not remove the vmcoreinfo reading and writing data.
Thanks for pointing it out, will send v2 post.
I understand the vmcoreinfo file generated by '-g' is used only for
makedumpfile, so it's OK if it doesn't contain KERNEL_IMAGE_SIZE since
crash doesn't refer to the file.
This patch doesn't modify the original vmcoreinfo section in dumpfile,
it sounds reasonable to me.


Thanks,
Atsushi Kumagai
Baoquan He
2016-11-10 02:06:43 UTC
Permalink
Post by Atsushi Kumagai
Hello Baoquan,
Post by Baoquan He
Post by Dave Anderson
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,
I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?
Aha, seems makedumpfile will get the vmcoreinfo offset and size, then
write the whole vmcoreinfo block into dumped vmcore header. But if
specify '-g' for makedumpfile to only generate a vmcoreinfo file, it
won't contain KERNEL_IMAGE_SIZE.
makedumpfile -g vmcoreinfo -x vmlinux
So I am not sure if you care about vmcoreinfo file, but you are right, I
should not remove the vmcoreinfo reading and writing data.
Thanks for pointing it out, will send v2 post.
I understand the vmcoreinfo file generated by '-g' is used only for
makedumpfile, so it's OK if it doesn't contain KERNEL_IMAGE_SIZE since
crash doesn't refer to the file.
This patch doesn't modify the original vmcoreinfo section in dumpfile,
it sounds reasonable to me.
Thanks for your comments, Atsushi! Then I am fine.

Thanks
Baoquan
Atsushi Kumagai
2016-11-10 03:58:06 UTC
Permalink
Post by Baoquan He
Post by Atsushi Kumagai
Post by Baoquan He
Post by Dave Anderson
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,
I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?
Aha, seems makedumpfile will get the vmcoreinfo offset and size, then
write the whole vmcoreinfo block into dumped vmcore header. But if
specify '-g' for makedumpfile to only generate a vmcoreinfo file, it
won't contain KERNEL_IMAGE_SIZE.
makedumpfile -g vmcoreinfo -x vmlinux
So I am not sure if you care about vmcoreinfo file, but you are right, I
should not remove the vmcoreinfo reading and writing data.
Thanks for pointing it out, will send v2 post.
I understand the vmcoreinfo file generated by '-g' is used only for
makedumpfile, so it's OK if it doesn't contain KERNEL_IMAGE_SIZE since
crash doesn't refer to the file.
This patch doesn't modify the original vmcoreinfo section in dumpfile,
it sounds reasonable to me.
Thanks for your comments, Atsushi! Then I am fine.
Additionally, I think it would be better to remove all of the code
you added in commit 56649f7b6bfe7.


Thanks,
Atsushi Kumagai
Baoquan He
2016-11-10 04:48:46 UTC
Permalink
Post by Atsushi Kumagai
Post by Baoquan He
Post by Atsushi Kumagai
Post by Baoquan He
Post by Dave Anderson
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,
I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?
Aha, seems makedumpfile will get the vmcoreinfo offset and size, then
write the whole vmcoreinfo block into dumped vmcore header. But if
specify '-g' for makedumpfile to only generate a vmcoreinfo file, it
won't contain KERNEL_IMAGE_SIZE.
makedumpfile -g vmcoreinfo -x vmlinux
So I am not sure if you care about vmcoreinfo file, but you are right, I
should not remove the vmcoreinfo reading and writing data.
Thanks for pointing it out, will send v2 post.
I understand the vmcoreinfo file generated by '-g' is used only for
makedumpfile, so it's OK if it doesn't contain KERNEL_IMAGE_SIZE since
crash doesn't refer to the file.
This patch doesn't modify the original vmcoreinfo section in dumpfile,
it sounds reasonable to me.
Thanks for your comments, Atsushi! Then I am fine.
Additionally, I think it would be better to remove all of the code
you added in commit 56649f7b6bfe7.
Check it again, seems patch 2/2 equals to reverting commit
56649f7b6bfe7. Did I miss anything?

Thanks
Baoquan
Atsushi Kumagai
2016-11-10 05:31:57 UTC
Permalink
Post by Baoquan He
Post by Atsushi Kumagai
Post by Baoquan He
Post by Atsushi Kumagai
Post by Baoquan He
Post by Dave Anderson
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,
I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?
Aha, seems makedumpfile will get the vmcoreinfo offset and size, then
write the whole vmcoreinfo block into dumped vmcore header. But if
specify '-g' for makedumpfile to only generate a vmcoreinfo file, it
won't contain KERNEL_IMAGE_SIZE.
makedumpfile -g vmcoreinfo -x vmlinux
So I am not sure if you care about vmcoreinfo file, but you are right, I
should not remove the vmcoreinfo reading and writing data.
Thanks for pointing it out, will send v2 post.
I understand the vmcoreinfo file generated by '-g' is used only for
makedumpfile, so it's OK if it doesn't contain KERNEL_IMAGE_SIZE since
crash doesn't refer to the file.
This patch doesn't modify the original vmcoreinfo section in dumpfile,
it sounds reasonable to me.
Thanks for your comments, Atsushi! Then I am fine.
Additionally, I think it would be better to remove all of the code
you added in commit 56649f7b6bfe7.
Check it again, seems patch 2/2 equals to reverting commit
56649f7b6bfe7. Did I miss anything?
You are right, it will be just a revert commit.

Thanks,
Atsushi Kumagai
Dave Anderson
2016-11-10 13:48:22 UTC
Permalink
----- Original Message -----
Post by Baoquan He
Post by Atsushi Kumagai
Hello Baoquan,
Post by Baoquan He
Post by Dave Anderson
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
Hi Bao,
I'm not clear on this. The crash utility still reads/needs KERNEL_IMAGE_SIZE
from the dumpfile's vmcoreinfo data. Is it being written to the vmcoreinfo
section somewhere else in the code?
Aha, seems makedumpfile will get the vmcoreinfo offset and size, then
write the whole vmcoreinfo block into dumped vmcore header. But if
specify '-g' for makedumpfile to only generate a vmcoreinfo file, it
won't contain KERNEL_IMAGE_SIZE.
makedumpfile -g vmcoreinfo -x vmlinux
So I am not sure if you care about vmcoreinfo file, but you are right, I
should not remove the vmcoreinfo reading and writing data.
Thanks for pointing it out, will send v2 post.
I understand the vmcoreinfo file generated by '-g' is used only for
makedumpfile, so it's OK if it doesn't contain KERNEL_IMAGE_SIZE since
crash doesn't refer to the file.
This patch doesn't modify the original vmcoreinfo section in dumpfile,
it sounds reasonable to me.
Thanks for your comments, Atsushi! Then I am fine.
Thanks
Baoquan
I concur -- I wasn't even aware of the "makedumpfile -g" option.

Sorry for the noise...
Dave
Baoquan He
2016-11-09 06:55:12 UTC
Permalink
Sorry, I am not familiar with DWARF format, and KERNEL_IMAGE_SIZE is a
defined MACRO. I don't know what DW_TAG_XXX_type should be taken to
retrieve it when specify "-g" to generate vmcoreinfo file from vmlinux.

So drop this patch for now though it doesn't affect Dave's Crash code,
will make time to try to add it later, I need work on other urgent
things now. Or someone who is interested can add this.

Now only patch 1/2 is available.

Thanks
Baoquan
Post by Baoquan He
The old MODULES_VADDR need be decided by KERNEL_IMAGE_SIZE when kaslr
enabled. Now MODULES_VADDR is not needed any more since Pratyush makes
all VA to PA converting done by page table lookup. So remove its related
code.
---
makedumpfile.c | 11 +----------
makedumpfile.h | 3 ---
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index a3f711e..1c95306 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1994,14 +1994,7 @@ get_value_for_old_linux(void)
NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) =
PAGE_BUDDY_MAPCOUNT_VALUE_v2_6_39_to_latest_version;
}
-#ifdef __x86_64__
- if (NUMBER(KERNEL_IMAGE_SIZE) == NOT_FOUND_NUMBER) {
- if (info->kernel_version < KERNEL_VERSION(2, 6, 26))
- NUMBER(KERNEL_IMAGE_SIZE) = KERNEL_IMAGE_SIZE_ORIG;
- else
- NUMBER(KERNEL_IMAGE_SIZE) = KERNEL_IMAGE_SIZE_2_6_26;
- }
-#endif
+
if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) {
if (info->kernel_version >= KERNEL_VERSION(2, 6, 27))
SIZE(pageflags) =
@@ -2258,7 +2251,6 @@ write_vmcoreinfo_data(void)
WRITE_NUMBER("PG_hwpoison", PG_hwpoison);
WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
- WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
WRITE_NUMBER("phys_base", phys_base);
WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
@@ -2606,7 +2598,6 @@ read_vmcoreinfo(void)
READ_SRCFILE("pud_t", pud_t);
READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
- READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
READ_NUMBER("phys_base", phys_base);
READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
diff --git a/makedumpfile.h b/makedumpfile.h
index 422b6be..b627e32 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -573,8 +573,6 @@ int get_va_bits_arm64(void);
#define VMEMMAP_END_2_6_31 (0xffffeaffffffffff) /* 2.6.31, or later */
#define __START_KERNEL_map (0xffffffff80000000)
-#define KERNEL_IMAGE_SIZE_ORIG (0x0000000008000000) /* 2.6.25, or former */
-#define KERNEL_IMAGE_SIZE_2_6_26 (0x0000000020000000) /* 2.6.26, or later */
#define KVBASE PAGE_OFFSET
#define _SECTION_SIZE_BITS (27)
#define _MAX_PHYSMEM_BITS_ORIG (40)
@@ -1713,7 +1711,6 @@ struct number_table {
long PG_hwpoison;
long PAGE_BUDDY_MAPCOUNT_VALUE;
- long KERNEL_IMAGE_SIZE;
long SECTION_SIZE_BITS;
long MAX_PHYSMEM_BITS;
long HUGETLB_PAGE_DTOR;
--
2.5.5
Baoquan He
2016-11-08 08:55:18 UTC
Permalink
Kernel code only exports virtual address of phys_base now and it's helpless
for Crash and Makedumpfile. Below patch which changes code to export value
of phys_base has been posted to upstream. So adapt code to get it.

kexec: Change to export the value of phys_base instead of symbol address
marc.info/?l=linux-kernel&m=147856863629624&w=2

Signed-off-by: Baoquan He <***@redhat.com>
---
arch/x86_64.c | 7 +++++--
makedumpfile.c | 5 ++---
makedumpfile.h | 2 +-
3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index 3ef33ae..8593c1a 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -61,7 +61,10 @@ get_phys_base_x86_64(void)
/*
* Get the relocatable offset
*/
- info->phys_base = 0; /* default/traditional */
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER) {
+ info->phys_base = NUMBER(phys_base);
+ return TRUE;
+ }

for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
if (virt_start >= __START_KERNEL_map) {
@@ -189,7 +192,7 @@ vtop4_x86_64(unsigned long vaddr)
unsigned long pte_paddr, pte;
unsigned long phys_base;

- if (SYMBOL(phys_base) != NOT_FOUND_SYMBOL)
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER)
phys_base = info->phys_base;
else
phys_base = 0;
diff --git a/makedumpfile.c b/makedumpfile.c
index b916dfb..a3f711e 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1507,7 +1507,6 @@ get_symbol_info(void)
SYMBOL_INIT(init_level4_pgt, "init_level4_pgt");
SYMBOL_INIT(vmlist, "vmlist");
SYMBOL_INIT(vmap_area_list, "vmap_area_list");
- SYMBOL_INIT(phys_base, "phys_base");
SYMBOL_INIT(node_online_map, "node_online_map");
SYMBOL_INIT(node_states, "node_states");
SYMBOL_INIT(node_memblk, "node_memblk");
@@ -2134,7 +2133,6 @@ write_vmcoreinfo_data(void)
WRITE_SYMBOL("init_level4_pgt", init_level4_pgt);
WRITE_SYMBOL("vmlist", vmlist);
WRITE_SYMBOL("vmap_area_list", vmap_area_list);
- WRITE_SYMBOL("phys_base", phys_base);
WRITE_SYMBOL("node_online_map", node_online_map);
WRITE_SYMBOL("node_states", node_states);
WRITE_SYMBOL("node_data", node_data);
@@ -2261,6 +2259,7 @@ write_vmcoreinfo_data(void)

WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ WRITE_NUMBER("phys_base", phys_base);

WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);

@@ -2488,7 +2487,6 @@ read_vmcoreinfo(void)
READ_SYMBOL("init_level4_pgt", init_level4_pgt);
READ_SYMBOL("vmlist", vmlist);
READ_SYMBOL("vmap_area_list", vmap_area_list);
- READ_SYMBOL("phys_base", phys_base);
READ_SYMBOL("node_online_map", node_online_map);
READ_SYMBOL("node_states", node_states);
READ_SYMBOL("node_data", node_data);
@@ -2609,6 +2607,7 @@ read_vmcoreinfo(void)

READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ READ_NUMBER("phys_base", phys_base);

READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);

diff --git a/makedumpfile.h b/makedumpfile.h
index 338c651..422b6be 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1394,7 +1394,6 @@ struct symbol_table {
unsigned long long init_level4_pgt;
unsigned long long vmlist;
unsigned long long vmap_area_list;
- unsigned long long phys_base;
unsigned long long node_online_map;
unsigned long long node_states;
unsigned long long node_memblk;
@@ -1718,6 +1717,7 @@ struct number_table {
long SECTION_SIZE_BITS;
long MAX_PHYSMEM_BITS;
long HUGETLB_PAGE_DTOR;
+ long phys_base;
};

struct srcfile_table {
--
2.5.5
Atsushi Kumagai
2016-11-10 03:57:51 UTC
Permalink
Post by Baoquan He
Kernel code only exports virtual address of phys_base now and it's helpless
for Crash and Makedumpfile. Below patch which changes code to export value
of phys_base has been posted to upstream. So adapt code to get it.
kexec: Change to export the value of phys_base instead of symbol address
marc.info/?l=linux-kernel&m=147856863629624&w=2
I suspect that this patch will break backward compatibility.
Post by Baoquan He
---
arch/x86_64.c | 7 +++++--
makedumpfile.c | 5 ++---
makedumpfile.h | 2 +-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86_64.c b/arch/x86_64.c
index 3ef33ae..8593c1a 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -61,7 +61,10 @@ get_phys_base_x86_64(void)
/*
* Get the relocatable offset
*/
- info->phys_base = 0; /* default/traditional */
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER) {
+ info->phys_base = NUMBER(phys_base);
+ return TRUE;
+ }
for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
if (virt_start >= __START_KERNEL_map) {
@@ -189,7 +192,7 @@ vtop4_x86_64(unsigned long vaddr)
unsigned long pte_paddr, pte;
unsigned long phys_base;
- if (SYMBOL(phys_base) != NOT_FOUND_SYMBOL)
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER)
phys_base = info->phys_base;
else
phys_base = 0;
In older kernel, even if get_phys_base_x86_64() calculates info->phys_base
from PT_LOAD, it will not be used since the kernels export *SYMBOL*(phys_base)
and phys_base will be always set to 0 here.
Is this check really needed ? Why don't you just set info->phys_base
to phys_base ? (Of course, info->phys_base should be initialized with 0
in get_phys_base_x86_64() in that case.)


Thanks,
Atsushi Kumagai
Post by Baoquan He
diff --git a/makedumpfile.c b/makedumpfile.c
index b916dfb..a3f711e 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1507,7 +1507,6 @@ get_symbol_info(void)
SYMBOL_INIT(init_level4_pgt, "init_level4_pgt");
SYMBOL_INIT(vmlist, "vmlist");
SYMBOL_INIT(vmap_area_list, "vmap_area_list");
- SYMBOL_INIT(phys_base, "phys_base");
SYMBOL_INIT(node_online_map, "node_online_map");
SYMBOL_INIT(node_states, "node_states");
SYMBOL_INIT(node_memblk, "node_memblk");
@@ -2134,7 +2133,6 @@ write_vmcoreinfo_data(void)
WRITE_SYMBOL("init_level4_pgt", init_level4_pgt);
WRITE_SYMBOL("vmlist", vmlist);
WRITE_SYMBOL("vmap_area_list", vmap_area_list);
- WRITE_SYMBOL("phys_base", phys_base);
WRITE_SYMBOL("node_online_map", node_online_map);
WRITE_SYMBOL("node_states", node_states);
WRITE_SYMBOL("node_data", node_data);
@@ -2261,6 +2259,7 @@ write_vmcoreinfo_data(void)
WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ WRITE_NUMBER("phys_base", phys_base);
WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
@@ -2488,7 +2487,6 @@ read_vmcoreinfo(void)
READ_SYMBOL("init_level4_pgt", init_level4_pgt);
READ_SYMBOL("vmlist", vmlist);
READ_SYMBOL("vmap_area_list", vmap_area_list);
- READ_SYMBOL("phys_base", phys_base);
READ_SYMBOL("node_online_map", node_online_map);
READ_SYMBOL("node_states", node_states);
READ_SYMBOL("node_data", node_data);
@@ -2609,6 +2607,7 @@ read_vmcoreinfo(void)
READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ READ_NUMBER("phys_base", phys_base);
READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
diff --git a/makedumpfile.h b/makedumpfile.h
index 338c651..422b6be 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1394,7 +1394,6 @@ struct symbol_table {
unsigned long long init_level4_pgt;
unsigned long long vmlist;
unsigned long long vmap_area_list;
- unsigned long long phys_base;
unsigned long long node_online_map;
unsigned long long node_states;
unsigned long long node_memblk;
@@ -1718,6 +1717,7 @@ struct number_table {
long SECTION_SIZE_BITS;
long MAX_PHYSMEM_BITS;
long HUGETLB_PAGE_DTOR;
+ long phys_base;
};
struct srcfile_table {
--
2.5.5
Baoquan He
2016-11-10 04:51:53 UTC
Permalink
Post by Atsushi Kumagai
Post by Baoquan He
Kernel code only exports virtual address of phys_base now and it's helpless
for Crash and Makedumpfile. Below patch which changes code to export value
of phys_base has been posted to upstream. So adapt code to get it.
kexec: Change to export the value of phys_base instead of symbol address
marc.info/?l=linux-kernel&m=147856863629624&w=2
I suspect that this patch will break backward compatibility.
Post by Baoquan He
---
arch/x86_64.c | 7 +++++--
makedumpfile.c | 5 ++---
makedumpfile.h | 2 +-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86_64.c b/arch/x86_64.c
index 3ef33ae..8593c1a 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -61,7 +61,10 @@ get_phys_base_x86_64(void)
/*
* Get the relocatable offset
*/
- info->phys_base = 0; /* default/traditional */
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER) {
+ info->phys_base = NUMBER(phys_base);
+ return TRUE;
+ }
for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
if (virt_start >= __START_KERNEL_map) {
@@ -189,7 +192,7 @@ vtop4_x86_64(unsigned long vaddr)
unsigned long pte_paddr, pte;
unsigned long phys_base;
- if (SYMBOL(phys_base) != NOT_FOUND_SYMBOL)
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER)
phys_base = info->phys_base;
else
phys_base = 0;
In older kernel, even if get_phys_base_x86_64() calculates info->phys_base
from PT_LOAD, it will not be used since the kernels export *SYMBOL*(phys_base)
and phys_base will be always set to 0 here.
Is this check really needed ? Why don't you just set info->phys_base
to phys_base ? (Of course, info->phys_base should be initialized with 0
in get_phys_base_x86_64() in that case.)
You are right. Will change as you suggested and repost.

Thanks
Baoquan
Baoquan He
2016-11-10 06:30:50 UTC
Permalink
Kernel code only exports virtual address of phys_base now and it's helpless
for Crash and Makedumpfile. Below patch which changes code to export value
of phys_base has been posted to upstream. So adapt code to get it.

kexec: Change to export the value of phys_base instead of symbol address
marc.info/?l=linux-kernel&m=147856863629624&w=2

Signed-off-by: Baoquan He <***@redhat.com>
---
v1->v2:
Patch v1 is not compatible with the old kernel. Change code in
get_phys_base_x86_64 and vtop4_x86_64 to avoid that according to
Atsushi's suggestion.

arch/x86_64.c | 12 +++++-------
makedumpfile.c | 5 ++---
makedumpfile.h | 2 +-
3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index 3ef33ae..010ea10 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -62,6 +62,10 @@ get_phys_base_x86_64(void)
* Get the relocatable offset
*/
info->phys_base = 0; /* default/traditional */
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER) {
+ info->phys_base = NUMBER(phys_base);
+ return TRUE;
+ }

for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
if (virt_start >= __START_KERNEL_map) {
@@ -187,12 +191,6 @@ vtop4_x86_64(unsigned long vaddr)
{
unsigned long page_dir, pml4, pgd_paddr, pgd_pte, pmd_paddr, pmd_pte;
unsigned long pte_paddr, pte;
- unsigned long phys_base;
-
- if (SYMBOL(phys_base) != NOT_FOUND_SYMBOL)
- phys_base = info->phys_base;
- else
- phys_base = 0;

if (SYMBOL(init_level4_pgt) == NOT_FOUND_SYMBOL) {
ERRMSG("Can't get the symbol of init_level4_pgt.\n");
@@ -202,7 +200,7 @@ vtop4_x86_64(unsigned long vaddr)
/*
* Get PGD.
*/
- page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + phys_base;
+ page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + info->phys_base;
page_dir += pml4_index(vaddr) * sizeof(unsigned long);
if (!readmem(PADDR, page_dir, &pml4, sizeof pml4)) {
ERRMSG("Can't get pml4 (page_dir:%lx).\n", page_dir);
diff --git a/makedumpfile.c b/makedumpfile.c
index b916dfb..a3f711e 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1507,7 +1507,6 @@ get_symbol_info(void)
SYMBOL_INIT(init_level4_pgt, "init_level4_pgt");
SYMBOL_INIT(vmlist, "vmlist");
SYMBOL_INIT(vmap_area_list, "vmap_area_list");
- SYMBOL_INIT(phys_base, "phys_base");
SYMBOL_INIT(node_online_map, "node_online_map");
SYMBOL_INIT(node_states, "node_states");
SYMBOL_INIT(node_memblk, "node_memblk");
@@ -2134,7 +2133,6 @@ write_vmcoreinfo_data(void)
WRITE_SYMBOL("init_level4_pgt", init_level4_pgt);
WRITE_SYMBOL("vmlist", vmlist);
WRITE_SYMBOL("vmap_area_list", vmap_area_list);
- WRITE_SYMBOL("phys_base", phys_base);
WRITE_SYMBOL("node_online_map", node_online_map);
WRITE_SYMBOL("node_states", node_states);
WRITE_SYMBOL("node_data", node_data);
@@ -2261,6 +2259,7 @@ write_vmcoreinfo_data(void)

WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ WRITE_NUMBER("phys_base", phys_base);

WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);

@@ -2488,7 +2487,6 @@ read_vmcoreinfo(void)
READ_SYMBOL("init_level4_pgt", init_level4_pgt);
READ_SYMBOL("vmlist", vmlist);
READ_SYMBOL("vmap_area_list", vmap_area_list);
- READ_SYMBOL("phys_base", phys_base);
READ_SYMBOL("node_online_map", node_online_map);
READ_SYMBOL("node_states", node_states);
READ_SYMBOL("node_data", node_data);
@@ -2609,6 +2607,7 @@ read_vmcoreinfo(void)

READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ READ_NUMBER("phys_base", phys_base);

READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);

diff --git a/makedumpfile.h b/makedumpfile.h
index 338c651..422b6be 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1394,7 +1394,6 @@ struct symbol_table {
unsigned long long init_level4_pgt;
unsigned long long vmlist;
unsigned long long vmap_area_list;
- unsigned long long phys_base;
unsigned long long node_online_map;
unsigned long long node_states;
unsigned long long node_memblk;
@@ -1718,6 +1717,7 @@ struct number_table {
long SECTION_SIZE_BITS;
long MAX_PHYSMEM_BITS;
long HUGETLB_PAGE_DTOR;
+ long phys_base;
};

struct srcfile_table {
--
2.5.5
Atsushi Kumagai
2016-11-11 08:12:29 UTC
Permalink
Post by Baoquan He
Kernel code only exports virtual address of phys_base now and it's helpless
for Crash and Makedumpfile. Below patch which changes code to export value
of phys_base has been posted to upstream. So adapt code to get it.
kexec: Change to export the value of phys_base instead of symbol address
marc.info/?l=linux-kernel&m=147856863629624&w=2
---
Patch v1 is not compatible with the old kernel. Change code in
get_phys_base_x86_64 and vtop4_x86_64 to avoid that according to
Atsushi's suggestion.
Looks good to me, I'll merge this patch into v1.6.1.
For 2/2, I'll revert 56649f7b6bfe7 with your patch's comment.


Thanks,
Atsushi Kumagai
Post by Baoquan He
arch/x86_64.c | 12 +++++-------
makedumpfile.c | 5 ++---
makedumpfile.h | 2 +-
3 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/arch/x86_64.c b/arch/x86_64.c
index 3ef33ae..010ea10 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -62,6 +62,10 @@ get_phys_base_x86_64(void)
* Get the relocatable offset
*/
info->phys_base = 0; /* default/traditional */
+ if (NUMBER(phys_base) != NOT_FOUND_NUMBER) {
+ info->phys_base = NUMBER(phys_base);
+ return TRUE;
+ }
for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
if (virt_start >= __START_KERNEL_map) {
@@ -187,12 +191,6 @@ vtop4_x86_64(unsigned long vaddr)
{
unsigned long page_dir, pml4, pgd_paddr, pgd_pte, pmd_paddr, pmd_pte;
unsigned long pte_paddr, pte;
- unsigned long phys_base;
-
- if (SYMBOL(phys_base) != NOT_FOUND_SYMBOL)
- phys_base = info->phys_base;
- else
- phys_base = 0;
if (SYMBOL(init_level4_pgt) == NOT_FOUND_SYMBOL) {
ERRMSG("Can't get the symbol of init_level4_pgt.\n");
@@ -202,7 +200,7 @@ vtop4_x86_64(unsigned long vaddr)
/*
* Get PGD.
*/
- page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + phys_base;
+ page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + info->phys_base;
page_dir += pml4_index(vaddr) * sizeof(unsigned long);
if (!readmem(PADDR, page_dir, &pml4, sizeof pml4)) {
ERRMSG("Can't get pml4 (page_dir:%lx).\n", page_dir);
diff --git a/makedumpfile.c b/makedumpfile.c
index b916dfb..a3f711e 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1507,7 +1507,6 @@ get_symbol_info(void)
SYMBOL_INIT(init_level4_pgt, "init_level4_pgt");
SYMBOL_INIT(vmlist, "vmlist");
SYMBOL_INIT(vmap_area_list, "vmap_area_list");
- SYMBOL_INIT(phys_base, "phys_base");
SYMBOL_INIT(node_online_map, "node_online_map");
SYMBOL_INIT(node_states, "node_states");
SYMBOL_INIT(node_memblk, "node_memblk");
@@ -2134,7 +2133,6 @@ write_vmcoreinfo_data(void)
WRITE_SYMBOL("init_level4_pgt", init_level4_pgt);
WRITE_SYMBOL("vmlist", vmlist);
WRITE_SYMBOL("vmap_area_list", vmap_area_list);
- WRITE_SYMBOL("phys_base", phys_base);
WRITE_SYMBOL("node_online_map", node_online_map);
WRITE_SYMBOL("node_states", node_states);
WRITE_SYMBOL("node_data", node_data);
@@ -2261,6 +2259,7 @@ write_vmcoreinfo_data(void)
WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ WRITE_NUMBER("phys_base", phys_base);
WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
@@ -2488,7 +2487,6 @@ read_vmcoreinfo(void)
READ_SYMBOL("init_level4_pgt", init_level4_pgt);
READ_SYMBOL("vmlist", vmlist);
READ_SYMBOL("vmap_area_list", vmap_area_list);
- READ_SYMBOL("phys_base", phys_base);
READ_SYMBOL("node_online_map", node_online_map);
READ_SYMBOL("node_states", node_states);
READ_SYMBOL("node_data", node_data);
@@ -2609,6 +2607,7 @@ read_vmcoreinfo(void)
READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
+ READ_NUMBER("phys_base", phys_base);
READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
diff --git a/makedumpfile.h b/makedumpfile.h
index 338c651..422b6be 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1394,7 +1394,6 @@ struct symbol_table {
unsigned long long init_level4_pgt;
unsigned long long vmlist;
unsigned long long vmap_area_list;
- unsigned long long phys_base;
unsigned long long node_online_map;
unsigned long long node_states;
unsigned long long node_memblk;
@@ -1718,6 +1717,7 @@ struct number_table {
long SECTION_SIZE_BITS;
long MAX_PHYSMEM_BITS;
long HUGETLB_PAGE_DTOR;
+ long phys_base;
};
struct srcfile_table {
--
2.5.5
Baoquan He
2016-11-11 09:49:55 UTC
Permalink
Post by Atsushi Kumagai
Post by Baoquan He
Kernel code only exports virtual address of phys_base now and it's helpless
for Crash and Makedumpfile. Below patch which changes code to export value
of phys_base has been posted to upstream. So adapt code to get it.
kexec: Change to export the value of phys_base instead of symbol address
marc.info/?l=linux-kernel&m=147856863629624&w=2
---
Patch v1 is not compatible with the old kernel. Change code in
get_phys_base_x86_64 and vtop4_x86_64 to avoid that according to
Atsushi's suggestion.
Looks good to me, I'll merge this patch into v1.6.1.
For 2/2, I'll revert 56649f7b6bfe7 with your patch's comment.
Thanks!

Loading...