* [PATCH RFC 2/9] xen: arm: Implement variable levels in dump_pt_walk
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-07-30 16:40 ` Julien Grall
2014-07-30 13:47 ` [PATCH RFC 3/9] xen: arm: handle concatenated root tables " Ian Campbell
` (7 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 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>
---
xen/arch/arm/mm.c | 62 ++++++++++++++++++++++++++++----------------
xen/arch/arm/p2m.c | 4 ++-
xen/include/asm-arm/page.h | 2 +-
3 files changed, 44 insertions(+), 24 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 0a243b0..fa6a729 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,50 @@ 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, 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, };
+ int level;
- if ( first_table_offset(addr) >= LPAE_ENTRIES )
- return;
+#ifdef CONFIG_ARM_32
+ BUG_ON(root_level < 1);
+#endif
+
+ mappings[root_level] = root;
+
+ for ( level = root_level; level < 4; 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;
+ if ( !mappings[level] )
+ {
+ printk("%s: Failed to map PT page\n", level_strs[level]);
+ break;
+ }
- 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;
+ pte = mappings[level][offsets[level]];
- 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);
+ printk("%s[0x%x] = 0x%"PRIpaddr"\n",
+ level_strs[level], offsets[level], pte.bits);
+ if ( !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 */
+ for ( level = root_level + 1 ; level < 4; level++ )
+ {
+ unmap_domain_page(mappings[level]);
+ }
}
void dump_hyp_walk(vaddr_t addr)
@@ -208,7 +226,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 61958ba..64efdce 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)
@@ -64,7 +66,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..d1f8d52 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, 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] 30+ messages in thread* Re: [PATCH RFC 2/9] xen: arm: Implement variable levels in dump_pt_walk
2014-07-30 13:47 ` [PATCH RFC 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
@ 2014-07-30 16:40 ` Julien Grall
0 siblings, 0 replies; 30+ messages in thread
From: Julien Grall @ 2014-07-30 16:40 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, Ian Campbell wrote:
> ---
> xen/arch/arm/mm.c | 62 ++++++++++++++++++++++++++++----------------
> xen/arch/arm/p2m.c | 4 ++-
> xen/include/asm-arm/page.h | 2 +-
> 3 files changed, 44 insertions(+), 24 deletions(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 0a243b0..fa6a729 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,50 @@ 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, 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, };
> + int level;
>
> - if ( first_table_offset(addr) >= LPAE_ENTRIES )
> - return;
> +#ifdef CONFIG_ARM_32
> + BUG_ON(root_level < 1);
> +#endif
> +
> + mappings[root_level] = root;
> +
> + for ( level = root_level; level < 4; 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;
> + if ( !mappings[level] )
> + {
> + printk("%s: Failed to map PT page\n", level_strs[level]);
> + break;
> + }
map_domain_page can't fail. So this test is not necessary.
>
> - 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;
> + pte = mappings[level][offsets[level]];
>
> - 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);
> + printk("%s[0x%x] = 0x%"PRIpaddr"\n",
> + level_strs[level], offsets[level], pte.bits);
> + if ( !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);
> + }
On 3rd level, pte.walk.table is always equal to 1. So for valid mapping,
you are mapping one more page than necessary.
> + /* mappings[root_level] is provided by the caller */
> + for ( level = root_level + 1 ; level < 4; level++ )
> + {
> + unmap_domain_page(mappings[level]);
unmap_domain_page doesn't handle NULL pointer. So if you loop has exited
before the last mapping, Xen may hangs.
> + }
> }
>
> void dump_hyp_walk(vaddr_t addr)
> @@ -208,7 +226,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 61958ba..64efdce 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)
> @@ -64,7 +66,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);
I've just noticed that we forgot to take the p2m lock here. So the P2M
can be modified under our feet. I'm not sure what are the implications
with the dump_pt_walk.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RFC 3/9] xen: arm: handle concatenated root tables in dump_pt_walk
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
2014-07-30 13:47 ` [PATCH RFC 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-07-30 16:58 ` Julien Grall
2014-07-30 13:47 ` [PATCH RFC 4/9] xen: arm: move setup_virt_paging to p2m.c Ian Campbell
` (6 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 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>
---
xen/arch/arm/mm.c | 45 +++++++++++++++++++++++++++++++-------------
xen/arch/arm/p2m.c | 13 +++----------
xen/include/asm-arm/page.h | 15 +++++++++++++--
3 files changed, 48 insertions(+), 25 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index fa6a729..4ff783a 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -167,50 +167,69 @@ static inline void check_memory_layout_alignment_constraints(void) {
#endif
}
-void dump_pt_walk(lpae_t *root, paddr_t addr, int root_level)
+void dump_pt_walk(paddr_t ttbr, paddr_t addr, int root_level, 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, };
- int level;
+ lpae_t pte, *mapping;
+ int level, root_table;
#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
+ * concetenate 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 < 4; level++ )
{
if ( offsets[level] > LPAE_ENTRIES )
break;
- if ( !mappings[level] )
+ if ( !mapping )
{
printk("%s: Failed to map PT page\n", level_strs[level]);
break;
}
- pte = mappings[level][offsets[level]];
+ pte = mapping[offsets[level]];
printk("%s[0x%x] = 0x%"PRIpaddr"\n",
level_strs[level], offsets[level], pte.bits);
+
if ( !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 */
- for ( level = root_level + 1 ; level < 4; level++ )
- {
- unmap_domain_page(mappings[level]);
- }
+ unmap_domain_page(mapping);
}
void dump_hyp_walk(vaddr_t addr)
@@ -226,7 +245,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 64efdce..6839acf 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)
{
@@ -52,22 +53,14 @@ void p2m_dump_info(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 d1f8d52..c1c8680 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -351,8 +351,19 @@ 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, 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,
+ int root_level, 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] 30+ messages in thread* Re: [PATCH RFC 3/9] xen: arm: handle concatenated root tables in dump_pt_walk
2014-07-30 13:47 ` [PATCH RFC 3/9] xen: arm: handle concatenated root tables " Ian Campbell
@ 2014-07-30 16:58 ` Julien Grall
2014-09-04 14:40 ` Ian Campbell
0 siblings, 1 reply; 30+ messages in thread
From: Julien Grall @ 2014-07-30 16:58 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, 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
> the walker code to select the correct page to map.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/arch/arm/mm.c | 45 +++++++++++++++++++++++++++++++-------------
> xen/arch/arm/p2m.c | 13 +++----------
> xen/include/asm-arm/page.h | 15 +++++++++++++--
> 3 files changed, 48 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index fa6a729..4ff783a 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -167,50 +167,69 @@ static inline void check_memory_layout_alignment_constraints(void) {
> #endif
> }
>
> -void dump_pt_walk(lpae_t *root, paddr_t addr, int root_level)
> +void dump_pt_walk(paddr_t ttbr, paddr_t addr, int root_level, int nr_root_tables)
I should have said that yon the previous patch...
Both root_level and nr_root_tables can't be negative. I would use
unsigned int here.
> {
> 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, };
> - int level;
> + lpae_t pte, *mapping;
> + int level, root_table;
unsigned int here too.
> #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
> + * concetenate a level-0 root.
concatenate
> + */
> + 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 < 4; level++ )
> {
> if ( offsets[level] > LPAE_ENTRIES )
> break;
>
> - if ( !mappings[level] )
> + if ( !mapping )
> {
> printk("%s: Failed to map PT page\n", level_strs[level]);
> break;
> }
>
> - pte = mappings[level][offsets[level]];
> + pte = mapping[offsets[level]];
>
> printk("%s[0x%x] = 0x%"PRIpaddr"\n",
> level_strs[level], offsets[level], pte.bits);
> +
Spurious change, maybe it belong to the previous patch?
[..]
> /* Map a 4k page in a fixmap entry */
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 64efdce..6839acf 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)
> {
> @@ -52,22 +53,14 @@ void p2m_dump_info(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,
You can directly use p2m->vttbr and avoid to call page_to_maddr.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH RFC 3/9] xen: arm: handle concatenated root tables in dump_pt_walk
2014-07-30 16:58 ` Julien Grall
@ 2014-09-04 14:40 ` Ian Campbell
2014-09-08 20:54 ` Julien Grall
0 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-09-04 14:40 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, tim, vijay.kilari, xen-devel
On Wed, 2014-07-30 at 17:58 +0100, Julien Grall wrote:
> > - 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,
>
> You can directly use p2m->vttbr and avoid to call page_to_maddr.
Not quite since vttbr contains the vmid encoding.
page_to_maddr is cheap enough for a debug path I think.
Ian.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RFC 3/9] xen: arm: handle concatenated root tables in dump_pt_walk
2014-09-04 14:40 ` Ian Campbell
@ 2014-09-08 20:54 ` Julien Grall
0 siblings, 0 replies; 30+ messages in thread
From: Julien Grall @ 2014-09-08 20:54 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, tim, vijay.kilari, xen-devel
Hi Ian,
On 04/09/14 07:40, Ian Campbell wrote:
> On Wed, 2014-07-30 at 17:58 +0100, Julien Grall wrote:
>>> - 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,
>>
>> You can directly use p2m->vttbr and avoid to call page_to_maddr.
>
> Not quite since vttbr contains the vmid encoding.
>
> page_to_maddr is cheap enough for a debug path I think.
Hmmm, right. Sorry for the noise.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RFC 4/9] xen: arm: move setup_virt_paging to p2m.c
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
2014-07-30 13:47 ` [PATCH RFC 2/9] xen: arm: Implement variable levels in dump_pt_walk Ian Campbell
2014-07-30 13:47 ` [PATCH RFC 3/9] xen: arm: handle concatenated root tables " Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-07-30 17:00 ` Julien Grall
2014-07-30 13:47 ` [PATCH RFC 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Ian Campbell
` (5 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 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>
---
xen/arch/arm/mm.c | 18 ------------------
xen/arch/arm/p2m.c | 18 ++++++++++++++++++
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 4ff783a..cf0b0cf 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -414,24 +414,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 6839acf..705b29b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1059,6 +1059,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
--
1.7.10.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH RFC 4/9] xen: arm: move setup_virt_paging to p2m.c
2014-07-30 13:47 ` [PATCH RFC 4/9] xen: arm: move setup_virt_paging to p2m.c Ian Campbell
@ 2014-07-30 17:00 ` Julien Grall
0 siblings, 0 replies; 30+ messages in thread
From: Julien Grall @ 2014-07-30 17:00 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, Ian Campbell wrote:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 6839acf..705b29b 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1059,6 +1059,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
>
To consistent, I would also move the definition of setup_virt_paging
from mm.h to p2m.h.
I will let you choose on this so:
Acked-by: Julien Grall <julien.grall@linaro.org>
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RFC 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
` (2 preceding siblings ...)
2014-07-30 13:47 ` [PATCH RFC 4/9] xen: arm: move setup_virt_paging to p2m.c Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-07-30 17:11 ` Julien Grall
2014-07-30 13:47 ` [PATCH RFC 6/9] xen: arm: handle variable p2m levels in p2m_lookup Ian Campbell
` (4 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 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>
---
xen/arch/arm/p2m.c | 16 +++++++++++++---
xen/arch/arm/setup.c | 4 ++--
xen/arch/arm/smpboot.c | 2 --
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 705b29b..225d125 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1059,8 +1059,17 @@ err:
return page;
}
+static void setup_virt_paging_one(void *data)
+{
+ unsigned long val = (unsigned long)data;
+ WRITE_SYSREG32(val, VTCR_EL2);
+ isb();
+}
+
void __cpuinit 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)
@@ -1070,11 +1079,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..ddaef7e 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();
@@ -838,6 +836,8 @@ void __init start_xen(unsigned long boot_phys_offset,
do_initcalls();
+ setup_virt_paging();
+
/* Create initial domain 0. */
dom0 = domain_create(0, 0, 0);
if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
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] 30+ messages in thread* Re: [PATCH RFC 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up
2014-07-30 13:47 ` [PATCH RFC 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Ian Campbell
@ 2014-07-30 17:11 ` Julien Grall
2014-09-04 14:50 ` Ian Campbell
0 siblings, 1 reply; 30+ messages in thread
From: Julien Grall @ 2014-07-30 17:11 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, 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.
NIT: I think it misses a newline on the second line. It looks strange to
see the first line short and not the second.
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> xen/arch/arm/p2m.c | 16 +++++++++++++---
> xen/arch/arm/setup.c | 4 ++--
> xen/arch/arm/smpboot.c | 2 --
> 3 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 705b29b..225d125 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1059,8 +1059,17 @@ err:
> return page;
> }
>
> +static void setup_virt_paging_one(void *data)
I would add __init.
> +{
> + unsigned long val = (unsigned long)data;
VTCR_EL2 is a 32 bit register. I would use uint32_t for the variable type.
> + WRITE_SYSREG32(val, VTCR_EL2);
> + isb();
> +}
> +
> void __cpuinit setup_virt_paging(void)
This can become __init now.
> {
> + unsigned long val;
> +
> /* Setup Stage 2 address translation */
> /* SH0=11 (Inner-shareable)
> * ORGN0=IRGN0=01 (Normal memory, Write-Back Write-Allocate Cacheable)
> @@ -1070,11 +1079,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..ddaef7e 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();
> @@ -838,6 +836,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>
> do_initcalls();
>
> + setup_virt_paging();
> +
Assuming there is no CPU hotplug, which IIRC is not yet support, this is
only depends on the SMP bring up. Can we move this before do_initcalls?
So if we need to initialize some code that is relying on VTCR_EL2, it
will be possible.
I was mostly thinking about IOMMU drivers where a similar trick will be
necessary sooner or later.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH RFC 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up
2014-07-30 17:11 ` Julien Grall
@ 2014-09-04 14:50 ` Ian Campbell
0 siblings, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2014-09-04 14:50 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, tim, vijay.kilari, xen-devel
On Wed, 2014-07-30 at 18:11 +0100, Julien Grall wrote:
> > +{
> > + unsigned long val = (unsigned long)data;
>
> VTCR_EL2 is a 32 bit register. I would use uint32_t for the variable type.
The problem with that is:
p2m.c: In function ‘setup_virt_paging_one’:
p2m.c:1117:20: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
uint32_t val = (uint32_t)data;
p2m.c: In function ‘setup_virt_paging’:
p2m.c:1139:27: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
setup_virt_paging_one((void *)val);
The fix for that is to sprinkle additional casts to (uintptr_t) before
all the (uint32_t) and (void*) casts, which IMHO is worse than passing
an unsigned long to WRITE_SYSREG32, which is mostly harmless I think.
> > @@ -838,6 +836,8 @@ void __init start_xen(unsigned long boot_phys_offset,
> >
> > do_initcalls();
> >
> > + setup_virt_paging();
> > +
>
> Assuming there is no CPU hotplug, which IIRC is not yet support, this is
> only depends on the SMP bring up. Can we move this before do_initcalls?
> So if we need to initialize some code that is relying on VTCR_EL2, it
> will be possible.
This seems to work. I'm not really sure what we would do about CPU
hotplug yet, especially of CPUs which would require a different choice
of VTCR_EL2 value. Cross that bridge when we get to it I think.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RFC 6/9] xen: arm: handle variable p2m levels in p2m_lookup
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
` (3 preceding siblings ...)
2014-07-30 13:47 ` [PATCH RFC 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-07-31 11:14 ` Julien Grall
2014-07-30 13:47 ` [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
` (3 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 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>
---
xen/arch/arm/p2m.c | 70 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 45 insertions(+), 25 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 225d125..557663f 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -149,45 +149,69 @@ 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;
p2m_type_t _t;
+ 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
+ * concetenate a level-0 root.
+ */
+ BUG_ON(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);
+ BUG_ON(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(pte.p2m.type != p2m_invalid);
@@ -195,10 +219,6 @@ done:
*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] 30+ messages in thread* Re: [PATCH RFC 6/9] xen: arm: handle variable p2m levels in p2m_lookup
2014-07-30 13:47 ` [PATCH RFC 6/9] xen: arm: handle variable p2m levels in p2m_lookup Ian Campbell
@ 2014-07-31 11:14 ` Julien Grall
2014-09-04 14:54 ` Ian Campbell
0 siblings, 1 reply; 30+ messages in thread
From: Julien Grall @ 2014-07-31 11:14 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, 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>
> ---
> xen/arch/arm/p2m.c | 70 +++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 225d125..557663f 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -149,45 +149,69 @@ 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;
> p2m_type_t _t;
> + int level, root_table;
I would use unsigned int here.
> +
> + 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
> + * concetenate a level-0 root.
concatenate
> + */
> + BUG_ON(P2M_ROOT_LEVEL == 0);
I don't think this check is necessary in a production build (ie
debug=n). If people is porting a new board, then it should be done in
debug mode.
Hence it should be a bit "faster" as this code will be called often.
I would switch this BUG_ON to ASSERT.
> + 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);
> + BUG_ON(level == 3);
Rather than checking level == 3, I would add
BUG_ON(level < 4) at the end of loop.
The only drawback to be there is we map one more page.
And same remark as the previous BUG_ON().
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH RFC 6/9] xen: arm: handle variable p2m levels in p2m_lookup
2014-07-31 11:14 ` Julien Grall
@ 2014-09-04 14:54 ` Ian Campbell
0 siblings, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2014-09-04 14:54 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, tim, vijay.kilari, xen-devel
On Thu, 2014-07-31 at 12:14 +0100, Julien Grall wrote:
> > + 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);
> > + BUG_ON(level == 3);
>
> Rather than checking level == 3, I would add
> BUG_ON(level < 4) at the end of loop.
You mean outside/before the loop?
That doesn't work if we hit a superpage mapping, since we will exit the
loop with level < 4. I think it needs to stay where it is.
> The only drawback to be there is we map one more page.
>
> And same remark as the previous BUG_ON().
I did make it an ASSERT(level < 3) though.
Ian.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
` (4 preceding siblings ...)
2014-07-30 13:47 ` [PATCH RFC 6/9] xen: arm: handle variable p2m levels in p2m_lookup Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-07-31 15:38 ` Julien Grall
2014-08-11 7:00 ` Vijay Kilari
2014-07-30 13:47 ` [PATCH RFC 8/9] xen: arm: support for up to 48-bit physical addressing on arm64 Ian Campbell
` (2 subsequent siblings)
8 siblings, 2 replies; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 UTC (permalink / raw)
To: xen-devel
Cc: julien.grall, tim, Ian Campbell, vijay.kilari, stefano.stabellini
As with prervious 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>
---
xen/arch/arm/p2m.c | 178 +++++++++++++++++++++++++---------------------------
1 file changed, 84 insertions(+), 94 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 557663f..e9938ae 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)
@@ -115,31 +114,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.
*
@@ -707,14 +681,13 @@ static int apply_p2m_changes(struct domain *d,
int mattr,
p2m_type_t t)
{
- int rc, ret;
+ int level, 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;
- 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;
@@ -726,9 +699,27 @@ static int apply_p2m_changes(struct domain *d,
spin_lock(&p2m->lock);
+ if ( P2M_ROOT_PAGES == 1 )
+ {
+ /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
+ mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+ }
+
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
@@ -748,73 +739,71 @@ 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 concetenate
+ * a level-0 root.
+ */
+ BUG_ON(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 */
-
- ret = apply_one_level(d, &first[first_table_offset(addr)],
- 1, 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 */
-
- ret = apply_one_level(d,&second[second_table_offset(addr)],
- 2, 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 */
}
-
- ret = apply_one_level(d, &third[third_table_offset(addr)],
- 3, 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 )
@@ -838,10 +827,11 @@ 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);
-
+ 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] 30+ messages in thread* Re: [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
2014-07-30 13:47 ` [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
@ 2014-07-31 15:38 ` Julien Grall
2014-08-11 7:00 ` Vijay Kilari
1 sibling, 0 replies; 30+ messages in thread
From: Julien Grall @ 2014-07-31 15:38 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, Ian Campbell wrote:
> As with prervious changes this involves conversion from a linear series of
previous
[..]
> @@ -726,9 +699,27 @@ static int apply_p2m_changes(struct domain *d,
>
> spin_lock(&p2m->lock);
>
> + if ( P2M_ROOT_PAGES == 1 )
> + {
> + /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
> + mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
> + {
> + rc = -EINVAL;
> + goto out;
> + }
> + }
> +
> 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
> @@ -748,73 +739,71 @@ 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 concetenate
concatenate
> + * a level-0 root.
> + */
> + BUG_ON(P2M_ROOT_LEVEL == 0);
My remark on #6 can be applied here. You are checking this every time
you loop rather that it's only set once at Xen boot.
If a developer decides to change those value after boot we will be in
trouble in other places.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
2014-07-30 13:47 ` [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
2014-07-31 15:38 ` Julien Grall
@ 2014-08-11 7:00 ` Vijay Kilari
2014-08-26 9:11 ` Vijay Kilari
1 sibling, 1 reply; 30+ messages in thread
From: Vijay Kilari @ 2014-08-11 7:00 UTC (permalink / raw)
To: Ian Campbell
Cc: Stefano Stabellini, Tim Deegan, Julien Grall,
xen-devel@lists.xen.org
On Wed, Jul 30, 2014 at 7:17 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> As with prervious 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>
> ---
> xen/arch/arm/p2m.c | 178 +++++++++++++++++++++++++---------------------------
> 1 file changed, 84 insertions(+), 94 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 557663f..e9938ae 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)
> @@ -115,31 +114,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.
> *
> @@ -707,14 +681,13 @@ static int apply_p2m_changes(struct domain *d,
> int mattr,
> p2m_type_t t)
> {
> - int rc, ret;
> + int level, 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;
> - 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;
>
> @@ -726,9 +699,27 @@ static int apply_p2m_changes(struct domain *d,
>
> spin_lock(&p2m->lock);
>
> + if ( P2M_ROOT_PAGES == 1 )
> + {
> + /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
> + mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
> + {
> + rc = -EINVAL;
In case of 48-bit platform in pa_range_info[] structure in
setup_virt_paging()
function, the root-order is set to 0. So for 48-bit platform P2M_ROOT_PAGES = 1
and hence Dom0 mapping always fails with below log
(XEN) P2M: 48-bit IPA with 48-bit PA
(XEN) P2M: 4 levels with order-0 root, VTCR 0x80053590
(XEN) *** LOADING DOMAIN 0 ***
(XEN) Loading kernel from boot module @ 0000000080080000
(XEN) Allocating 1:1 mappings totalling 128MB for dom0:
(XEN) Allocated 0x000000f0000000-0x000000f8000000 (128MB/128MB, order 15)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Failed map pages to DOM0: -22
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...
Regards
Vijay
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
2014-08-11 7:00 ` Vijay Kilari
@ 2014-08-26 9:11 ` Vijay Kilari
2014-09-04 14:01 ` Ian Campbell
0 siblings, 1 reply; 30+ messages in thread
From: Vijay Kilari @ 2014-08-26 9:11 UTC (permalink / raw)
To: Ian Campbell
Cc: Stefano Stabellini, Tim Deegan, Julien Grall,
xen-devel@lists.xen.org
On Mon, Aug 11, 2014 at 12:30 PM, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Wed, Jul 30, 2014 at 7:17 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
>> As with prervious 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>
>> ---
>> xen/arch/arm/p2m.c | 178 +++++++++++++++++++++++++---------------------------
>> 1 file changed, 84 insertions(+), 94 deletions(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index 557663f..e9938ae 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)
>> @@ -115,31 +114,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.
>> *
>> @@ -707,14 +681,13 @@ static int apply_p2m_changes(struct domain *d,
>> int mattr,
>> p2m_type_t t)
>> {
>> - int rc, ret;
>> + int level, 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;
>> - 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;
>>
>> @@ -726,9 +699,27 @@ static int apply_p2m_changes(struct domain *d,
>>
>> spin_lock(&p2m->lock);
>>
>> + if ( P2M_ROOT_PAGES == 1 )
>> + {
>> + /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
>> + mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
>> + {
>> + rc = -EINVAL;
Commenting below lines of code, where forcibly jumping to out if
P2M_ROOT_PAGES==1
I could boot my system with 48-bit PA
mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
/*{
printk("In %s line %d\n",__func__, __LINE__);
rc = -EINVAL;
goto out;
}*/
>
> In case of 48-bit platform in pa_range_info[] structure in
> setup_virt_paging()
> function, the root-order is set to 0. So for 48-bit platform P2M_ROOT_PAGES = 1
> and hence Dom0 mapping always fails with below log
>
> (XEN) P2M: 48-bit IPA with 48-bit PA
> (XEN) P2M: 4 levels with order-0 root, VTCR 0x80053590
> (XEN) *** LOADING DOMAIN 0 ***
> (XEN) Loading kernel from boot module @ 0000000080080000
> (XEN) Allocating 1:1 mappings totalling 128MB for dom0:
> (XEN) Allocated 0x000000f0000000-0x000000f8000000 (128MB/128MB, order 15)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Failed map pages to DOM0: -22
> (XEN) ****************************************
> (XEN)
> (XEN) Reboot in five seconds...
>
> Regards
> Vijay
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes
2014-08-26 9:11 ` Vijay Kilari
@ 2014-09-04 14:01 ` Ian Campbell
0 siblings, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2014-09-04 14:01 UTC (permalink / raw)
To: Vijay Kilari
Cc: Stefano Stabellini, Tim Deegan, Julien Grall,
xen-devel@lists.xen.org
(sorry for the delay replying)
On Tue, 2014-08-26 at 14:41 +0530, Vijay Kilari wrote:
> On Mon, Aug 11, 2014 at 12:30 PM, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> > On Wed, Jul 30, 2014 at 7:17 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> >> @@ -726,9 +699,27 @@ static int apply_p2m_changes(struct domain *d,
> >>
> >> spin_lock(&p2m->lock);
> >>
> >> + if ( P2M_ROOT_PAGES == 1 )
> >> + {
> >> + /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
> >> + mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
> >> + {
> >> + rc = -EINVAL;
>
> Commenting below lines of code, where forcibly jumping to out if
> P2M_ROOT_PAGES==1
> I could boot my system with 48-bit PA
>
> mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
> /*{
> printk("In %s line %d\n",__func__, __LINE__);
> rc = -EINVAL;
> goto out;
> }*/
Yeah, that block was totally bogus, not sure what I was thinking!
I suspect I meant to put an "if (!mappings[P2M_ROOT_LEVEL])" before it
but forget (or lost it during a rebase somehow), but as Julien points
out elsewhere map_domain_page cannot fail, so I've just nuked it like
you did.
Ian.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RFC 8/9] xen: arm: support for up to 48-bit physical addressing on arm64
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
` (5 preceding siblings ...)
2014-07-30 13:47 ` [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-08-07 15:33 ` Julien Grall
2014-07-30 13:47 ` [PATCH RFC 9/9] xen: arm: support for up to 48-bit IPA " Ian Campbell
2014-07-30 16:06 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Julien Grall
8 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 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>
---
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 | 45 ++++++++++++++++++++++++++++++++++++++-
4 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index ec57974..8fbaf25 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 dcb7071..28e3e77 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 c1c8680..43a1ee9 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 9d230f3..979a41d 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -84,6 +84,38 @@
#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)
+
+#define TCR_PS(x) ((x)<<16)
+
+#define TCR_TBI (_AC(0x1,UL)<<20)
+
+#ifdef CONFIG_ARM_64
+#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 */
@@ -188,8 +220,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] 30+ messages in thread* Re: [PATCH RFC 8/9] xen: arm: support for up to 48-bit physical addressing on arm64
2014-07-30 13:47 ` [PATCH RFC 8/9] xen: arm: support for up to 48-bit physical addressing on arm64 Ian Campbell
@ 2014-08-07 15:33 ` Julien Grall
0 siblings, 0 replies; 30+ messages in thread
From: Julien Grall @ 2014-08-07 15:33 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, Ian Campbell wrote:
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 9d230f3..979a41d 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -84,6 +84,38 @@
> #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)
> +
> +#define TCR_PS(x) ((x)<<16)
> +
> +#define TCR_TBI (_AC(0x1,UL)<<20)
TCR_PS and TCR_TBI are armv8 specific. I would only protect them by
#ifdef CONFIG_ARM_64
The rest of the patch looks good to me.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RFC 9/9] xen: arm: support for up to 48-bit IPA addressing on arm64
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
` (6 preceding siblings ...)
2014-07-30 13:47 ` [PATCH RFC 8/9] xen: arm: support for up to 48-bit physical addressing on arm64 Ian Campbell
@ 2014-07-30 13:47 ` Ian Campbell
2014-08-07 15:49 ` Julien Grall
2014-07-30 16:06 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Julien Grall
8 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 13:47 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 addreses. Therefore support for 48-bit addressing requres
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 synbolic 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>
---
xen/arch/arm/p2m.c | 82 +++++++++++++++++++++++++++++++--------
xen/include/asm-arm/p2m.h | 2 +-
xen/include/asm-arm/processor.h | 28 +++++++++++++
3 files changed, 94 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index e9938ae..0e8e8e0 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 int __read_mostly p2m_root_order;
+static 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)
@@ -164,6 +171,8 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
map = __map_domain_page(p2m->root + root_table);
+ BUG_ON(P2M_ROOT_LEVEL >= 4);
+
for ( level = P2M_ROOT_LEVEL ; level < 4 ; level++ )
{
mask = masks[level];
@@ -883,6 +892,7 @@ int p2m_alloc_table(struct domain *d)
{
struct p2m_domain *p2m = &d->arch.p2m;
struct page_info *page;
+ int i;
page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0);
if ( page == NULL )
@@ -891,8 +901,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;
@@ -1078,21 +1088,59 @@ static void setup_virt_paging_one(void *data)
void __cpuinit 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 pabits; /* Physical Address Size */
+ unsigned t0sz; /* Desired T0SZ, minimum in comment */
+ unsigned root_order; /* Page order of the root of the p2m */
+ unsigned 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 */
+ };
+
+ int cpu;
+ 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);
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 cfd0c52..a0cf2f4 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -13,7 +13,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 979a41d..a744a67 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -116,6 +116,34 @@
#define TCR_RES1 (_AC(1,UL)<<31)
#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)
+
+#define VTCR_TG0_4K (_AC(0x0,UL)<<14) /* arm64 only */
+#define VTCR_TG0_64K (_AC(0x1,UL)<<14)
+#define VTCR_TG0_16K (_AC(0x2,UL)<<14)
+
+#define VTCR_PS(x) ((x)<<16)
+
+#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] 30+ messages in thread* Re: [PATCH RFC 9/9] xen: arm: support for up to 48-bit IPA addressing on arm64
2014-07-30 13:47 ` [PATCH RFC 9/9] xen: arm: support for up to 48-bit IPA " Ian Campbell
@ 2014-08-07 15:49 ` Julien Grall
0 siblings, 0 replies; 30+ messages in thread
From: Julien Grall @ 2014-08-07 15:49 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, vijay.kilari, stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, Ian Campbell wrote:
> 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 addreses. Therefore support for 48-bit addressing requres
s/addreses/addresses/
s/requres/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 synbolic names for the VTCR_EL2 bits as we go improves the clarity
s/synbolic/symbolic/
> 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>
> ---
> xen/arch/arm/p2m.c | 82 +++++++++++++++++++++++++++++++--------
> xen/include/asm-arm/p2m.h | 2 +-
> xen/include/asm-arm/processor.h | 28 +++++++++++++
> 3 files changed, 94 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index e9938ae..0e8e8e0 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 int __read_mostly p2m_root_order;
unsigned int? It might even be better to use uint8_t as we won't never
have an order greater than 256. Even though I'm not sure if it will
improve performance.
> +static int __read_mostly p2m_root_level;
same here.
> +#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)
> @@ -164,6 +171,8 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
>
> map = __map_domain_page(p2m->root + root_table);
>
> + BUG_ON(P2M_ROOT_LEVEL >= 4);
> +
For ARM64, P2M_ROOT_LEVEL is set up once at startup and AFAIU should not
change.
Using BUG_ON seem excessive here. If you want to keep a test, I would
use ASSERT to avoid impacting hypervisor in production mode.
> for ( level = P2M_ROOT_LEVEL ; level < 4 ; level++ )
> {
> mask = masks[level];
> @@ -883,6 +892,7 @@ int p2m_alloc_table(struct domain *d)
> {
> struct p2m_domain *p2m = &d->arch.p2m;
> struct page_info *page;
> + int i;
unsigned int.
[..]
> #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 pabits; /* Physical Address Size */
missing the unsigned int by mistake? Or maybe uint8_t.
> + unsigned t0sz; /* Desired T0SZ, minimum in comment */
> + unsigned root_order; /* Page order of the root of the p2m */
> + unsigned 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 */
> + };
> +
> + int cpu;
unsigned int
> + int pa_range = 0x10; /* Larger than any possible value */
same here.
> +
> + 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 )
I think a space is missing around & for clarity.
> + 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;
Following my comment on BUG_ON earlier, I think you should check that we
correctly support the values set in p2m_root_{order,level} and bail out
if necessary.
[..]
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 979a41d..a744a67 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -116,6 +116,34 @@
> #define TCR_RES1 (_AC(1,UL)<<31)
> #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)
> +
> +#define VTCR_TG0_4K (_AC(0x0,UL)<<14) /* arm64 only */
> +#define VTCR_TG0_64K (_AC(0x1,UL)<<14)
> +#define VTCR_TG0_16K (_AC(0x2,UL)<<14)
> +
> +#define VTCR_PS(x) ((x)<<16)
All the define from the "/* arm64 only */" up to here are arm64 only.
With the comment at the end of the line it's no clear that it's actually
the case. It might be worse to add an #ifdef CONFIG_ARM_64
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root.
2014-07-30 13:47 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Ian Campbell
` (7 preceding siblings ...)
2014-07-30 13:47 ` [PATCH RFC 9/9] xen: arm: support for up to 48-bit IPA " Ian Campbell
@ 2014-07-30 16:06 ` Julien Grall
2014-07-30 16:19 ` Ian Campbell
8 siblings, 1 reply; 30+ messages in thread
From: Julien Grall @ 2014-07-30 16:06 UTC (permalink / raw)
To: Ian Campbell, xen-devel
Cc: Vijaya Kumar K, Ian Campbell, tim, vijay.kilari,
stefano.stabellini
Hi Ian,
On 07/30/2014 02:47 PM, 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 also switched from ->root_level to just ->root.
The last sentence doesn't seem to be relevant. At least I don't find any
such change in the patch.
Regards,
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
> 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 143199b..61958ba 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)
> {
> @@ -61,9 +61,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);
> }
> @@ -137,10 +137,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);
> }
> @@ -879,7 +879,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;
>
> @@ -889,9 +889,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
> @@ -968,9 +968,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);
>
> @@ -994,7 +994,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 06c93a0..cfd0c52 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -14,7 +14,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;
>
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root.
2014-07-30 16:06 ` [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root Julien Grall
@ 2014-07-30 16:19 ` Ian Campbell
2014-07-30 16:23 ` Julien Grall
0 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2014-07-30 16:19 UTC (permalink / raw)
To: Julien Grall
Cc: vijay.kilari, stefano.stabellini, Vijaya Kumar K, Ian Campbell,
tim, xen-devel
On Wed, 2014-07-30 at 17:06 +0100, Julien Grall wrote:
> Hi Ian,
>
> On 07/30/2014 02:47 PM, 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 also switched from ->root_level to just ->root.
>
> The last sentence doesn't seem to be relevant. At least I don't find any
> such change in the patch.
What I meant is that rather than switching from ->first_level to
->root_level (the "most trivial" change) I switched to just ->root.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RFC 1/9] xen: arm: rename p2m->first_level to p2m->root.
2014-07-30 16:19 ` Ian Campbell
@ 2014-07-30 16:23 ` Julien Grall
0 siblings, 0 replies; 30+ messages in thread
From: Julien Grall @ 2014-07-30 16:23 UTC (permalink / raw)
To: Ian Campbell
Cc: vijay.kilari, stefano.stabellini, Vijaya Kumar K, Ian Campbell,
tim, xen-devel
On 07/30/2014 05:19 PM, Ian Campbell wrote:
> On Wed, 2014-07-30 at 17:06 +0100, Julien Grall wrote:
>> Hi Ian,
>>
>> On 07/30/2014 02:47 PM, 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 also switched from ->root_level to just ->root.
>>
>> The last sentence doesn't seem to be relevant. At least I don't find any
>> such change in the patch.
>
> What I meant is that rather than switching from ->first_level to
> ->root_level (the "most trivial" change) I switched to just ->root.
It's not clear in the commit message as the title is "rename
p2m->first_level to p2m->root".
IHMO, the sentence seems to come out of nowhere.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 30+ messages in thread