Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-27 Thread Xunlei Pang 
On 04/27/2017 at 01:44 PM, Dave Young wrote:
> Hi Xunlei,
>
> On 04/27/17 at 01:25pm, Xunlei Pang wrote:
>> On 04/27/2017 at 11:06 AM, Dave Young wrote:
>>> [snip]
>>>  
>>>  static int __init crash_save_vmcoreinfo_init(void)
>>>  {
>>> +   /* One page should be enough for VMCOREINFO_BYTES under all 
>>> archs */
>> Can we add a comment in the VMCOREINFO_BYTES header file about the one
>> page assumption?
>>
>> Or just define the VMCOREINFO_BYTES as PAGE_SIZE instead of 4096
> Yes, I considered this before, but VMCOREINFO_BYTES is also used by 
> VMCOREINFO_NOTE_SIZE
> definition which is exported to sysfs, also some platform has larger page 
> size(64KB), so
> I didn't touch this 4096 value.
>
> I think I should use kmalloc() to allocate both of them, then move this 
> comment to Patch3 
> kimage_crash_copy_vmcoreinfo().
 But on the other hand, using a separate page for them seems safer compared 
 with
 using frequently-used slab, what's your opinion?
>>> I feel current page based way is better.
>>>
>>> For 64k page the vmcore note size will increase it seems fine. Do you
>>> have concern in mind?
>> Since tools are supposed to acquire vmcoreinfo note size from sysfs, it 
>> should be safe to do so,
>> except that there is some waste in memory for larger PAGE_SIZE.
> Either way is fine to me, I think it is up to your implementation, if
> choose page alloc then modify the macro with PAGE_SIZE looks better.

OK, I will use PAGE_SIZE then, thanks for your comments.

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


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

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-26 Thread Dave Young 
Hi Xunlei,

On 04/27/17 at 01:25pm, Xunlei Pang wrote:
> On 04/27/2017 at 11:06 AM, Dave Young wrote:
> > [snip]
> >  
> >  static int __init crash_save_vmcoreinfo_init(void)
> >  {
> > +   /* One page should be enough for VMCOREINFO_BYTES under all 
> > archs */
>  Can we add a comment in the VMCOREINFO_BYTES header file about the one
>  page assumption?
> 
>  Or just define the VMCOREINFO_BYTES as PAGE_SIZE instead of 4096
> >>> Yes, I considered this before, but VMCOREINFO_BYTES is also used by 
> >>> VMCOREINFO_NOTE_SIZE
> >>> definition which is exported to sysfs, also some platform has larger page 
> >>> size(64KB), so
> >>> I didn't touch this 4096 value.
> >>>
> >>> I think I should use kmalloc() to allocate both of them, then move this 
> >>> comment to Patch3 
> >>> kimage_crash_copy_vmcoreinfo().
> >> But on the other hand, using a separate page for them seems safer compared 
> >> with
> >> using frequently-used slab, what's your opinion?
> > I feel current page based way is better.
> >
> > For 64k page the vmcore note size will increase it seems fine. Do you
> > have concern in mind?
> 
> Since tools are supposed to acquire vmcoreinfo note size from sysfs, it 
> should be safe to do so,
> except that there is some waste in memory for larger PAGE_SIZE.

Either way is fine to me, I think it is up to your implementation, if
choose page alloc then modify the macro with PAGE_SIZE looks better.

Thanks
Dave

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

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-26 Thread Xunlei Pang 
On 04/27/2017 at 11:06 AM, Dave Young wrote:
> [snip]
>  
>  static int __init crash_save_vmcoreinfo_init(void)
>  {
> + /* One page should be enough for VMCOREINFO_BYTES under all archs */
 Can we add a comment in the VMCOREINFO_BYTES header file about the one
 page assumption?

 Or just define the VMCOREINFO_BYTES as PAGE_SIZE instead of 4096
>>> Yes, I considered this before, but VMCOREINFO_BYTES is also used by 
>>> VMCOREINFO_NOTE_SIZE
>>> definition which is exported to sysfs, also some platform has larger page 
>>> size(64KB), so
>>> I didn't touch this 4096 value.
>>>
>>> I think I should use kmalloc() to allocate both of them, then move this 
>>> comment to Patch3 
>>> kimage_crash_copy_vmcoreinfo().
>> But on the other hand, using a separate page for them seems safer compared 
>> with
>> using frequently-used slab, what's your opinion?
> I feel current page based way is better.
>
> For 64k page the vmcore note size will increase it seems fine. Do you
> have concern in mind?

Since tools are supposed to acquire vmcoreinfo note size from sysfs, it should 
be safe to do so,
except that there is some waste in memory for larger PAGE_SIZE.

Regards,
Xunlei

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

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-26 Thread Dave Young 
[snip]
> >>>  
> >>>  static int __init crash_save_vmcoreinfo_init(void)
> >>>  {
> >>> + /* One page should be enough for VMCOREINFO_BYTES under all archs */
> >> Can we add a comment in the VMCOREINFO_BYTES header file about the one
> >> page assumption?
> >>
> >> Or just define the VMCOREINFO_BYTES as PAGE_SIZE instead of 4096
> > Yes, I considered this before, but VMCOREINFO_BYTES is also used by 
> > VMCOREINFO_NOTE_SIZE
> > definition which is exported to sysfs, also some platform has larger page 
> > size(64KB), so
> > I didn't touch this 4096 value.
> >
> > I think I should use kmalloc() to allocate both of them, then move this 
> > comment to Patch3 
> > kimage_crash_copy_vmcoreinfo().
> 
> But on the other hand, using a separate page for them seems safer compared 
> with
> using frequently-used slab, what's your opinion?

I feel current page based way is better.

For 64k page the vmcore note size will increase it seems fine. Do you
have concern in mind?

Thanks

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

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-26 Thread Xunlei Pang 
On 04/26/2017 at 05:51 PM, Xunlei Pang wrote:
> On 04/26/2017 at 03:19 PM, Dave Young wrote:
>> Add ia64i list,  and s390 list although Michael has tested it
>>
>> On 04/20/17 at 07:39pm, Xunlei Pang wrote:
>>> As Eric said,
>>> "what we need to do is move the variable vmcoreinfo_note out
>>> of the kernel's .bss section.  And modify the code to regenerate
>>> and keep this information in something like the control page.
>>>
>>> Definitely something like this needs a page all to itself, and ideally
>>> far away from any other kernel data structures.  I clearly was not
>>> watching closely the data someone decided to keep this silly thing
>>> in the kernel's .bss section."
>>>
>>> This patch allocates extra pages for these vmcoreinfo_XXX variables,
>>> one advantage is that it enhances some safety of vmcoreinfo, because
>>> vmcoreinfo now is kept far away from other kernel data structures.
>>>
>>> Suggested-by: Eric Biederman 
>>> Cc: Michael Holzheu 
>>> Cc: Juergen Gross 
>>> Signed-off-by: Xunlei Pang 
>>> ---
>>> v3->v4:
>>> -Rebased on the latest linux-next
>>> -Handle S390 vmcoreinfo_note properly
>>> -Handle the newly-added xen/mmu_pv.c
>>>
>>>  arch/ia64/kernel/machine_kexec.c |  5 -
>>>  arch/s390/kernel/machine_kexec.c |  1 +
>>>  arch/s390/kernel/setup.c |  6 --
>>>  arch/x86/kernel/crash.c  |  2 +-
>>>  arch/x86/xen/mmu_pv.c|  4 ++--
>>>  include/linux/crash_core.h   |  2 +-
>>>  kernel/crash_core.c  | 27 +++
>>>  kernel/ksysfs.c  |  2 +-
>>>  8 files changed, 29 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/arch/ia64/kernel/machine_kexec.c 
>>> b/arch/ia64/kernel/machine_kexec.c
>>> index 599507b..c14815d 100644
>>> --- a/arch/ia64/kernel/machine_kexec.c
>>> +++ b/arch/ia64/kernel/machine_kexec.c
>>> @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
>>>  #endif
>>>  }
>>>  
>>> -phys_addr_t paddr_vmcoreinfo_note(void)
>>> -{
>>> -   return ia64_tpa((unsigned long)(char *)_note);
>>> -}
>>> -
>>> diff --git a/arch/s390/kernel/machine_kexec.c 
>>> b/arch/s390/kernel/machine_kexec.c
>>> index 49a6bd4..3d0b14a 100644
>>> --- a/arch/s390/kernel/machine_kexec.c
>>> +++ b/arch/s390/kernel/machine_kexec.c
>>> @@ -246,6 +246,7 @@ void arch_crash_save_vmcoreinfo(void)
>>> VMCOREINFO_SYMBOL(lowcore_ptr);
>>> VMCOREINFO_SYMBOL(high_memory);
>>> VMCOREINFO_LENGTH(lowcore_ptr, NR_CPUS);
>>> +   mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
>>>  }
>>>  
>>>  void machine_shutdown(void)
>>> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
>>> index 3ae756c..3d1d808 100644
>>> --- a/arch/s390/kernel/setup.c
>>> +++ b/arch/s390/kernel/setup.c
>>> @@ -496,11 +496,6 @@ static void __init setup_memory_end(void)
>>> pr_notice("The maximum memory size is %luMB\n", memory_end >> 20);
>>>  }
>>>  
>>> -static void __init setup_vmcoreinfo(void)
>>> -{
>>> -   mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
>>> -}
>>> -
>>>  #ifdef CONFIG_CRASH_DUMP
>>>  
>>>  /*
>>> @@ -939,7 +934,6 @@ void __init setup_arch(char **cmdline_p)
>>>  #endif
>>>  
>>> setup_resources();
>>> -   setup_vmcoreinfo();
>>> setup_lowcore();
>>> smp_fill_possible_mask();
>>> cpu_detect_mhz_feature();
>>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>>> index 22217ec..44404e2 100644
>>> --- a/arch/x86/kernel/crash.c
>>> +++ b/arch/x86/kernel/crash.c
>>> @@ -457,7 +457,7 @@ static int prepare_elf64_headers(struct crash_elf_data 
>>> *ced,
>>> bufp += sizeof(Elf64_Phdr);
>>> phdr->p_type = PT_NOTE;
>>> phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
>>> -   phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
>>> +   phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>>> (ehdr->e_phnum)++;
>>>  
>>>  #ifdef CONFIG_X86_64
>>> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
>>> index 9d9ae66..35543fa 100644
>>> --- a/arch/x86/xen/mmu_pv.c
>>> +++ b/arch/x86/xen/mmu_pv.c
>>> @@ -2723,8 +2723,8 @@ void xen_destroy_contiguous_region(phys_addr_t 
>>> pstart, unsigned int order)
>>>  phys_addr_t paddr_vmcoreinfo_note(void)
>>>  {
>>> if (xen_pv_domain())
>>> -   return virt_to_machine(_note).maddr;
>>> +   return virt_to_machine(vmcoreinfo_note).maddr;
>>> else
>>> -   return __pa_symbol(_note);
>>> +   return __pa(vmcoreinfo_note);
>>>  }
>>>  #endif /* CONFIG_KEXEC_CORE */
>>> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
>>> index eb71a70..ba283a2 100644
>>> --- a/include/linux/crash_core.h
>>> +++ b/include/linux/crash_core.h
>>> @@ -53,7 +53,7 @@
>>>  #define VMCOREINFO_PHYS_BASE(value) \
>>> vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
>>>  
>>> -extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
>>> +extern u32

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-26 Thread Xunlei Pang 
On 04/26/2017 at 03:19 PM, Dave Young wrote:
> Add ia64i list,  and s390 list although Michael has tested it
>
> On 04/20/17 at 07:39pm, Xunlei Pang wrote:
>> As Eric said,
>> "what we need to do is move the variable vmcoreinfo_note out
>> of the kernel's .bss section.  And modify the code to regenerate
>> and keep this information in something like the control page.
>>
>> Definitely something like this needs a page all to itself, and ideally
>> far away from any other kernel data structures.  I clearly was not
>> watching closely the data someone decided to keep this silly thing
>> in the kernel's .bss section."
>>
>> This patch allocates extra pages for these vmcoreinfo_XXX variables,
>> one advantage is that it enhances some safety of vmcoreinfo, because
>> vmcoreinfo now is kept far away from other kernel data structures.
>>
>> Suggested-by: Eric Biederman 
>> Cc: Michael Holzheu 
>> Cc: Juergen Gross 
>> Signed-off-by: Xunlei Pang 
>> ---
>> v3->v4:
>> -Rebased on the latest linux-next
>> -Handle S390 vmcoreinfo_note properly
>> -Handle the newly-added xen/mmu_pv.c
>>
>>  arch/ia64/kernel/machine_kexec.c |  5 -
>>  arch/s390/kernel/machine_kexec.c |  1 +
>>  arch/s390/kernel/setup.c |  6 --
>>  arch/x86/kernel/crash.c  |  2 +-
>>  arch/x86/xen/mmu_pv.c|  4 ++--
>>  include/linux/crash_core.h   |  2 +-
>>  kernel/crash_core.c  | 27 +++
>>  kernel/ksysfs.c  |  2 +-
>>  8 files changed, 29 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/ia64/kernel/machine_kexec.c 
>> b/arch/ia64/kernel/machine_kexec.c
>> index 599507b..c14815d 100644
>> --- a/arch/ia64/kernel/machine_kexec.c
>> +++ b/arch/ia64/kernel/machine_kexec.c
>> @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
>>  #endif
>>  }
>>  
>> -phys_addr_t paddr_vmcoreinfo_note(void)
>> -{
>> -return ia64_tpa((unsigned long)(char *)_note);
>> -}
>> -
>> diff --git a/arch/s390/kernel/machine_kexec.c 
>> b/arch/s390/kernel/machine_kexec.c
>> index 49a6bd4..3d0b14a 100644
>> --- a/arch/s390/kernel/machine_kexec.c
>> +++ b/arch/s390/kernel/machine_kexec.c
>> @@ -246,6 +246,7 @@ void arch_crash_save_vmcoreinfo(void)
>>  VMCOREINFO_SYMBOL(lowcore_ptr);
>>  VMCOREINFO_SYMBOL(high_memory);
>>  VMCOREINFO_LENGTH(lowcore_ptr, NR_CPUS);
>> +mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
>>  }
>>  
>>  void machine_shutdown(void)
>> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
>> index 3ae756c..3d1d808 100644
>> --- a/arch/s390/kernel/setup.c
>> +++ b/arch/s390/kernel/setup.c
>> @@ -496,11 +496,6 @@ static void __init setup_memory_end(void)
>>  pr_notice("The maximum memory size is %luMB\n", memory_end >> 20);
>>  }
>>  
>> -static void __init setup_vmcoreinfo(void)
>> -{
>> -mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
>> -}
>> -
>>  #ifdef CONFIG_CRASH_DUMP
>>  
>>  /*
>> @@ -939,7 +934,6 @@ void __init setup_arch(char **cmdline_p)
>>  #endif
>>  
>>  setup_resources();
>> -setup_vmcoreinfo();
>>  setup_lowcore();
>>  smp_fill_possible_mask();
>>  cpu_detect_mhz_feature();
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 22217ec..44404e2 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -457,7 +457,7 @@ static int prepare_elf64_headers(struct crash_elf_data 
>> *ced,
>>  bufp += sizeof(Elf64_Phdr);
>>  phdr->p_type = PT_NOTE;
>>  phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
>> -phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
>> +phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>>  (ehdr->e_phnum)++;
>>  
>>  #ifdef CONFIG_X86_64
>> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
>> index 9d9ae66..35543fa 100644
>> --- a/arch/x86/xen/mmu_pv.c
>> +++ b/arch/x86/xen/mmu_pv.c
>> @@ -2723,8 +2723,8 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, 
>> unsigned int order)
>>  phys_addr_t paddr_vmcoreinfo_note(void)
>>  {
>>  if (xen_pv_domain())
>> -return virt_to_machine(_note).maddr;
>> +return virt_to_machine(vmcoreinfo_note).maddr;
>>  else
>> -return __pa_symbol(_note);
>> +return __pa(vmcoreinfo_note);
>>  }
>>  #endif /* CONFIG_KEXEC_CORE */
>> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
>> index eb71a70..ba283a2 100644
>> --- a/include/linux/crash_core.h
>> +++ b/include/linux/crash_core.h
>> @@ -53,7 +53,7 @@
>>  #define VMCOREINFO_PHYS_BASE(value) \
>>  vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
>>  
>> -extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
>> +extern u32 *vmcoreinfo_note;
>>  extern size_t vmcoreinfo_size;
>>  extern size_t vmcoreinfo_max_size;
>>  
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-26 Thread Dave Young 
Add ia64i list,  and s390 list although Michael has tested it

On 04/20/17 at 07:39pm, Xunlei Pang wrote:
> As Eric said,
> "what we need to do is move the variable vmcoreinfo_note out
> of the kernel's .bss section.  And modify the code to regenerate
> and keep this information in something like the control page.
> 
> Definitely something like this needs a page all to itself, and ideally
> far away from any other kernel data structures.  I clearly was not
> watching closely the data someone decided to keep this silly thing
> in the kernel's .bss section."
> 
> This patch allocates extra pages for these vmcoreinfo_XXX variables,
> one advantage is that it enhances some safety of vmcoreinfo, because
> vmcoreinfo now is kept far away from other kernel data structures.
> 
> Suggested-by: Eric Biederman 
> Cc: Michael Holzheu 
> Cc: Juergen Gross 
> Signed-off-by: Xunlei Pang 
> ---
> v3->v4:
> -Rebased on the latest linux-next
> -Handle S390 vmcoreinfo_note properly
> -Handle the newly-added xen/mmu_pv.c
> 
>  arch/ia64/kernel/machine_kexec.c |  5 -
>  arch/s390/kernel/machine_kexec.c |  1 +
>  arch/s390/kernel/setup.c |  6 --
>  arch/x86/kernel/crash.c  |  2 +-
>  arch/x86/xen/mmu_pv.c|  4 ++--
>  include/linux/crash_core.h   |  2 +-
>  kernel/crash_core.c  | 27 +++
>  kernel/ksysfs.c  |  2 +-
>  8 files changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/ia64/kernel/machine_kexec.c 
> b/arch/ia64/kernel/machine_kexec.c
> index 599507b..c14815d 100644
> --- a/arch/ia64/kernel/machine_kexec.c
> +++ b/arch/ia64/kernel/machine_kexec.c
> @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
>  #endif
>  }
>  
> -phys_addr_t paddr_vmcoreinfo_note(void)
> -{
> - return ia64_tpa((unsigned long)(char *)_note);
> -}
> -
> diff --git a/arch/s390/kernel/machine_kexec.c 
> b/arch/s390/kernel/machine_kexec.c
> index 49a6bd4..3d0b14a 100644
> --- a/arch/s390/kernel/machine_kexec.c
> +++ b/arch/s390/kernel/machine_kexec.c
> @@ -246,6 +246,7 @@ void arch_crash_save_vmcoreinfo(void)
>   VMCOREINFO_SYMBOL(lowcore_ptr);
>   VMCOREINFO_SYMBOL(high_memory);
>   VMCOREINFO_LENGTH(lowcore_ptr, NR_CPUS);
> + mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
>  }
>  
>  void machine_shutdown(void)
> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
> index 3ae756c..3d1d808 100644
> --- a/arch/s390/kernel/setup.c
> +++ b/arch/s390/kernel/setup.c
> @@ -496,11 +496,6 @@ static void __init setup_memory_end(void)
>   pr_notice("The maximum memory size is %luMB\n", memory_end >> 20);
>  }
>  
> -static void __init setup_vmcoreinfo(void)
> -{
> - mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
> -}
> -
>  #ifdef CONFIG_CRASH_DUMP
>  
>  /*
> @@ -939,7 +934,6 @@ void __init setup_arch(char **cmdline_p)
>  #endif
>  
>   setup_resources();
> - setup_vmcoreinfo();
>   setup_lowcore();
>   smp_fill_possible_mask();
>   cpu_detect_mhz_feature();
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 22217ec..44404e2 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -457,7 +457,7 @@ static int prepare_elf64_headers(struct crash_elf_data 
> *ced,
>   bufp += sizeof(Elf64_Phdr);
>   phdr->p_type = PT_NOTE;
>   phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
> - phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
> + phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>   (ehdr->e_phnum)++;
>  
>  #ifdef CONFIG_X86_64
> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> index 9d9ae66..35543fa 100644
> --- a/arch/x86/xen/mmu_pv.c
> +++ b/arch/x86/xen/mmu_pv.c
> @@ -2723,8 +2723,8 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, 
> unsigned int order)
>  phys_addr_t paddr_vmcoreinfo_note(void)
>  {
>   if (xen_pv_domain())
> - return virt_to_machine(_note).maddr;
> + return virt_to_machine(vmcoreinfo_note).maddr;
>   else
> - return __pa_symbol(_note);
> + return __pa(vmcoreinfo_note);
>  }
>  #endif /* CONFIG_KEXEC_CORE */
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index eb71a70..ba283a2 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -53,7 +53,7 @@
>  #define VMCOREINFO_PHYS_BASE(value) \
>   vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
>  
> -extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
> +extern u32 *vmcoreinfo_note;
>  extern size_t vmcoreinfo_size;
>  extern size_t vmcoreinfo_max_size;
>  
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index fcbd568..0321f04 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -14,10 +14,10 @@
>  #include 
>  
>  /* vmcoreinfo stuff

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-24 Thread Michael Holzheu 
Am Thu, 20 Apr 2017 19:39:32 +0800
schrieb Xunlei Pang :

> As Eric said,
> "what we need to do is move the variable vmcoreinfo_note out
> of the kernel's .bss section.  And modify the code to regenerate
> and keep this information in something like the control page.
> 
> Definitely something like this needs a page all to itself, and ideally
> far away from any other kernel data structures.  I clearly was not
> watching closely the data someone decided to keep this silly thing
> in the kernel's .bss section."
> 
> This patch allocates extra pages for these vmcoreinfo_XXX variables,
> one advantage is that it enhances some safety of vmcoreinfo, because
> vmcoreinfo now is kept far away from other kernel data structures.
> 
> Suggested-by: Eric Biederman 
> Cc: Michael Holzheu 
> Cc: Juergen Gross 
> Signed-off-by: Xunlei Pang 

Now s390 seems to work with the patches. I tested kdump and
zfcpdump.

Tested-by: Michael Holzheu 


> ---
> v3->v4:
> -Rebased on the latest linux-next
> -Handle S390 vmcoreinfo_note properly
> -Handle the newly-added xen/mmu_pv.c
> 
>  arch/ia64/kernel/machine_kexec.c |  5 -
>  arch/s390/kernel/machine_kexec.c |  1 +
>  arch/s390/kernel/setup.c |  6 --
>  arch/x86/kernel/crash.c  |  2 +-
>  arch/x86/xen/mmu_pv.c|  4 ++--
>  include/linux/crash_core.h   |  2 +-
>  kernel/crash_core.c  | 27 +++
>  kernel/ksysfs.c  |  2 +-
>  8 files changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/ia64/kernel/machine_kexec.c 
> b/arch/ia64/kernel/machine_kexec.c
> index 599507b..c14815d 100644
> --- a/arch/ia64/kernel/machine_kexec.c
> +++ b/arch/ia64/kernel/machine_kexec.c
> @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
>  #endif
>  }
> 
> -phys_addr_t paddr_vmcoreinfo_note(void)
> -{
> - return ia64_tpa((unsigned long)(char *)_note);
> -}
> -
> diff --git a/arch/s390/kernel/machine_kexec.c 
> b/arch/s390/kernel/machine_kexec.c
> index 49a6bd4..3d0b14a 100644
> --- a/arch/s390/kernel/machine_kexec.c
> +++ b/arch/s390/kernel/machine_kexec.c
> @@ -246,6 +246,7 @@ void arch_crash_save_vmcoreinfo(void)
>   VMCOREINFO_SYMBOL(lowcore_ptr);
>   VMCOREINFO_SYMBOL(high_memory);
>   VMCOREINFO_LENGTH(lowcore_ptr, NR_CPUS);
> + mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
>  }
> 
>  void machine_shutdown(void)
> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
> index 3ae756c..3d1d808 100644
> --- a/arch/s390/kernel/setup.c
> +++ b/arch/s390/kernel/setup.c
> @@ -496,11 +496,6 @@ static void __init setup_memory_end(void)
>   pr_notice("The maximum memory size is %luMB\n", memory_end >> 20);
>  }
> 
> -static void __init setup_vmcoreinfo(void)
> -{
> - mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
> -}
> -
>  #ifdef CONFIG_CRASH_DUMP
> 
>  /*
> @@ -939,7 +934,6 @@ void __init setup_arch(char **cmdline_p)
>  #endif
> 
>   setup_resources();
> - setup_vmcoreinfo();
>   setup_lowcore();
>   smp_fill_possible_mask();
>   cpu_detect_mhz_feature();
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 22217ec..44404e2 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -457,7 +457,7 @@ static int prepare_elf64_headers(struct crash_elf_data 
> *ced,
>   bufp += sizeof(Elf64_Phdr);
>   phdr->p_type = PT_NOTE;
>   phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
> - phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
> + phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>   (ehdr->e_phnum)++;
> 
>  #ifdef CONFIG_X86_64
> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> index 9d9ae66..35543fa 100644
> --- a/arch/x86/xen/mmu_pv.c
> +++ b/arch/x86/xen/mmu_pv.c
> @@ -2723,8 +2723,8 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, 
> unsigned int order)
>  phys_addr_t paddr_vmcoreinfo_note(void)
>  {
>   if (xen_pv_domain())
> - return virt_to_machine(_note).maddr;
> + return virt_to_machine(vmcoreinfo_note).maddr;
>   else
> - return __pa_symbol(_note);
> + return __pa(vmcoreinfo_note);
>  }
>  #endif /* CONFIG_KEXEC_CORE */
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index eb71a70..ba283a2 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -53,7 +53,7 @@
>  #define VMCOREINFO_PHYS_BASE(value) \
>   vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
> 
> -extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
> +extern u32 *vmcoreinfo_note;
>  extern size_t vmcoreinfo_size;
>  extern size_t vmcoreinfo_max_size;
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index fcbd568..0321f04 100644
> ---

Re: [PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-20 Thread Juergen Gross 
On 20/04/17 13:39, Xunlei Pang wrote:
> As Eric said,
> "what we need to do is move the variable vmcoreinfo_note out
> of the kernel's .bss section.  And modify the code to regenerate
> and keep this information in something like the control page.
> 
> Definitely something like this needs a page all to itself, and ideally
> far away from any other kernel data structures.  I clearly was not
> watching closely the data someone decided to keep this silly thing
> in the kernel's .bss section."
> 
> This patch allocates extra pages for these vmcoreinfo_XXX variables,
> one advantage is that it enhances some safety of vmcoreinfo, because
> vmcoreinfo now is kept far away from other kernel data structures.
> 
> Suggested-by: Eric Biederman 
> Cc: Michael Holzheu 
> Cc: Juergen Gross 
> Signed-off-by: Xunlei Pang 

Xen parts: Reviewed-by: Juergen Gross 


Juergen

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

[PATCH v4 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

2017-04-20 Thread Xunlei Pang 
As Eric said,
"what we need to do is move the variable vmcoreinfo_note out
of the kernel's .bss section.  And modify the code to regenerate
and keep this information in something like the control page.

Definitely something like this needs a page all to itself, and ideally
far away from any other kernel data structures.  I clearly was not
watching closely the data someone decided to keep this silly thing
in the kernel's .bss section."

This patch allocates extra pages for these vmcoreinfo_XXX variables,
one advantage is that it enhances some safety of vmcoreinfo, because
vmcoreinfo now is kept far away from other kernel data structures.

Suggested-by: Eric Biederman 
Cc: Michael Holzheu 
Cc: Juergen Gross 
Signed-off-by: Xunlei Pang 
---
v3->v4:
-Rebased on the latest linux-next
-Handle S390 vmcoreinfo_note properly
-Handle the newly-added xen/mmu_pv.c

 arch/ia64/kernel/machine_kexec.c |  5 -
 arch/s390/kernel/machine_kexec.c |  1 +
 arch/s390/kernel/setup.c |  6 --
 arch/x86/kernel/crash.c  |  2 +-
 arch/x86/xen/mmu_pv.c|  4 ++--
 include/linux/crash_core.h   |  2 +-
 kernel/crash_core.c  | 27 +++
 kernel/ksysfs.c  |  2 +-
 8 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
index 599507b..c14815d 100644
--- a/arch/ia64/kernel/machine_kexec.c
+++ b/arch/ia64/kernel/machine_kexec.c
@@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
 #endif
 }
 
-phys_addr_t paddr_vmcoreinfo_note(void)
-{
-   return ia64_tpa((unsigned long)(char *)_note);
-}
-
diff --git a/arch/s390/kernel/machine_kexec.c b/arch/s390/kernel/machine_kexec.c
index 49a6bd4..3d0b14a 100644
--- a/arch/s390/kernel/machine_kexec.c
+++ b/arch/s390/kernel/machine_kexec.c
@@ -246,6 +246,7 @@ void arch_crash_save_vmcoreinfo(void)
VMCOREINFO_SYMBOL(lowcore_ptr);
VMCOREINFO_SYMBOL(high_memory);
VMCOREINFO_LENGTH(lowcore_ptr, NR_CPUS);
+   mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
 }
 
 void machine_shutdown(void)
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
index 3ae756c..3d1d808 100644
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -496,11 +496,6 @@ static void __init setup_memory_end(void)
pr_notice("The maximum memory size is %luMB\n", memory_end >> 20);
 }
 
-static void __init setup_vmcoreinfo(void)
-{
-   mem_assign_absolute(S390_lowcore.vmcore_info, paddr_vmcoreinfo_note());
-}
-
 #ifdef CONFIG_CRASH_DUMP
 
 /*
@@ -939,7 +934,6 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
setup_resources();
-   setup_vmcoreinfo();
setup_lowcore();
smp_fill_possible_mask();
cpu_detect_mhz_feature();
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 22217ec..44404e2 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -457,7 +457,7 @@ static int prepare_elf64_headers(struct crash_elf_data *ced,
bufp += sizeof(Elf64_Phdr);
phdr->p_type = PT_NOTE;
phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
-   phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
+   phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
(ehdr->e_phnum)++;
 
 #ifdef CONFIG_X86_64
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 9d9ae66..35543fa 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2723,8 +2723,8 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, 
unsigned int order)
 phys_addr_t paddr_vmcoreinfo_note(void)
 {
if (xen_pv_domain())
-   return virt_to_machine(_note).maddr;
+   return virt_to_machine(vmcoreinfo_note).maddr;
else
-   return __pa_symbol(_note);
+   return __pa(vmcoreinfo_note);
 }
 #endif /* CONFIG_KEXEC_CORE */
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index eb71a70..ba283a2 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -53,7 +53,7 @@
 #define VMCOREINFO_PHYS_BASE(value) \
vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
 
-extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
+extern u32 *vmcoreinfo_note;
 extern size_t vmcoreinfo_size;
 extern size_t vmcoreinfo_max_size;
 
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index fcbd568..0321f04 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -14,10 +14,10 @@
 #include 
 
 /* vmcoreinfo stuff */
-static unsigned char vmcoreinfo_data[VMCOREINFO_BYTES];
-u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
+static unsigned char *vmcoreinfo_data;
 size_t vmcoreinfo_size;
-size_t vmcoreinfo_max_size = sizeof(vmcoreinfo_data);
+size_t vmcoreinfo_max_size = VMCOREINFO_BYTES;
+u32 *vmcoreinfo_note;
 
 /*
  * parsing the