All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] xen: arm: support for > 40-bit physical addressing
@ 2014-09-04 16:13 Ian Campbell
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
  0 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:13 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Tim Deegan, vijay.kilari, Stefano Stabellini

ARM64 systems can support up to 48-bits of physical address, but
currently we configure both Stage-1 and Stage-2 for only 40-bits.
However there are systems with peripherals mapped higher up, in order to
support those we need to support 48-bits as output for stage-1 as well
as input and output for stage-2. Stage-2 support is needed in order to
map them 1:1 to dom0.

Unfortunately as the last commit message explains in a bit more detail
supporting larger input sizes on the p2m is a pain because it means we
can no longer statically decide to use a 3-level p2m with 2 concatenated
pages at the root and need to be able to support either that *or* a
4-level p2m with no root concatenation. There is simply no static choice
which supports both 40- and 48-bits of IPA.

So this series goes through the various functions which manipulate or
walk page tables and makes them able to support a dynamic starting level
and concatenation before switching arm64 to make the choice dependent on
the h/w capabilities.

This is somewhat loosely based on Vijay's original "Add 4-level page
table for stage 2 translation" series but due to the need to switch to
dynamic p2m levels not much of that remains.

I haven't actually been able to track down a system with >42-bit PASize.
Vijay, I'm hoping you can test this new version of the series on such a
system/model (thanks for your feed back on the previous iteration).

Compared with last time I've rebased onto Arianna's changes and taken
care of Julien's review comments.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root.
  2014-09-04 16:13 [PATCH v2 0/9] xen: arm: support for > 40-bit physical addressing Ian Campbell
@ 2014-09-04 16:14 ` Ian Campbell
  2014-09-04 16:14   ` [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
                     ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, Ian Campbell, vijay.kilari, stefano.stabellini,
	Vijaya Kumar K, Ian Campbell, tim

From: Ian Campbell <ian.campbell@citirx.com>

This was previously part of Vigaya's "xen/arm: Add 4-level page table
for stage 2 translation" but is split out here to make that patch
easier to read.

I went with ->root rather than ->root_level as the original did.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
---
v2: Clarified commit log
---
 xen/arch/arm/p2m.c                 |   24 ++++++++++++------------
 xen/drivers/passthrough/arm/smmu.c |    2 +-
 xen/include/asm-arm/p2m.h          |    2 +-
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index fc8c0dd..ec3b451 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -12,8 +12,8 @@
 #include <asm/page.h>
 
 /* First level P2M is 2 consecutive pages */
-#define P2M_FIRST_ORDER 1
-#define P2M_FIRST_ENTRIES (LPAE_ENTRIES<<P2M_FIRST_ORDER)
+#define P2M_ROOT_ORDER 1
+#define P2M_ROOT_ENTRIES (LPAE_ENTRIES<<P2M_ROOT_ORDER)
 
 static bool_t p2m_valid(lpae_t pte)
 {
@@ -65,9 +65,9 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
     }
 
     printk("P2M @ %p mfn:0x%lx\n",
-           p2m->first_level, page_to_mfn(p2m->first_level));
+           p2m->root, page_to_mfn(p2m->root));
 
-    first = __map_domain_page(p2m->first_level);
+    first = __map_domain_page(p2m->root);
     dump_pt_walk(first, addr);
     unmap_domain_page(first);
 }
@@ -141,10 +141,10 @@ static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
 {
     struct page_info *page;
 
-    if ( first_linear_offset(addr) >= P2M_FIRST_ENTRIES )
+    if ( first_linear_offset(addr) >= P2M_ROOT_ENTRIES )
         return NULL;
 
-    page = p2m->first_level + p2m_first_level_index(addr);
+    page = p2m->root + p2m_first_level_index(addr);
 
     return __map_domain_page(page);
 }
@@ -932,7 +932,7 @@ int p2m_alloc_table(struct domain *d)
     struct p2m_domain *p2m = &d->arch.p2m;
     struct page_info *page;
 
-    page = alloc_domheap_pages(NULL, P2M_FIRST_ORDER, 0);
+    page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0);
     if ( page == NULL )
         return -ENOMEM;
 
@@ -942,9 +942,9 @@ int p2m_alloc_table(struct domain *d)
     clear_and_clean_page(page);
     clear_and_clean_page(page + 1);
 
-    p2m->first_level = page;
+    p2m->root = page;
 
-    d->arch.vttbr = page_to_maddr(p2m->first_level)
+    d->arch.vttbr = page_to_maddr(p2m->root)
         | ((uint64_t)p2m->vmid&0xff)<<48;
 
     /* Make sure that all TLBs corresponding to the new VMID are flushed
@@ -1021,9 +1021,9 @@ void p2m_teardown(struct domain *d)
     while ( (pg = page_list_remove_head(&p2m->pages)) )
         free_domheap_page(pg);
 
-    free_domheap_pages(p2m->first_level, P2M_FIRST_ORDER);
+    free_domheap_pages(p2m->root, P2M_ROOT_ORDER);
 
-    p2m->first_level = NULL;
+    p2m->root = NULL;
 
     p2m_free_vmid(d);
 
@@ -1047,7 +1047,7 @@ int p2m_init(struct domain *d)
 
     d->arch.vttbr = 0;
 
-    p2m->first_level = NULL;
+    p2m->root = NULL;
 
     p2m->max_mapped_gfn = 0;
     p2m->lowest_mapped_gfn = ULONG_MAX;
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index f4eb2a2..42bde75 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -937,7 +937,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain_cfg *cfg)
     paddr_t p2maddr;
 
     ASSERT(cfg->domain != NULL);
-    p2maddr = page_to_maddr(cfg->domain->arch.p2m.first_level);
+    p2maddr = page_to_maddr(cfg->domain->arch.p2m.root);
 
     gr1_base = SMMU_GR1(smmu);
     cb_base = SMMU_CB_BASE(smmu) + SMMU_CB(smmu, cfg->cbndx);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 08ce07b..3648568 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -20,7 +20,7 @@ struct p2m_domain {
     struct page_list_head pages;
 
     /* Root of p2m page tables, 2 contiguous pages */
-    struct page_info *first_level;
+    struct page_info *root;
 
     /* Current VMID in use */
     uint8_t vmid;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-08 21:24     ` Julien Grall
  2014-09-08 21:33     ` Julien Grall
  2014-09-04 16:14   ` [PATCH v2 3/9] xen: arm: handle concatenated root tables " Ian Campbell
                     ` (7 subsequent siblings)
  8 siblings, 2 replies; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini

This allows us to correctly dump 64-bit hypervisor addresses, which use a 4
level table.

It also paves the way for boot-time selection of the number of levels to use in
the p2m, which is required to support both 40-bit and 48-bit systems.

To support multiple levels it is convenient to recast the page table walk as a
loop over the levels instead of the current open coding.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - map_domain_page cannot fail, so don't check
 - don't map an extra page for a valid L3 entry
 - avoid unmapping levels which we didn't map.
 - root_level argument is unsigned int.
 - fold in spurious whitespace change from next patch
---
 xen/arch/arm/mm.c          |   60 ++++++++++++++++++++++++++++----------------
 xen/arch/arm/p2m.c         |    4 ++-
 xen/include/asm-arm/page.h |    2 +-
 3 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 0a243b0..ab91275 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -84,10 +84,12 @@ lpae_t boot_third[LPAE_ENTRIES]  __attribute__((__aligned__(4096)));
  */
 
 #ifdef CONFIG_ARM_64
+#define HYP_PT_ROOT_LEVEL 0
 lpae_t xen_pgtable[LPAE_ENTRIES] __attribute__((__aligned__(4096)));
 lpae_t xen_first[LPAE_ENTRIES] __attribute__((__aligned__(4096)));
 #define THIS_CPU_PGTABLE xen_pgtable
 #else
+#define HYP_PT_ROOT_LEVEL 1
 /* Per-CPU pagetable pages */
 /* xen_pgtable == root of the trie (zeroeth level on 64-bit, first on 32-bit) */
 static DEFINE_PER_CPU(lpae_t *, xen_pgtable);
@@ -165,34 +167,48 @@ static inline void check_memory_layout_alignment_constraints(void) {
 #endif
 }
 
-void dump_pt_walk(lpae_t *first, paddr_t addr)
+void dump_pt_walk(lpae_t *root, paddr_t addr,
+                  unsigned int root_level)
 {
-    lpae_t *second = NULL, *third = NULL;
+    static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" };
+    const unsigned int offsets[4] = {
+        zeroeth_table_offset(addr),
+        first_table_offset(addr),
+        second_table_offset(addr),
+        third_table_offset(addr)
+    };
+    lpae_t pte, *mappings[4] = { 0, };
+    unsigned int level;
+
+    BUG_ON(!root);
+#ifdef CONFIG_ARM_32
+    BUG_ON(root_level < 1);
+#endif
 
-    if ( first_table_offset(addr) >= LPAE_ENTRIES )
-        return;
+    mappings[root_level] = root;
+
+    for ( level = root_level; ; level++ )
+    {
+        if ( offsets[level] > LPAE_ENTRIES )
+            break;
 
-    printk("1ST[0x%x] = 0x%"PRIpaddr"\n", first_table_offset(addr),
-           first[first_table_offset(addr)].bits);
-    if ( !first[first_table_offset(addr)].walk.valid ||
-         !first[first_table_offset(addr)].walk.table )
-        goto done;
+        pte = mappings[level][offsets[level]];
 
-    second = map_domain_page(first[first_table_offset(addr)].walk.base);
-    printk("2ND[0x%x] = 0x%"PRIpaddr"\n", second_table_offset(addr),
-           second[second_table_offset(addr)].bits);
-    if ( !second[second_table_offset(addr)].walk.valid ||
-         !second[second_table_offset(addr)].walk.table )
-        goto done;
+        printk("%s[0x%x] = 0x%"PRIpaddr"\n",
+               level_strs[level], offsets[level], pte.bits);
 
-    third = map_domain_page(second[second_table_offset(addr)].walk.base);
-    printk("3RD[0x%x] = 0x%"PRIpaddr"\n", third_table_offset(addr),
-           third[third_table_offset(addr)].bits);
+        if ( level == 3 || !pte.walk.valid || !pte.walk.table )
+            break;
 
-done:
-    if (third) unmap_domain_page(third);
-    if (second) unmap_domain_page(second);
+        mappings[level+1] = map_domain_page(pte.walk.base);
+    }
 
+    /* mappings[root_level] is provided by the caller so don't unmap that */
+    do
+    {
+        unmap_domain_page(mappings[level]);
+    }
+    while( level-- > root_level );
 }
 
 void dump_hyp_walk(vaddr_t addr)
@@ -208,7 +224,7 @@ void dump_hyp_walk(vaddr_t addr)
         BUG_ON( (lpae_t *)(unsigned long)(ttbr - phys_offset) != pgtable );
     else
         BUG_ON( virt_to_maddr(pgtable) != ttbr );
-    dump_pt_walk(pgtable, addr);
+    dump_pt_walk(pgtable, addr, HYP_PT_ROOT_LEVEL);
 }
 
 /* Map a 4k page in a fixmap entry */
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ec3b451..6a19e37 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -11,6 +11,8 @@
 #include <asm/hardirq.h>
 #include <asm/page.h>
 
+#define P2M_ROOT_LEVEL       1
+
 /* First level P2M is 2 consecutive pages */
 #define P2M_ROOT_ORDER 1
 #define P2M_ROOT_ENTRIES (LPAE_ENTRIES<<P2M_ROOT_ORDER)
@@ -68,7 +70,7 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
            p2m->root, page_to_mfn(p2m->root));
 
     first = __map_domain_page(p2m->root);
-    dump_pt_walk(first, addr);
+    dump_pt_walk(first, addr, P2M_ROOT_LEVEL);
     unmap_domain_page(first);
 }
 
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 739038a..4c21863 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -352,7 +352,7 @@ static inline void flush_xen_data_tlb_range_va(unsigned long va,
 void flush_page_to_ram(unsigned long mfn);
 
 /* Print a walk of an arbitrary page table */
-void dump_pt_walk(lpae_t *table, paddr_t addr);
+void dump_pt_walk(lpae_t *table, paddr_t addr, unsigned int root_level);
 
 /* Print a walk of the hypervisor's page tables for a virtual addr. */
 extern void dump_hyp_walk(vaddr_t addr);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 3/9] xen: arm: handle concatenated root tables in dump_pt_walk
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
  2014-09-04 16:14   ` [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-08 21:43     ` Julien Grall
  2014-09-04 16:14   ` [PATCH v2 4/9] xen: arm: move setup_virt_paging to p2m.[ch] from mm.[ch] Ian Campbell
                     ` (6 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini

ARM allows for the concatenation of pages at the root of a p2m (but not a
regular page table) in order to support a larger IPA space than the number of
levels in the P2M would normally support. We use this to support 40-bit guest
addresses.

Previously we were unable to dump IPAs which were outside the first page of the
root. To fix this we adjust dump_pt_walk to take the machine address of the
page table root instead of expecting the caller to have mapper it. This allows
the walker code to select the correct page to map.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - nr_root_tables is unsigned int
 - spell concatenate properly
---
 xen/arch/arm/mm.c          |   47 ++++++++++++++++++++++++++++++--------------
 xen/arch/arm/p2m.c         |   13 +++---------
 xen/include/asm-arm/page.h |   16 +++++++++++++--
 3 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index ab91275..da4522c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -167,32 +167,52 @@ static inline void check_memory_layout_alignment_constraints(void) {
 #endif
 }
 
-void dump_pt_walk(lpae_t *root, paddr_t addr,
-                  unsigned int root_level)
+void dump_pt_walk(paddr_t ttbr, paddr_t addr,
+                  unsigned int root_level,
+                  unsigned int nr_root_tables)
 {
     static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" };
+    const unsigned long root_pfn = paddr_to_pfn(ttbr);
     const unsigned int offsets[4] = {
         zeroeth_table_offset(addr),
         first_table_offset(addr),
         second_table_offset(addr),
         third_table_offset(addr)
     };
-    lpae_t pte, *mappings[4] = { 0, };
-    unsigned int level;
+    lpae_t pte, *mapping;
+    unsigned int level, root_table;
 
-    BUG_ON(!root);
 #ifdef CONFIG_ARM_32
     BUG_ON(root_level < 1);
 #endif
 
-    mappings[root_level] = root;
+    if ( nr_root_tables > 1 )
+    {
+        /*
+         * Concatenated root-level tables. The table number will be
+         * the offset at the previous level. It is not possible to
+         * concatenate a level-0 root.
+         */
+        BUG_ON(root_level == 0);
+        root_table = offsets[root_level - 1];
+        printk("Using concatenated root table %d\n", root_table);
+        if ( root_table >= nr_root_tables )
+        {
+            printk("Invalid root table offset\n");
+            return;
+        }
+    }
+    else
+        root_table = 0;
+
+    mapping = map_domain_page(root_pfn + root_table);
 
     for ( level = root_level; ; level++ )
     {
         if ( offsets[level] > LPAE_ENTRIES )
             break;
 
-        pte = mappings[level][offsets[level]];
+        pte = mapping[offsets[level]];
 
         printk("%s[0x%x] = 0x%"PRIpaddr"\n",
                level_strs[level], offsets[level], pte.bits);
@@ -200,15 +220,12 @@ void dump_pt_walk(lpae_t *root, paddr_t addr,
         if ( level == 3 || !pte.walk.valid || !pte.walk.table )
             break;
 
-        mappings[level+1] = map_domain_page(pte.walk.base);
+        /* For next iteration */
+        unmap_domain_page(mapping);
+        mapping = map_domain_page(pte.walk.base);
     }
 
-    /* mappings[root_level] is provided by the caller so don't unmap that */
-    do
-    {
-        unmap_domain_page(mappings[level]);
-    }
-    while( level-- > root_level );
+    unmap_domain_page(mapping);
 }
 
 void dump_hyp_walk(vaddr_t addr)
@@ -224,7 +241,7 @@ void dump_hyp_walk(vaddr_t addr)
         BUG_ON( (lpae_t *)(unsigned long)(ttbr - phys_offset) != pgtable );
     else
         BUG_ON( virt_to_maddr(pgtable) != ttbr );
-    dump_pt_walk(pgtable, addr, HYP_PT_ROOT_LEVEL);
+    dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
 }
 
 /* Map a 4k page in a fixmap entry */
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6a19e37..62547f2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -16,6 +16,7 @@
 /* First level P2M is 2 consecutive pages */
 #define P2M_ROOT_ORDER 1
 #define P2M_ROOT_ENTRIES (LPAE_ENTRIES<<P2M_ROOT_ORDER)
+#define P2M_ROOT_PAGES    (1<<P2M_ROOT_ORDER)
 
 static bool_t p2m_valid(lpae_t pte)
 {
@@ -56,22 +57,14 @@ void memory_type_changed(struct domain *d)
 void dump_p2m_lookup(struct domain *d, paddr_t addr)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
-    lpae_t *first;
 
     printk("dom%d IPA 0x%"PRIpaddr"\n", d->domain_id, addr);
 
-    if ( first_linear_offset(addr) > LPAE_ENTRIES )
-    {
-        printk("Cannot dump addresses in second of first level pages...\n");
-        return;
-    }
-
     printk("P2M @ %p mfn:0x%lx\n",
            p2m->root, page_to_mfn(p2m->root));
 
-    first = __map_domain_page(p2m->root);
-    dump_pt_walk(first, addr, P2M_ROOT_LEVEL);
-    unmap_domain_page(first);
+    dump_pt_walk(page_to_maddr(p2m->root), addr,
+                 P2M_ROOT_LEVEL, P2M_ROOT_PAGES);
 }
 
 static void p2m_load_VTTBR(struct domain *d)
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 4c21863..773822f 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -351,8 +351,20 @@ static inline void flush_xen_data_tlb_range_va(unsigned long va,
 /* Flush the dcache for an entire page. */
 void flush_page_to_ram(unsigned long mfn);
 
-/* Print a walk of an arbitrary page table */
-void dump_pt_walk(lpae_t *table, paddr_t addr, unsigned int root_level);
+/*
+ * Print a walk of a page table or p2m
+ *
+ * ttbr is the base address register (TTBR0_EL2 or VTTBR_EL2)
+ * addr is the PA or IPA to translate
+ * root_level is the starting level of the page table
+ *   (e.g. TCR_EL2.SL0 or VTCR_EL2.SL0 )
+ * nr_root_tables is the number of concatenated tables at the root.
+ *   this can only be != 1 for P2M walks starting at the first or
+ *   subsequent level.
+ */
+void dump_pt_walk(paddr_t ttbr, paddr_t addr,
+                  unsigned int root_level,
+                  unsigned int nr_root_tables);
 
 /* Print a walk of the hypervisor's page tables for a virtual addr. */
 extern void dump_hyp_walk(vaddr_t addr);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 4/9] xen: arm: move setup_virt_paging to p2m.[ch] from mm.[ch]
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
  2014-09-04 16:14   ` [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
  2014-09-04 16:14   ` [PATCH v2 3/9] xen: arm: handle concatenated root tables " Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-04 16:14   ` [PATCH v2 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Ian Campbell
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini

This file is where most of the P2M logic lives and this function will
eventually need to poke at some internals, so move it.

This is pure code motion.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
v2: also move from mm.h to p2m.h
---
 xen/arch/arm/mm.c         |   18 ------------------
 xen/arch/arm/p2m.c        |   18 ++++++++++++++++++
 xen/include/asm-arm/mm.h  |    2 --
 xen/include/asm-arm/p2m.h |    3 +++
 4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index da4522c..6d9eb67 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -410,24 +410,6 @@ void __init arch_init_memory(void)
     BUG_ON(IS_ERR(dom_cow));
 }
 
-void __cpuinit setup_virt_paging(void)
-{
-    /* Setup Stage 2 address translation */
-    /* SH0=11 (Inner-shareable)
-     * ORGN0=IRGN0=01 (Normal memory, Write-Back Write-Allocate Cacheable)
-     * SL0=01 (Level-1)
-     * ARVv7: T0SZ=(1)1000 = -8 (32-(-8) = 40 bit physical addresses)
-     * ARMv8: T0SZ=01 1000 = 24 (64-24   = 40 bit physical addresses)
-     *        PS=010 == 40 bits
-     */
-#ifdef CONFIG_ARM_32
-    WRITE_SYSREG32(0x80003558, VTCR_EL2);
-#else
-    WRITE_SYSREG32(0x80023558, VTCR_EL2);
-#endif
-    isb();
-}
-
 static inline lpae_t pte_of_xenaddr(vaddr_t va)
 {
     paddr_t ma = va + phys_offset;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 62547f2..6cd13c2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1112,6 +1112,24 @@ err:
     return page;
 }
 
+void __cpuinit setup_virt_paging(void)
+{
+    /* Setup Stage 2 address translation */
+    /* SH0=11 (Inner-shareable)
+     * ORGN0=IRGN0=01 (Normal memory, Write-Back Write-Allocate Cacheable)
+     * SL0=01 (Level-1)
+     * ARVv7: T0SZ=(1)1000 = -8 (32-(-8) = 40 bit physical addresses)
+     * ARMv8: T0SZ=01 1000 = 24 (64-24   = 40 bit physical addresses)
+     *        PS=010 == 40 bits
+     */
+#ifdef CONFIG_ARM_32
+    WRITE_SYSREG32(0x80003558, VTCR_EL2);
+#else
+    WRITE_SYSREG32(0x80023558, VTCR_EL2);
+#endif
+    isb();
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 9fa80a4..840a805 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -155,8 +155,6 @@ extern void remove_early_mappings(void);
 extern int __cpuinit init_secondary_pagetables(int cpu);
 /* Switch secondary CPUS to its own pagetables and finalise MMU setup */
 extern void __cpuinit mmu_init_secondary_cpu(void);
-/* Second stage paging setup, to be called on all CPUs */
-extern void __cpuinit setup_virt_paging(void);
 /* Set up the xenheap: up to 1GB of contiguous, always-mapped memory.
  * Base must be 32MB aligned and size a multiple of 32MB. */
 extern void setup_xenheap_mappings(unsigned long base_mfn, unsigned long nr_mfns);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 3648568..53aa0fc 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -72,6 +72,9 @@ typedef enum {
 /* Initialise vmid allocator */
 void p2m_vmid_allocator_init(void);
 
+/* Second stage paging setup, to be called on all CPUs */
+void __cpuinit setup_virt_paging(void);
+
 /* Init the datastructures for later use by the p2m code */
 int p2m_init(struct domain *d);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
                     ` (2 preceding siblings ...)
  2014-09-04 16:14   ` [PATCH v2 4/9] xen: arm: move setup_virt_paging to p2m.[ch] from mm.[ch] Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-08 21:46     ` Julien Grall
  2014-09-04 16:14   ` [PATCH v2 6/9] xen: arm: handle variable p2m levels in p2m_lookup Ian Campbell
                     ` (4 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini

Currently we retain the hardcoded values but soon we will want to calculate the
correct values based upon the CPU properties common to all processors, which
are only available once they are all up.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - rewrap commit message.
 - setup_virt_paging(_one) are __init now.
 - setup_virt_paging called before do_initcalls().
---
 xen/arch/arm/p2m.c     |   18 ++++++++++++++----
 xen/arch/arm/setup.c   |    4 ++--
 xen/arch/arm/smpboot.c |    2 --
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6cd13c2..6e64f94 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1112,8 +1112,17 @@ err:
     return page;
 }
 
-void __cpuinit setup_virt_paging(void)
+static void __init setup_virt_paging_one(void *data)
 {
+    unsigned long val = (unsigned long)data;
+    WRITE_SYSREG32(val, VTCR_EL2);
+    isb();
+}
+
+void __init setup_virt_paging(void)
+{
+    unsigned long val;
+
     /* Setup Stage 2 address translation */
     /* SH0=11 (Inner-shareable)
      * ORGN0=IRGN0=01 (Normal memory, Write-Back Write-Allocate Cacheable)
@@ -1123,11 +1132,12 @@ void __cpuinit setup_virt_paging(void)
      *        PS=010 == 40 bits
      */
 #ifdef CONFIG_ARM_32
-    WRITE_SYSREG32(0x80003558, VTCR_EL2);
+    val = 0x80003558;
 #else
-    WRITE_SYSREG32(0x80023558, VTCR_EL2);
+    val = 0x80023558;
 #endif
-    isb();
+    setup_virt_paging_one((void *)val);
+    smp_call_function(setup_virt_paging_one, (void *)val, 1);
 }
 
 /*
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 446b4dc..35ee7f0 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -788,8 +788,6 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     gic_init();
 
-    setup_virt_paging();
-
     p2m_vmid_allocator_init();
 
     softirq_init();
@@ -836,6 +834,8 @@ void __init start_xen(unsigned long boot_phys_offset,
     printk("Brought up %ld CPUs\n", (long)num_online_cpus());
     /* TODO: smp_cpus_done(); */
 
+    setup_virt_paging();
+
     do_initcalls();
 
     /* Create initial domain 0. */
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index cf149da..ee395a1 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -279,8 +279,6 @@ void __cpuinit start_secondary(unsigned long boot_phys_offset,
 
     init_traps();
 
-    setup_virt_paging();
-
     mmu_init_secondary_cpu();
 
     gic_init_secondary_cpu();
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 6/9] xen: arm: handle variable p2m levels in p2m_lookup
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
                     ` (3 preceding siblings ...)
  2014-09-04 16:14   ` [PATCH v2 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-08 22:05     ` Julien Grall
  2014-09-04 16:14   ` [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
                     ` (3 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini

This paves the way for boot-time selection of the number of levels to
use in the p2m, which is required to support both 40-bit and 48-bit
systems. For now the starting level remains a compile time constant.

Implemented by turning the linear sequence of lookups into a loop.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - pt level vars are unsigned int
 - spell concatenate properly
 - ASSERT() rather than BUG_ON()
---
 xen/arch/arm/p2m.c |   73 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6e64f94..ea5e342 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -153,56 +153,77 @@ static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
 paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
-    lpae_t pte, *first = NULL, *second = NULL, *third = NULL;
+    const unsigned int offsets[4] = {
+        zeroeth_table_offset(paddr),
+        first_table_offset(paddr),
+        second_table_offset(paddr),
+        third_table_offset(paddr)
+    };
+    const paddr_t masks[4] = {
+        ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
+    };
+    lpae_t pte, *map;
     paddr_t maddr = INVALID_PADDR;
-    paddr_t mask;
+    paddr_t mask = 0;
     p2m_type_t _t;
+    unsigned int level, root_table;
+
+    BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
 
     /* Allow t to be NULL */
     t = t ?: &_t;
 
     *t = p2m_invalid;
 
+    if ( P2M_ROOT_PAGES > 1 )
+    {
+        /*
+         * Concatenated root-level tables. The table number will be
+         * the offset at the previous level. It is not possible to
+         * concatenate a level-0 root.
+         */
+        ASSERT(P2M_ROOT_LEVEL > 0);
+        root_table = offsets[P2M_ROOT_LEVEL - 1];
+        if ( root_table >= P2M_ROOT_PAGES )
+            goto err;
+    }
+    else
+        root_table = 0;
+
     spin_lock(&p2m->lock);
 
-    first = p2m_map_first(p2m, paddr);
-    if ( !first )
-        goto err;
+    map = __map_domain_page(p2m->root + root_table);
 
-    mask = FIRST_MASK;
-    pte = first[first_table_offset(paddr)];
-    if ( !p2m_table(pte) )
-        goto done;
+    for ( level = P2M_ROOT_LEVEL ; level < 4 ; level++ )
+    {
+        mask = masks[level];
 
-    mask = SECOND_MASK;
-    second = map_domain_page(pte.p2m.base);
-    pte = second[second_table_offset(paddr)];
-    if ( !p2m_table(pte) )
-        goto done;
+        pte = map[offsets[level]];
 
-    mask = THIRD_MASK;
+        if ( level == 3 && !p2m_table(pte) )
+            /* Invalid, clobber the pte */
+            pte.bits = 0;
+        if ( level == 3 || !p2m_table(pte) )
+            /* Done */
+            break;
 
-    BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
+        ASSERT(level < 3);
 
-    third = map_domain_page(pte.p2m.base);
-    pte = third[third_table_offset(paddr)];
+        /* Map for next level */
+        unmap_domain_page(map);
+        map = map_domain_page(pte.p2m.base);
+    }
 
-    /* This bit must be one in the level 3 entry */
-    if ( !p2m_table(pte) )
-        pte.bits = 0;
+    unmap_domain_page(map);
 
-done:
     if ( p2m_valid(pte) )
     {
+        ASSERT(mask);
         ASSERT(pte.p2m.type != p2m_invalid);
         maddr = (pte.bits & PADDR_MASK & mask) | (paddr & ~mask);
         *t = pte.p2m.type;
     }
 
-    if (third) unmap_domain_page(third);
-    if (second) unmap_domain_page(second);
-    if (first) unmap_domain_page(first);
-
 err:
     spin_unlock(&p2m->lock);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
                     ` (4 preceding siblings ...)
  2014-09-04 16:14   ` [PATCH v2 6/9] xen: arm: handle variable p2m levels in p2m_lookup Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-05 23:32     ` Arianna Avanzini
  2014-09-08 22:22     ` Julien Grall
  2014-09-04 16:14   ` [PATCH v2 8/9] xen: arm: support for up to 48-bit physical addressing on arm64 Ian Campbell
                     ` (2 subsequent siblings)
  8 siblings, 2 replies; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: Ian Campbell, vijay.kilari, stefano.stabellini, julien.grall,
	tim, Arianna Avanzini

As with previous changes this involves conversion from a linear series of
lookups into a loop over the levels.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Arianna Avanzini <avanzini.arianna@gmail.com>
---
v2:
 - Rebase, in particular over 4b25423aee13 "arch/arm: unmap partially-mapped
   memory regions" which had some interesting interactions (which I hope I've
   gotten right!)
 - Spell previous and concatenate correctly.
 - ASSERT rather than BUG_ON for concatenated level zero root.
---
 xen/arch/arm/p2m.c |  172 ++++++++++++++++++++++++----------------------------
 1 file changed, 78 insertions(+), 94 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ea5e342..8e330c7 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -15,7 +15,6 @@
 
 /* First level P2M is 2 consecutive pages */
 #define P2M_ROOT_ORDER 1
-#define P2M_ROOT_ENTRIES (LPAE_ENTRIES<<P2M_ROOT_ORDER)
 #define P2M_ROOT_PAGES    (1<<P2M_ROOT_ORDER)
 
 static bool_t p2m_valid(lpae_t pte)
@@ -119,31 +118,6 @@ void flush_tlb_domain(struct domain *d)
         p2m_load_VTTBR(current->domain);
 }
 
-static int p2m_first_level_index(paddr_t addr)
-{
-    /*
-     * 1st pages are concatenated so zeroeth offset gives us the
-     * index of the 1st page
-     */
-    return zeroeth_table_offset(addr);
-}
-
-/*
- * Map whichever of the first pages contain addr. The caller should
- * then use first_table_offset as an index.
- */
-static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
-{
-    struct page_info *page;
-
-    if ( first_linear_offset(addr) >= P2M_ROOT_ENTRIES )
-        return NULL;
-
-    page = p2m->root + p2m_first_level_index(addr);
-
-    return __map_domain_page(page);
-}
-
 /*
  * Lookup the MFN corresponding to a domain's PFN.
  *
@@ -733,13 +707,12 @@ static int apply_p2m_changes(struct domain *d,
 {
     int rc, ret;
     struct p2m_domain *p2m = &d->arch.p2m;
-    lpae_t *first = NULL, *second = NULL, *third = NULL;
+    lpae_t *mappings[4] = { NULL, };
     paddr_t addr, orig_maddr = maddr;
     unsigned int level = 0;
-    unsigned long cur_first_page = ~0,
-                  cur_first_offset = ~0,
-                  cur_second_offset = ~0;
-    unsigned long count = 0;
+    unsigned int cur_root_table = ~0;
+    unsigned int cur_offset[4] = { ~0, };
+    unsigned int count = 0;
     bool_t flush = false;
     bool_t flush_pt;
 
@@ -751,9 +724,21 @@ static int apply_p2m_changes(struct domain *d,
 
     spin_lock(&p2m->lock);
 
+    /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
+    if ( P2M_ROOT_PAGES == 1 )
+        mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
+
     addr = start_gpaddr;
     while ( addr < end_gpaddr )
     {
+        int root_table;
+        const unsigned int offsets[4] = {
+            zeroeth_table_offset(addr),
+            first_table_offset(addr),
+            second_table_offset(addr),
+            third_table_offset(addr)
+        };
+
         /*
          * Arbitrarily, preempt every 512 operations or 8192 nops.
          * 512*P2M_ONE_PROGRESS == 8192*P2M_ONE_PROGRESS_NOP == 0x2000
@@ -773,76 +758,72 @@ static int apply_p2m_changes(struct domain *d,
             count = 0;
         }
 
-        if ( cur_first_page != p2m_first_level_index(addr) )
+        if ( P2M_ROOT_PAGES > 1 )
         {
-            if ( first ) unmap_domain_page(first);
-            first = p2m_map_first(p2m, addr);
-            if ( !first )
+            int i;
+            /*
+             * Concatenated root-level tables. The table number will be the
+             * offset at the previous level. It is not possible to concatenate
+             * a level-0 root.
+             */
+            ASSERT(P2M_ROOT_LEVEL > 0);
+            root_table = offsets[P2M_ROOT_LEVEL - 1];
+            if ( root_table >= P2M_ROOT_PAGES )
             {
                 rc = -EINVAL;
                 goto out;
             }
-            cur_first_page = p2m_first_level_index(addr);
-            /* Any mapping further down is now invalid */
-            cur_first_offset = cur_second_offset = ~0;
-        }
-
-        /* We only use a 3 level p2m at the moment, so no level 0,
-         * current hardware doesn't support super page mappings at
-         * level 0 anyway */
-
-        level = 1;
-        ret = apply_one_level(d, &first[first_table_offset(addr)],
-                              level, flush_pt, op,
-                              start_gpaddr, end_gpaddr,
-                              &addr, &maddr, &flush,
-                              mattr, t);
-        if ( ret < 0 ) { rc = ret ; goto out; }
-        count += ret;
-        if ( ret != P2M_ONE_DESCEND ) continue;
-
-        BUG_ON(!p2m_valid(first[first_table_offset(addr)]));
 
-        if ( cur_first_offset != first_table_offset(addr) )
-        {
-            if (second) unmap_domain_page(second);
-            second = map_domain_page(first[first_table_offset(addr)].p2m.base);
-            cur_first_offset = first_table_offset(addr);
-            /* Any mapping further down is now invalid */
-            cur_second_offset = ~0;
+            if ( cur_root_table != root_table )
+            {
+                if ( mappings[P2M_ROOT_LEVEL] )
+                    unmap_domain_page(mappings[P2M_ROOT_LEVEL]);
+                mappings[P2M_ROOT_LEVEL] =
+                    __map_domain_page(p2m->root + root_table);
+                if ( !mappings[P2M_ROOT_LEVEL] )
+                {
+                    rc = -EINVAL;
+                    goto out;
+                }
+                cur_root_table = root_table;
+                /* Any mapping further down is now invalid */
+                for ( i = P2M_ROOT_LEVEL; i < 4; i++ )
+                    cur_offset[i] = ~0;
+            }
         }
-        /* else: second already valid */
-
-        level = 2;
-        ret = apply_one_level(d,&second[second_table_offset(addr)],
-                              level, flush_pt, op,
-                              start_gpaddr, end_gpaddr,
-                              &addr, &maddr, &flush,
-                              mattr, t);
-        if ( ret < 0 ) { rc = ret ; goto out; }
-        count += ret;
-        if ( ret != P2M_ONE_DESCEND ) continue;
 
-        BUG_ON(!p2m_valid(second[second_table_offset(addr)]));
-
-        if ( cur_second_offset != second_table_offset(addr) )
+        for ( level = P2M_ROOT_LEVEL; level < 4; level++ )
         {
-            /* map third level */
-            if (third) unmap_domain_page(third);
-            third = map_domain_page(second[second_table_offset(addr)].p2m.base);
-            cur_second_offset = second_table_offset(addr);
+            unsigned offset = offsets[level];
+            lpae_t *entry = &mappings[level][offset];
+
+            ret = apply_one_level(d, entry,
+                                  level, flush_pt, op,
+                                  start_gpaddr, end_gpaddr,
+                                  &addr, &maddr, &flush,
+                                  mattr, t);
+            if ( ret < 0 ) { rc = ret ; goto out; }
+            count += ret;
+            /* L3 had better have done something! We cannot descend any further */
+            BUG_ON(level == 3 && ret == P2M_ONE_DESCEND);
+            if ( ret != P2M_ONE_DESCEND ) break;
+
+            BUG_ON(!p2m_valid(*entry));
+
+            if ( cur_offset[level] != offset )
+            {
+                /* Update mapping for next level */
+                int i;
+                if ( mappings[level+1] )
+                    unmap_domain_page(mappings[level+1]);
+                mappings[level+1] = map_domain_page(entry->p2m.base);
+                cur_offset[level] = offset;
+                /* Any mapping further down is now invalid */
+                for ( i = level+1; i < 4; i++ )
+                    cur_offset[i] = ~0;
+            }
+            /* else: next level already valid */
         }
-
-        level = 3;
-        ret = apply_one_level(d, &third[third_table_offset(addr)],
-                              level, flush_pt, op,
-                              start_gpaddr, end_gpaddr,
-                              &addr, &maddr, &flush,
-                              mattr, t);
-        if ( ret < 0 ) { rc = ret ; goto out; }
-        /* L3 had better have done something! We cannot descend any further */
-        BUG_ON(ret == P2M_ONE_DESCEND);
-        count += ret;
     }
 
     if ( flush )
@@ -866,9 +847,6 @@ static int apply_p2m_changes(struct domain *d,
     rc = 0;
 
 out:
-    if (third) unmap_domain_page(third);
-    if (second) unmap_domain_page(second);
-    if (first) unmap_domain_page(first);
     if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) &&
          addr != start_gpaddr )
     {
@@ -884,6 +862,12 @@ out:
                           mattr, p2m_invalid);
     }
 
+    for ( level = P2M_ROOT_LEVEL; level < 4; level ++ )
+    {
+        if ( mappings[level] )
+            unmap_domain_page(mappings[level]);
+    }
+
     spin_unlock(&p2m->lock);
 
     return rc;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 8/9] xen: arm: support for up to 48-bit physical addressing on arm64
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
                     ` (5 preceding siblings ...)
  2014-09-04 16:14   ` [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-08 23:28     ` Julien Grall
  2014-09-04 16:14   ` [PATCH v2 9/9] xen: arm: support for up to 48-bit IPA " Ian Campbell
  2014-09-08 21:05   ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Julien Grall
  8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini

This only affects Xen's own stage one paging.

- Use symbolic names for TCR bits for clarity.
- Update PADDR_BITS
- Base field of LPAE PT structs is now 36 bits (and therefore
  unsigned long long for arm32 compatibility)
- TCR_EL2.PS is set from ID_AA64MMFR0_EL1.PASize.
- Provide decode of ID_AA64MMFR0_EL1 in CPU info

Parts of this are derived from "xen/arm: Add 4-level page table for
stage 2 translation" by Vijaya Kumar K.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - TCR_{PS,TBI} under CONFIG_ARM_64
---
 xen/arch/arm/arm32/head.S       |    2 +-
 xen/arch/arm/arm64/head.S       |   10 +++++---
 xen/include/asm-arm/page.h      |   16 ++++++++-----
 xen/include/asm-arm/processor.h |   48 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 16d76f4..5c0263e 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -227,7 +227,7 @@ cpu_init_done:
          * PT walks use Inner-Shareable accesses,
          * PT walks are write-back, write-allocate in both cache levels,
          * Full 32-bit address space goes through this table. */
-        ldr   r0, =0x80003500
+        ldr   r0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
         mcr   CP32(r0, HTCR)
 
         /* Set up the HSCTLR:
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 43b5e72..d22af1c 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -224,13 +224,17 @@ skip_bss:
         ldr   x0, =MAIRVAL
         msr   mair_el2, x0
 
-        /* Set up the HTCR:
-         * PASize -- 40 bits / 1TB
+        /* Set up TCR_EL2:
+         * PS -- Based on ID_AA64MMFR0_EL1.PARange
          * Top byte is used
          * PT walks use Inner-Shareable accesses,
          * PT walks are write-back, write-allocate in both cache levels,
          * Full 64-bit address space goes through this table. */
-        ldr   x0, =0x80823500
+        ldr   x0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
+        /* ID_AA64MMFR0_EL1[3:0] (PARange) corresponds to TCR_EL2[18:16] (PS) */
+        mrs   x1, ID_AA64MMFR0_EL1
+        bfi   x0, x1, #16, #3
+
         msr   tcr_el2, x0
 
         /* Set up the SCTLR_EL2:
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 773822f..d758b61 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -6,7 +6,11 @@
 #include <public/xen.h>
 #include <asm/processor.h>
 
+#ifdef CONFIG_ARM_64
+#define PADDR_BITS              48
+#else
 #define PADDR_BITS              40
+#endif
 #define PADDR_MASK              ((1ULL << PADDR_BITS)-1)
 
 #define VADDR_BITS              32
@@ -114,8 +118,8 @@ typedef struct __packed {
     unsigned long ng:1;         /* Not-Global */
 
     /* The base address must be appropriately aligned for Block entries */
-    unsigned long base:28;      /* Base address of block or next table */
-    unsigned long sbz:12;       /* Must be zero */
+    unsigned long long base:36; /* Base address of block or next table */
+    unsigned long sbz:4;        /* Must be zero */
 
     /* These seven bits are only used in Block entries and are ignored
      * in Table entries. */
@@ -149,8 +153,8 @@ typedef struct __packed {
     unsigned long sbz4:1;
 
     /* The base address must be appropriately aligned for Block entries */
-    unsigned long base:28;      /* Base address of block or next table */
-    unsigned long sbz3:12;
+    unsigned long long base:36; /* Base address of block or next table */
+    unsigned long sbz3:4;
 
     /* These seven bits are only used in Block entries and are ignored
      * in Table entries. */
@@ -174,9 +178,9 @@ typedef struct __packed {
     unsigned long pad2:10;
 
     /* The base address must be appropriately aligned for Block entries */
-    unsigned long base:28;      /* Base address of block or next table */
+    unsigned long long base:36; /* Base address of block or next table */
 
-    unsigned long pad1:24;
+    unsigned long pad1:16;
 } lpae_walk_t;
 
 typedef union {
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 0cc5b6d..044bd98 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -87,6 +87,41 @@
 #define HCR_SWIO        (_AC(1,UL)<<1) /* Set/Way Invalidation Override */
 #define HCR_VM          (_AC(1,UL)<<0) /* Virtual MMU Enable */
 
+/* TCR: Stage 1 Translation Control */
+
+#define TCR_T0SZ(x)     ((x)<<0)
+
+#define TCR_IRGN0_NC    (_AC(0x0,UL)<<8)
+#define TCR_IRGN0_WBWA  (_AC(0x1,UL)<<8)
+#define TCR_IRGN0_WT    (_AC(0x2,UL)<<8)
+#define TCR_IRGN0_WB    (_AC(0x3,UL)<<8)
+
+#define TCR_ORGN0_NC    (_AC(0x0,UL)<<10)
+#define TCR_ORGN0_WBWA  (_AC(0x1,UL)<<10)
+#define TCR_ORGN0_WT    (_AC(0x2,UL)<<10)
+#define TCR_ORGN0_WB    (_AC(0x3,UL)<<10)
+
+#define TCR_SH0_NS      (_AC(0x0,UL)<<12)
+#define TCR_SH0_OS      (_AC(0x2,UL)<<12)
+#define TCR_SH0_IS      (_AC(0x3,UL)<<12)
+
+#define TCR_TG0_4K      (_AC(0x0,UL)<<14)
+#define TCR_TG0_64K     (_AC(0x1,UL)<<14)
+#define TCR_TG0_16K     (_AC(0x2,UL)<<14)
+
+#ifdef CONFIG_ARM_64
+
+#define TCR_PS(x)       ((x)<<16)
+#define TCR_TBI         (_AC(0x1,UL)<<20)
+
+#define TCR_RES1        (_AC(1,UL)<<31|_AC(1,UL)<<23)
+
+#else
+
+#define TCR_RES1        (_AC(1,UL)<<31)
+
+#endif
+
 /* HCPTR Hyp. Coprocessor Trap Register */
 #define HCPTR_TTA       ((_AC(1,U)<<20))        /* Trap trace registers */
 #define HCPTR_CP(x)     ((_AC(1,U)<<(x)))       /* Trap Coprocessor x */
@@ -191,8 +226,19 @@ struct cpuinfo_arm {
         uint64_t bits[2];
     } aux64;
 
-    struct {
+    union {
         uint64_t bits[2];
+        struct {
+            unsigned long pa_range:4;
+            unsigned long asid_bits:4;
+            unsigned long bigend:4;
+            unsigned long secure_ns:4;
+            unsigned long bigend_el0:4;
+            unsigned long tgranule_16K:4;
+            unsigned long tgranule_64K:4;
+            unsigned long tgranule_4K:4;
+            unsigned long __res0:32;
+       };
     } mm64;
 
     struct {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 9/9] xen: arm: support for up to 48-bit IPA addressing on arm64
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
                     ` (6 preceding siblings ...)
  2014-09-04 16:14   ` [PATCH v2 8/9] xen: arm: support for up to 48-bit physical addressing on arm64 Ian Campbell
@ 2014-09-04 16:14   ` Ian Campbell
  2014-09-08 23:42     ` Julien Grall
  2014-09-08 21:05   ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Julien Grall
  8 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2014-09-04 16:14 UTC (permalink / raw)
  To: xen-devel
  Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini

Currently we support only 40-bits. This is insufficient on systems where
peripherals which need to be 1:1 mapped to dom0 are above the 40-bit limit.

Unfortunately the hardware requirements are such that this means that the
number of levels in the P2M is not static and must vary with the number of
implemented physical address bits. This is described in ARM DDI 0487A.b Table
D4-5. In short there is no single p2m configuration which supports everything
from 40- to 48- bits.

For example a system which supports up to 40-bit addressing will only support 3
level p2m (maximum SL0 is 1 == 3 levels), requiring a concatenated page table
root consisting of two pages to make the full 40-bits of addressing.

A maximum of 16 pages can be concatenated meaning that a 3 level p2m can only
support up to 43-bit addresses. Therefore support for 48-bit addressing
requires SL0==2 (4 levels of paging).

After the previous patches our various p2m lookup and manipulation functions
already support starting at arbitrary level and with arbitrary root
concatenation. All that remains is to determine the correct settings from
ID_AA64MMFR0_EL1.PARange for which we use a lookup table.

As well as supporting 44 and 48 bit addressing we can also reduce the order of
the first level for systems which support only 32 or 36 physical address bits,
saving a page.

Systems with 42-bits are an interesting case, since they only support 3 levels
of paging, implying that 8 pages are required at the root level. So far I am
not aware of any systems with peripheral located so high up (the only 42-bit
system I've seen has nothing above 40-bits), so such systems remain configured
for 40-bit IPA with a pair of pages at the root of the p2m.

Switching to symbolic names for the VTCR_EL2 bits as we go improves the clarity
of the result.

Parts of this are derived from "xen/arm: Add 4-level page table for
stage 2 translation" by Vijaya Kumar K.

arm32 remains with the static 3-level, 2 page root configuration.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - lots of spelling in the commit log.
 - Use ASSERT not BUG_ON in hotpaths.
 - Lots of unsigned int cleanups
 - Check for invalid p2m_root_level/p2m_root_order combinations at
   setup_virt_paging time.
 - CONFIG_ARM_64 for 64-bit only VTCR bit definitions
---
 xen/arch/arm/p2m.c              |   84 +++++++++++++++++++++++++++++++--------
 xen/include/asm-arm/p2m.h       |    2 +-
 xen/include/asm-arm/processor.h |   32 +++++++++++++++
 3 files changed, 100 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 8e330c7..d6a0c27 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -11,10 +11,17 @@
 #include <asm/hardirq.h>
 #include <asm/page.h>
 
-#define P2M_ROOT_LEVEL       1
+#ifdef CONFIG_ARM_64
+static unsigned int __read_mostly p2m_root_order;
+static unsigned int __read_mostly p2m_root_level;
+#define P2M_ROOT_ORDER    p2m_root_order
+#define P2M_ROOT_LEVEL p2m_root_level
+#else
+/* First level P2M is alway 2 consecutive pages */
+#define P2M_ROOT_LEVEL 1
+#define P2M_ROOT_ORDER    1
+#endif
 
-/* First level P2M is 2 consecutive pages */
-#define P2M_ROOT_ORDER 1
 #define P2M_ROOT_PAGES    (1<<P2M_ROOT_ORDER)
 
 static bool_t p2m_valid(lpae_t pte)
@@ -168,6 +175,8 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
 
     map = __map_domain_page(p2m->root + root_table);
 
+    ASSERT(P2M_ROOT_LEVEL < 4);
+
     for ( level = P2M_ROOT_LEVEL ; level < 4 ; level++ )
     {
         mask = masks[level];
@@ -931,6 +940,7 @@ int p2m_alloc_table(struct domain *d)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
     struct page_info *page;
+    unsigned int i;
 
     page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0);
     if ( page == NULL )
@@ -939,8 +949,8 @@ int p2m_alloc_table(struct domain *d)
     spin_lock(&p2m->lock);
 
     /* Clear both first level pages */
-    clear_and_clean_page(page);
-    clear_and_clean_page(page + 1);
+    for ( i = 0; i < P2M_ROOT_PAGES; i++ )
+        clear_and_clean_page(page + i);
 
     p2m->root = page;
 
@@ -1126,21 +1136,61 @@ static void __init setup_virt_paging_one(void *data)
 
 void __init setup_virt_paging(void)
 {
-    unsigned long val;
-
     /* Setup Stage 2 address translation */
-    /* SH0=11 (Inner-shareable)
-     * ORGN0=IRGN0=01 (Normal memory, Write-Back Write-Allocate Cacheable)
-     * SL0=01 (Level-1)
-     * ARVv7: T0SZ=(1)1000 = -8 (32-(-8) = 40 bit physical addresses)
-     * ARMv8: T0SZ=01 1000 = 24 (64-24   = 40 bit physical addresses)
-     *        PS=010 == 40 bits
-     */
+    unsigned long val = VTCR_RES1|VTCR_SH0_IS|VTCR_ORGN0_WBWA|VTCR_IRGN0_WBWA;
+
 #ifdef CONFIG_ARM_32
-    val = 0x80003558;
-#else
-    val = 0x80023558;
+    printk("P2M: 40-bit IPA\n");
+    val |= VTCR_T0SZ(0x18); /* 40 bit IPA */
+#else /* CONFIG_ARM_64 */
+    const struct {
+        unsigned int pabits; /* Physical Address Size */
+        unsigned int t0sz;   /* Desired T0SZ, minimum in comment */
+        unsigned int root_order; /* Page order of the root of the p2m */
+        unsigned int sl0;    /* Desired SL0, maximum in comment */
+    } pa_range_info[] = {
+        /* T0SZ minimum and SL0 maximum from ARM DDI 0487A.b Table D4-5 */
+        /*      PA size, t0sz(min), root-order, sl0(max) */
+        [0] = { 32,      32/*32*/,  0,          1 },
+        [1] = { 36,      28/*28*/,  0,          1 },
+        [2] = { 40,      24/*24*/,  1,          1 },
+        [3] = { 42,      24/*22*/,  1,          1 },
+        [4] = { 44,      20/*20*/,  0,          2 },
+        [5] = { 48,      16/*16*/,  0,          2 },
+        [6] = { 0 }, /* Invalid */
+        [7] = { 0 }  /* Invalid */
+    };
+
+    unsigned int cpu;
+    unsigned int pa_range = 0x10; /* Larger than any possible value */
+
+    for_each_online_cpu ( cpu )
+    {
+        struct cpuinfo_arm *info = &cpu_data[cpu];
+        if ( info->mm64.pa_range < pa_range )
+            pa_range = info->mm64.pa_range;
+    }
+
+    /* pa_range is 4 bits, but the defined encodings are only 3 bits */
+    if ( pa_range&0x8 || !pa_range_info[pa_range].pabits )
+        panic("Unknown encoding of ID_AA64MMFR0_EL1.PARange %x\n", pa_range);
+
+    val |= VTCR_PS(pa_range);
+    val |= VTCR_TG0_4K;
+    val |= VTCR_SL0(pa_range_info[pa_range].sl0);
+    val |= VTCR_T0SZ(pa_range_info[pa_range].t0sz);
+
+    p2m_root_order = pa_range_info[pa_range].root_order;
+    p2m_root_level = 2 - pa_range_info[pa_range].sl0;
+
+    printk("P2M: %d-bit IPA with %d-bit PA\n",
+           64 - pa_range_info[pa_range].t0sz,
+           pa_range_info[pa_range].pabits);
 #endif
+    printk("P2M: %d levels with order-%d root, VTCR 0x%lx\n",
+           4 - P2M_ROOT_LEVEL, P2M_ROOT_ORDER, val);
+    /* It is not allowed to concatenate a level zero root */
+    BUG_ON( P2M_ROOT_LEVEL == 0 && P2M_ROOT_ORDER > 0 );
     setup_virt_paging_one((void *)val);
     smp_call_function(setup_virt_paging_one, (void *)val, 1);
 }
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 53aa0fc..7264a17 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -19,7 +19,7 @@ struct p2m_domain {
     /* Pages used to construct the p2m */
     struct page_list_head pages;
 
-    /* Root of p2m page tables, 2 contiguous pages */
+    /* The root of the p2m tree. May be concatenated */
     struct page_info *root;
 
     /* Current VMID in use */
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 044bd98..41dd45a 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -122,6 +122,38 @@
 
 #endif
 
+/* VTCR: Stage 2 Translation Control */
+
+#define VTCR_T0SZ(x)    ((x)<<0)
+
+#define VTCR_SL0(x)     ((x)<<6)
+
+#define VTCR_IRGN0_NC   (_AC(0x0,UL)<<8)
+#define VTCR_IRGN0_WBWA (_AC(0x1,UL)<<8)
+#define VTCR_IRGN0_WT   (_AC(0x2,UL)<<8)
+#define VTCR_IRGN0_WB   (_AC(0x3,UL)<<8)
+
+#define VTCR_ORGN0_NC   (_AC(0x0,UL)<<10)
+#define VTCR_ORGN0_WBWA (_AC(0x1,UL)<<10)
+#define VTCR_ORGN0_WT   (_AC(0x2,UL)<<10)
+#define VTCR_ORGN0_WB   (_AC(0x3,UL)<<10)
+
+#define VTCR_SH0_NS     (_AC(0x0,UL)<<12)
+#define VTCR_SH0_OS     (_AC(0x2,UL)<<12)
+#define VTCR_SH0_IS     (_AC(0x3,UL)<<12)
+
+#ifdef CONFIG_ARM_64
+
+#define VTCR_TG0_4K     (_AC(0x0,UL)<<14)
+#define VTCR_TG0_64K    (_AC(0x1,UL)<<14)
+#define VTCR_TG0_16K    (_AC(0x2,UL)<<14)
+
+#define VTCR_PS(x)      ((x)<<16)
+
+#endif
+
+#define VTCR_RES1       (_AC(1,UL)<<31)
+
 /* HCPTR Hyp. Coprocessor Trap Register */
 #define HCPTR_TTA       ((_AC(1,U)<<20))        /* Trap trace registers */
 #define HCPTR_CP(x)     ((_AC(1,U)<<(x)))       /* Trap Coprocessor x */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
  2014-09-04 16:14   ` [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
@ 2014-09-05 23:32     ` Arianna Avanzini
  2014-09-08  8:59       ` Ian Campbell
  2014-09-08 22:22     ` Julien Grall
  1 sibling, 1 reply; 25+ messages in thread
From: Arianna Avanzini @ 2014-09-05 23:32 UTC (permalink / raw)
  To: Ian Campbell
  Cc: stefano.stabellini, tim, vijay.kilari, julien.grall, xen-devel

On Thu, Sep 04, 2014 at 05:14:15PM +0100, Ian Campbell wrote:
> As with previous changes this involves conversion from a linear series of
> lookups into a loop over the levels.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Arianna Avanzini <avanzini.arianna@gmail.com>

(For the little it's worth, everything looks fine to me)

> ---
> v2:
>  - Rebase, in particular over 4b25423aee13 "arch/arm: unmap partially-mapped
>    memory regions" which had some interesting interactions (which I hope I've
>    gotten right!)
>  - Spell previous and concatenate correctly.
>  - ASSERT rather than BUG_ON for concatenated level zero root.
> ---
>  xen/arch/arm/p2m.c |  172 ++++++++++++++++++++++++----------------------------
>  1 file changed, 78 insertions(+), 94 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index ea5e342..8e330c7 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -15,7 +15,6 @@
>  
>  /* First level P2M is 2 consecutive pages */
>  #define P2M_ROOT_ORDER 1
> -#define P2M_ROOT_ENTRIES (LPAE_ENTRIES<<P2M_ROOT_ORDER)
>  #define P2M_ROOT_PAGES    (1<<P2M_ROOT_ORDER)
>  
>  static bool_t p2m_valid(lpae_t pte)
> @@ -119,31 +118,6 @@ void flush_tlb_domain(struct domain *d)
>          p2m_load_VTTBR(current->domain);
>  }
>  
> -static int p2m_first_level_index(paddr_t addr)
> -{
> -    /*
> -     * 1st pages are concatenated so zeroeth offset gives us the
> -     * index of the 1st page
> -     */
> -    return zeroeth_table_offset(addr);
> -}
> -
> -/*
> - * Map whichever of the first pages contain addr. The caller should
> - * then use first_table_offset as an index.
> - */
> -static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
> -{
> -    struct page_info *page;
> -
> -    if ( first_linear_offset(addr) >= P2M_ROOT_ENTRIES )
> -        return NULL;
> -
> -    page = p2m->root + p2m_first_level_index(addr);
> -
> -    return __map_domain_page(page);
> -}
> -
>  /*
>   * Lookup the MFN corresponding to a domain's PFN.
>   *
> @@ -733,13 +707,12 @@ static int apply_p2m_changes(struct domain *d,
>  {
>      int rc, ret;
>      struct p2m_domain *p2m = &d->arch.p2m;
> -    lpae_t *first = NULL, *second = NULL, *third = NULL;
> +    lpae_t *mappings[4] = { NULL, };
>      paddr_t addr, orig_maddr = maddr;
>      unsigned int level = 0;
> -    unsigned long cur_first_page = ~0,
> -                  cur_first_offset = ~0,
> -                  cur_second_offset = ~0;
> -    unsigned long count = 0;
> +    unsigned int cur_root_table = ~0;
> +    unsigned int cur_offset[4] = { ~0, };
> +    unsigned int count = 0;
>      bool_t flush = false;
>      bool_t flush_pt;
>  
> @@ -751,9 +724,21 @@ static int apply_p2m_changes(struct domain *d,
>  
>      spin_lock(&p2m->lock);
>  
> +    /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
> +    if ( P2M_ROOT_PAGES == 1 )
> +        mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
> +
>      addr = start_gpaddr;
>      while ( addr < end_gpaddr )
>      {
> +        int root_table;
> +        const unsigned int offsets[4] = {
> +            zeroeth_table_offset(addr),
> +            first_table_offset(addr),
> +            second_table_offset(addr),
> +            third_table_offset(addr)
> +        };
> +
>          /*
>           * Arbitrarily, preempt every 512 operations or 8192 nops.
>           * 512*P2M_ONE_PROGRESS == 8192*P2M_ONE_PROGRESS_NOP == 0x2000
> @@ -773,76 +758,72 @@ static int apply_p2m_changes(struct domain *d,
>              count = 0;
>          }
>  
> -        if ( cur_first_page != p2m_first_level_index(addr) )
> +        if ( P2M_ROOT_PAGES > 1 )
>          {
> -            if ( first ) unmap_domain_page(first);
> -            first = p2m_map_first(p2m, addr);
> -            if ( !first )
> +            int i;
> +            /*
> +             * Concatenated root-level tables. The table number will be the
> +             * offset at the previous level. It is not possible to concatenate
> +             * a level-0 root.
> +             */
> +            ASSERT(P2M_ROOT_LEVEL > 0);
> +            root_table = offsets[P2M_ROOT_LEVEL - 1];
> +            if ( root_table >= P2M_ROOT_PAGES )
>              {
>                  rc = -EINVAL;
>                  goto out;
>              }
> -            cur_first_page = p2m_first_level_index(addr);
> -            /* Any mapping further down is now invalid */
> -            cur_first_offset = cur_second_offset = ~0;
> -        }
> -
> -        /* We only use a 3 level p2m at the moment, so no level 0,
> -         * current hardware doesn't support super page mappings at
> -         * level 0 anyway */
> -
> -        level = 1;
> -        ret = apply_one_level(d, &first[first_table_offset(addr)],
> -                              level, flush_pt, op,
> -                              start_gpaddr, end_gpaddr,
> -                              &addr, &maddr, &flush,
> -                              mattr, t);
> -        if ( ret < 0 ) { rc = ret ; goto out; }
> -        count += ret;
> -        if ( ret != P2M_ONE_DESCEND ) continue;
> -
> -        BUG_ON(!p2m_valid(first[first_table_offset(addr)]));
>  
> -        if ( cur_first_offset != first_table_offset(addr) )
> -        {
> -            if (second) unmap_domain_page(second);
> -            second = map_domain_page(first[first_table_offset(addr)].p2m.base);
> -            cur_first_offset = first_table_offset(addr);
> -            /* Any mapping further down is now invalid */
> -            cur_second_offset = ~0;
> +            if ( cur_root_table != root_table )
> +            {
> +                if ( mappings[P2M_ROOT_LEVEL] )
> +                    unmap_domain_page(mappings[P2M_ROOT_LEVEL]);
> +                mappings[P2M_ROOT_LEVEL] =
> +                    __map_domain_page(p2m->root + root_table);
> +                if ( !mappings[P2M_ROOT_LEVEL] )
> +                {
> +                    rc = -EINVAL;
> +                    goto out;
> +                }
> +                cur_root_table = root_table;
> +                /* Any mapping further down is now invalid */
> +                for ( i = P2M_ROOT_LEVEL; i < 4; i++ )
> +                    cur_offset[i] = ~0;
> +            }
>          }
> -        /* else: second already valid */
> -
> -        level = 2;
> -        ret = apply_one_level(d,&second[second_table_offset(addr)],
> -                              level, flush_pt, op,
> -                              start_gpaddr, end_gpaddr,
> -                              &addr, &maddr, &flush,
> -                              mattr, t);
> -        if ( ret < 0 ) { rc = ret ; goto out; }
> -        count += ret;
> -        if ( ret != P2M_ONE_DESCEND ) continue;
>  
> -        BUG_ON(!p2m_valid(second[second_table_offset(addr)]));
> -
> -        if ( cur_second_offset != second_table_offset(addr) )
> +        for ( level = P2M_ROOT_LEVEL; level < 4; level++ )
>          {
> -            /* map third level */
> -            if (third) unmap_domain_page(third);
> -            third = map_domain_page(second[second_table_offset(addr)].p2m.base);
> -            cur_second_offset = second_table_offset(addr);
> +            unsigned offset = offsets[level];
> +            lpae_t *entry = &mappings[level][offset];
> +
> +            ret = apply_one_level(d, entry,
> +                                  level, flush_pt, op,
> +                                  start_gpaddr, end_gpaddr,
> +                                  &addr, &maddr, &flush,
> +                                  mattr, t);
> +            if ( ret < 0 ) { rc = ret ; goto out; }
> +            count += ret;
> +            /* L3 had better have done something! We cannot descend any further */
> +            BUG_ON(level == 3 && ret == P2M_ONE_DESCEND);
> +            if ( ret != P2M_ONE_DESCEND ) break;
> +
> +            BUG_ON(!p2m_valid(*entry));
> +
> +            if ( cur_offset[level] != offset )
> +            {
> +                /* Update mapping for next level */
> +                int i;
> +                if ( mappings[level+1] )
> +                    unmap_domain_page(mappings[level+1]);
> +                mappings[level+1] = map_domain_page(entry->p2m.base);
> +                cur_offset[level] = offset;
> +                /* Any mapping further down is now invalid */
> +                for ( i = level+1; i < 4; i++ )
> +                    cur_offset[i] = ~0;
> +            }
> +            /* else: next level already valid */
>          }
> -
> -        level = 3;
> -        ret = apply_one_level(d, &third[third_table_offset(addr)],
> -                              level, flush_pt, op,
> -                              start_gpaddr, end_gpaddr,
> -                              &addr, &maddr, &flush,
> -                              mattr, t);
> -        if ( ret < 0 ) { rc = ret ; goto out; }
> -        /* L3 had better have done something! We cannot descend any further */
> -        BUG_ON(ret == P2M_ONE_DESCEND);
> -        count += ret;
>      }
>  
>      if ( flush )
> @@ -866,9 +847,6 @@ static int apply_p2m_changes(struct domain *d,
>      rc = 0;
>  
>  out:
> -    if (third) unmap_domain_page(third);
> -    if (second) unmap_domain_page(second);
> -    if (first) unmap_domain_page(first);
>      if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) &&
>           addr != start_gpaddr )
>      {
> @@ -884,6 +862,12 @@ out:
>                            mattr, p2m_invalid);
>      }
>  
> +    for ( level = P2M_ROOT_LEVEL; level < 4; level ++ )
> +    {
> +        if ( mappings[level] )
> +            unmap_domain_page(mappings[level]);
> +    }
> +
>      spin_unlock(&p2m->lock);
>  
>      return rc;
> -- 
> 1.7.10.4
> 

-- 
/*
 * Arianna Avanzini
 * avanzini.arianna@gmail.com
 * 73628@studenti.unimore.it
 */

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
  2014-09-05 23:32     ` Arianna Avanzini
@ 2014-09-08  8:59       ` Ian Campbell
  0 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2014-09-08  8:59 UTC (permalink / raw)
  To: Arianna Avanzini
  Cc: stefano.stabellini, tim, vijay.kilari, julien.grall, xen-devel

On Sat, 2014-09-06 at 01:32 +0200, Arianna Avanzini wrote:
> On Thu, Sep 04, 2014 at 05:14:15PM +0100, Ian Campbell wrote:
> > As with previous changes this involves conversion from a linear series of
> > lookups into a loop over the levels.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Arianna Avanzini <avanzini.arianna@gmail.com>
> 
> (For the little it's worth, everything looks fine to me)

On the contrary it's good to know I didn't break your patch, thanks.

(and more generally opinions on patches up to a full blown reviewed-by
are always welcome from anyone).

Ian.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root.
  2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
                     ` (7 preceding siblings ...)
  2014-09-04 16:14   ` [PATCH v2 9/9] xen: arm: support for up to 48-bit IPA " Ian Campbell
@ 2014-09-08 21:05   ` Julien Grall
  8 siblings, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 21:05 UTC (permalink / raw)
  To: Ian Campbell, xen-devel
  Cc: Vijaya Kumar K, Ian Campbell, tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> From: Ian Campbell <ian.campbell@citirx.com>
>
> This was previously part of Vigaya's "xen/arm: Add 4-level page table
> for stage 2 translation" but is split out here to make that patch
> easier to read.
>
> I went with ->root rather than ->root_level as the original did.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk
  2014-09-04 16:14   ` [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
@ 2014-09-08 21:24     ` Julien Grall
  2014-09-08 22:12       ` Julien Grall
  2014-09-09 15:04       ` Ian Campbell
  2014-09-08 21:33     ` Julien Grall
  1 sibling, 2 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 21:24 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> -void dump_pt_walk(lpae_t *first, paddr_t addr)
> +void dump_pt_walk(lpae_t *root, paddr_t addr,
> +                  unsigned int root_level)
>   {
> -    lpae_t *second = NULL, *third = NULL;
> +    static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" };
> +    const unsigned int offsets[4] = {
> +        zeroeth_table_offset(addr),
> +        first_table_offset(addr),
> +        second_table_offset(addr),
> +        third_table_offset(addr)
> +    };
> +    lpae_t pte, *mappings[4] = { 0, };

Xen never maps the next level when level == 3, shoudn't mappings[3] enough?

[..]

> +    /* mappings[root_level] is provided by the caller so don't unmap that */
> +    do
> +    {
> +        unmap_domain_page(mappings[level]);
> +    }
> +    while( level-- > root_level );

NIT: It looks like we commonly use the coding style

do
{
} while ( ... )

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk
  2014-09-04 16:14   ` [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
  2014-09-08 21:24     ` Julien Grall
@ 2014-09-08 21:33     ` Julien Grall
  1 sibling, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 21:33 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini

Hi Ian,

I forgot one point on my previous mail.

On 04/09/14 09:14, Ian Campbell wrote:
> -    if ( first_table_offset(addr) >= LPAE_ENTRIES )
> -        return;
> +    mappings[root_level] = root;
> +
> +    for ( level = root_level; ; level++ )
> +    {

I would add the check "level < 3" or an ASSERT in the beginning of the 
loop. So we could catch a possible overflow on offsets later if someone 
decide to modify it.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 3/9] xen: arm: handle concatenated root tables in dump_pt_walk
  2014-09-04 16:14   ` [PATCH v2 3/9] xen: arm: handle concatenated root tables " Ian Campbell
@ 2014-09-08 21:43     ` Julien Grall
  0 siblings, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 21:43 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> ARM allows for the concatenation of pages at the root of a p2m (but not a
> regular page table) in order to support a larger IPA space than the number of
> levels in the P2M would normally support. We use this to support 40-bit guest
> addresses.
>
> Previously we were unable to dump IPAs which were outside the first page of the
> root. To fix this we adjust dump_pt_walk to take the machine address of the
> page table root instead of expecting the caller to have mapper it. This allows

NIT: s/mapper/mapped/ ?

[..]

> -    mappings[root_level] = root;
> +    if ( nr_root_tables > 1 )
> +    {
> +        /*
> +         * Concatenated root-level tables. The table number will be
> +         * the offset at the previous level. It is not possible to
> +         * concatenate a level-0 root.
> +         */
> +        BUG_ON(root_level == 0);
> +        root_table = offsets[root_level - 1];
> +        printk("Using concatenated root table %d\n", root_table);

NIT: %u

Other than this 2 minor comments:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up
  2014-09-04 16:14   ` [PATCH v2 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Ian Campbell
@ 2014-09-08 21:46     ` Julien Grall
  0 siblings, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 21:46 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> Currently we retain the hardcoded values but soon we will want to calculate the
> correct values based upon the CPU properties common to all processors, which
> are only available once they are all up.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 6/9] xen: arm: handle variable p2m levels in p2m_lookup
  2014-09-04 16:14   ` [PATCH v2 6/9] xen: arm: handle variable p2m levels in p2m_lookup Ian Campbell
@ 2014-09-08 22:05     ` Julien Grall
  0 siblings, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 22:05 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> This paves the way for boot-time selection of the number of levels to
> use in the p2m, which is required to support both 40-bit and 48-bit
> systems. For now the starting level remains a compile time constant.
>
> Implemented by turning the linear sequence of lookups into a loop.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk
  2014-09-08 21:24     ` Julien Grall
@ 2014-09-08 22:12       ` Julien Grall
  2014-09-09 15:04       ` Ian Campbell
  1 sibling, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 22:12 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini



On 08/09/14 14:24, Julien Grall wrote:
> Hi Ian,
>
> On 04/09/14 09:14, Ian Campbell wrote:
>> -void dump_pt_walk(lpae_t *first, paddr_t addr)
>> +void dump_pt_walk(lpae_t *root, paddr_t addr,
>> +                  unsigned int root_level)
>>   {
>> -    lpae_t *second = NULL, *third = NULL;
>> +    static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" };
>> +    const unsigned int offsets[4] = {
>> +        zeroeth_table_offset(addr),
>> +        first_table_offset(addr),
>> +        second_table_offset(addr),
>> +        third_table_offset(addr)
>> +    };
>> +    lpae_t pte, *mappings[4] = { 0, };
>
> Xen never maps the next level when level == 3, shoudn't mappings[3] enough?

After thinking, mappings[4] is fine here. Sorry for the noise.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
  2014-09-04 16:14   ` [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
  2014-09-05 23:32     ` Arianna Avanzini
@ 2014-09-08 22:22     ` Julien Grall
  1 sibling, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 22:22 UTC (permalink / raw)
  To: Ian Campbell, xen-devel
  Cc: Arianna Avanzini, tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> +            if ( cur_root_table != root_table )
> +            {
> +                if ( mappings[P2M_ROOT_LEVEL] )
> +                    unmap_domain_page(mappings[P2M_ROOT_LEVEL]);
> +                mappings[P2M_ROOT_LEVEL] =
> +                    __map_domain_page(p2m->root + root_table);
> +                if ( !mappings[P2M_ROOT_LEVEL] )

__map_domain_page can never fail, so you can drop this check.

With this change:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 8/9] xen: arm: support for up to 48-bit physical addressing on arm64
  2014-09-04 16:14   ` [PATCH v2 8/9] xen: arm: support for up to 48-bit physical addressing on arm64 Ian Campbell
@ 2014-09-08 23:28     ` Julien Grall
  0 siblings, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 23:28 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> This only affects Xen's own stage one paging.
>
> - Use symbolic names for TCR bits for clarity.
> - Update PADDR_BITS
> - Base field of LPAE PT structs is now 36 bits (and therefore
>    unsigned long long for arm32 compatibility)
> - TCR_EL2.PS is set from ID_AA64MMFR0_EL1.PASize.
> - Provide decode of ID_AA64MMFR0_EL1 in CPU info
>
> Parts of this are derived from "xen/arm: Add 4-level page table for
> stage 2 translation" by Vijaya Kumar K.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 9/9] xen: arm: support for up to 48-bit IPA addressing on arm64
  2014-09-04 16:14   ` [PATCH v2 9/9] xen: arm: support for up to 48-bit IPA " Ian Campbell
@ 2014-09-08 23:42     ` Julien Grall
  0 siblings, 0 replies; 25+ messages in thread
From: Julien Grall @ 2014-09-08 23:42 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini

Hi Ian,

On 04/09/14 09:14, Ian Campbell wrote:
> +    for_each_online_cpu ( cpu )
> +    {
> +        struct cpuinfo_arm *info = &cpu_data[cpu];

NIT: const struct


Other than that:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk
  2014-09-08 21:24     ` Julien Grall
  2014-09-08 22:12       ` Julien Grall
@ 2014-09-09 15:04       ` Ian Campbell
  2014-09-09 15:08         ` Ian Campbell
  2014-09-09 16:04         ` Andrew Cooper
  1 sibling, 2 replies; 25+ messages in thread
From: Ian Campbell @ 2014-09-09 15:04 UTC (permalink / raw)
  To: Julien Grall; +Cc: stefano.stabellini, tim, vijay.kilari, xen-devel

On Mon, 2014-09-08 at 14:24 -0700, Julien Grall wrote:
> > +    /* mappings[root_level] is provided by the caller so don't unmap that */
> > +    do
> > +    {
> > +        unmap_domain_page(mappings[level]);
> > +    }
> > +    while( level-- > root_level );
> 
> NIT: It looks like we commonly use the coding style
> 
> do
> {
> } while ( ... )

Seems we do. Seems inconsistent with if() and while/do loops to me. Oh
well.

Ian.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk
  2014-09-09 15:04       ` Ian Campbell
@ 2014-09-09 15:08         ` Ian Campbell
  2014-09-09 16:04         ` Andrew Cooper
  1 sibling, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2014-09-09 15:08 UTC (permalink / raw)
  To: Julien Grall; +Cc: stefano.stabellini, tim, vijay.kilari, xen-devel

On Tue, 2014-09-09 at 16:04 +0100, Ian Campbell wrote:
> On Mon, 2014-09-08 at 14:24 -0700, Julien Grall wrote:
> > > +    /* mappings[root_level] is provided by the caller so don't unmap that */
> > > +    do
> > > +    {
> > > +        unmap_domain_page(mappings[level]);
> > > +    }
> > > +    while( level-- > root_level );
> > 
> > NIT: It looks like we commonly use the coding style
> > 
> > do
> > {
> > } while ( ... )
> 
> Seems we do. Seems inconsistent with if() and while/do loops to me. Oh
> well.

Actually now I go to look for the line in question it seems like we
actually have a pretty arbitrary mixture. I'm going to stick with the
extra newline (I did fix the missing space after the while though).

Ian.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk
  2014-09-09 15:04       ` Ian Campbell
  2014-09-09 15:08         ` Ian Campbell
@ 2014-09-09 16:04         ` Andrew Cooper
  1 sibling, 0 replies; 25+ messages in thread
From: Andrew Cooper @ 2014-09-09 16:04 UTC (permalink / raw)
  To: Ian Campbell, Julien Grall
  Cc: xen-devel, tim, vijay.kilari, stefano.stabellini

On 09/09/14 16:04, Ian Campbell wrote:
> On Mon, 2014-09-08 at 14:24 -0700, Julien Grall wrote:
>>> +    /* mappings[root_level] is provided by the caller so don't unmap that */
>>> +    do
>>> +    {
>>> +        unmap_domain_page(mappings[level]);
>>> +    }
>>> +    while( level-- > root_level );
>> NIT: It looks like we commonly use the coding style
>>
>> do
>> {
>> } while ( ... )
> Seems we do. Seems inconsistent with if() and while/do loops to me. Oh
> well.
>
> Ian.

It is to visually differentiate

if ( ... )
{
    ....
}
while ( something unrelated, with a suspicious looking semicolon );

from
do
{
    ....
}
while ( something related to the loop );

~Andrew

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2014-09-09 16:04 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-04 16:13 [PATCH v2 0/9] xen: arm: support for > 40-bit physical addressing Ian Campbell
2014-09-04 16:14 ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
2014-09-04 16:14   ` [PATCH v2 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
2014-09-08 21:24     ` Julien Grall
2014-09-08 22:12       ` Julien Grall
2014-09-09 15:04       ` Ian Campbell
2014-09-09 15:08         ` Ian Campbell
2014-09-09 16:04         ` Andrew Cooper
2014-09-08 21:33     ` Julien Grall
2014-09-04 16:14   ` [PATCH v2 3/9] xen: arm: handle concatenated root tables " Ian Campbell
2014-09-08 21:43     ` Julien Grall
2014-09-04 16:14   ` [PATCH v2 4/9] xen: arm: move setup_virt_paging to p2m.[ch] from mm.[ch] Ian Campbell
2014-09-04 16:14   ` [PATCH v2 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Ian Campbell
2014-09-08 21:46     ` Julien Grall
2014-09-04 16:14   ` [PATCH v2 6/9] xen: arm: handle variable p2m levels in p2m_lookup Ian Campbell
2014-09-08 22:05     ` Julien Grall
2014-09-04 16:14   ` [PATCH v2 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
2014-09-05 23:32     ` Arianna Avanzini
2014-09-08  8:59       ` Ian Campbell
2014-09-08 22:22     ` Julien Grall
2014-09-04 16:14   ` [PATCH v2 8/9] xen: arm: support for up to 48-bit physical addressing on arm64 Ian Campbell
2014-09-08 23:28     ` Julien Grall
2014-09-04 16:14   ` [PATCH v2 9/9] xen: arm: support for up to 48-bit IPA " Ian Campbell
2014-09-08 23:42     ` Julien Grall
2014-09-08 21:05   ` [PATCH v2 1/9] xen: arm: rename p2m->first_level to p2m->root Julien Grall

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.