[PATCH 0/7] Xen fixes and minor cleanups

[PATCH 0/7] Xen fixes and minor cleanups

[Daniel Kiper]

Hi,

This patch series contains following Xen fixes and cleanups:
  - xen: Improve calculation of beginning of virtual address space,
  - elf: Increase buf size in get_pt_note_info(),
  - xen: Take into account new frame table address since Xen 4.3,
  - xen: Enforce page size only when xen-syms file is used,
  - Mute some compiler warnings,
  - Use elf_getshdrstrndx() instead of elf_getshstrndx(),
  - Do not break progress messages.

All patches were tested with Xen versions up to 4.3.

Daniel

[PATCH 1/7] xen: Improve calculation of beginning of virtual address space

[Daniel Kiper]

Xen commit 4b28bf6ae90bd83fd1113d8bdc53c3266ffeb328 (x86: re-introduce
map_domain_page() et al) once again altered virtual address space.
Current algorithm calculating its start could not cope with that change.
New version establishes this value on the base of domain_list placement
and is more generic.

Similar patch was applied to crash tool.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 arch/x86_64.c  |   16 +++++++++++-----
 makedumpfile.h |    2 --
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index 8e61b20..d864a06 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -372,14 +372,20 @@ int get_xen_basic_info_x86_64(void)
 		info->xen_phys_start = info->xen_crash_info.v2->xen_phys_start;
 	}
 
+	info->xen_virt_start = SYMBOL(domain_list);
+
+	/*
+	 * Xen virtual mapping is aligned to 1 GiB boundary.
+	 * domain_list lives in bss which sits no more than
+	 * 1 GiB below beginning of virtual address space.
+	 */
+	info->xen_virt_start &= 0xffffffffc0000000;
+
 	if (info->xen_crash_info.com &&
-	    info->xen_crash_info.com->xen_major_version >= 4) {
-		info->xen_virt_start = XEN_VIRT_START_V4;
+	    info->xen_crash_info.com->xen_major_version >= 4)
 		info->directmap_virt_end = DIRECTMAP_VIRT_END_V4;
-	} else {
-		info->xen_virt_start = XEN_VIRT_START_V3;
+	else
 		info->directmap_virt_end = DIRECTMAP_VIRT_END_V3;
-	}
 
 	if (SYMBOL(pgd_l4) == NOT_FOUND_SYMBOL) {
 		ERRMSG("Can't get pml4.\n");
diff --git a/makedumpfile.h b/makedumpfile.h
index a5826e0..1a87500 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1473,8 +1473,6 @@ int get_xen_info_x86(void);
 #define DIRECTMAP_VIRT_END_V3 (0xffff840000000000)
 #define DIRECTMAP_VIRT_END_V4 (0xffff880000000000)
 #define DIRECTMAP_VIRT_END    (info->directmap_virt_end)
-#define XEN_VIRT_START_V3     (0xffff828c80000000)
-#define XEN_VIRT_START_V4     (0xffff82c480000000)
 #define XEN_VIRT_START        (info->xen_virt_start)
 #define XEN_VIRT_END          (XEN_VIRT_START + (1UL << 30))
 #define FRAMETABLE_VIRT_START 0xffff82f600000000
-- 
1.7.10.4

[PATCH 2/7] elf: Increase buf size in get_pt_note_info()

[Daniel Kiper]

get_pt_note_info() always ignores VMCOREINFO_XEN note
because buf size is too small. It does not have place
for \0 char which marks EOS. This patch fixes that bug
and VMCOREINFO_XEN note living in /proc/vmcore file
could be properly detected now.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 elf_info.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/elf_info.c b/elf_info.c
index 0c1e36a..70a6dd2 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -310,7 +310,7 @@ get_pt_note_info(void)
 {
 	int n_type, size_name, size_desc;
 	off_t offset, offset_desc;
-	char buf[VMCOREINFO_XEN_NOTE_NAME_BYTES];
+	char buf[VMCOREINFO_XEN_NOTE_NAME_BYTES + 1];
 	char note[MAX_SIZE_NHDR];
 
 	nr_cpus = 0;
-- 
1.7.10.4

Re: [Xen-devel] [PATCH 2/7] elf: Increase buf size in get_pt_note_info()

[Andrew Cooper]

On 12/07/13 14:48, Daniel Kiper wrote:
> get_pt_note_info() always ignores VMCOREINFO_XEN note
> because buf size is too small. It does not have place
> for \0 char which marks EOS. This patch fixes that bug
> and VMCOREINFO_XEN note living in /proc/vmcore file
> could be properly detected now.
>
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
>  elf_info.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/elf_info.c b/elf_info.c
> index 0c1e36a..70a6dd2 100644
> --- a/elf_info.c
> +++ b/elf_info.c
> @@ -310,7 +310,7 @@ get_pt_note_info(void)
>  {
>  	int n_type, size_name, size_desc;
>  	off_t offset, offset_desc;
> -	char buf[VMCOREINFO_XEN_NOTE_NAME_BYTES];
> +	char buf[VMCOREINFO_XEN_NOTE_NAME_BYTES + 1];
>  	char note[MAX_SIZE_NHDR];
>  
>  	nr_cpus = 0;

Elf Note namesz is defined to include the terminating null character, so
I would argue that VMCOREINFO_XEN_NOTE_NAME_BYTES is off-by-one.

~Andrew

Re: [Xen-devel] [PATCH 2/7] elf: Increase buf size in get_pt_note_info()

[Daniel Kiper]

On Fri, Jul 12, 2013 at 03:04:41PM +0100, Andrew Cooper wrote:
> On 12/07/13 14:48, Daniel Kiper wrote:
> > get_pt_note_info() always ignores VMCOREINFO_XEN note
> > because buf size is too small. It does not have place
> > for \0 char which marks EOS. This patch fixes that bug
> > and VMCOREINFO_XEN note living in /proc/vmcore file
> > could be properly detected now.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> >  elf_info.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/elf_info.c b/elf_info.c
> > index 0c1e36a..70a6dd2 100644
> > --- a/elf_info.c
> > +++ b/elf_info.c
> > @@ -310,7 +310,7 @@ get_pt_note_info(void)
> >  {
> >  	int n_type, size_name, size_desc;
> >  	off_t offset, offset_desc;
> > -	char buf[VMCOREINFO_XEN_NOTE_NAME_BYTES];
> > +	char buf[VMCOREINFO_XEN_NOTE_NAME_BYTES + 1];
> >  	char note[MAX_SIZE_NHDR];
> >
> >  	nr_cpus = 0;
>
> Elf Note namesz is defined to include the terminating null character, so
> I would argue that VMCOREINFO_XEN_NOTE_NAME_BYTES is off-by-one.

I have checked that once again. By mistake I have assumed that
sizeof("string") does not take into account \0 char (like strlen()).
It means that in real buf could accommodate VMCOREINFO_XEN string
with EOS and relevant condition should be fixed instead of buf size.
I will fix it in second patch series release.

Thanks,

Daniel

[PATCH 3/7] xen: Take into account new frame table address since Xen 4.3

[Daniel Kiper]

Since Xen commit a8d2b06db7826063df9d04be9d6f928bf2189bd0
(x86: extend frame table virtual space) frame table has
new address. Take into account that thing.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 arch/x86_64.c  |   11 +++++++++--
 makedumpfile.h |   21 +++++++++++----------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index d864a06..771d457 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -401,8 +401,15 @@ int get_xen_basic_info_x86_64(void)
 			return FALSE;
 		}
 		info->frame_table_vaddr = frame_table_vaddr;
-	} else
-		info->frame_table_vaddr = FRAMETABLE_VIRT_START;
+	} else {
+		if (info->xen_crash_info.com &&
+		    ((info->xen_crash_info.com->xen_major_version == 4 &&
+		      info->xen_crash_info.com->xen_minor_version >= 3) ||
+		      info->xen_crash_info.com->xen_major_version > 4))
+			info->frame_table_vaddr = FRAMETABLE_VIRT_START_V4_3;
+		else
+			info->frame_table_vaddr = FRAMETABLE_VIRT_START_V3;
+	}
 
 	if (!info->xen_crash_info.com ||
 	    info->xen_crash_info.com->xen_major_version < 4) {
diff --git a/makedumpfile.h b/makedumpfile.h
index 1a87500..1789a11 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1466,16 +1466,17 @@ int get_xen_info_x86(void);
 #define ENTRY_MASK		(~0xfff0000000000fffULL)
 #define MAX_X86_64_FRAMES	(info->page_size / sizeof(unsigned long))
 
-#define PAGE_OFFSET_XEN_DOM0  (0xffff880000000000) /* different from linux */
-#define HYPERVISOR_VIRT_START (0xffff800000000000)
-#define HYPERVISOR_VIRT_END   (0xffff880000000000)
-#define DIRECTMAP_VIRT_START  (0xffff830000000000)
-#define DIRECTMAP_VIRT_END_V3 (0xffff840000000000)
-#define DIRECTMAP_VIRT_END_V4 (0xffff880000000000)
-#define DIRECTMAP_VIRT_END    (info->directmap_virt_end)
-#define XEN_VIRT_START        (info->xen_virt_start)
-#define XEN_VIRT_END          (XEN_VIRT_START + (1UL << 30))
-#define FRAMETABLE_VIRT_START 0xffff82f600000000
+#define PAGE_OFFSET_XEN_DOM0		(0xffff880000000000) /* different from linux */
+#define HYPERVISOR_VIRT_START		(0xffff800000000000)
+#define HYPERVISOR_VIRT_END		(0xffff880000000000)
+#define DIRECTMAP_VIRT_START		(0xffff830000000000)
+#define DIRECTMAP_VIRT_END_V3		(0xffff840000000000)
+#define DIRECTMAP_VIRT_END_V4		(0xffff880000000000)
+#define DIRECTMAP_VIRT_END		(info->directmap_virt_end)
+#define XEN_VIRT_START			(info->xen_virt_start)
+#define XEN_VIRT_END			(XEN_VIRT_START + (1UL << 30))
+#define FRAMETABLE_VIRT_START_V3	0xffff82f600000000
+#define FRAMETABLE_VIRT_START_V4_3	0xffff82e000000000
 
 #define is_xen_vaddr(x) \
 	((x) >= HYPERVISOR_VIRT_START && (x) < HYPERVISOR_VIRT_END)
-- 
1.7.10.4

[PATCH 4/7] xen: Enforce page size only when xen-syms file is used

[Daniel Kiper]

Enforce page size only when xen-syms file is used.
Otherwise its size could be read from VMCOREINFO
file or /proc/vmcore file.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 makedumpfile.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index b42565c..b4abbe5 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -7324,8 +7324,6 @@ initial_xen(void)
 #endif
 	if (!init_xen_crash_info())
 		return FALSE;
-	if (!fallback_to_current_page_size())
-		return FALSE;
 	/*
 	 * Get the debug information for analysis from the vmcoreinfo file
 	 */
@@ -7340,6 +7338,8 @@ initial_xen(void)
 		set_dwarf_debuginfo("xen-syms", NULL,
 				    info->name_xen_syms, info->fd_xen_syms);
 
+		if (!fallback_to_current_page_size())
+			return FALSE;
 		if (!get_symbol_info_xen())
 			return FALSE;
 		if (!get_structure_info_xen())
-- 
1.7.10.4

[PATCH 5/7] Mute some compiler warnings

[Daniel Kiper]

This patch mutes follwing compiler warnings:
  - warning: assignment discards ‘const’ qualifier from
    pointer target type [enabled by default],
  - warning: variable ‘page_offset’ set but not used
    [-Wunused-but-set-variable].

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 dwarf_info.c  |    4 ++--
 sadump_info.c |    2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/dwarf_info.c b/dwarf_info.c
index 6e21b8a..86cae69 100644
--- a/dwarf_info.c
+++ b/dwarf_info.c
@@ -1427,7 +1427,7 @@ get_die_member(unsigned long long die_off, int index, long *offset,
 	if (!get_data_member_location(die, offset))
 		*offset = 0;
 
-	*name = dwarf_diename(die);
+	*name = (char *)dwarf_diename(die);
 	/*
 	 * Duplicate the string before we pass it to eppic layer. The
 	 * original string returned by dwarf layer will become invalid
@@ -1513,7 +1513,7 @@ get_die_name(unsigned long long die_off)
 		return NULL;
 	}
 
-	name = dwarf_diename(&result);
+	name = (char *)dwarf_diename(&result);
 	if (name)
 		name = strdup(name);
 	clean_dwfl_info();
diff --git a/sadump_info.c b/sadump_info.c
index be6cf55..01cf5eb 100644
--- a/sadump_info.c
+++ b/sadump_info.c
@@ -948,7 +948,6 @@ int
 readpage_sadump(unsigned long long paddr, void *bufptr)
 {
 	unsigned long long pfn, block, whole_offset, perdisk_offset;
-	ulong page_offset;
 	int fd_memory;
 
 	if (si->kdump_backed_up &&
@@ -957,7 +956,6 @@ readpage_sadump(unsigned long long paddr, void *bufptr)
 		paddr += si->backup_offset - si->backup_src_start;
 
 	pfn = paddr_to_pfn(paddr);
-	page_offset = paddr % info->page_size;
 
 	if (pfn >= si->sh_memory->max_mapnr)
 		return FALSE;
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH 5/7] Mute some compiler warnings

[Andrew Cooper]

On 12/07/13 14:48, Daniel Kiper wrote:
> This patch mutes follwing compiler warnings:
>   - warning: assignment discards ‘const’ qualifier from
>     pointer target type [enabled by default],
>   - warning: variable ‘page_offset’ set but not used
>     [-Wunused-but-set-variable].
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
>  dwarf_info.c  |    4 ++--
>  sadump_info.c |    2 --
>  2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/dwarf_info.c b/dwarf_info.c
> index 6e21b8a..86cae69 100644
> --- a/dwarf_info.c
> +++ b/dwarf_info.c
> @@ -1427,7 +1427,7 @@ get_die_member(unsigned long long die_off, int index, long *offset,
>  	if (!get_data_member_location(die, offset))
>  		*offset = 0;
>  
> -	*name = dwarf_diename(die);
> +	*name = (char *)dwarf_diename(die);

Discarding a const qualifier like this is usually a good sign that you
are misusing name in the first place.

Indeed, name is merely non-const to strdup() and reuse the same variable.

This would be more clearly done using

const char * diename = dwarf_diename(&result) ;

and

name = strdup(diename);

~Andrew

>  	/*
>  	 * Duplicate the string before we pass it to eppic layer. The
>  	 * original string returned by dwarf layer will become invalid
> @@ -1513,7 +1513,7 @@ get_die_name(unsigned long long die_off)
>  		return NULL;
>  	}
>  
> -	name = dwarf_diename(&result);
> +	name = (char *)dwarf_diename(&result);
>  	if (name)
>  		name = strdup(name);
>  	clean_dwfl_info();
> diff --git a/sadump_info.c b/sadump_info.c
> index be6cf55..01cf5eb 100644
> --- a/sadump_info.c
> +++ b/sadump_info.c
> @@ -948,7 +948,6 @@ int
>  readpage_sadump(unsigned long long paddr, void *bufptr)
>  {
>  	unsigned long long pfn, block, whole_offset, perdisk_offset;
> -	ulong page_offset;
>  	int fd_memory;
>  
>  	if (si->kdump_backed_up &&
> @@ -957,7 +956,6 @@ readpage_sadump(unsigned long long paddr, void *bufptr)
>  		paddr += si->backup_offset - si->backup_src_start;
>  
>  	pfn = paddr_to_pfn(paddr);
> -	page_offset = paddr % info->page_size;
>  
>  	if (pfn >= si->sh_memory->max_mapnr)
>  		return FALSE;


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

[PATCH 6/7] Use elf_getshdrstrndx() instead of elf_getshstrndx()

[Daniel Kiper]

Use elf_getshdrstrndx() instead of elf_getshstrndx()
because later is marked as deprecated.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 dwarf_info.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dwarf_info.c b/dwarf_info.c
index 86cae69..3678cbe 100644
--- a/dwarf_info.c
+++ b/dwarf_info.c
@@ -944,7 +944,7 @@ get_debug_info(void)
 	elfd = dwarf_info.elfd;
 	dwarfd = dwarf_info.dwarfd;
 
-	if (elf_getshstrndx(elfd, &shstrndx) < 0) {
+	if (elf_getshdrstrndx(elfd, &shstrndx) < 0) {
 		ERRMSG("Can't get the section index of the string table.\n");
 		goto out;
 	}
-- 
1.7.10.4

[PATCH 7/7] Do not break progress messages

[Daniel Kiper]

DEBUG_MSG() displaying erase info size breaks progress messages.
Fix this by moving relevant DEBUG_MSG() to write_elf_eraseinfo().

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 makedumpfile.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index b4abbe5..a3bfc80 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -5045,7 +5045,6 @@ write_elf_header(struct cache_data *cd_header)
 	 * function.
 	 */
 	info->size_elf_eraseinfo = size_eraseinfo;
-	DEBUG_MSG("erase info size: %lu\n", info->size_elf_eraseinfo);
 
 	/*
 	 * Write a PT_NOTE header.
@@ -6326,6 +6325,8 @@ write_elf_eraseinfo(struct cache_data *cd_header)
 	off_t offset_eraseinfo;
 	unsigned long note_header_size, size_written, size_note;
 
+	DEBUG_MSG("erase info size: %lu\n", info->size_elf_eraseinfo);
+
 	if (!info->size_elf_eraseinfo)
 		return TRUE;
 
-- 
1.7.10.4

Re: [Xen-devel] [PATCH 0/7] Xen fixes and minor cleanups

[Andrew Cooper]

On 12/07/13 14:48, Daniel Kiper wrote:
> Hi,
>
> This patch series contains following Xen fixes and cleanups:
>   - xen: Improve calculation of beginning of virtual address space,
>   - elf: Increase buf size in get_pt_note_info(),
>   - xen: Take into account new frame table address since Xen 4.3,
>   - xen: Enforce page size only when xen-syms file is used,
>   - Mute some compiler warnings,
>   - Use elf_getshdrstrndx() instead of elf_getshstrndx(),
>   - Do not break progress messages.
>
> All patches were tested with Xen versions up to 4.3.
>
> Daniel

I presume these are all kexec-tools fixes, given the files patched, but
no specific statement one way or another?

~Andrew

>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel-GuqFBffKawuEi8DpZVb4nw@public.gmane.org
> http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 0/7] Xen fixes and minor cleanups

[Daniel Kiper]

On Fri, Jul 12, 2013 at 03:01:22PM +0100, Andrew Cooper wrote:
> On 12/07/13 14:48, Daniel Kiper wrote:
> > Hi,
> >
> > This patch series contains following Xen fixes and cleanups:
> >   - xen: Improve calculation of beginning of virtual address space,
> >   - elf: Increase buf size in get_pt_note_info(),
> >   - xen: Take into account new frame table address since Xen 4.3,
> >   - xen: Enforce page size only when xen-syms file is used,
> >   - Mute some compiler warnings,
> >   - Use elf_getshdrstrndx() instead of elf_getshstrndx(),
> >   - Do not break progress messages.
> >
> > All patches were tested with Xen versions up to 4.3.
> >
> > Daniel
>
> I presume these are all kexec-tools fixes, given the files patched, but
> no specific statement one way or another?

Sorry, I forgot to mention that this is makedumpfile patch series.

Daniel