makedumpfile PATCH] Fix the use of Xen physical and machine addresses
Eric DeVolder
eric.devolder at oracle.com
Tue May 30 12:30:43 PDT 2017
Hi,
Testing is underway. Generally working but I do have a couple of fails
I'm looking into:
> FAIL xen-4.1.2-rc3-pre_domU-pv_xl_linux-2.6.39-x86_64.tar.bz2
> makedumpfile: get_elf_info: Can't get the number of PT_LOAD.
I will report back once this has been root caused.
Regards,
eric
On 05/23/2017 04:34 AM, Daniel Kiper wrote:
> Hi Eric,
>
> May I ask you to do the tests of this patch with our (Xen) dumpfiles?
> After that please drop the summary of the results here. If you have
> any questions drop me a line.
>
> Daniel
>
> ----- Forwarded message from Petr Tesarik <ptesarik at suse.cz> -----
>
> Date: Tue, 23 May 2017 07:46:40 +0200
> From: Petr Tesarik <ptesarik at suse.cz>
> To: Atsushi Kumagai <ats-kumagai at wm.jp.nec.com>
> Cc: kexec at lists.infradead.org, Daniel Kiper <daniel.kiper at oracle.com>
> Subject: [makedumpfile PATCH] Fix the use of Xen physical and machine addresses
> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.31; x86_64-suse-linux-gnu)
>
> There is currently some support for physical-to-machine translation in
> makedumpfile, but it is inconsistent.
>
> Most importantly, vaddr_to_paddr() may return either a physical address
> or a machine address:
>
> 1. If the return value is calculated directly (by subtracting direct
> mapping offset, or by consulting ELF headers), then it is a guest
> physical address (as seen by the Linux kernel).
>
> 2. If the return value is taken from page tables, then it is a machine
> address (as seen by the CPU).
>
> Interestingly, makedumpfile never uses guest physical addresses, except
> in __exclude_unnecessary_pages(), which already performs the required
> ptom conversion explicitly.
>
> So, the best solution is to treat PADDR as "the address seen by the CPU"
> (be it on bare metal or under Xen) and get rid of MADDR_XEN. For addresses
> that are translated directly, vaddr_to_paddr() return value must be
> converted using ptom_xen(), but this call is in fact merely moved from
> readmem().
>
> This patch has been tested on a few bare metal and Xen dumps (x86_64 and
> x86).
>
> Signed-off-by: Petr Tesarik <ptesarik at suse.com>
> ---
> arch/ia64.c | 8 +++++---
> arch/x86.c | 19 +++++++++++++------
> arch/x86_64.c | 19 ++++++++++++-------
> makedumpfile.c | 45 ++++++++-------------------------------------
> makedumpfile.h | 2 +-
> 5 files changed, 39 insertions(+), 54 deletions(-)
>
> diff --git a/arch/ia64.c b/arch/ia64.c
> index e629f94..6c33cc7 100644
> --- a/arch/ia64.c
> +++ b/arch/ia64.c
> @@ -243,6 +243,8 @@ vtop_ia64(unsigned long vaddr)
> if (!is_vmalloc_addr_ia64(vaddr)) {
> paddr = vaddr - info->kernel_start +
> (info->phys_base & KERNEL_TR_PAGE_MASK);
> + if (is_xen_memory())
> + paddr = ptom_xen(paddr);
> return paddr;
> }
>
> @@ -301,7 +303,7 @@ kvtop_xen_ia64(unsigned long kvaddr)
>
> dirp = SYMBOL(frametable_pg_dir) - DIRECTMAP_VIRT_START;
> dirp += ((addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> dirp = entry & _PFN_MASK;
> @@ -309,7 +311,7 @@ kvtop_xen_ia64(unsigned long kvaddr)
> return NOT_PADDR;
>
> dirp += ((addr >> PMD_SHIFT) & (PTRS_PER_PMD - 1)) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> dirp = entry & _PFN_MASK;
> @@ -317,7 +319,7 @@ kvtop_xen_ia64(unsigned long kvaddr)
> return NOT_PADDR;
>
> dirp += ((addr >> PAGESHIFT()) & (PTRS_PER_PTE - 1)) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_P))
> diff --git a/arch/x86.c b/arch/x86.c
> index 1b4d2b6..3fdae93 100644
> --- a/arch/x86.c
> +++ b/arch/x86.c
> @@ -233,8 +233,11 @@ vaddr_to_paddr_x86(unsigned long vaddr)
> {
> unsigned long long paddr;
>
> - if ((paddr = vtop_x86_remap(vaddr)) != NOT_PADDR)
> + if ((paddr = vtop_x86_remap(vaddr)) != NOT_PADDR) {
> + if (is_xen_memory())
> + paddr = ptom_xen(paddr);
> return paddr;
> + }
>
> if ((paddr = vaddr_to_paddr_general(vaddr)) != NOT_PADDR)
> return paddr;
> @@ -247,8 +250,12 @@ vaddr_to_paddr_x86(unsigned long vaddr)
> ERRMSG("Can't get necessary information for vmalloc translation.\n");
> return NOT_PADDR;
> }
> - if (!is_vmalloc_addr_x86(vaddr))
> - return (vaddr - info->kernel_start);
> + if (!is_vmalloc_addr_x86(vaddr)) {
> + paddr = vaddr - info->kernel_start;
> + if (is_xen_memory())
> + paddr = ptom_xen(paddr);
> + return paddr;
> + }
>
> if (vt.mem_flags & MEMORY_X86_PAE) {
> paddr = vtop_x86_PAE(vaddr);
> @@ -280,7 +287,7 @@ kvtop_xen_x86(unsigned long kvaddr)
> if ((dirp = kvtop_xen_x86(SYMBOL(pgd_l3))) == NOT_PADDR)
> return NOT_PADDR;
> dirp += pgd_index_PAE(kvaddr) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_PRESENT))
> @@ -288,7 +295,7 @@ kvtop_xen_x86(unsigned long kvaddr)
>
> dirp = entry & ENTRY_MASK;
> dirp += pmd_index(kvaddr) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_PRESENT))
> @@ -301,7 +308,7 @@ kvtop_xen_x86(unsigned long kvaddr)
>
> dirp = entry & ENTRY_MASK;
> dirp += pte_index(kvaddr) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_PRESENT)) {
> diff --git a/arch/x86_64.c b/arch/x86_64.c
> index e978a36..daa2e86 100644
> --- a/arch/x86_64.c
> +++ b/arch/x86_64.c
> @@ -112,7 +112,7 @@ get_machdep_info_x86_64(void)
> ERRMSG("Can't get p2m_mfn address.\n");
> return FALSE;
> }
> - if (!readmem(MADDR_XEN, pfn_to_paddr(p2m_mfn),
> + if (!readmem(PADDR, pfn_to_paddr(p2m_mfn),
> &frame_mfn, PAGESIZE())) {
> ERRMSG("Can't read p2m_mfn.\n");
> return FALSE;
> @@ -126,7 +126,7 @@ get_machdep_info_x86_64(void)
> if (!frame_mfn[i])
> break;
>
> - if (!readmem(MADDR_XEN, pfn_to_paddr(frame_mfn[i]), &buf,
> + if (!readmem(PADDR, pfn_to_paddr(frame_mfn[i]), &buf,
> PAGESIZE())) {
> ERRMSG("Can't get frame_mfn[%d].\n", i);
> return FALSE;
> @@ -154,7 +154,7 @@ get_machdep_info_x86_64(void)
> if (!frame_mfn[i])
> break;
>
> - if (!readmem(MADDR_XEN, pfn_to_paddr(frame_mfn[i]),
> + if (!readmem(PADDR, pfn_to_paddr(frame_mfn[i]),
> &info->p2m_mfn_frame_list[i * MFNS_PER_FRAME],
> mfns[i] * sizeof(unsigned long))) {
> ERRMSG("Can't get p2m_mfn_frame_list.\n");
> @@ -211,6 +211,11 @@ vtop4_x86_64(unsigned long vaddr)
> * Get PGD.
> */
> page_dir = SYMBOL(init_level4_pgt) - __START_KERNEL_map + info->phys_base;
> + if (is_xen_memory()) {
> + page_dir = ptom_xen(page_dir);
> + if (page_dir == NOT_PADDR)
> + return NOT_PADDR;
> + }
> page_dir += pml4_index(vaddr) * sizeof(unsigned long);
> if (!readmem(PADDR, page_dir, &pml4, sizeof pml4)) {
> ERRMSG("Can't get pml4 (page_dir:%lx).\n", page_dir);
> @@ -303,7 +308,7 @@ kvtop_xen_x86_64(unsigned long kvaddr)
> if ((dirp = kvtop_xen_x86_64(SYMBOL(pgd_l4))) == NOT_PADDR)
> return NOT_PADDR;
> dirp += pml4_index(kvaddr) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_PRESENT))
> @@ -311,7 +316,7 @@ kvtop_xen_x86_64(unsigned long kvaddr)
>
> dirp = entry & ENTRY_MASK;
> dirp += pgd_index(kvaddr) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_PRESENT))
> @@ -323,7 +328,7 @@ kvtop_xen_x86_64(unsigned long kvaddr)
>
> dirp = entry & ENTRY_MASK;
> dirp += pmd_index(kvaddr) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_PRESENT))
> @@ -335,7 +340,7 @@ kvtop_xen_x86_64(unsigned long kvaddr)
>
> dirp = entry & ENTRY_MASK;
> dirp += pte_index(kvaddr) * sizeof(unsigned long long);
> - if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
> + if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
> return NOT_PADDR;
>
> if (!(entry & _PAGE_PRESENT)) {
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 301772a..954a772 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -149,7 +149,7 @@ ptom_xen(unsigned long long paddr)
> }
> maddr = pfn_to_paddr(info->p2m_mfn_frame_list[mfn_idx])
> + sizeof(unsigned long) * frame_idx;
> - if (!readmem(MADDR_XEN, maddr, &mfn, sizeof(mfn))) {
> + if (!readmem(PADDR, maddr, &mfn, sizeof(mfn))) {
> ERRMSG("Can't get mfn.\n");
> return NOT_PADDR;
> }
> @@ -211,7 +211,7 @@ get_dom0_mapnr()
> unsigned i;
>
> maddr = pfn_to_paddr(info->p2m_mfn_frame_list[mfn_idx]);
> - if (!readmem(MADDR_XEN, maddr, &mfns, sizeof(mfns))) {
> + if (!readmem(PADDR, maddr, &mfns, sizeof(mfns))) {
> ERRMSG("Can't read %ld domain-0 mfns at 0x%llu\n",
> (long)MFNS_PER_FRAME, maddr);
> return FALSE;
> @@ -924,7 +924,7 @@ int
> readmem(int type_addr, unsigned long long addr, void *bufptr, size_t size)
> {
> size_t read_size, size_orig = size;
> - unsigned long long paddr, maddr = NOT_PADDR;
> + unsigned long long paddr;
> unsigned long long pgaddr;
> void *pgbuf;
> struct cache_entry *cached;
> @@ -937,25 +937,9 @@ next_page:
> addr);
> goto error;
> }
> - if (is_xen_memory()) {
> - if ((maddr = ptom_xen(paddr)) == NOT_PADDR) {
> - ERRMSG("Can't convert a physical address(%llx) to machine address.\n",
> - paddr);
> - return FALSE;
> - }
> - paddr = maddr;
> - }
> break;
> case PADDR:
> paddr = addr;
> - if (is_xen_memory()) {
> - if ((maddr = ptom_xen(paddr)) == NOT_PADDR) {
> - ERRMSG("Can't convert a physical address(%llx) to machine address.\n",
> - paddr);
> - return FALSE;
> - }
> - paddr = maddr;
> - }
> break;
> case VADDR_XEN:
> if ((paddr = kvtop_xen(addr)) == NOT_PADDR) {
> @@ -964,9 +948,6 @@ next_page:
> goto error;
> }
> break;
> - case MADDR_XEN:
> - paddr = addr;
> - break;
> default:
> ERRMSG("Invalid address type (%d).\n", type_addr);
> goto error;
> @@ -5493,18 +5474,10 @@ exclude_zero_pages_cyclic(struct cycle *cycle)
> if (!is_dumpable(info->bitmap2, pfn, cycle))
> continue;
>
> - if (is_xen_memory()) {
> - if (!readmem(MADDR_XEN, paddr, buf, info->page_size)) {
> - ERRMSG("Can't get the page data(pfn:%llx, max_mapnr:%llx).\n",
> - pfn, info->max_mapnr);
> - return FALSE;
> - }
> - } else {
> - if (!readmem(PADDR, paddr, buf, info->page_size)) {
> - ERRMSG("Can't get the page data(pfn:%llx, max_mapnr:%llx).\n",
> - pfn, info->max_mapnr);
> - return FALSE;
> - }
> + if (!readmem(PADDR, paddr, buf, info->page_size)) {
> + ERRMSG("Can't get the page data(pfn:%llx, max_mapnr:%llx).\n",
> + pfn, info->max_mapnr);
> + return FALSE;
> }
> if (is_zero_page(buf, info->page_size)) {
> if (clear_bit_on_2nd_bitmap(pfn, cycle))
> @@ -7145,11 +7118,9 @@ int
> read_pfn(mdf_pfn_t pfn, unsigned char *buf)
> {
> unsigned long long paddr;
> - int type_addr;
>
> paddr = pfn_to_paddr(pfn);
> - type_addr = is_xen_memory() ? MADDR_XEN : PADDR;
> - if (!readmem(type_addr, paddr, buf, info->page_size)) {
> + if (!readmem(PADDR, paddr, buf, info->page_size)) {
> ERRMSG("Can't get the page data.\n");
> return FALSE;
> }
> diff --git a/makedumpfile.h b/makedumpfile.h
> index e32e567..1158487 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -130,7 +130,6 @@ enum {
> VADDR,
> PADDR,
> VADDR_XEN,
> - MADDR_XEN
> };
>
> /*
> @@ -2151,6 +2150,7 @@ mdf_pfn_t get_num_dumpable_cyclic(void);
> mdf_pfn_t get_num_dumpable_cyclic_withsplit(void);
> int get_loads_dumpfile_cyclic(void);
> int initial_xen(void);
> +unsigned long long ptom_xen(unsigned long long paddr);
> unsigned long long get_free_memory_size(void);
> int calculate_cyclic_buffer_size(void);
> int prepare_splitblock_table(void);
>
More information about the kexec
mailing list