[PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Ian Campbell]

This allows us to support sparse physical address maps which we previously
could not because the frametable would end up taking up an enourmous fraction
of RAM.

On a fast model which has RAM at 0x80000000-0x100000000 and
0x880000000-0x900000000 this reduces the size of the frametable from
478M to 84M.

This is very much a WIP. Things left to do:
  - Make it work for arm32 (completely broken)
  - Should xenheap_mfn_start/frametable_base_pdx be an (optional?) part of the
    common infra?
  - Sort out the semtantics of mfn/pdx vs frame table base, I suspect these are
    a bit confused right now...
  - Probably lots of bugs...

Posting because I'm going to be away but it will hopefully be useful for the
UEFI and grub+multiboot work being done by Linaro.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/Rules.mk        |    1 +
 xen/arch/arm/mm.c            |   18 +++++++------
 xen/arch/arm/setup.c         |   58 ++++++++++++++++++++++++++++++++----------
 xen/include/asm-arm/config.h |    4 ++-
 xen/include/asm-arm/mm.h     |   31 +++++++++++-----------
 xen/include/asm-arm/numa.h   |    2 +-
 6 files changed, 76 insertions(+), 38 deletions(-)

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 8658176..26fafa2 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -10,6 +10,7 @@ HAS_DEVICE_TREE := y
 HAS_VIDEO := y
 HAS_ARM_HDLCD := y
 HAS_PASSTHROUGH := y
+HAS_PDX := y
 
 CFLAGS += -I$(BASEDIR)/include
 
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index fa6a729..bc399c0 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -140,7 +140,7 @@ unsigned long xenheap_mfn_start __read_mostly = ~0UL;
 unsigned long xenheap_mfn_end __read_mostly;
 unsigned long xenheap_virt_end __read_mostly;
 
-unsigned long frametable_base_mfn __read_mostly;
+unsigned long frametable_base_pdx __read_mostly;
 unsigned long frametable_virt_end __read_mostly;
 
 unsigned long max_page;
@@ -683,7 +683,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
     /* Align to previous 1GB boundary */
     base_mfn &= ~((FIRST_SIZE>>PAGE_SHIFT)-1);
 
-    offset = base_mfn - xenheap_mfn_start;
+    offset = pfn_to_pdx(base_mfn - xenheap_mfn_start);
     vaddr = DIRECTMAP_VIRT_START + offset*PAGE_SIZE;
 
     while ( base_mfn < end_mfn )
@@ -734,7 +734,8 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
 void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
 {
     unsigned long nr_pages = (pe - ps) >> PAGE_SHIFT;
-    unsigned long frametable_size = nr_pages * sizeof(struct page_info);
+    unsigned long nr_pdxs = pfn_to_pdx(nr_pages);
+    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
     unsigned long base_mfn;
 #ifdef CONFIG_ARM_64
     lpae_t *second, pte;
@@ -742,7 +743,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     int i;
 #endif
 
-    frametable_base_mfn = ps >> PAGE_SHIFT;
+    frametable_base_pdx = pfn_to_pdx(ps >> PAGE_SHIFT);
 
     /* Round up to 32M boundary */
     frametable_size = (frametable_size + 0x1ffffff) & ~0x1ffffff;
@@ -763,11 +764,12 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     create_32mb_mappings(xen_second, FRAMETABLE_VIRT_START, base_mfn, frametable_size >> PAGE_SHIFT);
 #endif
 
-    memset(&frame_table[0], 0, nr_pages * sizeof(struct page_info));
-    memset(&frame_table[nr_pages], -1,
-           frametable_size - (nr_pages * sizeof(struct page_info)));
+    memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
+    memset(&frame_table[nr_pdxs], -1,
+           frametable_size - (nr_pdxs * sizeof(struct page_info)));
+
+    frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pdxs * sizeof(struct page_info));
 
-    frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pages * sizeof(struct page_info));
 }
 
 void *__init arch_vmap_virt_end(void)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 446b4dc..dbd92bb 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -423,6 +423,46 @@ static paddr_t __init get_xen_paddr(void)
     return paddr;
 }
 
+/* Sets all bits from the most-significant 1-bit down to the LSB */
+static u64 __init fill_mask(u64 mask)
+{
+    while (mask & (mask + 1))
+        mask |= mask + 1;
+    return mask;
+}
+
+static void init_pdx(void)
+{
+    u64 mask = fill_mask(bootinfo.mem.bank[0].start - 1);
+    int bank;
+
+    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
+    {
+        paddr_t bank_start = bootinfo.mem.bank[bank].start;
+        paddr_t bank_size = bootinfo.mem.bank[bank].size;
+        paddr_t bank_end = bank_start + bank_size;
+
+        set_pdx_range(paddr_to_pfn(bank_start),
+                      paddr_to_pfn(bank_end));
+
+
+        mask |= bank_start | fill_mask(bank_start ^ (bank_end - 1));
+    }
+
+    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
+    {
+        paddr_t bank_start = bootinfo.mem.bank[bank].start;
+        paddr_t bank_size = bootinfo.mem.bank[bank].size;
+        paddr_t bank_end = bank_start + bank_size;
+
+        if (~mask &
+            fill_mask(bank_start ^ (bank_end - 1)))
+            mask = 0;
+    }
+
+    pfn_pdx_hole_setup(mask >> PAGE_SHIFT);
+}
+
 #ifdef CONFIG_ARM_32
 static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
 {
@@ -439,6 +479,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     if ( !bootinfo.mem.nr_banks )
         panic("No memory bank");
 
+    init_pdx();
+
     /*
      * We are going to accumulate two regions here.
      *
@@ -468,6 +510,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
         paddr_t new_ram_start = min(ram_start,bank_start);
         paddr_t new_ram_end = max(ram_end,bank_end);
 
+#error ARM32 not converted to PDX yet
         /*
          * If the new bank is contiguous with the initial contiguous
          * region then incorporate it into the contiguous region.
@@ -616,6 +659,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     unsigned long dtb_pages;
     void *fdt;
 
+    init_pdx();
+
     total_pages = 0;
     for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
     {
@@ -628,19 +673,6 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
         paddr_t new_ram_start = min(ram_start,bank_start);
         paddr_t new_ram_end = max(ram_end,bank_end);
 
-        /*
-         * We allow non-contigious regions so long as at least half of
-         * the total RAM region actually contains RAM. We actually
-         * fudge this slightly and require that adding the current
-         * bank does not cause us to violate this restriction.
-         *
-         * This restriction ensures that the frametable (which is not
-         * currently sparse) does not consume all available RAM.
-         */
-        if ( bank > 0 && 2 * new_ram_size < new_ram_end - new_ram_start )
-            /* Would create memory map which is too sparse, so stop here. */
-            break;
-
         ram_start = new_ram_start;
         ram_end = new_ram_end;
         ram_size = new_ram_size;
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 1c3abcf..e525c7f 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -149,7 +149,9 @@
 #define VMAP_VIRT_END    (VMAP_VIRT_START + GB(1) - 1)
 
 #define FRAMETABLE_VIRT_START  GB(32)
-#define FRAMETABLE_VIRT_END    (FRAMETABLE_VIRT_START + GB(32) - 1)
+#define FRAMETABLE_SIZE        GB(32)
+#define FRAMETABLE_NR          (FRAMETABLE_SIZE / sizeof(*frame_table))
+#define FRAMETABLE_VIRT_END    (FRAMETABLE_VIRT_START + FRAMETABLE_SIZE - 1)
 
 #define DIRECTMAP_VIRT_START   SLOT0(256)
 #define DIRECTMAP_SIZE         (SLOT0_ENTRY_SIZE * (265-256))
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 9fa80a4..77901f7 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -6,6 +6,7 @@
 #include <asm/page.h>
 #include <public/xen.h>
 #include <xen/domain_page.h>
+#include <xen/pdx.h>
 
 /* Align Xen to a 2 MiB boundary. */
 #define XEN_PADDR_ALIGN (1 << 21)
@@ -140,12 +141,14 @@ extern void share_xen_page_with_privileged_guests(
     struct page_info *page, int readonly);
 
 #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
-/* MFN of the first page in the frame table. */
-extern unsigned long frametable_base_mfn;
+/* PDX of the first page in the frame table. */
+extern unsigned long frametable_base_pdx;
 
 extern unsigned long max_page;
 extern unsigned long total_pages;
 
+#define PDX_GROUP_SHIFT SECOND_SHIFT
+
 /* Boot-time pagetable setup */
 extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr);
 /* Remove early mappings */
@@ -184,20 +187,15 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len)
     return ioremap_attr(start, len, PAGE_HYPERVISOR_WC);
 }
 
+/* XXX -- account for base */
 #define mfn_valid(mfn)        ({                                              \
     unsigned long __m_f_n = (mfn);                                            \
-    likely(__m_f_n >= frametable_base_mfn && __m_f_n < max_page);             \
+    likely(pfn_to_pdx(__m_f_n) >= frametable_base_pdx && __mfn_valid(__m_f_n)); \
 })
 
-#define max_pdx                 max_page
-#define pfn_to_pdx(pfn)         (pfn)
-#define pdx_to_pfn(pdx)         (pdx)
-#define virt_to_pdx(va)         virt_to_mfn(va)
-#define pdx_to_virt(pdx)        mfn_to_virt(pdx)
-
 /* Convert between machine frame numbers and page-info structures. */
-#define mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) - frametable_base_mfn))
-#define page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table) + frametable_base_mfn)
+#define mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) - frametable_base_pdx))
+#define page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table) + frametable_base_pdx)
 #define __page_to_mfn(pg)  page_to_mfn(pg)
 #define __mfn_to_page(mfn) mfn_to_page(mfn)
 
@@ -221,6 +219,7 @@ static inline paddr_t __virt_to_maddr(vaddr_t va)
 #define virt_to_maddr(va)   __virt_to_maddr((vaddr_t)(va))
 
 #ifdef CONFIG_ARM_32
+#error ARM32 not converted to PDX yet
 static inline void *maddr_to_virt(paddr_t ma)
 {
     ASSERT(is_xen_heap_mfn(ma >> PAGE_SHIFT));
@@ -230,9 +229,11 @@ static inline void *maddr_to_virt(paddr_t ma)
 #else
 static inline void *maddr_to_virt(paddr_t ma)
 {
-    ASSERT((ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
-    ma -= pfn_to_paddr(xenheap_mfn_start);
-    return (void *)(unsigned long) ma + DIRECTMAP_VIRT_START;
+    ASSERT(pfn_to_pdx(ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
+    return (void *)(DIRECTMAP_VIRT_START -
+                    pfn_to_paddr(xenheap_mfn_start) +
+                    ((ma & ma_va_bottom_mask) |
+                     ((ma & ma_top_mask) >> pfn_pdx_hole_shift)));
 }
 #endif
 
@@ -264,7 +265,7 @@ static inline struct page_info *virt_to_page(const void *v)
     return frame_table
         + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT)
         + xenheap_mfn_start
-        - frametable_base_mfn;
+        - frametable_base_pdx;
 }
 
 static inline void *page_to_virt(const struct page_info *pg)
diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
index 2c019d7..06a9d5a 100644
--- a/xen/include/asm-arm/numa.h
+++ b/xen/include/asm-arm/numa.h
@@ -12,7 +12,7 @@ static inline __attribute__((pure)) int phys_to_nid(paddr_t addr)
 
 /* XXX: implement NUMA support */
 #define node_spanned_pages(nid) (total_pages)
-#define node_start_pfn(nid) (frametable_base_mfn)
+#define node_start_pfn(nid) (pdx_to_pfn(frametable_base_pdx))
 #define __node_distance(a, b) (20)
 
 #endif /* __ARCH_ARM_NUMA_H */
-- 
1.7.10.4

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Roy Franz]

[-- Attachment #1.1: Type: text/plain, Size: 12519 bytes --]

On Thu, Jul 31, 2014 at 4:01 AM, Ian Campbell <ian.campbell@citrix.com>
wrote:

> This allows us to support sparse physical address maps which we previously
> could not because the frametable would end up taking up an enourmous
> fraction
> of RAM.
>
> On a fast model which has RAM at 0x80000000-0x100000000 and
> 0x880000000-0x900000000 this reduces the size of the frametable from
> 478M to 84M.
>
> This is very much a WIP. Things left to do:
>   - Make it work for arm32 (completely broken)
>   - Should xenheap_mfn_start/frametable_base_pdx be an (optional?) part of
> the
>     common infra?
>   - Sort out the semtantics of mfn/pdx vs frame table base, I suspect
> these are
>     a bit confused right now...
>   - Probably lots of bugs...
>
> Posting because I'm going to be away but it will hopefully be useful for
> the
> UEFI and grub+multiboot work being done by Linaro.
>

This works for me on the foundation model - it now looks like XEN is using
all of the memory, and there are no obvious problems.


>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/Rules.mk        |    1 +
>  xen/arch/arm/mm.c            |   18 +++++++------
>  xen/arch/arm/setup.c         |   58
> ++++++++++++++++++++++++++++++++----------
>  xen/include/asm-arm/config.h |    4 ++-
>  xen/include/asm-arm/mm.h     |   31 +++++++++++-----------
>  xen/include/asm-arm/numa.h   |    2 +-
>  6 files changed, 76 insertions(+), 38 deletions(-)
>
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 8658176..26fafa2 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -10,6 +10,7 @@ HAS_DEVICE_TREE := y
>  HAS_VIDEO := y
>  HAS_ARM_HDLCD := y
>  HAS_PASSTHROUGH := y
> +HAS_PDX := y
>
>  CFLAGS += -I$(BASEDIR)/include
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index fa6a729..bc399c0 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -140,7 +140,7 @@ unsigned long xenheap_mfn_start __read_mostly = ~0UL;
>  unsigned long xenheap_mfn_end __read_mostly;
>  unsigned long xenheap_virt_end __read_mostly;
>
> -unsigned long frametable_base_mfn __read_mostly;
> +unsigned long frametable_base_pdx __read_mostly;
>  unsigned long frametable_virt_end __read_mostly;
>
>  unsigned long max_page;
> @@ -683,7 +683,7 @@ void __init setup_xenheap_mappings(unsigned long
> base_mfn,
>      /* Align to previous 1GB boundary */
>      base_mfn &= ~((FIRST_SIZE>>PAGE_SHIFT)-1);
>
> -    offset = base_mfn - xenheap_mfn_start;
> +    offset = pfn_to_pdx(base_mfn - xenheap_mfn_start);
>      vaddr = DIRECTMAP_VIRT_START + offset*PAGE_SIZE;
>
>      while ( base_mfn < end_mfn )
> @@ -734,7 +734,8 @@ void __init setup_xenheap_mappings(unsigned long
> base_mfn,
>  void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>  {
>      unsigned long nr_pages = (pe - ps) >> PAGE_SHIFT;
> -    unsigned long frametable_size = nr_pages * sizeof(struct page_info);
> +    unsigned long nr_pdxs = pfn_to_pdx(nr_pages);
> +    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
>      unsigned long base_mfn;
>  #ifdef CONFIG_ARM_64
>      lpae_t *second, pte;
> @@ -742,7 +743,7 @@ void __init setup_frametable_mappings(paddr_t ps,
> paddr_t pe)
>      int i;
>  #endif
>
> -    frametable_base_mfn = ps >> PAGE_SHIFT;
> +    frametable_base_pdx = pfn_to_pdx(ps >> PAGE_SHIFT);
>
>      /* Round up to 32M boundary */
>      frametable_size = (frametable_size + 0x1ffffff) & ~0x1ffffff;
> @@ -763,11 +764,12 @@ void __init setup_frametable_mappings(paddr_t ps,
> paddr_t pe)
>      create_32mb_mappings(xen_second, FRAMETABLE_VIRT_START, base_mfn,
> frametable_size >> PAGE_SHIFT);
>  #endif
>
> -    memset(&frame_table[0], 0, nr_pages * sizeof(struct page_info));
> -    memset(&frame_table[nr_pages], -1,
> -           frametable_size - (nr_pages * sizeof(struct page_info)));
> +    memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
> +    memset(&frame_table[nr_pdxs], -1,
> +           frametable_size - (nr_pdxs * sizeof(struct page_info)));
> +
> +    frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pdxs *
> sizeof(struct page_info));
>
> -    frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pages *
> sizeof(struct page_info));
>  }
>
>  void *__init arch_vmap_virt_end(void)
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 446b4dc..dbd92bb 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -423,6 +423,46 @@ static paddr_t __init get_xen_paddr(void)
>      return paddr;
>  }
>
> +/* Sets all bits from the most-significant 1-bit down to the LSB */
> +static u64 __init fill_mask(u64 mask)
> +{
> +    while (mask & (mask + 1))
> +        mask |= mask + 1;
> +    return mask;
> +}
> +
> +static void init_pdx(void)
> +{
> +    u64 mask = fill_mask(bootinfo.mem.bank[0].start - 1);
> +    int bank;
> +
> +    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> +    {
> +        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> +        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> +        paddr_t bank_end = bank_start + bank_size;
> +
> +        set_pdx_range(paddr_to_pfn(bank_start),
> +                      paddr_to_pfn(bank_end));
> +
> +
> +        mask |= bank_start | fill_mask(bank_start ^ (bank_end - 1));
> +    }
> +
> +    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> +    {
> +        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> +        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> +        paddr_t bank_end = bank_start + bank_size;
> +
> +        if (~mask &
> +            fill_mask(bank_start ^ (bank_end - 1)))
> +            mask = 0;
> +    }
> +
> +    pfn_pdx_hole_setup(mask >> PAGE_SHIFT);
> +}
> +
>  #ifdef CONFIG_ARM_32
>  static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
>  {
> @@ -439,6 +479,8 @@ static void __init setup_mm(unsigned long dtb_paddr,
> size_t dtb_size)
>      if ( !bootinfo.mem.nr_banks )
>          panic("No memory bank");
>
> +    init_pdx();
> +
>      /*
>       * We are going to accumulate two regions here.
>       *
> @@ -468,6 +510,7 @@ static void __init setup_mm(unsigned long dtb_paddr,
> size_t dtb_size)
>          paddr_t new_ram_start = min(ram_start,bank_start);
>          paddr_t new_ram_end = max(ram_end,bank_end);
>
> +#error ARM32 not converted to PDX yet
>          /*
>           * If the new bank is contiguous with the initial contiguous
>           * region then incorporate it into the contiguous region.
> @@ -616,6 +659,8 @@ static void __init setup_mm(unsigned long dtb_paddr,
> size_t dtb_size)
>      unsigned long dtb_pages;
>      void *fdt;
>
> +    init_pdx();
> +
>      total_pages = 0;
>      for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
>      {
> @@ -628,19 +673,6 @@ static void __init setup_mm(unsigned long dtb_paddr,
> size_t dtb_size)
>          paddr_t new_ram_start = min(ram_start,bank_start);
>          paddr_t new_ram_end = max(ram_end,bank_end);
>
> -        /*
> -         * We allow non-contigious regions so long as at least half of
> -         * the total RAM region actually contains RAM. We actually
> -         * fudge this slightly and require that adding the current
> -         * bank does not cause us to violate this restriction.
> -         *
> -         * This restriction ensures that the frametable (which is not
> -         * currently sparse) does not consume all available RAM.
> -         */
> -        if ( bank > 0 && 2 * new_ram_size < new_ram_end - new_ram_start )
> -            /* Would create memory map which is too sparse, so stop here.
> */
> -            break;
> -
>          ram_start = new_ram_start;
>          ram_end = new_ram_end;
>          ram_size = new_ram_size;
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 1c3abcf..e525c7f 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -149,7 +149,9 @@
>  #define VMAP_VIRT_END    (VMAP_VIRT_START + GB(1) - 1)
>
>  #define FRAMETABLE_VIRT_START  GB(32)
> -#define FRAMETABLE_VIRT_END    (FRAMETABLE_VIRT_START + GB(32) - 1)
> +#define FRAMETABLE_SIZE        GB(32)
> +#define FRAMETABLE_NR          (FRAMETABLE_SIZE / sizeof(*frame_table))
> +#define FRAMETABLE_VIRT_END    (FRAMETABLE_VIRT_START + FRAMETABLE_SIZE -
> 1)
>
>  #define DIRECTMAP_VIRT_START   SLOT0(256)
>  #define DIRECTMAP_SIZE         (SLOT0_ENTRY_SIZE * (265-256))
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index 9fa80a4..77901f7 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -6,6 +6,7 @@
>  #include <asm/page.h>
>  #include <public/xen.h>
>  #include <xen/domain_page.h>
> +#include <xen/pdx.h>
>
>  /* Align Xen to a 2 MiB boundary. */
>  #define XEN_PADDR_ALIGN (1 << 21)
> @@ -140,12 +141,14 @@ extern void share_xen_page_with_privileged_guests(
>      struct page_info *page, int readonly);
>
>  #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
> -/* MFN of the first page in the frame table. */
> -extern unsigned long frametable_base_mfn;
> +/* PDX of the first page in the frame table. */
> +extern unsigned long frametable_base_pdx;
>
>  extern unsigned long max_page;
>  extern unsigned long total_pages;
>
> +#define PDX_GROUP_SHIFT SECOND_SHIFT
> +
>  /* Boot-time pagetable setup */
>  extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t
> xen_paddr);
>  /* Remove early mappings */
> @@ -184,20 +187,15 @@ static inline void __iomem *ioremap_wc(paddr_t
> start, size_t len)
>      return ioremap_attr(start, len, PAGE_HYPERVISOR_WC);
>  }
>
> +/* XXX -- account for base */
>  #define mfn_valid(mfn)        ({
>      \
>      unsigned long __m_f_n = (mfn);
>      \
> -    likely(__m_f_n >= frametable_base_mfn && __m_f_n < max_page);
>     \
> +    likely(pfn_to_pdx(__m_f_n) >= frametable_base_pdx &&
> __mfn_valid(__m_f_n)); \
>  })
>
> -#define max_pdx                 max_page
> -#define pfn_to_pdx(pfn)         (pfn)
> -#define pdx_to_pfn(pdx)         (pdx)
> -#define virt_to_pdx(va)         virt_to_mfn(va)
> -#define pdx_to_virt(pdx)        mfn_to_virt(pdx)
> -
>  /* Convert between machine frame numbers and page-info structures. */
> -#define mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) -
> frametable_base_mfn))
> -#define page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table)
> + frametable_base_mfn)
> +#define mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) -
> frametable_base_pdx))
> +#define page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table)
> + frametable_base_pdx)
>  #define __page_to_mfn(pg)  page_to_mfn(pg)
>  #define __mfn_to_page(mfn) mfn_to_page(mfn)
>
> @@ -221,6 +219,7 @@ static inline paddr_t __virt_to_maddr(vaddr_t va)
>  #define virt_to_maddr(va)   __virt_to_maddr((vaddr_t)(va))
>
>  #ifdef CONFIG_ARM_32
> +#error ARM32 not converted to PDX yet
>  static inline void *maddr_to_virt(paddr_t ma)
>  {
>      ASSERT(is_xen_heap_mfn(ma >> PAGE_SHIFT));
> @@ -230,9 +229,11 @@ static inline void *maddr_to_virt(paddr_t ma)
>  #else
>  static inline void *maddr_to_virt(paddr_t ma)
>  {
> -    ASSERT((ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
> -    ma -= pfn_to_paddr(xenheap_mfn_start);
> -    return (void *)(unsigned long) ma + DIRECTMAP_VIRT_START;
> +    ASSERT(pfn_to_pdx(ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
> +    return (void *)(DIRECTMAP_VIRT_START -
> +                    pfn_to_paddr(xenheap_mfn_start) +
> +                    ((ma & ma_va_bottom_mask) |
> +                     ((ma & ma_top_mask) >> pfn_pdx_hole_shift)));
>  }
>  #endif
>
> @@ -264,7 +265,7 @@ static inline struct page_info *virt_to_page(const
> void *v)
>      return frame_table
>          + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT)
>          + xenheap_mfn_start
> -        - frametable_base_mfn;
> +        - frametable_base_pdx;
>  }
>
>  static inline void *page_to_virt(const struct page_info *pg)
> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> index 2c019d7..06a9d5a 100644
> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -12,7 +12,7 @@ static inline __attribute__((pure)) int
> phys_to_nid(paddr_t addr)
>
>  /* XXX: implement NUMA support */
>  #define node_spanned_pages(nid) (total_pages)
> -#define node_start_pfn(nid) (frametable_base_mfn)
> +#define node_start_pfn(nid) (pdx_to_pfn(frametable_base_pdx))
>  #define __node_distance(a, b) (20)
>
>  #endif /* __ARCH_ARM_NUMA_H */
> --
> 1.7.10.4
>
>

[-- Attachment #1.2: Type: text/html, Size: 14877 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Julien Grall]

Hi Ian,

On 07/31/2014 12:01 PM, Ian Campbell wrote:
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 446b4dc..dbd92bb 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -423,6 +423,46 @@ static paddr_t __init get_xen_paddr(void)
>      return paddr;
>  }
>  
> +/* Sets all bits from the most-significant 1-bit down to the LSB */
> +static u64 __init fill_mask(u64 mask)
> +{
> +    while (mask & (mask + 1))

Coding style: Missing spaces

> +        mask |= mask + 1;
> +    return mask;
> +}
> +
> +static void init_pdx(void)
> +{
> +    u64 mask = fill_mask(bootinfo.mem.bank[0].start - 1);
> +    int bank;
> +
> +    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> +    {
> +        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> +        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> +        paddr_t bank_end = bank_start + bank_size;
> +
> +        set_pdx_range(paddr_to_pfn(bank_start),
> +                      paddr_to_pfn(bank_end));
> +
> +
> +        mask |= bank_start | fill_mask(bank_start ^ (bank_end - 1));
> +    }
> +
> +    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> +    {
> +        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> +        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> +        paddr_t bank_end = bank_start + bank_size;
> +
> +        if (~mask &
> +            fill_mask(bank_start ^ (bank_end - 1)))

Coding style.

I don't really understand what the function is achieving. Could you
explain a bit more?

Regards,

-- 
Julien Grall

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Ian Campbell]

On Thu, 2014-08-07 at 17:03 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 07/31/2014 12:01 PM, Ian Campbell wrote:
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 446b4dc..dbd92bb 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -423,6 +423,46 @@ static paddr_t __init get_xen_paddr(void)
> >      return paddr;
> >  }
> >  
> > +/* Sets all bits from the most-significant 1-bit down to the LSB */
> > +static u64 __init fill_mask(u64 mask)
> > +{
> > +    while (mask & (mask + 1))
> 
> Coding style: Missing spaces
> 
> > +        mask |= mask + 1;
> > +    return mask;
> > +}
> > +
> > +static void init_pdx(void)
> > +{
> > +    u64 mask = fill_mask(bootinfo.mem.bank[0].start - 1);
> > +    int bank;
> > +
> > +    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> > +    {
> > +        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> > +        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> > +        paddr_t bank_end = bank_start + bank_size;
> > +
> > +        set_pdx_range(paddr_to_pfn(bank_start),
> > +                      paddr_to_pfn(bank_end));
> > +
> > +
> > +        mask |= bank_start | fill_mask(bank_start ^ (bank_end - 1));
> > +    }
> > +
> > +    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> > +    {
> > +        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> > +        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> > +        paddr_t bank_end = bank_start + bank_size;
> > +
> > +        if (~mask &
> > +            fill_mask(bank_start ^ (bank_end - 1)))
> 
> Coding style.
> 
> I don't really understand what the function is achieving. Could you
> explain a bit more?

This is largely derived from an x86 equivalent (see srat_parse_regions,
they are different because one walks the device tree RAM and the other
the SRAT), so I hope Jan will correct me if I'm wrong about the
following:

What it is doing is calculating a mask which corresponds to to the bits
that are active address bits across the valid memory addresses, i.e. a
bit which is necessary to unambiguously represent some valid address is
1 and a bit which is the same for all addresses is 0.

Using that mask we then find a large run of zeroes from he middle of the
mask which by construction do not actually get used for addressing. By
omitting those bits from the PFN we obtain a PDX which is the
(losslessly) compressed form.

I have a feeling I've not explained that so well...

Ian.

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Jan Beulich]

>>> On 04.09.14 at 18:40, <Ian.Campbell@citrix.com> wrote:
> On Thu, 2014-08-07 at 17:03 +0100, Julien Grall wrote:
>> I don't really understand what the function is achieving. Could you
>> explain a bit more?
> 
> This is largely derived from an x86 equivalent (see srat_parse_regions,
> they are different because one walks the device tree RAM and the other
> the SRAT), so I hope Jan will correct me if I'm wrong about the
> following:
> 
> What it is doing is calculating a mask which corresponds to to the bits
> that are active address bits across the valid memory addresses, i.e. a
> bit which is necessary to unambiguously represent some valid address is
> 1 and a bit which is the same for all addresses is 0.
> 
> Using that mask we then find a large run of zeroes from he middle of the
> mask which by construction do not actually get used for addressing. By
> omitting those bits from the PFN we obtain a PDX which is the
> (losslessly) compressed form.
> 
> I have a feeling I've not explained that so well...

You've done a very good job in fact.

Jan

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Ian Campbell]

On Fri, 2014-09-05 at 09:20 +0100, Jan Beulich wrote:
> >>> On 04.09.14 at 18:40, <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2014-08-07 at 17:03 +0100, Julien Grall wrote:
> >> I don't really understand what the function is achieving. Could you
> >> explain a bit more?
> > 
> > This is largely derived from an x86 equivalent (see srat_parse_regions,
> > they are different because one walks the device tree RAM and the other
> > the SRAT), so I hope Jan will correct me if I'm wrong about the
> > following:
> > 
> > What it is doing is calculating a mask which corresponds to to the bits
> > that are active address bits across the valid memory addresses, i.e. a
> > bit which is necessary to unambiguously represent some valid address is
> > 1 and a bit which is the same for all addresses is 0.
> > 
> > Using that mask we then find a large run of zeroes from he middle of the
> > mask which by construction do not actually get used for addressing. By
> > omitting those bits from the PFN we obtain a PDX which is the
> > (losslessly) compressed form.
> > 
> > I have a feeling I've not explained that so well...
> 
> You've done a very good job in fact.

Thanks ;-)

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Julien Grall]

On 04/09/14 09:40, Ian Campbell wrote:
>> I don't really understand what the function is achieving. Could you
>> explain a bit more?
>
> This is largely derived from an x86 equivalent (see srat_parse_regions,
> they are different because one walks the device tree RAM and the other
> the SRAT), so I hope Jan will correct me if I'm wrong about the
> following:
>
> What it is doing is calculating a mask which corresponds to to the bits
> that are active address bits across the valid memory addresses, i.e. a
> bit which is necessary to unambiguously represent some valid address is
> 1 and a bit which is the same for all addresses is 0.
>
> Using that mask we then find a large run of zeroes from he middle of the
> mask which by construction do not actually get used for addressing. By
> omitting those bits from the PFN we obtain a PDX which is the
> (losslessly) compressed form.

Many thank for the explanation!

-- 
Julien Grall

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Ian Campbell]

On Mon, 2014-09-08 at 17:32 -0700, Julien Grall wrote:
> 
> On 04/09/14 09:40, Ian Campbell wrote:
> >> I don't really understand what the function is achieving. Could you
> >> explain a bit more?
> >
> > This is largely derived from an x86 equivalent (see srat_parse_regions,
> > they are different because one walks the device tree RAM and the other
> > the SRAT), so I hope Jan will correct me if I'm wrong about the
> > following:
> >
> > What it is doing is calculating a mask which corresponds to to the bits
> > that are active address bits across the valid memory addresses, i.e. a
> > bit which is necessary to unambiguously represent some valid address is
> > 1 and a bit which is the same for all addresses is 0.
> >
> > Using that mask we then find a large run of zeroes from he middle of the
> > mask which by construction do not actually get used for addressing. By
> > omitting those bits from the PFN we obtain a PDX which is the
> > (losslessly) compressed form.
> 
> Many thank for the explanation!

No problem. Do you have an opinion on the patch now?

Ian.

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Roy Franz]

On Tue, Sep 9, 2014 at 5:16 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2014-09-08 at 17:32 -0700, Julien Grall wrote:
>>
>> On 04/09/14 09:40, Ian Campbell wrote:
>> >> I don't really understand what the function is achieving. Could you
>> >> explain a bit more?
>> >
>> > This is largely derived from an x86 equivalent (see srat_parse_regions,
>> > they are different because one walks the device tree RAM and the other
>> > the SRAT), so I hope Jan will correct me if I'm wrong about the
>> > following:
>> >
>> > What it is doing is calculating a mask which corresponds to to the bits
>> > that are active address bits across the valid memory addresses, i.e. a
>> > bit which is necessary to unambiguously represent some valid address is
>> > 1 and a bit which is the same for all addresses is 0.
>> >
>> > Using that mask we then find a large run of zeroes from he middle of the
>> > mask which by construction do not actually get used for addressing. By
>> > omitting those bits from the PFN we obtain a PDX which is the
>> > (losslessly) compressed form.
>>
>> Many thank for the explanation!
>
> No problem. Do you have an opinion on the patch now?
>
> Ian.
>
This patchset would be nice to have merged, as without it half of the
memory on the ARM v8 models (and I think Juno board as well)
is ignored.  I've been running with with patchset for the last month
without problem.

Roy

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Julien Grall]

Hi Ian,

On 09/09/14 05:16, Ian Campbell wrote:
> On Mon, 2014-09-08 at 17:32 -0700, Julien Grall wrote:
>>
>> On 04/09/14 09:40, Ian Campbell wrote:
>>>> I don't really understand what the function is achieving. Could you
>>>> explain a bit more?
>>>
>>> This is largely derived from an x86 equivalent (see srat_parse_regions,
>>> they are different because one walks the device tree RAM and the other
>>> the SRAT), so I hope Jan will correct me if I'm wrong about the
>>> following:
>>>
>>> What it is doing is calculating a mask which corresponds to to the bits
>>> that are active address bits across the valid memory addresses, i.e. a
>>> bit which is necessary to unambiguously represent some valid address is
>>> 1 and a bit which is the same for all addresses is 0.
>>>
>>> Using that mask we then find a large run of zeroes from he middle of the
>>> mask which by construction do not actually get used for addressing. By
>>> omitting those bits from the PFN we obtain a PDX which is the
>>> (losslessly) compressed form.
>>
>> Many thank for the explanation!
>
> No problem. Do you have an opinion on the patch now?

I don't have any things to add for now on this patch. The ARM64 part 
looks good to me. I'm waiting to see the ARM32 one.

Regards,

-- 
Julien Grall

Re: [PATCH RFC 2/2] xen: arm: Enable physical address space compression (PDX) on arm64

[Ian Campbell]

On Tue, 2014-09-09 at 13:59 -0700, Julien Grall wrote:
> Hi Ian,
> 
> On 09/09/14 05:16, Ian Campbell wrote:
> > On Mon, 2014-09-08 at 17:32 -0700, Julien Grall wrote:
> >>
> >> On 04/09/14 09:40, Ian Campbell wrote:
> >>>> I don't really understand what the function is achieving. Could you
> >>>> explain a bit more?
> >>>
> >>> This is largely derived from an x86 equivalent (see srat_parse_regions,
> >>> they are different because one walks the device tree RAM and the other
> >>> the SRAT), so I hope Jan will correct me if I'm wrong about the
> >>> following:
> >>>
> >>> What it is doing is calculating a mask which corresponds to to the bits
> >>> that are active address bits across the valid memory addresses, i.e. a
> >>> bit which is necessary to unambiguously represent some valid address is
> >>> 1 and a bit which is the same for all addresses is 0.
> >>>
> >>> Using that mask we then find a large run of zeroes from he middle of the
> >>> mask which by construction do not actually get used for addressing. By
> >>> omitting those bits from the PFN we obtain a PDX which is the
> >>> (losslessly) compressed form.
> >>
> >> Many thank for the explanation!
> >
> > No problem. Do you have an opinion on the patch now?
> 
> I don't have any things to add for now on this patch. The ARM64 part 
> looks good to me. I'm waiting to see the ARM32 one.

Damn, I new I'd forgotten to do something!

I'll take a look at arm32 asap....

Ian.