Re: [PATCH v5 02/13] kexec_file: Change kexec_add_buffer to take kexec_buf as argument.

2016-08-16 Thread Balbir Singh


On 17/08/16 04:49, Thiago Jung Bauermann wrote:
> Am Dienstag, 16 August 2016, 16:15:55 schrieb Balbir Singh:
>> On 16/08/16 00:49, Thiago Jung Bauermann wrote:
>>> Am Montag, 15 August 2016, 17:30:49 schrieb Balbir Singh:
 On Thu, Aug 11, 2016 at 08:08:07PM -0300, Thiago Jung Bauermann wrote:
> Adapt all callers to the new function prototype.

 Could you please expand on this?
>>>
>>> Is the following better?
>>>
>>> Adapt all callers to set up a kexec_buf to pass to kexec_add_buffer.
>>
>> Yes and the reason for doing so? Consolidation/clarity of implementation?
> 
> Indeed. What about this commit message?
> 
> Subject: [PATCH v5 02/13] kexec_file: Change kexec_add_buffer to take  
>  kexec_buf as argument.
> 
> This is done to simplify the kexec_add_buffer argument list.
> Adapt all callers to set up a kexec_buf to pass to kexec_add_buffer.
> 
> In addition, change the type of kexec_buf.buffer from char * to void *.
> There is no particular reason for it to be a char *, and the change
> allows us to get rid of 3 existing casts to char * in the code.
> 
> Signed-off-by: Thiago Jung Bauermann 
> Acked-by: Dave Young 
> Acked-by: Balbir Singh 
> 


Looks good

Balbir

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


[PATCH v2 1/2] kexec: Introduce "/sys/kernel/kexec_crash_low_size"

2016-08-16 Thread Xunlei Pang
"/sys/kernel/kexec_crash_size" only handles crashk_res, it
is fine in most cases, but sometimes we have crashk_low_res.
For example, when "crashkernel=size[KMG],high" combined with
"crashkernel=size[KMG],low" is used for 64-bit x86.

Like crashk_res, we introduce the corresponding sysfs file
"/sys/kernel/kexec_crash_low_size" for crashk_low_res.

So, the exact total reserved memory is the sum of the two.

crashk_low_res can also be shrunk via this new interface,
and users should be aware of what they are doing.

Suggested-by: Dave Young 
Signed-off-by: Xunlei Pang 
---
 include/linux/kexec.h |  4 ++--
 kernel/kexec_core.c   | 23 ---
 kernel/ksysfs.c   | 25 +++--
 3 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d743777..4f271fc 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -304,8 +304,8 @@ int parse_crashkernel_high(char *cmdline, unsigned long 
long system_ram,
unsigned long long *crash_size, unsigned long long *crash_base);
 int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
unsigned long long *crash_size, unsigned long long *crash_base);
-int crash_shrink_memory(unsigned long new_size);
-size_t crash_get_memory_size(void);
+int crash_shrink_memory(struct resource *res, unsigned long new_size);
+size_t crash_get_memory_size(struct resource *res);
 void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
 
 int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..707d18e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -925,13 +925,13 @@ void crash_kexec(struct pt_regs *regs)
}
 }
 
-size_t crash_get_memory_size(void)
+size_t crash_get_memory_size(struct resource *res)
 {
size_t size = 0;
 
mutex_lock(_mutex);
-   if (crashk_res.end != crashk_res.start)
-   size = resource_size(_res);
+   if (res->end != res->start)
+   size = resource_size(res);
mutex_unlock(_mutex);
return size;
 }
@@ -945,7 +945,7 @@ void __weak crash_free_reserved_phys_range(unsigned long 
begin,
free_reserved_page(boot_pfn_to_page(addr >> PAGE_SHIFT));
 }
 
-int crash_shrink_memory(unsigned long new_size)
+int crash_shrink_memory(struct resource *res, unsigned long new_size)
 {
int ret = 0;
unsigned long start, end;
@@ -958,8 +958,9 @@ int crash_shrink_memory(unsigned long new_size)
ret = -ENOENT;
goto unlock;
}
-   start = crashk_res.start;
-   end = crashk_res.end;
+
+   start = res->start;
+   end = res->end;
old_size = (end == 0) ? 0 : end - start + 1;
if (new_size >= old_size) {
ret = (new_size == old_size) ? 0 : -EINVAL;
@@ -975,17 +976,17 @@ int crash_shrink_memory(unsigned long new_size)
start = roundup(start, KEXEC_CRASH_MEM_ALIGN);
end = roundup(start + new_size, KEXEC_CRASH_MEM_ALIGN);
 
-   crash_free_reserved_phys_range(end, crashk_res.end);
+   crash_free_reserved_phys_range(end, res->end);
 
-   if ((start == end) && (crashk_res.parent != NULL))
-   release_resource(_res);
+   if ((start == end) && (res->parent != NULL))
+   release_resource(res);
 
ram_res->start = end;
-   ram_res->end = crashk_res.end;
+   ram_res->end = res->end;
ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
ram_res->name = "System RAM";
 
-   crashk_res.end = end - 1;
+   res->end = end - 1;
 
insert_resource(_resource, ram_res);
 
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index ee1bc1b..3336fd5 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -105,10 +105,30 @@ static ssize_t kexec_crash_loaded_show(struct kobject 
*kobj,
 }
 KERNEL_ATTR_RO(kexec_crash_loaded);
 
+static ssize_t kexec_crash_low_size_show(struct kobject *kobj,
+  struct kobj_attribute *attr, char *buf)
+{
+   return sprintf(buf, "%zu\n", crash_get_memory_size(_low_res));
+}
+static ssize_t kexec_crash_low_size_store(struct kobject *kobj,
+  struct kobj_attribute *attr,
+  const char *buf, size_t count)
+{
+   unsigned long cnt;
+   int ret;
+
+   if (kstrtoul(buf, 0, ))
+   return -EINVAL;
+
+   ret = crash_shrink_memory(_low_res, cnt);
+   return ret < 0 ? ret : count;
+}
+KERNEL_ATTR_RW(kexec_crash_low_size);
+
 static ssize_t kexec_crash_size_show(struct kobject *kobj,
   struct kobj_attribute *attr, char *buf)
 {
-   return sprintf(buf, "%zu\n", crash_get_memory_size());
+   return sprintf(buf, "%zu\n", crash_get_memory_size(_res));
 }
 static ssize_t 

[PATCH v2 2/2] kexec: Consider crashk_low_res in sanity_check_segment_list()

2016-08-16 Thread Xunlei Pang
We have crashk_res only in most cases, but sometimes we have
crashk_low_res.

For example, on 64-bit x86 systems, when "crashkernel=32M,high"
combined with "crashkernel=128M,low" is used, so some segments
may have the chance to be loaded into crashk_low_res area. We
can't fail it as a memory violation in these cases.

Thus, we add the case to regard the segment as valid if it is
within crashk_low_res.

Signed-off-by: Xunlei Pang 
---
 kernel/kexec_core.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 707d18e..9012a60 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -248,9 +248,14 @@ int sanity_check_segment_list(struct kimage *image)
mstart = image->segment[i].mem;
mend = mstart + image->segment[i].memsz - 1;
/* Ensure we are within the crash kernel limits */
-   if ((mstart < phys_to_boot_phys(crashk_res.start)) ||
-   (mend > phys_to_boot_phys(crashk_res.end)))
-   return -EADDRNOTAVAIL;
+   if ((mstart >= phys_to_boot_phys(crashk_res.start)) &&
+   (mend <= phys_to_boot_phys(crashk_res.end)))
+   continue;
+   if ((mstart >= phys_to_boot_phys(crashk_low_res.start)) 
&&
+   (mend <= phys_to_boot_phys(crashk_low_res.end)))
+   continue;
+
+   return -EADDRNOTAVAIL;
}
}
 
-- 
1.8.3.1


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


Re: [PATCH v5 02/13] kexec_file: Change kexec_add_buffer to take kexec_buf as argument.

2016-08-16 Thread Thiago Jung Bauermann
Am Dienstag, 16 August 2016, 16:15:55 schrieb Balbir Singh:
> On 16/08/16 00:49, Thiago Jung Bauermann wrote:
> > Am Montag, 15 August 2016, 17:30:49 schrieb Balbir Singh:
> >> On Thu, Aug 11, 2016 at 08:08:07PM -0300, Thiago Jung Bauermann wrote:
> >>> Adapt all callers to the new function prototype.
> >> 
> >> Could you please expand on this?
> > 
> > Is the following better?
> > 
> > Adapt all callers to set up a kexec_buf to pass to kexec_add_buffer.
> 
> Yes and the reason for doing so? Consolidation/clarity of implementation?

Indeed. What about this commit message?

Subject: [PATCH v5 02/13] kexec_file: Change kexec_add_buffer to take  
 kexec_buf as argument.

This is done to simplify the kexec_add_buffer argument list.
Adapt all callers to set up a kexec_buf to pass to kexec_add_buffer.

In addition, change the type of kexec_buf.buffer from char * to void *.
There is no particular reason for it to be a char *, and the change
allows us to get rid of 3 existing casts to char * in the code.

Signed-off-by: Thiago Jung Bauermann 
Acked-by: Dave Young 
Acked-by: Balbir Singh 

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


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


Re: [PATCH] kexec: Account crashk_low_res to kexec_crash_size

2016-08-16 Thread Dave Young
Hi,

On 08/15/16 at 04:05pm, Xunlei Pang wrote:
> On 2016/08/15 at 15:17, Dave Young wrote:
> > Hi Xunlei,
> >
> > On 08/13/16 at 04:26pm, Xunlei Pang wrote:
> >> "/sys/kernel/kexec_crash_size" only includes crashk_res, it
> >> is fine in most cases, but sometimes we have crashk_low_res.
> >> For example, when "crashkernel=size[KMG],high" combined with
> >> "crashkernel=size[KMG],low" is used for 64-bit x86.
> >>
> >> Let "/sys/kernel/kexec_crash_size" reflect all the reserved
> >> memory including crashk_low_res, this is more understandable
> >> from its naming.
> > Maybe export another file for the kexec_crash_low_size so that
> > we can clearly get how much the low area is.
> 
> I'm fine with it.
> 
> >> Although we can get all the crash memory from "/proc/iomem"
> >> by filtering all "Crash kernel" keyword, it is more convenient
> >> to utilize this file, and the two ways should stay consistent.
> > Shrink low area does not make much sense, one may either use it or
> > shrink it to 0.
> >
> > Actually think more about it, the crashk_low is only for x86,
> > it might be even better to move it to x86 code instead of in
> > common code.
> >
> > Opinion?
> 
> crashk_low is defined in kernel/kexec_core.c, it's an architecture 
> independent definition
> though it's only used by x86 currently, maybe it can be used by others in the 
> future.
> It's why I'm not handling it specifically for x86.

Ok, we can leave with it since it is in common code from the very
beginning but I doubt that any other arches will use it.

> 
> I just tested the original proc interface further, and it can be shrinked to 
> be zero.
> So I guess we can ease the restriction on shrinking the low area as well.
> 
> What do you think?

Ok, agreed.

Thanks
Dave

> 
> Regards,
> Xunlei
> 
> >
> > Thanks
> > Dave
> >> Note that write to "/sys/kernel/kexec_crash_size" is to shrink
> >> the reserved memory, and we want to shrink crashk_res only.
> >> So we add some additional check in crash_shrink_memory() since
> >> crashk_low_res now is involved.
> >>
> >> Signed-off-by: Xunlei Pang 
> >> ---
> >>  kernel/kexec_core.c | 15 ++-
> >>  1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> >> index 5616755..d5ae780 100644
> >> --- a/kernel/kexec_core.c
> >> +++ b/kernel/kexec_core.c
> >> @@ -932,6 +932,8 @@ size_t crash_get_memory_size(void)
> >>mutex_lock(_mutex);
> >>if (crashk_res.end != crashk_res.start)
> >>size = resource_size(_res);
> >> +  if (crashk_low_res.end != crashk_low_res.start)
> >> +  size += resource_size(_low_res);
> >>mutex_unlock(_mutex);
> >>return size;
> >>  }
> >> @@ -949,7 +951,7 @@ int crash_shrink_memory(unsigned long new_size)
> >>  {
> >>int ret = 0;
> >>unsigned long start, end;
> >> -  unsigned long old_size;
> >> +  unsigned long low_size, old_size;
> >>struct resource *ram_res;
> >>  
> >>mutex_lock(_mutex);
> >> @@ -958,6 +960,17 @@ int crash_shrink_memory(unsigned long new_size)
> >>ret = -ENOENT;
> >>goto unlock;
> >>}
> >> +
> >> +  start = crashk_low_res.start;
> >> +  end = crashk_low_res.end;
> >> +  low_size = (end == 0) ? 0 : end - start + 1;
> >> +  /* Do not shrink crashk_low_res. */
> >> +  if (new_size <= low_size) {
> >> +  ret = -EINVAL;
> >> +  goto unlock;
> >> +  }
> >> +
> >> +  new_size -= low_size;
> >>start = crashk_res.start;
> >>end = crashk_res.end;
> >>old_size = (end == 0) ? 0 : end - start + 1;
> >> -- 
> >> 1.8.3.1
> >>
> >>
> >> ___
> >> 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
> 

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


Re: [PATCH v24 5/9] arm64: kdump: add kdump support

2016-08-16 Thread James Morse
Hi Pratyush,

On 11/08/16 11:03, Pratyush Anand wrote:
> On 10/08/2016:11:48:27 PM, Pratyush Anand wrote:
>> On 10/08/2016:05:38:05 PM, James Morse wrote:
>>> =%<=
>>> diff --git a/arch/arm64/kernel/crash_dump.c b/arch/arm64/kernel/crash_dump.c
>>> index 2dc54d129be1..784d4c30b534 100644
>>> --- a/arch/arm64/kernel/crash_dump.c
>>> +++ b/arch/arm64/kernel/crash_dump.c
>>> @@ -37,6 +37,11 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
>>> if (!csize)
>>> return 0;
>>>
>>> +   if (memblock_is_memory(pfn << PAGE_SHIFT) &&
>>> +   !memblock_is_map_memory(pfn << PAGE_SHIFT))
>>> +   /* skip this nomap memory region, reserved by firmware */
>>> +   return 0;
> 
> This should return 0 or -EINVAL? because, its caller does not care properly
> about 0 return value (when csize is non-zero). So either we need to return
> -EINVAL or we need to fix it's caller so that pread() would know that required
> number of data were not read.

I blindly followed 'number of bytes copied' -> 0. It worked for me, but may not
be correct.

remap_oldmem_pfn_checked() looks like it substitutes the zero page in this (or
at least a similar) case, maybe we should do the same for nomap pages.


> 
>>> +
>>> vaddr = ioremap_cache(__pfn_to_phys(pfn), PAGE_SIZE);
>>> if (!vaddr)
>>> return -ENOMEM;
>>> =%<=
>>
>> In any case kernel must not panic, so I think we must have above hunk. 
>> However,
>> we also need to look into kexec-tools that why it is asking kernel to copy 
>> those
>> unneeded chunks.
>>
>> I will test tomorrow with above hunk.
> 
> After that hunk it did not crash but vmcore-dmesg fails with following 
> message:
> "No program header covering vaddr 0x401ff0found kexec bug?"
> 
> It happened because vmcore-dmesg is sending wrong offset to the pread(), and 
> so
> it did not crash after the above kernel hunk but it still read garbage wrong
> log_buf virtual address pointer.
> 
> vmcore-dmesg is sending wrong offset because page_offset(vp_offset) 
> calculation
> is not perfect for my case, explained here [1].
> 
> So, if I correct page_offset(vp_offset) (as arm64_mem.page_offset = 
> ehdr.e_entry
> - "kernel Code Start PA" + phys_offset), then vmcore-dmesg and vmcore copy
> worked fine, however if I use makedumpfile to copy(compressed) data from
> /proc/vmcore then it still generates "synchronous external abort". I think, it

At a guess makedumpfile is mmap()ing /proc/vmcore so it can use multiple
threads to read (then compress) the data. This bypasses the check added to
copy_oldmem_page(). We probably need to provide a remap_oldmem_pfn_range() that
checks whether the range contains nomap pages.

I will try and send a fixup patch to do this later this week, (unless someone
beats me to it!)


> generated because it would have found garbage data in EFI memory region.

If it was marked as belonging to efi in the efi memory map, the kernel shouldn't
be touching it. If you add 'efi=debug' to your kernel cmdline you get a table of
the addresses and properties.


Thanks,

James


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


Re: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

2016-08-16 Thread Andrew Morton
On Sat, 13 Aug 2016 00:18:23 -0300 Thiago Jung Bauermann 
 wrote:

> kexec_update_segment allows a given segment in kexec_image to have
> its contents updated. This is useful if the current kernel wants to
> send information to the next kernel that is up-to-date at the time of
> reboot.
> 
> ...
>
> @@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage 
> *image,
>   return page;
>  }
>  
> +/**
> + * kexec_update_segment - update the contents of a kimage segment
> + * @buffer:  New contents of the segment.
> + * @bufsz:   @buffer size.
> + * @load_addr:   Segment's physical address in the next kernel.
> + * @memsz:   Segment size.
> + *
> + * This function assumes kexec_mutex is held.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int kexec_update_segment(const char *buffer, unsigned long bufsz,
> +  unsigned long load_addr, unsigned long memsz)
> +{
> + int i;
> + unsigned long entry;
> + unsigned long *ptr = NULL;
> + void *dest = NULL;
> +
> + if (kexec_image == NULL) {
> + pr_err("Can't update segment: no kexec image loaded.\n");
> + return -EINVAL;
> + }
> +
> + /*
> +  * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
> +  * we have to do it here as well.
> +  */
> + memsz = ALIGN(memsz, PAGE_SIZE);
> +
> + for (i = 0; i < kexec_image->nr_segments; i++)
> + /* We only support updating whole segments. */
> + if (load_addr == kexec_image->segment[i].mem &&
> + memsz == kexec_image->segment[i].memsz) {
> + if (kexec_image->segment[i].do_checksum) {
> + pr_err("Trying to update non-modifiable 
> segment.\n");
> + return -EINVAL;
> + }
> +
> + break;
> + }
> + if (i == kexec_image->nr_segments) {
> + pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
> +load_addr, memsz);
> + return -EINVAL;
> + }
> +
> + for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
> +  entry = *ptr++) {
> + void *addr = (void *) (entry & PAGE_MASK);
> +
> + switch (entry & IND_FLAGS) {
> + case IND_DESTINATION:
> + dest = addr;
> + break;
> + case IND_INDIRECTION:
> + ptr = __va(addr);
> + break;
> + case IND_SOURCE:
> + /* Shouldn't happen, but verify just to be safe. */
> + if (dest == NULL) {
> + pr_err("Invalid kexec entries list.");
> + return -EINVAL;
> + }
> +
> + if (dest == (void *) load_addr) {
> + struct page *page;
> + char *ptr;
> + size_t uchunk, mchunk;
> +
> + page = kmap_to_page(addr);
> +
> + ptr = kmap(page);

kmap_atomic() could be used here, and it is appreciably faster.


> + ptr += load_addr & ~PAGE_MASK;
> + mchunk = min_t(size_t, memsz,
> +PAGE_SIZE - (load_addr & 
> ~PAGE_MASK));
> + uchunk = min(bufsz, mchunk);
> + memcpy(ptr, buffer, uchunk);
> +
> + kunmap(page);
> +
> + bufsz -= uchunk;
> + load_addr += mchunk;
> + buffer += mchunk;
> + memsz -= mchunk;
> + }
> + dest += PAGE_SIZE;
> + }
> +
> + /* Shouldn't happen, but verify just to be safe. */
> + if (ptr == NULL) {
> + pr_err("Invalid kexec entries list.");
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +


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


Re: [PATCH v5 04/13] powerpc: Factor out relocation code from module_64.c to elf_util_64.c.

2016-08-16 Thread Balbir Singh


On 16/08/16 09:25, Thiago Jung Bauermann wrote:
> Am Montag, 15 August 2016, 17:46:34 schrieb Balbir Singh:
>> On Thu, Aug 11, 2016 at 08:08:09PM -0300, Thiago Jung Bauermann wrote:
>>> +/**
>>> + * elf64_apply_relocate_add - apply 64 bit RELA relocations
>>> + * @elf_info:  Support information for the ELF binary being 
> relocated.
>>> + * @strtab:String table for the associated symbol 
> table.
>>> + * @symindex:  Section header index for the associated 
> symbol table.
>>> + * @relsec:Section header index for the relocations to 
> apply.
>>> + * @obj_name:  The name of the ELF binary, for information 
> messages.
>>> + */
>>> +int elf64_apply_relocate_add(const struct elf_info *elf_info,
>>> +const char *strtab, unsigned int symindex,
>>> +unsigned int relsec, const char *obj_name)
>>> +{
>>> +   unsigned int i;
>>> +   Elf64_Shdr *sechdrs = elf_info->sechdrs;
>>> +   Elf64_Rela *rela = (void *)sechdrs[relsec].sh_addr;
>>> +   Elf64_Sym *sym;
>>> +   unsigned long *location;
>>> +   unsigned long value;
>>> +
>>
>> For the relocatable kernel we expect only
>>
>> R_PPC64_RELATIVE
>> R_PPC64_NONE
>> R_PPC64_ADDR64
>>
>> In the future we can use this to check/assert the usage of this
>> for the core kernel (vmlinux) when loaded.
>>
>> Did we check elf64_apply_relocate_add with zImage and vmlinux?
> 
> kexec_file_load doesn't call call elf64_apply_relocate_add on the kernel 
> image, it only uses it to relocate the purgatory. So whether it is loading a 
> zImage or a vmlinux file, the function will work in the same way since the 
> purgatory binary is the same regardless of the kernel image format.

Thanks for clarifying.

> 
> For the same reason, as it currently stands kexec_file_load can't check the 
> relocation types used in the kernel image. But it is possible to add such a 
> check/assertion in kexec_elf_64.c:build_elf_exec_info if we want.
> 
> I tested kexec_file_load on both relocatable and non-relocatable vmlinux and 
> it works correctly.
> 
> I hadn't tested with zImage yet. I just did, and I had two problems:
> 
> 1. For some reason, it has an INTERP segment. This patch series doesn't 
> support loading program interpreters for ELF binaries, so 
> kexec_elf_64.c:build_elf_exec_info refuses to load them.
> 
> 2. If I disable the check for the INTERP segment, the zImage file loads 
> correctly, but then I get an exception during reboot when loading the kexec 
> image, right before jumping into the purgatory. I suspect this is because 
> the LOAD segment has a virtual address of 0, and the first kernel is not 
> coping well with that. But I still have to debug it further.
> 
> Is there a reason for the zImage ELF header to request an interpreter and to 
> have a virtual address of 0?
> 

Not that I am aware of. 

Balbir Singh

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


Re: [PATCH v5 02/13] kexec_file: Change kexec_add_buffer to take kexec_buf as argument.

2016-08-16 Thread Balbir Singh


On 16/08/16 00:49, Thiago Jung Bauermann wrote:
> Am Montag, 15 August 2016, 17:30:49 schrieb Balbir Singh:
>> On Thu, Aug 11, 2016 at 08:08:07PM -0300, Thiago Jung Bauermann wrote:
>>> Adapt all callers to the new function prototype.
>>
>> Could you please expand on this?
> 
> Is the following better?
> 
> Adapt all callers to set up a kexec_buf to pass to kexec_add_buffer.

Yes and the reason for doing so? Consolidation/clarity of implementation?

> 
>> Looks good otherwise
>>
>> Acked-by: Balbir Singh 
> 
> Thank you for reviewing this series!
> 

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


Re: [PATCH 3/3] close_dump_bitmap: simplify logic

2016-08-16 Thread Petr Tesarik
On Tue, 16 Aug 2016 00:37:09 +
Atsushi Kumagai  wrote:

> >> > The boolean expression replicates the logic of open_dump_bitmap().
> >> > It's simpler and less error-prone to simply check if fd_bitmap is
> >> > valid.
> >> >
> >> > Signed-off-by: Martin Wilck 
> >> > ---
> >> >  makedumpfile.c | 3 +--
> >> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >> >
> >> > diff --git a/makedumpfile.c b/makedumpfile.c
> >> > index 43278f1..771ab7c 100644
> >> > --- a/makedumpfile.c
> >> > +++ b/makedumpfile.c
> >> > @@ -8579,8 +8579,7 @@ close_dump_file(void)
> >> >  void
> >> >  close_dump_bitmap(void)
> >> >  {
> >> > -if (!info->working_dir && !info->flag_reassemble && !info-
> >> > >flag_refiltering
> >> > -&& !info->flag_sadump && !info->flag_mem_usage)
> >> > +if (!info->fd_bitmap)
> >>
> >> Strictly speaking, zero is a valid FD. I can see that it is highly
> >> unlikely to be the bitmap FD, but it would be a nice cleanup to
> >> initialize fd_bitmap to a negative number and check info->fd_bitmap <
> >> 0.
> >> I'm just not sure where to put the initializition...
> >
> >
> >> > OTOH I know I'm asking you to fix something that you didn't break.
> >
> >I had the same thought, and the same excuse not to address it in this
> >patch set. If you grep makedumpfile.c for "fd_bitmap", you'll see many
> >checks like "if (info->fd_bitmap)". I just followed that pattern for
> >now.
> 
> I see, it would be better to make the checks strict on this occasion.
> So, could you work for that cleanup before your three patches as an
> additional cleanup patch ?

OK, I take it. ;-)

Martin, do you mind rebasing your patch when I'm done with the cleanup?

Petr T

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