RE: [Makedumpfile PATCH] elf_info: fix file_size if segment is excluded

[Atsushi Kumagai]

Hello Pratyush,

Thanks for your report, I have just one question.

>> exclude_segment() is called for Crash Kernel whose range is
>> 2b00-350f.
>>
>> We see following after exclude_segment()
>>
>> LOAD (2)
>>  phys_start : 10
>>  phys_end   : 2aff
>>  virt_start : 8a5a4010
>>  virt_end   : 8a5a6aff
>>  file_offset: a5a40102000
>>  file_size  : dfefd000
>> LOAD (3)
>>  phys_start : 3510
>>  phys_end   : dfffd000
>>  virt_start : 8a5a7510
>>  virt_end   : 8a5b1fffd000
>>  file_offset: a5a75102000
>>  file_size  : 0
>>
>> Since file_size is calculated wrong therefore readpage_elf() does not
>> behave correctly.
>>
>> This patch fixes above wrong behavior.
>>
>> Signed-off-by: Pratyush Anand 
>> ---
>> elf_info.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/elf_info.c b/elf_info.c
>> index 8e2437622141..6bf5e3373595 100644
>> --- a/elf_info.c
>> +++ b/elf_info.c
>> @@ -826,9 +826,12 @@ static int exclude_segment(struct pt_load_segment 
>> **pt_loads,
>>temp_seg.virt_end = vend;
>>temp_seg.file_offset = (*pt_loads)[i].file_offset
>>+ temp_seg.virt_start - (*pt_loads)[i].virt_start;
>> +temp_seg.file_size = temp_seg.phys_end
>> +- temp_seg.phys_start;
>>
>>(*pt_loads)[i].virt_end = kvstart - 1;
>>(*pt_loads)[i].phys_end =  start - 1;
>> +(*pt_loads)[i].file_size -= temp_seg.file_size;

I think we should additionally subtract (end - start), right ? 
This code seems to leave Crash Kernel region for the former half of PT_LOAD.

Thanks,
Atsushi Kumagai

>>tidx = i+1;
>>} else if (kvstart != vstart) {
>> --
>> 2.9.3
>>


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

Re: [PATCH v5 15/32] efi: Update efi_mem_type() to return an error rather than 0

[Tom Lendacky]

On 5/7/2017 12:18 PM, Borislav Petkov wrote:

On Tue, Apr 18, 2017 at 04:19:00PM -0500, Tom Lendacky wrote:

The efi_mem_type() function currently returns a 0, which maps to
EFI_RESERVED_TYPE, if the function is unable to find a memmap entry for
the supplied physical address. Returning EFI_RESERVED_TYPE implies that
a memmap entry exists, when it doesn't.  Instead of returning 0, change
the function to return a negative error value when no memmap entry is
found.

Signed-off-by: Tom Lendacky 
---


...


diff --git a/include/linux/efi.h b/include/linux/efi.h
index cd768a1..a27bb3f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -973,7 +973,7 @@ static inline void efi_esrt_init(void) { }
 extern int efi_config_parse_tables(void *config_tables, int count, int sz,
   efi_config_table_type_t *arch_tables);
 extern u64 efi_get_iobase (void);
-extern u32 efi_mem_type (unsigned long phys_addr);
+extern int efi_mem_type (unsigned long phys_addr);


WARNING: space prohibited between function name and open parenthesis '('
#101: FILE: include/linux/efi.h:976:
+extern int efi_mem_type (unsigned long phys_addr);

Please integrate scripts/checkpatch.pl in your patch creation workflow.
Some of the warnings/errors *actually* make sense.


I do/did run scripts/checkpatch.pl against all my patches. In this case
I chose to keep the space in order to stay consistent with some of the
surrounding functions.  No problem though, I can remove the space.

Thanks,
Tom



I know, the other function prototypes have a space too but that's not
our coding style. Looks like this trickled in from ia64, from looking at
arch/ia64/kernel/efi.c.



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

Re: [Makedumpfile PATCH] elf_info: fix file_size if segment is excluded

[Baoquan He]

Good catch!
Ack

发自我的 iPhone

> 在 2017年5月8日，下午8:41，Pratyush Anand  写道：
> 
> I received following on a specific x86_64 hp virtual machine while
> executing `makedumpfile --mem-usage /proc/kcore`.
> 
> vtop4_x86_64: Can't get a valid pte.
> readmem: Can't convert a virtual address(88115860) to physical 
> address.
> readmem: type_addr: 0, addr:88115860, size:128
> get_nodes_online: Can't get the node online map.
> 
> With some debug print in vtop4_x86_64() I noticed that pte value is read
> as 0, while crash reads the value correctly:
> 
> from makedumpfile:
> vaddr=88115860
> page_dir=59eaff8
> pml4=59ed067
> pgd_paddr=59edff0
> pgd_pte=59ee063
> pmd_paddr=59ee200
> pmd_pte=3642f063
> pte_paddr=3642f8a8
> pte=0
> 
> from crash
> crash> vtop 88115860
> VIRTUAL   PHYSICAL
> 88115860  5b15860
> 
> PML4 DIRECTORY: 87fea000
> PAGE DIRECTORY: 59ed067
>   PUD: 59edff0 => 59ee063
>   PMD: 59ee200 => 3642f063
>   PTE: 3642f8a8 => 5b15163
>  PAGE: 5b15000
> 
> With some more debug prints in elf_info.c
> 
> Before calling exclude_segment()
> 
> LOAD (2)
>  phys_start : 10
>  phys_end   : dfffd000
>  virt_start : 8a5a4010
>  virt_end   : 8a5b1fffd000
>  file_offset: a5a40102000
>  file_size  : dfefd000
> 
> exclude_segment() is called for Crash Kernel whose range is
> 2b00-350f.
> 
> We see following after exclude_segment()
> 
> LOAD (2)
>  phys_start : 10
>  phys_end   : 2aff
>  virt_start : 8a5a4010
>  virt_end   : 8a5a6aff
>  file_offset: a5a40102000
>  file_size  : dfefd000
> LOAD (3)
>  phys_start : 3510
>  phys_end   : dfffd000
>  virt_start : 8a5a7510
>  virt_end   : 8a5b1fffd000
>  file_offset: a5a75102000
>  file_size  : 0
> 
> Since file_size is calculated wrong therefore readpage_elf() does not
> behave correctly.
> 
> This patch fixes above wrong behavior.
> 
> Signed-off-by: Pratyush Anand 
> ---
> elf_info.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/elf_info.c b/elf_info.c
> index 8e2437622141..6bf5e3373595 100644
> --- a/elf_info.c
> +++ b/elf_info.c
> @@ -826,9 +826,12 @@ static int exclude_segment(struct pt_load_segment 
> **pt_loads,
>temp_seg.virt_end = vend;
>temp_seg.file_offset = (*pt_loads)[i].file_offset
>+ temp_seg.virt_start - (*pt_loads)[i].virt_start;
> +temp_seg.file_size = temp_seg.phys_end
> +- temp_seg.phys_start;
> 
>(*pt_loads)[i].virt_end = kvstart - 1;
>(*pt_loads)[i].phys_end =  start - 1;
> +(*pt_loads)[i].file_size -= temp_seg.file_size;
> 
>tidx = i+1;
>} else if (kvstart != vstart) {
> -- 
> 2.9.3
> 
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[Makedumpfile PATCH] elf_info: fix file_size if segment is excluded

[Pratyush Anand]

I received following on a specific x86_64 hp virtual machine while
executing `makedumpfile --mem-usage /proc/kcore`.

vtop4_x86_64: Can't get a valid pte.
readmem: Can't convert a virtual address(88115860) to physical address.
readmem: type_addr: 0, addr:88115860, size:128
get_nodes_online: Can't get the node online map.

With some debug print in vtop4_x86_64() I noticed that pte value is read
as 0, while crash reads the value correctly:

from makedumpfile:
vaddr=88115860
page_dir=59eaff8
pml4=59ed067
pgd_paddr=59edff0
pgd_pte=59ee063
pmd_paddr=59ee200
pmd_pte=3642f063
pte_paddr=3642f8a8
pte=0

from crash
crash> vtop 88115860
VIRTUAL   PHYSICAL
88115860  5b15860

PML4 DIRECTORY: 87fea000
PAGE DIRECTORY: 59ed067
   PUD: 59edff0 => 59ee063
   PMD: 59ee200 => 3642f063
   PTE: 3642f8a8 => 5b15163
  PAGE: 5b15000

With some more debug prints in elf_info.c

Before calling exclude_segment()

LOAD (2)
  phys_start : 10
  phys_end   : dfffd000
  virt_start : 8a5a4010
  virt_end   : 8a5b1fffd000
  file_offset: a5a40102000
  file_size  : dfefd000

exclude_segment() is called for Crash Kernel whose range is
2b00-350f.

We see following after exclude_segment()

LOAD (2)
  phys_start : 10
  phys_end   : 2aff
  virt_start : 8a5a4010
  virt_end   : 8a5a6aff
  file_offset: a5a40102000
  file_size  : dfefd000
LOAD (3)
  phys_start : 3510
  phys_end   : dfffd000
  virt_start : 8a5a7510
  virt_end   : 8a5b1fffd000
  file_offset: a5a75102000
  file_size  : 0

Since file_size is calculated wrong therefore readpage_elf() does not
behave correctly.

This patch fixes above wrong behavior.

Signed-off-by: Pratyush Anand 
---
 elf_info.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/elf_info.c b/elf_info.c
index 8e2437622141..6bf5e3373595 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -826,9 +826,12 @@ static int exclude_segment(struct pt_load_segment 
**pt_loads,
temp_seg.virt_end = vend;
temp_seg.file_offset = 
(*pt_loads)[i].file_offset
+ temp_seg.virt_start - 
(*pt_loads)[i].virt_start;
+   temp_seg.file_size = temp_seg.phys_end
+   - temp_seg.phys_start;
 
(*pt_loads)[i].virt_end = kvstart - 1;
(*pt_loads)[i].phys_end =  start - 1;
+   (*pt_loads)[i].file_size -= temp_seg.file_size;
 
tidx = i+1;
} else if (kvstart != vstart) {
-- 
2.9.3


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

Re: [PATCH v3 2/2] x86_64/kexec: Use PUD level 1GB page for identity mapping if available

[Xunlei Pang]

On 05/08/2017 at 02:29 PM, Ingo Molnar wrote:
> * Xunlei Pang  wrote:
>
>> On 05/05/2017 at 05:20 PM, Ingo Molnar wrote:
>>> * Xunlei Pang  wrote:
>>>
 On 05/05/2017 at 02:52 PM, Ingo Molnar wrote:
> * Xunlei Pang  wrote:
>
>> @@ -122,6 +122,10 @@ static int init_pgtable(struct kimage *image, 
>> unsigned long start_pgtable)
>>  
>>  level4p = (pgd_t *)__va(start_pgtable);
>>  clear_page(level4p);
>> +
>> +if (direct_gbpages)
>> +info.direct_gbpages = true;
> No, this should be keyed off the CPU feature (X86_FEATURE_GBPAGES) 
> automatically, 
> not set blindly! AFAICS this patch will crash kexec on any CPU that does 
> not 
> support gbpages.
 It should be fine, probe_page_size_mask() already takes care of this:
 if (direct_gbpages && boot_cpu_has(X86_FEATURE_GBPAGES)) {
 printk(KERN_INFO "Using GB pages for direct mapping\n");
 page_size_mask |= 1 << PG_LEVEL_1G;
 } else {
 direct_gbpages = 0;
 }

 So if X86_FEATURE_GBPAGES is not supported, direct_gbpages will be set to 
 0.
>>> So why is the introduction of the info.direct_gbpages flag necessary? 
>>> AFAICS it 
>>> just duplicates the kernel's direct_gbpages flag. One outcome is that 
>>> hibernation 
>>> won't use gbpages, which is silly.
>> boot/compressed/pagetable.c also uses kernel_ident_mapping_init() for kaslr, 
>> at 
>> the moment we don't have "direct_gbpages" definition or X86_FEATURE_GBPAGES 
>> feature detection.
>>
>> I thought that we can change the other call sites when found really needed.
> Ok, you are right - I'll use the original patches as submitted, with the 
> updated 
> changelogs.

Thanks!

Regards,
Xunlei

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

Re: [PATCH v3 2/2] x86_64/kexec: Use PUD level 1GB page for identity mapping if available

[Ingo Molnar]

* Xunlei Pang  wrote:

> On 05/05/2017 at 05:20 PM, Ingo Molnar wrote:
> > * Xunlei Pang  wrote:
> >
> >> On 05/05/2017 at 02:52 PM, Ingo Molnar wrote:
> >>> * Xunlei Pang  wrote:
> >>>
>  @@ -122,6 +122,10 @@ static int init_pgtable(struct kimage *image, 
>  unsigned long start_pgtable)
>   
>   level4p = (pgd_t *)__va(start_pgtable);
>   clear_page(level4p);
>  +
>  +if (direct_gbpages)
>  +info.direct_gbpages = true;
> >>> No, this should be keyed off the CPU feature (X86_FEATURE_GBPAGES) 
> >>> automatically, 
> >>> not set blindly! AFAICS this patch will crash kexec on any CPU that does 
> >>> not 
> >>> support gbpages.
> >> It should be fine, probe_page_size_mask() already takes care of this:
> >> if (direct_gbpages && boot_cpu_has(X86_FEATURE_GBPAGES)) {
> >> printk(KERN_INFO "Using GB pages for direct mapping\n");
> >> page_size_mask |= 1 << PG_LEVEL_1G;
> >> } else {
> >> direct_gbpages = 0;
> >> }
> >>
> >> So if X86_FEATURE_GBPAGES is not supported, direct_gbpages will be set to 
> >> 0.
> > So why is the introduction of the info.direct_gbpages flag necessary? 
> > AFAICS it 
> > just duplicates the kernel's direct_gbpages flag. One outcome is that 
> > hibernation 
> > won't use gbpages, which is silly.
> 
> boot/compressed/pagetable.c also uses kernel_ident_mapping_init() for kaslr, 
> at 
> the moment we don't have "direct_gbpages" definition or X86_FEATURE_GBPAGES 
> feature detection.
> 
> I thought that we can change the other call sites when found really needed.

Ok, you are right - I'll use the original patches as submitted, with the 
updated 
changelogs.

Thanks,

Ingo

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