All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@linaro.org>
To: Ian Campbell <ian.campbell@citrix.com>
Cc: stefano.stabellini@eu.citrix.com, tim@xen.org,
	andre.przywara@linaro.org, xen-devel@lists.xen.org
Subject: Re: [PATCH 6/7] xen/arm: Support dtb /memreserve/ regions
Date: Thu, 12 Sep 2013 14:03:14 +0100	[thread overview]
Message-ID: <5231BB92.3090508@linaro.org> (raw)
In-Reply-To: <1378989775-28294-6-git-send-email-ian.campbell@citrix.com>

On 09/12/2013 01:42 PM, Ian Campbell wrote:
> This requires a mapping of the DTB during setup_mm. Previosly this was in the
> BOOT_MISC slot, which is clobbered by setup_pagetables. Split it out into its
> own slot which can be preserved.
> 
> Also handle this regions are part of consider_modules() and next_modules() to
> ensure we do not locate any part of Xen or the heaps over them.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/arm32/head.S    |    2 +-
>  xen/arch/arm/arm32/traps.c   |    3 +++
>  xen/arch/arm/arm64/head.S    |    2 +-
>  xen/arch/arm/mm.c            |   10 +++++++-
>  xen/arch/arm/setup.c         |   58 ++++++++++++++++++++++++++++++++++++++++--
>  xen/arch/arm/traps.c         |    3 ++-
>  xen/common/device_tree.c     |   13 +++++++++-
>  xen/include/asm-arm/config.h |    7 ++---
>  xen/include/asm-arm/mm.h     |    2 ++
>  9 files changed, 90 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 79e95b6..5072e2a 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -301,7 +301,7 @@ cpu_init_done:
>          orr   r2, r2, #PT_UPPER(MEM)
>          orr   r2, r2, #PT_LOWER(MEM) /* r2:r3 := 2MB RAM incl. DTB */
>          add   r4, r4, #8
> -        strd  r2, r3, [r1, r4]       /* Map it in the early boot slot */
> +        strd  r2, r3, [r1, r4]       /* Map it in the early fdt slot */
>  
>  pt_ready:
>          PRINT("- Turning on paging -\r\n")
> diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
> index ff0b945..e8dd9f5 100644
> --- a/xen/arch/arm/arm32/traps.c
> +++ b/xen/arch/arm/arm32/traps.c
> @@ -22,6 +22,7 @@
>  #include <public/xen.h>
>  
>  #include <asm/processor.h>
> +#include <asm/early_printk.h>
>  
>  asmlinkage void do_trap_undefined_instruction(struct cpu_user_regs *regs)
>  {
> @@ -40,6 +41,8 @@ asmlinkage void do_trap_prefetch_abort(struct cpu_user_regs *regs)
>  
>  asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
>  {
> +    early_printk("Data Abort at %"PRIvaddr" DFAR %"PRIx32"\n",
> +                 regs->pc, READ_CP32(DFAR));

early_printk belongs to __init and will be discarded by
free_init_memory. Any data abort after this call will result to a
"double" hang.

>      do_unexpected_trap("Data Abort", regs);
>  }
>  
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 21b7e4d..33ff489 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -255,7 +255,7 @@ skip_bss:
>          mov   x3, #PT_MEM            /* x2 := 2MB RAM incl. DTB */
>          orr   x2, x2, x3
>          add   x4, x4, #8
> -        str   x2, [x1, x4]           /* Map it in the early boot slot */
> +        str   x2, [x1, x4]           /* Map it in the early fdt slot */
>  
>  pt_ready:
>          PRINT("- Turning on paging -\r\n")
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 69c157a..86e3207 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -361,6 +361,13 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
>      return mfn_to_xen_entry(mfn);
>  }
>  
> +void __init remove_early_mappings(void)
> +{
> +    lpae_t pte = {0};
> +    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
> +    flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, SECOND_SIZE);
> +}
> +
>  /* Boot-time pagetable setup.
>   * Changes here may need matching changes in head.S */
>  void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> @@ -401,7 +408,8 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>          p[second_linear_offset(va)].bits = 0;
>      }
>      for ( i = 0; i < 4 * LPAE_ENTRIES; i++)
> -        if ( p[i].pt.valid )
> +        /* The FDT is not relocated */
> +        if ( p[i].pt.valid && i != second_linear_offset(BOOT_FDT_VIRT_START) )
>              p[i].pt.base += (phys_offset - boot_phys_offset) >> PAGE_SHIFT;
>  
>      /* Change pagetables to the copy in the relocated Xen */
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 95f22a1..d88f121 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -35,6 +35,7 @@
>  #include <xen/cpu.h>
>  #include <xen/pfn.h>
>  #include <xen/vmap.h>
> +#include <xen/libfdt/libfdt.h>
>  #include <asm/page.h>
>  #include <asm/current.h>
>  #include <asm/setup.h>
> @@ -161,6 +162,8 @@ void __init discard_initial_modules(void)
>      }
>  
>      mi->nr_mods = 0;
> +
> +    remove_early_mappings();
>  }
>  
>  /*
> @@ -177,6 +180,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
>  {
>      const struct dt_module_info *mi = &early_info.modules;
>      int i;
> +    int nr_rsvd;
>  
>      s = (s+align-1) & ~(align-1);
>      e = e & ~(align-1);
> @@ -184,6 +188,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
>      if ( s > e ||  e - s < size )
>          return 0;
>  
> +    /* First check the boot modules */
>      for ( i = first_mod; i <= mi->nr_mods; i++ )
>      {
>          paddr_t mod_s = mi->module[i].start;
> @@ -199,6 +204,32 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
>          }
>      }
>  
> +    /* Now check any fdt reserved areas. */
> +
> +    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
> +
> +    for ( ; i < mi->nr_mods + nr_rsvd; i++ )
> +    {
> +        paddr_t mod_s, mod_e;
> +
> +        if ( fdt_get_mem_rsv(device_tree_flattened,
> +                             i - mi->nr_mods,
> +                             &mod_s, &mod_e ) < 0 )
> +            /* If we can't read it, pretend it doesn't exist... */
> +            continue;
> +
> +        /* fdt_get_mem_rsv returns length */
> +        mod_e += mod_s;
> +
> +        if ( s < mod_e && mod_s < e )
> +        {
> +            mod_e = consider_modules(mod_e, e, size, align, i+1);
> +            if ( mod_e )
> +                return mod_e;
> +
> +            return consider_modules(s, mod_s, size, align, i+1);
> +        }
> +    }
>      return e;
>  }
>  
> @@ -212,7 +243,29 @@ static paddr_t __init next_module(paddr_t s, paddr_t *n)
>  {
>      struct dt_module_info *mi = &early_info.modules;
>      paddr_t lowest = ~(paddr_t)0;
> -    int i;
> +    int i, nr_rsvd;
> +
> +    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
> +
> +    for ( i=0 ; i < nr_rsvd; i++ )
> +    {
> +        paddr_t mod_s, mod_e;
> +
> +        if ( fdt_get_mem_rsv(device_tree_flattened, i,
> +                             &mod_s, &mod_e ) < 0 )
> +            /* If we can't read it, pretend it doesn't exist... */
> +            continue;
> +
> +        /* fdt_get_mem_rsv returns length */
> +        mod_e += mod_s;
> +
> +        if ( mod_s < s )
> +            continue;
> +        if ( mod_s > lowest )
> +            continue;
> +        lowest = mod_s;
> +        *n = mod_e;
> +    }

You use this same loop for ... fdt_get ... on different place. Can  you
add a function fdt_for_each_memreserve?
>  
>      for ( i = 0; i <= mi->nr_mods; i++ )
>      {
> @@ -226,6 +279,7 @@ static paddr_t __init next_module(paddr_t s, paddr_t *n)
>          lowest = mod_s;
>          *n = mod_e;
>      }
> +

Spurious line?

>      return lowest;
>  }
>  
> @@ -517,7 +571,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>      smp_clear_cpu_maps();
>  
>      /* This is mapped by head.S */
> -    device_tree_flattened = (void *)BOOT_MISC_VIRT_START
> +    device_tree_flattened = (void *)BOOT_FDT_VIRT_START
>          + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
>      fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
>  
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 0e9a141..a78c1e0 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -35,6 +35,7 @@
>  #include <asm/regs.h>
>  #include <asm/cpregs.h>
>  #include <asm/psci.h>
> +#include <asm/early_printk.h>
>  
>  #include "decode.h"
>  #include "io.h"
> @@ -810,7 +811,7 @@ void vcpu_show_execution_state(struct vcpu *v)
>  
>  void do_unexpected_trap(const char *msg, struct cpu_user_regs *regs)
>  {
> -    printk("CPU%d: Unexpected Trap: %s\n", smp_processor_id(), msg);
> +    early_printk("CPU%d: Unexpected Trap: %s\n", smp_processor_id(), msg);

Same issue as do data_abort_trap

>      show_execution_state(regs);
>      while(1);
>  }
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 9e0c224..132a2bd 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -489,7 +489,7 @@ static void __init early_print_info(void)
>  {
>      struct dt_mem_info *mi = &early_info.mem;
>      struct dt_module_info *mods = &early_info.modules;
> -    int i;
> +    int i, nr_rsvd;
>  
>      for ( i = 0; i < mi->nr_banks; i++ )
>          early_printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n",
> @@ -502,6 +502,17 @@ static void __init early_print_info(void)
>                       mods->module[i].start,
>                       mods->module[i].start + mods->module[i].size,
>                       mods->module[i].cmdline);
> +    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
> +    for ( i = 0; i < nr_rsvd; i++ )
> +    {
> +        paddr_t s, e;
> +        if ( fdt_get_mem_rsv(device_tree_flattened, i, &s, &e) < 0 )
> +            continue;
> +        /* fdt_get_mem_rsv returns length */
> +        e += s;
> +        early_printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n",
> +                     i, s, e);
> +    }
>  }
>  
>  /**
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 624c73e..efeb952 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -80,10 +80,10 @@
>   *   0  -   2M   Unmapped
>   *   2M -   4M   Xen text, data, bss
>   *   4M -   6M   Fixmap: special-purpose 4K mapping slots
> - *   6M -   8M   Early boot misc (see below)
> + *   6M -   8M   Early boot mapping of FDT
> + *   8M -  10M   Early boot misc (see below)
>   *
>   * The early boot misc area is used:
> - *   - in head.S for the DTB for device_tree_early_init().
>   *   - in setup_pagetables() when relocating Xen.
>   *
>   * ARM32 layout:
> @@ -116,7 +116,8 @@
>  
>  #define XEN_VIRT_START         _AT(vaddr_t,0x00200000)
>  #define FIXMAP_ADDR(n)        (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE)
> -#define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00600000)
> +#define BOOT_FDT_VIRT_START    _AT(vaddr_t,0x00600000)
> +#define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00800000)
>  
>  #define HYPERVISOR_VIRT_START  XEN_VIRT_START
>  
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index 97c2ee0..467687a 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -155,6 +155,8 @@ extern unsigned long total_pages;
>  
>  /* Boot-time pagetable setup */
>  extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr);
> +/* Remove early mappings */
> +extern void remove_early_mappings(void);
>  /* Allocate and initialise pagetables for a secondary CPU */
>  extern int __cpuinit init_secondary_pagetables(int cpu);
>  /* Switch secondary CPUS to its own pagetables and finalise MMU setup */
> 


-- 
Julien Grall

  reply	other threads:[~2013-09-12 13:03 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-12 12:42 [PATCH 0/7] xen: arm: memory mangement fixes / improvements Ian Campbell
2013-09-12 12:42 ` [PATCH 1/7] xen/arm: ensure the xenheap is 32MB aligned Ian Campbell
2013-09-12 12:42 ` [PATCH 2/7] xen/arm: DOMHEAP_SECOND_PAGES is arm32 specific Ian Campbell
2013-09-12 12:42 ` [PATCH 3/7] xen/arm: Reserve FDT via early module mechanism Ian Campbell
2013-09-12 13:39   ` Julien Grall
2013-09-12 13:56     ` Ian Campbell
2013-09-12 12:42 ` [PATCH 4/7] xen/arm: cope with modules outside of "visible" RAM Ian Campbell
2013-09-12 12:42 ` [PATCH 5/7] xen: support RAM at addresses 0 and 4096 Ian Campbell
2013-09-12 13:25   ` Jan Beulich
2013-09-12 13:54     ` Ian Campbell
2013-09-13 11:20     ` Ian Campbell
2013-09-13 11:28       ` Keir Fraser
2013-09-12 12:42 ` [PATCH 6/7] xen/arm: Support dtb /memreserve/ regions Ian Campbell
2013-09-12 13:03   ` Julien Grall [this message]
2013-09-12 13:47     ` Ian Campbell
2013-09-12 12:42 ` [PATCH 7/7] xen/arm: rename boot misc region to boot reloc now it has a single purpose Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5231BB92.3090508@linaro.org \
    --to=julien.grall@linaro.org \
    --cc=andre.przywara@linaro.org \
    --cc=ian.campbell@citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.