x86_64: Fix page_offset for randomized base enabled

Signed-off-by Pratyush Anand <[email protected]>

---
 arch/x86_64.c  | 10 ++++++++--
 makedumpfile.h |  4 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

arch/x86_64.c +8 -2

index ddf7be6bc57b..3a53b4fa03ed 100644
@@ -187,6 +187,12 @@ 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");

@@ -196,9 +202,9 @@ vtop4_x86_64(unsigned long vaddr)
        /*
         * Get PGD.
         */
-       page_dir  = SYMBOL(init_level4_pgt);
+       page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + phys_base;
        page_dir += pml4_index(vaddr) * sizeof(unsigned long);
-       if (!readmem(VADDR, page_dir, &pml4, sizeof pml4)) {
+       if (!readmem(PADDR, page_dir, &pml4, sizeof pml4)) {
                ERRMSG("Can't get pml4 (page_dir:%lx).\n", page_dir);
                return NOT_PADDR;
        }

makedumpfile.h +2 -3

index f0154226bcb8..f64652e34901 100644
@@ -876,12 +876,12 @@ int is_vmalloc_addr_x86_64(ulong vaddr);
 int get_phys_base_x86_64(void);
 int get_machdep_info_x86_64(void);
 int get_versiondep_info_x86_64(void);
-unsigned long long vaddr_to_paddr_x86_64(unsigned long vaddr);
+unsigned long long vtop4_x86_64(unsigned long vaddr);
 #define find_vmemmap()         find_vmemmap_x86_64()
 #define get_phys_base()                get_phys_base_x86_64()
 #define get_machdep_info()     get_machdep_info_x86_64()
 #define get_versiondep_info()  get_versiondep_info_x86_64()
-#define vaddr_to_paddr(X)      vaddr_to_paddr_x86_64(X)
+#define vaddr_to_paddr(X)      vtop4_x86_64(X)
 #define is_phys_addr(X)                (!is_vmalloc_addr_x86_64(X))
 #endif /* x86_64 */
 
--

2.7.4

Patch 1/4 fixes page_offset calculation, so that it is correctly calculated on KASLR enabled kernel as well.
Patch 2/4 simplifies VA to PA translation. New code has been benchmarked against old code on a 4T system.
Patch 3/4 and 4/4 is removal of (now) unnecessary code.

I think, we should find a way to kill find_vememmap() as well, so that VMEMMAP_START can be removed. I have very limited idea about x86, so unable to do that as of now.

Pratyush Anand (4):
x86_64: Calculate page_offset from pt_load
x86_64: translate all VA to PA using page table values
x86_64: kill is_vmalloc_addr_x86_64()
x86_64: kill some unused initialization

 arch/x86_64.c  | 84 ++++++++++++++++++++--------------------------------------
 makedumpfile.h |  9 +++----
 2 files changed, 32 insertions(+), 61 deletions(-)

Hello,

Quoted 16 lines · Pratyush Anand

Patch 1/4 fixes page_offset calculation, so that it is correctly calculated on KASLR enabled kernel as well.
Patch 2/4 simplifies VA to PA translation. New code has been benchmarked against old code on a 4T system.
Patch 3/4 and 4/4 is removal of (now) unnecessary code.

I think, we should find a way to kill find_vememmap() as well, so that VMEMMAP_START can be removed. I have very limited idea about x86, so unable to do that as of now.

Pratyush Anand (4):
x86_64: Calculate page_offset from pt_load
x86_64: translate all VA to PA using page table values
x86_64: kill is_vmalloc_addr_x86_64()
x86_64: kill some unused initialization

Quoted 4 lines

 arch/x86_64.c  | 84 ++++++++++++++++++++--------------------------------------
 makedumpfile.h |  9 +++----
 2 files changed, 32 insertions(+), 61 deletions(-)

Cross-posting but FYI, this patch fixes the issue reported in the following thread : makedumpfile issues many readpage_elf: Attempt to read non-existent page[1]

It corresponds to the commit identified by my kernel bisection.

I will wait until it is accepted in Kumagai-san's tree to include it in Debian/Ubuntu.

Currently we translate some of the VA areas using linear mapping while some other(which can not be linearly mapped) using page table.

However, we will have entry of a page in the page table irrespective of its virtual region. So, we can always look into page table for any VA to PA translation. This approach will solve lot of complexity in makedumpfile. It will in turn remove dependency over variables like VMALLOC_START, MODULES_VADDR etc whose definition keeps changing in newer kernel version.

Moreover, I do not see any side effect of this approach in terms of execution timing. I tested with IBM x3950 X6 machine having 4136359 MB of memory. These are the results of makedumpfile execution time:

Without this patch:
===================
With -d 31:
Trial 1: 237.59526248 S
Trial 2: 235.236914962 S
Trail 3: 237.678712045 S

With -d 1:
Trial 1: 2548.905296877 S
Trial 2: 2549.759881756 S

With this patch:
===================
With -d 31:
Trial 1: 232.713841516 S
Trial 2: 228.45697177 S
Trail 3: 232.942262441 S

With -d 1:
Trial 1: 2768.424565806 S
Trial 2: 2749.622115455 S
Trail 3: 2537.770359073 S

Signed-off-by Pratyush Anand <[email protected]>

---
arch/x86_64.c | 42 ++++++++---------------------------------- makedumpfile.h | 4 ++--

2 files changed, 10 insertions(+), 36 deletions(-)

arch/x86_64.c +6 -0

index a96fd8ae00a1..fe2764a8bec2 100644
@@ -203,6 +203,12 @@ 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");

arch/x86_64.c +1 -33

 	page_dir += pml4_index(vaddr) * sizeof(unsigned long);
-	if (!readmem(VADDR, page_dir, &pml4, sizeof pml4)) {
+	if (!readmem(PADDR, page_dir, &pml4, sizeof pml4)) {
 		ERRMSG("Can't get pml4 (page_dir:%lx).\n", page_dir);
 		return NOT_PADDR;
 	}

@@ -285,38 +291,6 @@ vtop4_x86_64(unsigned long vaddr)
 	return (pte & ENTRY_MASK) + PAGEOFFSET(vaddr);
 }
 
-unsigned long long
-vaddr_to_paddr_x86_64(unsigned long vaddr)
-{
-	unsigned long phys_base;
-	unsigned long long paddr;
-
-	/*
-	 * Check the relocatable kernel.
-	 */
-	if (SYMBOL(phys_base) != NOT_FOUND_SYMBOL)
-		phys_base = info->phys_base;
-	else
-		phys_base = 0;
-
-	if (is_vmalloc_addr_x86_64(vaddr)) {
-		if ((paddr = vtop4_x86_64(vaddr)) == NOT_PADDR) {
-			ERRMSG("Can't convert a virtual address(%lx) to " \
-			    "physical address.\n", vaddr);
-			return NOT_PADDR;
-		}
-	} else if (vaddr >= __START_KERNEL_map) {
-		paddr = vaddr - __START_KERNEL_map + phys_base;
-
-	} else {
-		if (is_xen_memory())
-			paddr = vaddr - PAGE_OFFSET_XEN_DOM0;
-		else
-			paddr = vaddr - PAGE_OFFSET;
-	}
-	return paddr;
-}
-
 /*
 * for Xen extraction
 */

makedumpfile.h +2 -3

index a5955ff750e5..13559651feb6 100644
@@ -863,12 +863,12 @@ int is_vmalloc_addr_x86_64(ulong vaddr);
 int get_phys_base_x86_64(void);
 int get_machdep_info_x86_64(void);
 int get_versiondep_info_x86_64(void);
-unsigned long long vaddr_to_paddr_x86_64(unsigned long vaddr);
+unsigned long long vtop4_x86_64(unsigned long vaddr);
 #define find_vmemmap()		find_vmemmap_x86_64()
 #define get_phys_base()		get_phys_base_x86_64()
 #define get_machdep_info()	get_machdep_info_x86_64()
 #define get_versiondep_info()	get_versiondep_info_x86_64()
-#define vaddr_to_paddr(X)	vaddr_to_paddr_x86_64(X)
+#define vaddr_to_paddr(X)	vtop4_x86_64(X)
 #define is_phys_addr(X)		(!is_vmalloc_addr_x86_64(X))
 #endif /* x86_64 */
 
--

2.7.4

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Hello Pratysh,

Quoted 31 lines

Currently we translate some of the VA areas using linear mapping while some other(which can not be linearly mapped) using page table.

However, we will have entry of a page in the page table irrespective of its virtual region. So, we can always look into page table for any VA to PA translation. This approach will solve lot of complexity in makedumpfile. It will in turn remove dependency over variables like VMALLOC_START, MODULES_VADDR etc whose definition keeps changing in newer kernel version.

Moreover, I do not see any side effect of this approach in terms of execution timing. I tested with IBM x3950 X6 machine having 4136359 MB of memory. These are the results of makedumpfile execution time:

Without this patch:
===================
With -d 31:
Trial 1: 237.59526248 S
Trial 2: 235.236914962 S
Trail 3: 237.678712045 S

With -d 1:
Trial 1: 2548.905296877 S
Trial 2: 2549.759881756 S

With this patch:
===================
With -d 31:
Trial 1: 232.713841516 S
Trial 2: 228.45697177 S
Trail 3: 232.942262441 S

Quoted 4 lines

With -d 1:
Trial 1: 2768.424565806 S
Trial 2: 2749.622115455 S
Trail 3: 2537.770359073 S

Could you increase the number of trials ?

If the average time is close to the results of Trial 1 (2768s) and 2 (2749s), the regression rate is 8% and it sounds neither large nor small. If the average is a level of 2500s like Trial 3, it's ideal.

Quoted 21 lines

Signed-off-by Pratyush Anand <[email protected]>

---
arch/x86_64.c | 42 ++++++++---------------------------------- makedumpfile.h | 4 ++--

2 files changed, 10 insertions(+), 36 deletions(-)

arch/x86_64.c +6 -0

index a96fd8ae00a1..fe2764a8bec2 100644
@@ -203,6 +203,12 @@ 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;
 

arch/x86_64.c vtop4_x86_64(unsigned long vaddr) · line 221

Quoted 8 lines

arch/x86_64.c +1 -1

 	if (SYMBOL(init_level4_pgt) == NOT_FOUND_SYMBOL) {
 		ERRMSG("Can't get the symbol of init_level4_pgt.\n");

@@ -212,9 +218,9 @@ vtop4_x86_64(unsigned long vaddr)
 	/*
 	 * Get PGD.
 	 */
-	page_dir  = SYMBOL(init_level4_pgt);
+	page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + phys_base;

I want to confirm that this VA to PA translation is always safe, otherwise we should do the condition check which was done in vaddr_to_paddr_x86_64(), isn't it ?

Thanks,
Atsushi Kumagai

Quoted 64 lines

arch/x86_64.c +1 -33

 	page_dir += pml4_index(vaddr) * sizeof(unsigned long);
-	if (!readmem(VADDR, page_dir, &pml4, sizeof pml4)) {
+	if (!readmem(PADDR, page_dir, &pml4, sizeof pml4)) {
 		ERRMSG("Can't get pml4 (page_dir:%lx).\n", page_dir);
 		return NOT_PADDR;
 	}

@@ -285,38 +291,6 @@ vtop4_x86_64(unsigned long vaddr)
 	return (pte & ENTRY_MASK) + PAGEOFFSET(vaddr);
 }
 
-unsigned long long
-vaddr_to_paddr_x86_64(unsigned long vaddr)
-{
-	unsigned long phys_base;
-	unsigned long long paddr;
-
-	/*
-	 * Check the relocatable kernel.
-	 */
-	if (SYMBOL(phys_base) != NOT_FOUND_SYMBOL)
-		phys_base = info->phys_base;
-	else
-		phys_base = 0;
-
-	if (is_vmalloc_addr_x86_64(vaddr)) {
-		if ((paddr = vtop4_x86_64(vaddr)) == NOT_PADDR) {
-			ERRMSG("Can't convert a virtual address(%lx) to " \
-			    "physical address.\n", vaddr);
-			return NOT_PADDR;
-		}
-	} else if (vaddr >= __START_KERNEL_map) {
-		paddr = vaddr - __START_KERNEL_map + phys_base;
-
-	} else {
-		if (is_xen_memory())
-			paddr = vaddr - PAGE_OFFSET_XEN_DOM0;
-		else
-			paddr = vaddr - PAGE_OFFSET;
-	}
-	return paddr;
-}
-
 /*
 * for Xen extraction
 */

makedumpfile.h +2 -2

index a5955ff750e5..13559651feb6 100644
@@ -863,12 +863,12 @@ int is_vmalloc_addr_x86_64(ulong vaddr);
 int get_phys_base_x86_64(void);
 int get_machdep_info_x86_64(void);
 int get_versiondep_info_x86_64(void);
-unsigned long long vaddr_to_paddr_x86_64(unsigned long vaddr);
+unsigned long long vtop4_x86_64(unsigned long vaddr);
 #define find_vmemmap()		find_vmemmap_x86_64()
 #define get_phys_base()		get_phys_base_x86_64()
 #define get_machdep_info()	get_machdep_info_x86_64()
 #define get_versiondep_info()	get_versiondep_info_x86_64()
-#define vaddr_to_paddr(X)	vaddr_to_paddr_x86_64(X)
+#define vaddr_to_paddr(X)	vtop4_x86_64(X)
 #define is_phys_addr(X)		(!is_vmalloc_addr_x86_64(X))
 #endif /* x86_64 */
 

Quoted 8 lines

--
2.7.4

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Quoted 6 lines

Quoted 4 lines

With -d 1:
Trial 1: 2768.424565806 S
Trial 2: 2749.622115455 S
Trail 3: 2537.770359073 S

Could you increase the number of trials ?

OK, I can do that. Might take some time, as I will have to arrange that high memory machine again.

Quoted 25 lines

If the average time is close to the results of Trial 1 (2768s) and 2 (2749s), the regression rate is 8% and it sounds neither large nor small. If the average is a level of 2500s like Trial 3, it's ideal.

Quoted 21 lines

Signed-off-by Pratyush Anand <[email protected]>

---
arch/x86_64.c | 42 ++++++++---------------------------------- makedumpfile.h | 4 ++--

2 files changed, 10 insertions(+), 36 deletions(-)

arch/x86_64.c +6 -0

index a96fd8ae00a1..fe2764a8bec2 100644
@@ -203,6 +203,12 @@ 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;
 

Quoted 13 lines

arch/x86_64.c vtop4_x86_64(unsigned long vaddr) · line 221

Quoted 8 lines

arch/x86_64.c +1 -1

 	if (SYMBOL(init_level4_pgt) == NOT_FOUND_SYMBOL) {
 		ERRMSG("Can't get the symbol of init_level4_pgt.\n");

@@ -212,9 +218,9 @@ vtop4_x86_64(unsigned long vaddr)
 	/*
 	 * Get PGD.
 	 */
-	page_dir  = SYMBOL(init_level4_pgt);
+	page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + phys_base;

I want to confirm that this VA to PA translation is always safe, otherwise we should do the condition check which was done in vaddr_to_paddr_x86_64(), isn't it ?

I think this should be safe, however x86 expert can comment better. Baoquan any comment here?

Quoted 49 lines

Quoted 6 lines

Quoted 4 lines

With -d 1:
Trial 1: 2768.424565806 S
Trial 2: 2749.622115455 S
Trail 3: 2537.770359073 S

Could you increase the number of trials ?

OK, I can do that. Might take some time, as I will have to arrange that high memory machine again.

Quoted 25 lines

If the average time is close to the results of Trial 1 (2768s) and 2 (2749s), the regression rate is 8% and it sounds neither large nor small. If the average is a level of 2500s like Trial 3, it's ideal.

Quoted 21 lines

Signed-off-by Pratyush Anand <[email protected]>

---
arch/x86_64.c | 42 ++++++++---------------------------------- makedumpfile.h | 4 ++--

2 files changed, 10 insertions(+), 36 deletions(-)

arch/x86_64.c +6 -0

index a96fd8ae00a1..fe2764a8bec2 100644
@@ -203,6 +203,12 @@ 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;
 

Quoted 13 lines

arch/x86_64.c vtop4_x86_64(unsigned long vaddr) · line 221

Quoted 8 lines

arch/x86_64.c +1 -1

 	if (SYMBOL(init_level4_pgt) == NOT_FOUND_SYMBOL) {
 		ERRMSG("Can't get the symbol of init_level4_pgt.\n");

@@ -212,9 +218,9 @@ vtop4_x86_64(unsigned long vaddr)
 	/*
 	 * Get PGD.
 	 */
-	page_dir  = SYMBOL(init_level4_pgt);
+	page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + phys_base;

I want to confirm that this VA to PA translation is always safe, otherwise we should do the condition check which was done in vaddr_to_paddr_x86_64(), isn't it ?

Quoted 2 lines

I think this should be safe, however x86 expert can comment better. Baoquan any comment here?

Yes, I think this is safe. Below is the physical to virtual address translation function in x86 64. And init_level4_pgt is a global variable located in kernel text region.

arch/x86/include/asm/page_64.h

static inline unsigned long __phys_addr_nodebug(unsigned long x)

{
       unsigned long y = x - __START_KERNEL_map;

       /* use the carry flag to determine if x was < __START_KERNEL_map */
       x = y + ((x > y) ? phys_base : (__START_KERNEL_map - PAGE_OFFSET));

       return x;
}

Patch 1/4 fixes page_offset calculation, so that it is correctly calculated on KASLR enabled kernel as well.
Patch 2/4 simplifies VA to PA translation. New code has been benchmarked against old code on a 4T system.
Patch 3/4 and 4/4 is removal of (now) unnecessary code.

I think, we should find a way to kill find_vememmap() as well, so that VMEMMAP_START can be removed. I have very limited idea about x86, so unable to do that as of now.

Pratyush Anand (4):
x86_64: Calculate page_offset from pt_load
x86_64: translate all VA to PA using page table values
x86_64: kill is_vmalloc_addr_x86_64()
x86_64: kill some unused initialization

 arch/x86_64.c  | 84 ++++++++++++++++++++--------------------------------------
 makedumpfile.h |  9 +++----
 2 files changed, 32 insertions(+), 61 deletions(-)

Hi Pratyush,

Quoted 16 lines · Pratyush Anand

Patch 1/4 fixes page_offset calculation, so that it is correctly calculated on KASLR enabled kernel as well.
Patch 2/4 simplifies VA to PA translation. New code has been benchmarked against old code on a 4T system.
Patch 3/4 and 4/4 is removal of (now) unnecessary code.

I think, we should find a way to kill find_vememmap() as well, so that VMEMMAP_START can be removed. I have very limited idea about x86, so unable to do that as of now.

Pratyush Anand (4):
x86_64: Calculate page_offset from pt_load
x86_64: translate all VA to PA using page table values
x86_64: kill is_vmalloc_addr_x86_64()
x86_64: kill some unused initialization

Quoted 4 lines

 arch/x86_64.c  | 84 ++++++++++++++++++++--------------------------------------
 makedumpfile.h |  9 +++----
 2 files changed, 32 insertions(+), 61 deletions(-)

According to our test, with these patches the dumped vmcore is correct which means simple crash test bt works. But the saved vmcore size is larger than before.

I collected two --message-level 31 logs with/without your patches, the kernel kaslr was disabled during my test so that we can focus on the vmcore size issue, it looks like mem_map address was not collected correctly.

[please ignore the patched log extra debug message I added]

patched-makedumpfile.txt renamed from unpatched-makedumpfile.txt +35 -29

@@ -1,4 +1,4 @@
-[127.0.0.1-2016-10-27-10:27:03]# /tmp/makedumpfile -l -d 31 --message-level 31 vmcore vmcore.m.1
+[127.0.0.1-2016-10-27-10:27:03]# /home/dyoung/git/makedumpfile/makedumpfile -l -d 31 --message-level 31 vmcore vmcore.m
 sadump: does not have partition header
 sadump: read dump device as unknown format
 sadump: unknown format

@@ -36,70 +36,76 @@
 
 Memory type  : SPARSEMEM_EX
 
+printing memmap .....
 mem_map (0)
-  mem_map    : ffffea0000000000
+  mem_map    : 0
   pfn_start  : 0
   pfn_end    : 8000
+printing memmap .....
 mem_map (1)
-  mem_map    : ffffea0000200000
+  mem_map    : 0
   pfn_start  : 8000
   pfn_end    : 10000
+printing memmap .....
 mem_map (2)
-  mem_map    : ffffea0000400000
+  mem_map    : 0
   pfn_start  : 10000
   pfn_end    : 18000
+printing memmap .....
 mem_map (3)
-  mem_map    : ffffea0000600000
+  mem_map    : 0
   pfn_start  : 18000
   pfn_end    : 20000
+printing memmap .....
 mem_map (4)
-  mem_map    : ffffea0000800000
+  mem_map    : 0
   pfn_start  : 20000
   pfn_end    : 28000
+printing memmap .....
 mem_map (5)
-  mem_map    : ffffea0000a00000
+  mem_map    : 0
   pfn_start  : 28000
   pfn_end    : 30000
+printing memmap .....
 mem_map (6)
-  mem_map    : ffffea0000c00000
+  mem_map    : 0
   pfn_start  : 30000
   pfn_end    : 38000
+printing memmap .....
 mem_map (7)
-  mem_map    : ffffea0000e00000
+  mem_map    : 0
   pfn_start  : 38000
   pfn_end    : 3ffda
 mmap() is available on the kernel.
 Checking for memory holes          : [100.0 %] |STEP [Checking for memory holes  ] : 0.000030 seconds
-Excluding unnecessary pages        : [100.0 %] \STEP [Excluding unnecessary pages] : 0.007547 seconds
-Checking for memory holes          : [100.0 %] -STEP [Checking for memory holes  ] : 0.000016 seconds
-Checking for memory holes          : [100.0 %] /STEP [Checking for memory holes  ] : 0.000009 seconds
-Excluding unnecessary pages        : [100.0 %] |STEP [Excluding unnecessary pages] : 0.006623 seconds
-Copying data                       : [100.0 %] \STEP [Copying data               ] : 0.184442 seconds
-Copying data                       : [100.0 %] -STEP [Copying data               ] : 0.000041 seconds
+Excluding unnecessary pages        : [100.0 %] \STEP [Excluding unnecessary pages] : 0.000007 seconds
+Checking for memory holes          : [100.0 %] -STEP [Checking for memory holes  ] : 0.000014 seconds
+Checking for memory holes          : [100.0 %] /STEP [Checking for memory holes  ] : 0.000008 seconds
+Excluding unnecessary pages        : [100.0 %] |STEP [Excluding unnecessary pages] : 0.000007 seconds
+Copying data                       : [100.0 %] /STEP [Copying data               ] : 1.421661 seconds
+Copying data                       : [100.0 %] |STEP [Copying data               ] : 0.000036 seconds
 
 Writing erase info...
-offset_eraseinfo: dd5c4e, size_eraseinfo: 0
+offset_eraseinfo: 888c149, size_eraseinfo: 0
 
 Original pages  : 0x0000000000030c7d
-  Excluded pages   : 0x000000000002cf58
-    Pages filled with zero  : 0x00000000000002be
-    Non-private cache pages : 0x0000000000006574
-    Private cache pages     : 0x0000000000000f27
-    User process data pages : 0x0000000000003481
-    Free pages              : 0x000000000002237e
+  Excluded pages   : 0x000000000001d534
+    Pages filled with zero  : 0x000000000001d534
+    Non-private cache pages : 0x0000000000000000
+    Private cache pages     : 0x0000000000000000
+    User process data pages : 0x0000000000000000
+    Free pages              : 0x0000000000000000
     Hwpoison pages          : 0x0000000000000000
-  Remaining pages  : 0x0000000000003d25
-  (The number of pages is reduced to 7%.)
+  Remaining pages  : 0x0000000000013749
+  (The number of pages is reduced to 39%.)
 Memory Hole     : 0x000000000000f35d
 --------------------------------------------------
 Total pages     : 0x000000000003ffda
 
-Cache hit: 29946, miss: 47, hit rate: 99.8%
+Cache hit: 196285, miss: 201, hit rate: 99.9%
 
 
-The dumpfile is saved to vmcore.m.1.
+The dumpfile is saved to vmcore.m.
 
 makedumpfile Completed.
-[root@dhcp-128-65 127.0.0.1-2016-10-27-10:27:03]# ls -lth vmcore.m*
--rw------- 1 root root  14M Oct 27 10:30 vmcore.m.1
--rw------- 1 root root 137M Oct 27 10:28 vmcore.m
+[root@dhcp-128-65 127.0.0.1-2016-10-27-10:27:03]# 

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Hi Pratyush,

Quoted 16 lines · Pratyush Anand

Patch 1/4 fixes page_offset calculation, so that it is correctly calculated on KASLR enabled kernel as well.
Patch 2/4 simplifies VA to PA translation. New code has been benchmarked against old code on a 4T system.
Patch 3/4 and 4/4 is removal of (now) unnecessary code.

I think, we should find a way to kill find_vememmap() as well, so that VMEMMAP_START can be removed. I have very limited idea about x86, so unable to do that as of now.

Pratyush Anand (4):
x86_64: Calculate page_offset from pt_load
x86_64: translate all VA to PA using page table values
x86_64: kill is_vmalloc_addr_x86_64()
x86_64: kill some unused initialization

Quoted 4 lines

 arch/x86_64.c  | 84 ++++++++++++++++++++--------------------------------------
 makedumpfile.h |  9 +++----
 2 files changed, 32 insertions(+), 61 deletions(-)

According to our test, with these patches the dumped vmcore is correct which means simple crash test bt works. But the saved vmcore size is larger than before.

I collected two --message-level 31 logs with/without your patches, the kernel kaslr was disabled during my test so that we can focus on the vmcore size issue, it looks like mem_map address was not collected correctly.

[please ignore the patched log extra debug message I added]

patched-makedumpfile.txt renamed from unpatched-makedumpfile.txt +35 -29

@@ -1,4 +1,4 @@
-[127.0.0.1-2016-10-27-10:27:03]# /tmp/makedumpfile -l -d 31 --message-level 31 vmcore vmcore.m.1
+[127.0.0.1-2016-10-27-10:27:03]# /home/dyoung/git/makedumpfile/makedumpfile -l -d 31 --message-level 31 vmcore vmcore.m
 sadump: does not have partition header
 sadump: read dump device as unknown format
 sadump: unknown format

@@ -36,70 +36,76 @@
 
 Memory type  : SPARSEMEM_EX
 
+printing memmap .....
 mem_map (0)
-  mem_map    : ffffea0000000000
+  mem_map    : 0
   pfn_start  : 0
   pfn_end    : 8000
+printing memmap .....
 mem_map (1)
-  mem_map    : ffffea0000200000
+  mem_map    : 0
   pfn_start  : 8000
   pfn_end    : 10000
+printing memmap .....
 mem_map (2)
-  mem_map    : ffffea0000400000
+  mem_map    : 0
   pfn_start  : 10000
   pfn_end    : 18000
+printing memmap .....
 mem_map (3)
-  mem_map    : ffffea0000600000
+  mem_map    : 0
   pfn_start  : 18000
   pfn_end    : 20000
+printing memmap .....
 mem_map (4)
-  mem_map    : ffffea0000800000
+  mem_map    : 0
   pfn_start  : 20000
   pfn_end    : 28000
+printing memmap .....
 mem_map (5)
-  mem_map    : ffffea0000a00000
+  mem_map    : 0
   pfn_start  : 28000
   pfn_end    : 30000
+printing memmap .....
 mem_map (6)
-  mem_map    : ffffea0000c00000
+  mem_map    : 0
   pfn_start  : 30000
   pfn_end    : 38000
+printing memmap .....
 mem_map (7)
-  mem_map    : ffffea0000e00000
+  mem_map    : 0
   pfn_start  : 38000
   pfn_end    : 3ffda
 mmap() is available on the kernel.
 Checking for memory holes          : [100.0 %] |STEP [Checking for memory holes  ] : 0.000030 seconds
-Excluding unnecessary pages        : [100.0 %] \STEP [Excluding unnecessary pages] : 0.007547 seconds
-Checking for memory holes          : [100.0 %] -STEP [Checking for memory holes  ] : 0.000016 seconds
-Checking for memory holes          : [100.0 %] /STEP [Checking for memory holes  ] : 0.000009 seconds
-Excluding unnecessary pages        : [100.0 %] |STEP [Excluding unnecessary pages] : 0.006623 seconds
-Copying data                       : [100.0 %] \STEP [Copying data               ] : 0.184442 seconds
-Copying data                       : [100.0 %] -STEP [Copying data               ] : 0.000041 seconds
+Excluding unnecessary pages        : [100.0 %] \STEP [Excluding unnecessary pages] : 0.000007 seconds
+Checking for memory holes          : [100.0 %] -STEP [Checking for memory holes  ] : 0.000014 seconds
+Checking for memory holes          : [100.0 %] /STEP [Checking for memory holes  ] : 0.000008 seconds
+Excluding unnecessary pages        : [100.0 %] |STEP [Excluding unnecessary pages] : 0.000007 seconds
+Copying data                       : [100.0 %] /STEP [Copying data               ] : 1.421661 seconds
+Copying data                       : [100.0 %] |STEP [Copying data               ] : 0.000036 seconds
 
 Writing erase info...
-offset_eraseinfo: dd5c4e, size_eraseinfo: 0
+offset_eraseinfo: 888c149, size_eraseinfo: 0
 
 Original pages  : 0x0000000000030c7d
-  Excluded pages   : 0x000000000002cf58
-    Pages filled with zero  : 0x00000000000002be
-    Non-private cache pages : 0x0000000000006574
-    Private cache pages     : 0x0000000000000f27
-    User process data pages : 0x0000000000003481
-    Free pages              : 0x000000000002237e
+  Excluded pages   : 0x000000000001d534
+    Pages filled with zero  : 0x000000000001d534
+    Non-private cache pages : 0x0000000000000000
+    Private cache pages     : 0x0000000000000000
+    User process data pages : 0x0000000000000000
+    Free pages              : 0x0000000000000000
     Hwpoison pages          : 0x0000000000000000
-  Remaining pages  : 0x0000000000003d25
-  (The number of pages is reduced to 7%.)
+  Remaining pages  : 0x0000000000013749
+  (The number of pages is reduced to 39%.)
 Memory Hole     : 0x000000000000f35d
 --------------------------------------------------
 Total pages     : 0x000000000003ffda
 
-Cache hit: 29946, miss: 47, hit rate: 99.8%
+Cache hit: 196285, miss: 201, hit rate: 99.9%
 
 
-The dumpfile is saved to vmcore.m.1.
+The dumpfile is saved to vmcore.m.
 
 makedumpfile Completed.
-[root@dhcp-128-65 127.0.0.1-2016-10-27-10:27:03]# ls -lth vmcore.m*
--rw------- 1 root root  14M Oct 27 10:30 vmcore.m.1
--rw------- 1 root root 137M Oct 27 10:28 vmcore.m
+[root@dhcp-128-65 127.0.0.1-2016-10-27-10:27:03]# 

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

According to our test, with these patches the dumped vmcore is correct which means simple crash test bt works. But the saved vmcore size is larger than before.

I collected two --message-level 31 logs with/without your patches, the kernel kaslr was disabled during my test so that we can focus on the vmcore size issue, it looks like mem_map address was not collected correctly.

[please ignore the patched log extra debug message I added]

page_offset can always be calculated as 'virtual - physical' for a direct mapping area on x86. Therefore, remove the version dependent calculation and use this method.

Signed-off-by Pratyush Anand <[email protected]>

---
 arch/x86_64.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

arch/x86_64.c +0 -0

index ddf7be6bc57b..a96fd8ae00a1 100644
@@ -44,6 +44,24 @@ get_xen_p2m_mfn(void)
 	return NOT_FOUND_LONG_VALUE;
 }

arch/x86_64.c +9 -0

+			info->page_offset = virt_start - phys_start;
+			return TRUE;
+		}
+	}
+
+	ERRMSG("Can't get any pt_load to calculate page offset.\n");
+	return FALSE;
+}
+
 int
 get_phys_base_x86_64(void)
 {

@@ -159,10 +177,8 @@ get_versiondep_info_x86_64(void)
 	else
 		info->max_physmem_bits  = _MAX_PHYSMEM_BITS_2_6_31;

I put the cped-vmcore/vmlinux here:
https://people.redhat.com/~ruyang/test/

Adding Dave Anderson for any comments about the vmcore correctness from crash point of view..

That being said, my recent 4.8 and 4.9 KASLR testing has been on live systems and compressed kdumps, so the old tried-and-true manner of calculating the phys_base from the ELF PT_LOAD segments apparently no longer works with KASLR.

It would be so much more helpful if the VMCOREINFO data in the ELF header stored the actual phys_base value instead of its symbol value:

crash> help -D
...
SYMBOL(phys_base)=ffffffffa740b010
...

which is completely useless unless the phys_base value is known.

Anyway, can you send me the makedumpfile code that calculates the phys_base value?

Dave

----- Original Message -----

Quoted 14 lines

That being said, my recent 4.8 and 4.9 KASLR testing has been on live systems and compressed kdumps, so the old tried-and-true manner of calculating the phys_base from the ELF PT_LOAD segments apparently no longer works with KASLR.

It would be so much more helpful if the VMCOREINFO data in the ELF header stored the actual phys_base value instead of its symbol value:

crash> help -D
...
SYMBOL(phys_base)=ffffffffa740b010
...

Quoted 6 lines

which is completely useless unless the phys_base value is known.

Anyway, can you send me the makedumpfile code that calculates the phys_base value?

Dave

As it turns out, the problem with the crash utility is that it has to calculate phys_base well before it even knows the kernel has been relocated by KASLR. So when it sees the __START_KERNEL_map PT_LOAD segment, it mistakes
it for the kernel modules' virtual address region and skips it.

The kernel has this:

  #if defined(CONFIG_RANDOMIZE_BASE)
  #define KERNEL_IMAGE_SIZE       (1024 * 1024 * 1024)
  #else
  #define KERNEL_IMAGE_SIZE       (512 * 1024 * 1024)
  #endif

and then this:

  #define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)

So with KASLR, MODULES_VADDR gets pushed up from the traditional ffffffffa0000000 up to ffffffffc0000000.

So I'm curious as to what you use in makedumpfile to determine whether CONFIG_RANDOMIZE_BASE has been configured?

Thanks,
Dave

As it turns out, the problem with the crash utility is that it has to calculate phys_base well before it even knows the kernel has been relocated by KASLR. So when it sees the __START_KERNEL_map PT_LOAD segment, it mistakes it for the kernel modules' virtual address region and skips it.

The kernel has this:

  #if defined(CONFIG_RANDOMIZE_BASE)
  #define KERNEL_IMAGE_SIZE       (1024 * 1024 * 1024)
  #else
  #define KERNEL_IMAGE_SIZE       (512 * 1024 * 1024)
  #endif

and then this:

  #define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)

So with KASLR, MODULES_VADDR gets pushed up from the traditional ffffffffa0000000 up to ffffffffc0000000.

So I'm curious as to what you use in makedumpfile to determine whether CONFIG_RANDOMIZE_BASE has been configured?

----- Original Message -----

Quoted 43 lines

----- Original Message -----

Quoted 14 lines

That being said, my recent 4.8 and 4.9 KASLR testing has been on live systems and compressed kdumps, so the old tried-and-true manner of calculating the phys_base from the ELF PT_LOAD segments apparently no longer works with KASLR.

It would be so much more helpful if the VMCOREINFO data in the ELF header stored the actual phys_base value instead of its symbol value:

crash> help -D
...
SYMBOL(phys_base)=ffffffffa740b010
...

Quoted 6 lines

which is completely useless unless the phys_base value is known.

Anyway, can you send me the makedumpfile code that calculates the phys_base value?

Dave

As it turns out, the problem with the crash utility is that it has to calculate phys_base well before it even knows the kernel has been relocated by KASLR. So when it sees the __START_KERNEL_map PT_LOAD segment, it mistakes
it for the kernel modules' virtual address region and skips it.

The kernel has this:

  #if defined(CONFIG_RANDOMIZE_BASE)
  #define KERNEL_IMAGE_SIZE       (1024 * 1024 * 1024)
  #else
  #define KERNEL_IMAGE_SIZE       (512 * 1024 * 1024)
  #endif

and then this:

  #define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)

Quoted 8 lines

So with KASLR, MODULES_VADDR gets pushed up from the traditional ffffffffa0000000 up to ffffffffc0000000.

So I'm curious as to what you use in makedumpfile to determine whether CONFIG_RANDOMIZE_BASE has been configured?

Thanks,
Dave

Hey, sorry, I didn't notice that this was added upstream:

commit 1303a27c9c32020a3b6ac89be270d2ab1f28be24
Author: Baoquan He <[email protected]>
Date: Wed Sep 9 15:39:03 2015 -0700

kexec: export KERNEL_IMAGE_SIZE to vmcoreinfo

With that in place, it will be an easy fix for the crash utility.

Thanks,
Dave

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Quoted 16 lines

page_offset can always be calculated as 'virtual - physical' for a direct mapping area on x86. Therefore, remove the version dependent calculation and use this method.

Signed-off-by Pratyush Anand <[email protected]>

---
 arch/x86_64.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

arch/x86_64.c +0 -0

index ddf7be6bc57b..a96fd8ae00a1 100644
@@ -44,6 +44,24 @@ get_xen_p2m_mfn(void)
 	return NOT_FOUND_LONG_VALUE;
 }

arch/x86_64.c get_xen_p2m_mfn(void) · line 55

Quoted 10 lines

arch/x86_64.c +9 -0

 
+static int
+get_page_offset_x86_64(void)
+{
+	int i;
+	unsigned long long phys_start;
+	unsigned long long virt_start;
+
+	for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
+		if (virt_start >= __START_KERNEL_map) {

OK..So, this is the problem. We should have

if (virt_start < __START_KERNEL_map) {

Kernel text region lies above __START_KERNEL_map, which is linearly mapped however not a direct mapping. Direct mapping region lies below it instead. So, page_offset can only be calculated with a region which is below __START_KERNEL_map.

Thanks Baoquan for finding it.

Quoted 23 lines

arch/x86_64.c +11 -4

+			info->page_offset = virt_start - phys_start;
+			return TRUE;
+		}
+	}
+
+	ERRMSG("Can't get any pt_load to calculate page offset.\n");
+	return FALSE;
+}
+
 int
 get_phys_base_x86_64(void)
 {

@@ -159,10 +177,8 @@ get_versiondep_info_x86_64(void)
 	else
 		info->max_physmem_bits  = _MAX_PHYSMEM_BITS_2_6_31;
 
-	if (info->kernel_version < KERNEL_VERSION(2, 6, 27))
-		info->page_offset = __PAGE_OFFSET_ORIG;
-	else
-		info->page_offset = __PAGE_OFFSET_2_6_27;
+	if (!get_page_offset_x86_64())
+		return FALSE;
 

arch/x86_64.c get_versiondep_info_x86_64(void) · line 184

Quoted 3 lines

arch/x86_64.c +0 -0

 	if (info->kernel_version < KERNEL_VERSION(2, 6, 31)) {
 		info->vmalloc_start = VMALLOC_START_ORIG;

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Hello Pratyush,

Quoted 37 lines · Pratyush Anand

Quoted 16 lines

page_offset can always be calculated as 'virtual - physical' for a direct mapping area on x86. Therefore, remove the version dependent calculation and use this method.

Signed-off-by Pratyush Anand <[email protected]>

---
 arch/x86_64.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

arch/x86_64.c +0 -0

index ddf7be6bc57b..a96fd8ae00a1 100644
@@ -44,6 +44,24 @@ get_xen_p2m_mfn(void)
 	return NOT_FOUND_LONG_VALUE;
 }

arch/x86_64.c get_xen_p2m_mfn(void) · line 55

Quoted 10 lines

arch/x86_64.c +9 -0

 
+static int
+get_page_offset_x86_64(void)
+{
+	int i;
+	unsigned long long phys_start;
+	unsigned long long virt_start;
+
+	for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
+		if (virt_start >= __START_KERNEL_map) {

OK..So, this is the problem. We should have

if (virt_start < __START_KERNEL_map) {

Kernel text region lies above __START_KERNEL_map, which is linearly mapped however not a direct mapping. Direct mapping region lies below it instead. So, page_offset can only be calculated with a region which is below __START_KERNEL_map.

Quoted 1 line

Thanks Baoquan for finding it.

Could you explain the issue in more detail, what is the difference between "linear map" and "direct map" ?

BTW, I confirmed that the difference of v2 patch is only this and the result of benchmark looks great, thank you very much.

Regards,
Atsushi Kumagai

Quoted 31 lines

Quoted 16 lines

arch/x86_64.c +9 -0

+			info->page_offset = virt_start - phys_start;
+			return TRUE;
+		}
+	}
+
+	ERRMSG("Can't get any pt_load to calculate page offset.\n");
+	return FALSE;
+}
+
 int
 get_phys_base_x86_64(void)
 {

@@ -159,10 +177,8 @@ get_versiondep_info_x86_64(void)
 	else
 		info->max_physmem_bits  = _MAX_PHYSMEM_BITS_2_6_31;

arch/x86_64.c get_versiondep_info_x86_64(void) · line 184

Quoted 10 lines

arch/x86_64.c +2 -4

 
-	if (info->kernel_version < KERNEL_VERSION(2, 6, 27))
-		info->page_offset = __PAGE_OFFSET_ORIG;
-	else
-		info->page_offset = __PAGE_OFFSET_2_6_27;
+	if (!get_page_offset_x86_64())
+		return FALSE;
 
 	if (info->kernel_version < KERNEL_VERSION(2, 6, 31)) {
 		info->vmalloc_start = VMALLOC_START_ORIG;

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Quoted 39 lines

Hello Pratyush,

Quoted 37 lines · Pratyush Anand

Quoted 16 lines

page_offset can always be calculated as 'virtual - physical' for a direct mapping area on x86. Therefore, remove the version dependent calculation and use this method.

Signed-off-by Pratyush Anand <[email protected]>

---
 arch/x86_64.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

arch/x86_64.c +0 -0

index ddf7be6bc57b..a96fd8ae00a1 100644
@@ -44,6 +44,24 @@ get_xen_p2m_mfn(void)
 	return NOT_FOUND_LONG_VALUE;
 }

arch/x86_64.c get_xen_p2m_mfn(void) · line 55

Quoted 10 lines

arch/x86_64.c +9 -0

 
+static int
+get_page_offset_x86_64(void)
+{
+	int i;
+	unsigned long long phys_start;
+	unsigned long long virt_start;
+
+	for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
+		if (virt_start >= __START_KERNEL_map) {

OK..So, this is the problem. We should have

if (virt_start < __START_KERNEL_map) {

Kernel text region lies above __START_KERNEL_map, which is linearly mapped however not a direct mapping. Direct mapping region lies below it instead. So, page_offset can only be calculated with a region which is below __START_KERNEL_map.

Quoted 4 lines

Quoted 1 line

Thanks Baoquan for finding it.

Could you explain the issue in more detail, what is the difference between "linear map" and "direct map" ?

Well, these could be easily misunderstood. Virtual address in kernel text region or within direct mapping which is from PAGE_OFFSET are all linear mapping. Because the related PA can be calculated by:
PA= VA - PAGE_OFFSET; //y=x - a;
PA= VA - __START_KERNEL_map + phys_base; //y=x-a+b

In user-space kexec-tools we created pt_load program segments for kernel text region and crash memory regions. The first program header is kernel text region. So here if we want to get page_offset, just need to ignore the 1st one, kernel text region.

Quoted 4 lines

BTW, I confirmed that the difference of v2 patch is only this and the result of benchmark looks great, thank you very much.

Quoted 34 lines

Regards,
Atsushi Kumagai

Quoted 31 lines

Quoted 16 lines

arch/x86_64.c +9 -0

+			info->page_offset = virt_start - phys_start;
+			return TRUE;
+		}
+	}
+
+	ERRMSG("Can't get any pt_load to calculate page offset.\n");
+	return FALSE;
+}
+
 int
 get_phys_base_x86_64(void)
 {

@@ -159,10 +177,8 @@ get_versiondep_info_x86_64(void)
 	else
 		info->max_physmem_bits  = _MAX_PHYSMEM_BITS_2_6_31;

arch/x86_64.c get_versiondep_info_x86_64(void) · line 184

Quoted 10 lines

arch/x86_64.c +2 -4

 
-	if (info->kernel_version < KERNEL_VERSION(2, 6, 27))
-		info->page_offset = __PAGE_OFFSET_ORIG;
-	else
-		info->page_offset = __PAGE_OFFSET_2_6_27;
+	if (!get_page_offset_x86_64())
+		return FALSE;
 
 	if (info->kernel_version < KERNEL_VERSION(2, 6, 31)) {
 		info->vmalloc_start = VMALLOC_START_ORIG;

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec