Xen devel - Oct 2013 - [PATCH v2 0/6] xen: arm: fixups for systems with RAM above 4GB

This is primarily an attempt to get arm64 Xen working on systems which
do not have any RAM at all below 4GB but there are small fixes for
systems with highmem generally.

I''ve been testing this with a hack DTB which uses only the AEM
fastmodel''s 36-bit alias of DRAM and a hacked up boot-wrapper to load
at
the appropriate addresses etc.

The first patch "xen: correct xenheap_bits after "xen: support RAM at
addresses 0 and 4096" has been previously posted as a standalone patch.

Since v1 I have fixed the issue with not being able to actually build a
guest (needed to set HCR_EL2.RW correctly). This version now loads the
dom0 kernel successfully.

I''ve also pushed to xenbits:

The following changes since commit 1e143e2ae8be3ba86c2e931a1ee8d91efca08f89:

  libxl: correctly handle libxl_get_cpu_topology failure in libxl_{cpu,
node}map_to_{node, cpu}map (2013-10-03 14:45:54 +0100)

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/xen.git no-low-ram-v2

for you to fetch changes up to 619d081e1df07d3abad2c73eca494b27ac1b266b:

  xen: arm: Ensure HCR_EL2.RW is set correctly when building dom0 (2013-10-10
15:28:03 +0100)

----------------------------------------------------------------
Ian Campbell (6):
      xen: correct xenheap_bits after "xen: support RAM at addresses 0 and
4096"
      xen: arm: Enable 40 bit addressing in VTCR for arm64
      xen: arm: map entire memory banks on arm64
      xen: arm: make sure pagetable mask macros have appropriate size
      xen: arm: correctly round down MFN to 1GB boundary make sure pagetable
mask macros as physaddr size
      xen: arm: Ensure HCR_EL2.RW is set correctly when building dom0

 xen/arch/arm/domain_build.c |   10 +++++++---
 xen/arch/arm/mm.c           |   13 ++++++++++---
 xen/arch/arm/setup.c        |    4 ++--
 xen/common/page_alloc.c     |    2 +-
 xen/include/asm-arm/page.h  |   20 ++++++++++----------
 5 files changed, 30 insertions(+), 19 deletions(-)


Ian.

This is incorrect after commit 1aac966e24e which shuffled the zones up by one.
I''ve observed failures on arm64 systems with RAM at
0x8,00000000-0x8,7fffffff
since xenheap_bits ends up as 35 instead of 36 (which is the zone with all the
RAM).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
I suppose that MEMZONE_XEN is not really useful when !CONFIG_SEPARATE_XENHEAP
so in principal 1aac966e24e could be make conditional, but in reality
MEMZONE_XEN is at least referenced when !CONFIG_SEPARATE_XENHEAP so at least
some other cleanup would be needed. This fix seems simpler/clearer.
---
 xen/common/page_alloc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index fb8187b..4c17fbd 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1364,7 +1364,7 @@ static unsigned int __read_mostly xenheap_bits;
 
 void __init xenheap_max_mfn(unsigned long mfn)
 {
-    xenheap_bits = fls(mfn) + PAGE_SHIFT - 1;
+    xenheap_bits = fls(mfn) + PAGE_SHIFT;
 }
 
 void init_xenheap_pages(paddr_t ps, paddr_t pe)
-- 
1.7.10.4

This requires setting the v8 specific VTCR_EL2.PS field. These bits are
UNK/SBZP on v7.

Also the TS0SZ field is described slightly differently for v8, so update the
comment to reflect this.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/mm.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 474dfef..16bbb39 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -369,9 +369,16 @@ void __cpuinit setup_virt_paging(void)
     /* Setup Stage 2 address translation */
     /* SH0=00, ORGN0=IRGN0=01
      * SL0=01 (Level-1)
-     * T0SZ=(1)1000 = -8 (40 bit physical addresses)
+     * 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
      */
-    WRITE_SYSREG32(0x80002558, VTCR_EL2); isb();
+#ifdef CONFIG_ARM_32
+    WRITE_SYSREG32(0x80002558, VTCR_EL2);
+#else
+    WRITE_SYSREG32(0x80022558, VTCR_EL2);
+#endif
+    isb();
 }
 
 static inline lpae_t pte_of_xenaddr(vaddr_t va)
-- 
1.7.10.4

Currently we only map regions which are not part of boot modules. However we
subsequently free at least some of those modules to the heaps in
discard_initial_modules and if we were unluckly with sizing/location we might
end up adding unmapped pages to the heap.

The heaps on 64-bit use 1GB mappings, so in practice this is probably pretty
unlikely and I''ve not actually seen it.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/setup.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 49f344c..6300802 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -519,6 +519,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t
dtb_size)
 
         xenheap_pages += (bank_size >> PAGE_SHIFT);
 
+        setup_xenheap_mappings(bank_start>>PAGE_SHIFT,
bank_size>>PAGE_SHIFT);
+
         /* XXX we assume that the ram regions are ordered */
         s = bank_start;
         while ( s < bank_end )
@@ -535,8 +537,6 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t
dtb_size)
             if ( e > bank_end )
                 e = bank_end;
 
-            setup_xenheap_mappings(s>>PAGE_SHIFT,
(e-s)>>PAGE_SHIFT);
-
             xenheap_mfn_end = e;
 
             dt_unreserved_regions(s, e, init_boot_pages, 0);
-- 
1.7.10.4

{ZEROETH,FIRST,SECOND,THIRD}_MASK are used with physical addresses which may
be larger than 32 bits. Therefore ensure that they are wide enough by casting
to paddr_t otherwise we may truncate addresses on 32-bit.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/include/asm-arm/page.h |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 3d0f8a9..d468418 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -323,17 +323,17 @@ static inline int gva_to_ipa(vaddr_t va, paddr_t *paddr)
 #define LPAE_ENTRIES    (1u << LPAE_SHIFT)
 #define LPAE_ENTRY_MASK (LPAE_ENTRIES - 1)
 
-#define THIRD_SHIFT  PAGE_SHIFT
-#define THIRD_SIZE   (1u << THIRD_SHIFT)
-#define THIRD_MASK   (~(THIRD_SIZE - 1))
-#define SECOND_SHIFT (THIRD_SHIFT + LPAE_SHIFT)
-#define SECOND_SIZE  (1u << SECOND_SHIFT)
-#define SECOND_MASK  (~(SECOND_SIZE - 1))
-#define FIRST_SHIFT  (SECOND_SHIFT + LPAE_SHIFT)
-#define FIRST_SIZE   (1u << FIRST_SHIFT)
-#define FIRST_MASK   (~(FIRST_SIZE - 1))
+#define THIRD_SHIFT    (PAGE_SHIFT)
+#define THIRD_SIZE     ((paddr_t)1 << THIRD_SHIFT)
+#define THIRD_MASK     (~(THIRD_SIZE - 1))
+#define SECOND_SHIFT   (THIRD_SHIFT + LPAE_SHIFT)
+#define SECOND_SIZE    ((paddr_t)1 << SECOND_SHIFT)
+#define SECOND_MASK    (~(SECOND_SIZE - 1))
+#define FIRST_SHIFT    (SECOND_SHIFT + LPAE_SHIFT)
+#define FIRST_SIZE     ((paddr_t)1 << FIRST_SHIFT)
+#define FIRST_MASK     (~(FIRST_SIZE - 1))
 #define ZEROETH_SHIFT  (FIRST_SHIFT + LPAE_SHIFT)
-#define ZEROETH_SIZE   (1u << ZEROETH_SHIFT)
+#define ZEROETH_SIZE   ((paddr_t)1 << ZEROETH_SHIFT)
 #define ZEROETH_MASK   (~(ZEROETH_SIZE - 1))
 
 /* Calculate the offsets into the pagetables for a given VA */
-- 
1.7.10.4

~FIRST_MASK is nothing like correct for rounding down an MFN. It is the
inverse *and* an address not a framenumber so wrong in every dimension! We
cannot use FIRST_MASK since that would mask off any zeroeth level bits.
Instead calculate the correct value from FIRST_SIZE.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/mm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 16bbb39..cc72e78 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -639,7 +639,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
     end_mfn = base_mfn + nr_mfns;
 
     /* Align to previous 1GB boundary */
-    base_mfn &= ~FIRST_MASK;
+    base_mfn &= ~((FIRST_SIZE>>PAGE_SHIFT)-1);
 
     offset = base_mfn - xenheap_mfn_start;
     vaddr = DIRECTMAP_VIRT_START + offset*PAGE_SIZE;
-- 
1.7.10.4

copy_to_user and friends rely on this, since the address transalation
functions (guest VA -> MFN) will truncate VA to the appropriate size.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index fb1fa56..e3ea3f0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -875,6 +875,13 @@ int construct_dom0(struct domain *d)
 
     /* The following loads use the domain''s p2m */
     p2m_load_VTTBR(d);
+#ifdef CONFIG_ARM_64
+    d->arch.type = kinfo.type;
+    if ( is_pv32_domain(d) )
+        WRITE_SYSREG(READ_SYSREG(HCR_EL2) & ~HCR_RW, HCR_EL2);
+    else
+        WRITE_SYSREG(READ_SYSREG(HCR_EL2) | HCR_RW, HCR_EL2);
+#endif
 
     kernel_load(&kinfo);
     dtb_load(&kinfo);
@@ -888,9 +895,6 @@ int construct_dom0(struct domain *d)
 
     regs->pc = (register_t)kinfo.entry;
 
-#ifdef CONFIG_ARM_64
-    d->arch.type = kinfo.type;
-#endif
 
     if ( is_pv32_domain(d) )
     {
-- 
1.7.10.4

On 10/10/2013 15:43, "Ian Campbell" <ian.campbell@citrix.com>
wrote:
> This is incorrect after commit 1aac966e24e which shuffled the zones up by
one.
> I''ve observed failures on arm64 systems with RAM at
0x8,00000000-0x8,7fffffff
> since xenheap_bits ends up as 35 instead of 36 (which is the zone with all
the
> RAM).
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
Acked-by: Keir Fraser <keir@xen.org>
> ---
> I suppose that MEMZONE_XEN is not really useful when
!CONFIG_SEPARATE_XENHEAP
> so in principal 1aac966e24e could be make conditional, but in reality
> MEMZONE_XEN is at least referenced when !CONFIG_SEPARATE_XENHEAP so at
least
> some other cleanup would be needed. This fix seems simpler/clearer.
> ---
>  xen/common/page_alloc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index fb8187b..4c17fbd 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1364,7 +1364,7 @@ static unsigned int __read_mostly xenheap_bits;
>  
>  void __init xenheap_max_mfn(unsigned long mfn)
>  {
> -    xenheap_bits = fls(mfn) + PAGE_SHIFT - 1;
> +    xenheap_bits = fls(mfn) + PAGE_SHIFT;
>  }
>  
>  void init_xenheap_pages(paddr_t ps, paddr_t pe)

Keir acked the generic bit, but ping on the ARM specific patches?

Thanks,
Ian.

On Thu, 2013-10-10 at 15:43 +0100, Ian Campbell wrote:
> This is primarily an attempt to get arm64 Xen working on systems which
> do not have any RAM at all below 4GB but there are small fixes for
> systems with highmem generally.
> 
> I''ve been testing this with a hack DTB which uses only the AEM
> fastmodel''s 36-bit alias of DRAM and a hacked up boot-wrapper to
load at
> the appropriate addresses etc.
> 
> The first patch "xen: correct xenheap_bits after "xen: support
RAM at
> addresses 0 and 4096" has been previously posted as a standalone
patch.
> 
> Since v1 I have fixed the issue with not being able to actually build a
> guest (needed to set HCR_EL2.RW correctly). This version now loads the
> dom0 kernel successfully.
> 
> I''ve also pushed to xenbits:
> 
> The following changes since commit
1e143e2ae8be3ba86c2e931a1ee8d91efca08f89:
> 
>   libxl: correctly handle libxl_get_cpu_topology failure in libxl_{cpu,
node}map_to_{node, cpu}map (2013-10-03 14:45:54 +0100)
> 
> are available in the git repository at:
> 
>   git://xenbits.xen.org/people/ianc/xen.git no-low-ram-v2
> 
> for you to fetch changes up to 619d081e1df07d3abad2c73eca494b27ac1b266b:
> 
>   xen: arm: Ensure HCR_EL2.RW is set correctly when building dom0
(2013-10-10 15:28:03 +0100)
> 
> ----------------------------------------------------------------
> Ian Campbell (6):
>       xen: correct xenheap_bits after "xen: support RAM at addresses 0
and 4096"
>       xen: arm: Enable 40 bit addressing in VTCR for arm64
>       xen: arm: map entire memory banks on arm64
>       xen: arm: make sure pagetable mask macros have appropriate size
>       xen: arm: correctly round down MFN to 1GB boundary make sure
pagetable mask macros as physaddr size
>       xen: arm: Ensure HCR_EL2.RW is set correctly when building dom0
> 
>  xen/arch/arm/domain_build.c |   10 +++++++---
>  xen/arch/arm/mm.c           |   13 ++++++++++---
>  xen/arch/arm/setup.c        |    4 ++--
>  xen/common/page_alloc.c     |    2 +-
>  xen/include/asm-arm/page.h  |   20 ++++++++++----------
>  5 files changed, 30 insertions(+), 19 deletions(-)
> 
> 
> Ian.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On 10/10/2013 03:43 PM, Ian Campbell wrote:
> {ZEROETH,FIRST,SECOND,THIRD}_MASK are used with physical addresses which
may
> be larger than 32 bits. Therefore ensure that they are wide enough by
casting
> to paddr_t otherwise we may truncate addresses on 32-bit.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

-- 
Julien Grall

On 10/10/2013 03:43 PM, Ian Campbell wrote:
> This requires setting the v8 specific VTCR_EL2.PS field. These bits are
> UNK/SBZP on v7.
>
> Also the TS0SZ field is described slightly differently for v8, so update
the
> comment to reflect this.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

-- 
Julien Grall

On 10/10/2013 03:43 PM, Ian Campbell wrote:
> copy_to_user and friends rely on this, since the address transalation
> functions (guest VA -> MFN) will truncate VA to the appropriate size.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
> ---
>   xen/arch/arm/domain_build.c |   10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index fb1fa56..e3ea3f0 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -875,6 +875,13 @@ int construct_dom0(struct domain *d)
>
>       /* The following loads use the domain''s p2m */
>       p2m_load_VTTBR(d);
> +#ifdef CONFIG_ARM_64
> +    d->arch.type = kinfo.type;
> +    if ( is_pv32_domain(d) )
> +        WRITE_SYSREG(READ_SYSREG(HCR_EL2) & ~HCR_RW, HCR_EL2);
> +    else
> +        WRITE_SYSREG(READ_SYSREG(HCR_EL2) | HCR_RW, HCR_EL2);
> +#endif
>
>       kernel_load(&kinfo);
>       dtb_load(&kinfo);
> @@ -888,9 +895,6 @@ int construct_dom0(struct domain *d)
>
>       regs->pc = (register_t)kinfo.entry;
>
> -#ifdef CONFIG_ARM_64
> -    d->arch.type = kinfo.type;
> -#endif
>
>       if ( is_pv32_domain(d) )
>       {
>
-- 
Julien Grall

On 10/10/2013 03:43 PM, Ian Campbell wrote:
> copy_to_user and friends rely on this, since the address transalation
I forgot the minor typo in trans(a)lation
> functions (guest VA -> MFN) will truncate VA to the appropriate size.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>   xen/arch/arm/domain_build.c |   10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index fb1fa56..e3ea3f0 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -875,6 +875,13 @@ int construct_dom0(struct domain *d)
>
>       /* The following loads use the domain''s p2m */
>       p2m_load_VTTBR(d);
> +#ifdef CONFIG_ARM_64
> +    d->arch.type = kinfo.type;
> +    if ( is_pv32_domain(d) )
> +        WRITE_SYSREG(READ_SYSREG(HCR_EL2) & ~HCR_RW, HCR_EL2);
> +    else
> +        WRITE_SYSREG(READ_SYSREG(HCR_EL2) | HCR_RW, HCR_EL2);
> +#endif
>
>       kernel_load(&kinfo);
>       dtb_load(&kinfo);
> @@ -888,9 +895,6 @@ int construct_dom0(struct domain *d)
>
>       regs->pc = (register_t)kinfo.entry;
>
> -#ifdef CONFIG_ARM_64
> -    d->arch.type = kinfo.type;
> -#endif
>
>       if ( is_pv32_domain(d) )
>       {
>
-- 
Julien Grall

On 10/10/2013 03:43 PM, Ian Campbell wrote:
> ~FIRST_MASK is nothing like correct for rounding down an MFN. It is the
> inverse *and* an address not a framenumber so wrong in every dimension! We
> cannot use FIRST_MASK since that would mask off any zeroeth level bits.
> Instead calculate the correct value from FIRST_SIZE.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
> ---
>   xen/arch/arm/mm.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 16bbb39..cc72e78 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -639,7 +639,7 @@ void __init setup_xenheap_mappings(unsigned long
base_mfn,
>       end_mfn = base_mfn + nr_mfns;
>
>       /* Align to previous 1GB boundary */
> -    base_mfn &= ~FIRST_MASK;
> +    base_mfn &= ~((FIRST_SIZE>>PAGE_SHIFT)-1);
>
>       offset = base_mfn - xenheap_mfn_start;
>       vaddr = DIRECTMAP_VIRT_START + offset*PAGE_SIZE;
>
-- 
Julien Grall

On 10/10/2013 03:43 PM, Ian Campbell wrote:
> Currently we only map regions which are not part of boot modules. However
we
> subsequently free at least some of those modules to the heaps in
> discard_initial_modules and if we were unluckly with sizing/location we
might
> end up adding unmapped pages to the heap.
>
> The heaps on 64-bit use 1GB mappings, so in practice this is probably
pretty
> unlikely and I''ve not actually seen it.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
> ---
>   xen/arch/arm/setup.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 49f344c..6300802 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -519,6 +519,8 @@ static void __init setup_mm(unsigned long dtb_paddr,
size_t dtb_size)
>
>           xenheap_pages += (bank_size >> PAGE_SHIFT);
>
> +        setup_xenheap_mappings(bank_start>>PAGE_SHIFT,
bank_size>>PAGE_SHIFT);
> +
>           /* XXX we assume that the ram regions are ordered */
>           s = bank_start;
>           while ( s < bank_end )
> @@ -535,8 +537,6 @@ static void __init setup_mm(unsigned long dtb_paddr,
size_t dtb_size)
>               if ( e > bank_end )
>                   e = bank_end;
>
> -            setup_xenheap_mappings(s>>PAGE_SHIFT,
(e-s)>>PAGE_SHIFT);
> -
>               xenheap_mfn_end = e;
>
>               dt_unreserved_regions(s, e, init_boot_pages, 0);
>
-- 
Julien Grall

On 10/21/2013 10:35 AM, Ian Campbell wrote:
> Keir acked the generic bit, but ping on the ARM specific patches?
I have acked patches #2-#6.

Cheers,

-- 
Julien Grall

On Wed, 2013-10-23 at 14:55 +0100, Julien Grall wrote:
> 
> On 10/21/2013 10:35 AM, Ian Campbell wrote:
> > Keir acked the generic bit, but ping on the ARM specific patches?
> 
> I have acked patches #2-#6.
Thanks. Keir also acked #1 so I have applied.