All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] relocate Xen in over 4GB space for arm32
@ 2015-04-08 12:36 Iurii Konovalenko
  2015-04-08 12:36 ` [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space Iurii Konovalenko
  2015-04-08 12:36 ` [PATCH v1 2/2] arm: skip verifying memory continuity Iurii Konovalenko
  0 siblings, 2 replies; 16+ messages in thread
From: Iurii Konovalenko @ 2015-04-08 12:36 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, ian.campbell, stefano.stabellini

From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>

The following patch series adds ability to relocate Xen in over 4GB space
for 32-bit arm  cores with LPAE.

Iurii Konovalenko (2):
  arm: Add ability to relocate Xen in over 4GB space
  arm: skip verifying memory continuity

 xen/Rules.mk                   |  1 +
 xen/arch/arm/arm32/head.S      | 21 ++++++++++++++++++++-
 xen/arch/arm/platforms/rcar2.c |  9 ++++++++-
 xen/arch/arm/setup.c           | 21 ++++++++++++++++-----
 xen/arch/arm/smpboot.c         | 33 +++++++++++++++++++++++++++++----
 5 files changed, 74 insertions(+), 11 deletions(-)

-- 
1.9.1

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

* [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 12:36 [PATCH v1 0/2] relocate Xen in over 4GB space for arm32 Iurii Konovalenko
@ 2015-04-08 12:36 ` Iurii Konovalenko
  2015-04-08 16:05   ` Julien Grall
  2015-04-15 16:41   ` Ian Campbell
  2015-04-08 12:36 ` [PATCH v1 2/2] arm: skip verifying memory continuity Iurii Konovalenko
  1 sibling, 2 replies; 16+ messages in thread
From: Iurii Konovalenko @ 2015-04-08 12:36 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, ian.campbell, stefano.stabellini

From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>

Primary CPU relocate Xen in over 4GB space and wake up seondary CPUs.
Secondary CPUs run on unrelocated copy of Xen until turning on MMU.
After turning on MMU secondary CPUs run on relocated copy of Xen.

To add ability to relocate Xen in over 4GB space add following to
compilation command: ARM32_RELOCATE_OVER_4GB=y

Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
Signed-off-by: Andrii Anisov <andrii.anisov@globallogic.com>
---
 xen/Rules.mk                   |  1 +
 xen/arch/arm/arm32/head.S      | 21 ++++++++++++++++++++-
 xen/arch/arm/platforms/rcar2.c |  9 ++++++++-
 xen/arch/arm/setup.c           | 17 ++++++++++++++++-
 xen/arch/arm/smpboot.c         | 33 +++++++++++++++++++++++++++++----
 5 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index feb08d6..fbd34a5 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -64,6 +64,7 @@ CFLAGS-$(HAS_PCI)       += -DHAS_PCI
 CFLAGS-$(HAS_IOPORTS)   += -DHAS_IOPORTS
 CFLAGS-$(HAS_PDX)       += -DHAS_PDX
 CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
+CFLAGS-$(ARM32_RELOCATE_OVER_4GB) += -DARM32_RELOCATE_OVER_4GB
 
 ifneq ($(max_phys_cpus),)
 CFLAGS-y                += -DMAX_PHYS_CPUS=$(max_phys_cpus)
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 5c0263e..26be1d0 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -262,8 +262,21 @@ cpu_init_done:
         add   r4, r4, r10            /* r4 := paddr (boot_pagetable) */
         mov   r5, #0                 /* r4:r5 is paddr (boot_pagetable) */
         mcrr  CP64(r4, r5, HTTBR)
+#ifdef ARM32_RELOCATE_OVER_4GB
+        teq   r7, #0
+        beq   1f                     /* construct pagetable if CPU0 */
 
-        /* Setup boot_pgtable: */
+        /*Skip constructing TLBs for secondary CPUs, use constructed by CPU0*/
+        PRINT("- Skip construction pagetable, using CPU0 table @")
+        mov   r0, r5
+        bl    putn
+        mov   r0, r4
+        bl    putn
+        PRINT("  -\r\n")
+        b   skip_constructing
+#endif
+
+1:      /* Setup boot_pgtable: */
         ldr   r1, =boot_second
         add   r1, r1, r10            /* r1 := paddr (boot_second) */
 
@@ -346,6 +359,7 @@ virtphys_clash:
         PRINT("- Unable to build boot page tables - virt and phys addresses clash. -\r\n")
         b     fail
 
+skip_constructing:
 1:
         PRINT("- Turning on paging -\r\n")
 
@@ -427,6 +441,11 @@ paging:
          * setup in init_secondary_pagetables. */
 
         ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by CPU 0 */
+#ifdef ARM32_RELOCATE_OVER_4GB
+        ldr   r1, =_start
+        sub r4, r1
+        add r4, #BOOT_RELOC_VIRT_START
+#endif
         ldrd  r4, r5, [r4]           /* Actual value */
         dsb
         mcrr  CP64(r4, r5, HTTBR)
diff --git a/xen/arch/arm/platforms/rcar2.c b/xen/arch/arm/platforms/rcar2.c
index aef544c..4365166 100644
--- a/xen/arch/arm/platforms/rcar2.c
+++ b/xen/arch/arm/platforms/rcar2.c
@@ -25,6 +25,9 @@
 #define RCAR2_RAM_SIZE                         0x1000
 #define RCAR2_SMP_START_OFFSET                 0xFFC
 
+#ifdef ARM32_RELOCATE_OVER_4GB
+extern paddr_t xen_relocation_offset;
+#endif
 static int __init rcar2_smp_init(void)
 {
     void __iomem *pram;
@@ -38,7 +41,11 @@ static int __init rcar2_smp_init(void)
     }
 
     /* setup reset vectors */
-    writel(__pa(init_secondary), pram + RCAR2_SMP_START_OFFSET);
+    writel(__pa(init_secondary)
+#ifdef ARM32_RELOCATE_OVER_4GB
+            - xen_relocation_offset
+#endif
+            , pram + RCAR2_SMP_START_OFFSET);
     iounmap(pram);
 
     sev();
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 4ec7c13..66e2834 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -407,7 +407,7 @@ static paddr_t __init get_xen_paddr(void)
             if ( !e )
                 continue;
 
-#ifdef CONFIG_ARM_32
+#if defined (CONFIG_ARM_32) && !defined(ARM32_RELOCATE_OVER_4GB)
             /* Xen must be under 4GB */
             if ( e > 0x100000000ULL )
                 e = 0x100000000ULL;
@@ -699,6 +699,9 @@ void __init setup_cache(void)
     cacheline_bytes = 1U << (4 + (ccsid & 0x7));
 }
 
+#ifdef ARM32_RELOCATE_OVER_4GB
+paddr_t xen_relocation_offset;
+#endif
 /* C entry point for boot CPU */
 void __init start_xen(unsigned long boot_phys_offset,
                       unsigned long fdt_paddr,
@@ -740,8 +743,20 @@ void __init start_xen(unsigned long boot_phys_offset,
                              (paddr_t)(uintptr_t)(_end - _start + 1), NULL);
     BUG_ON(!xen_bootmodule);
 
+#ifdef ARM32_RELOCATE_OVER_4GB
+    //save physical address of init_secondary
+    xen_relocation_offset = __pa(init_secondary);
+#endif
     xen_paddr = get_xen_paddr();
     setup_pagetables(boot_phys_offset, xen_paddr);
+#ifdef ARM32_RELOCATE_OVER_4GB
+    //Now Xen is relocated
+    //Calculate offset of Xen relocation
+    //It is difference between new physical address of init_secondary an old one
+    //This offset is needed in several places when we have to write to old Xen location
+    //(secondary CPUs run on old-located Xen and rely on some variables from CPU0)
+    xen_relocation_offset = __pa(init_secondary) - xen_relocation_offset;
+#endif
 
     /* Update Xen's address now that we have relocated. */
     printk("Update BOOTMOD_XEN from %"PRIpaddr"-%"PRIpaddr" => %"PRIpaddr"-%"PRIpaddr"\n",
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index a96cda2..731144c 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -31,6 +31,9 @@
 #include <xen/console.h>
 #include <asm/gic.h>
 #include <asm/psci.h>
+#ifdef ARM32_RELOCATE_OVER_4GB
+#include <xen/vmap.h>
+#endif
 
 cpumask_t cpu_online_map;
 cpumask_t cpu_present_map;
@@ -353,17 +356,33 @@ int __init cpu_up_send_sgi(int cpu)
     return 0;
 }
 
+#ifdef ARM32_RELOCATE_OVER_4GB
+extern paddr_t xen_relocation_offset;
+#endif
 /* Bring up a remote CPU */
 int __cpu_up(unsigned int cpu)
 {
     int rc;
     s_time_t deadline;
+#ifdef ARM32_RELOCATE_OVER_4GB
+    paddr_t p_info = __pa(&smp_up_cpu) - xen_relocation_offset;
+    unsigned long* info = ioremap_nocache(p_info, sizeof(unsigned long));
+#else
+    unsigned long* info = &smp_up_cpu;
+#endif
 
     printk("Bringing up CPU%d\n", cpu);
 
     rc = init_secondary_pagetables(cpu);
     if ( rc < 0 )
+#ifdef ARM32_RELOCATE_OVER_4GB
+    {
+        iounmap(info);
+        return rc;
+    }
+#else
         return rc;
+#endif
 
     console_start_sync(); /* Secondary may use early_printk */
 
@@ -374,8 +393,8 @@ int __cpu_up(unsigned int cpu)
     init_data.cpuid = cpu;
 
     /* Open the gate for this CPU */
-    smp_up_cpu = cpu_logical_map(cpu);
-    clean_dcache(smp_up_cpu);
+    *info = cpu_logical_map(cpu);
+    clean_dcache(*info);
 
     rc = arch_cpu_up(cpu);
 
@@ -383,6 +402,9 @@ int __cpu_up(unsigned int cpu)
 
     if ( rc < 0 )
     {
+#ifdef ARM32_RELOCATE_OVER_4GB
+        iounmap(info);
+#endif
         printk("Failed to bring up CPU%d\n", cpu);
         return rc;
     }
@@ -406,8 +428,11 @@ int __cpu_up(unsigned int cpu)
      */
     init_data.stack = NULL;
     init_data.cpuid = ~0;
-    smp_up_cpu = MPIDR_INVALID;
-    clean_dcache(smp_up_cpu);
+    *info = MPIDR_INVALID;
+    clean_dcache(*info);
+#ifdef ARM32_RELOCATE_OVER_4GB
+    iounmap(info);
+#endif
 
     if ( !cpu_online(cpu) )
     {
-- 
1.9.1

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

* [PATCH v1 2/2] arm: skip verifying memory continuity
  2015-04-08 12:36 [PATCH v1 0/2] relocate Xen in over 4GB space for arm32 Iurii Konovalenko
  2015-04-08 12:36 ` [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space Iurii Konovalenko
@ 2015-04-08 12:36 ` Iurii Konovalenko
  2015-04-08 16:20   ` Julien Grall
  1 sibling, 1 reply; 16+ messages in thread
From: Iurii Konovalenko @ 2015-04-08 12:36 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, ian.campbell, stefano.stabellini

From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>

Odd check.

Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
Signed-off-by: Andrii Anisov <andrii.anisov@globallogic.com>
---
 xen/arch/arm/setup.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 66e2834..a7fcbb6 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -394,10 +394,6 @@ static paddr_t __init get_xen_paddr(void)
         const struct membank *bank = &mi->bank[i];
         paddr_t s, e;
 
-        /* We can only deal with contiguous memory at the moment */
-        if ( last_end != bank->start )
-            break;
-
         last_end = bank->start + bank->size;
 
         if ( bank->size >= min_size )
-- 
1.9.1

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 12:36 ` [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space Iurii Konovalenko
@ 2015-04-08 16:05   ` Julien Grall
  2015-04-08 17:24     ` Andrii Anisov
                       ` (3 more replies)
  2015-04-15 16:41   ` Ian Campbell
  1 sibling, 4 replies; 16+ messages in thread
From: Julien Grall @ 2015-04-08 16:05 UTC (permalink / raw)
  To: Iurii Konovalenko, xen-devel
  Cc: julien.grall, tim, ian.campbell, stefano.stabellini

Hi Iurii,

OOI, can you give more context why you need to relocate Xen over 4GB?

On 08/04/15 13:36, Iurii Konovalenko wrote:
> From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
> 
> Primary CPU relocate Xen in over 4GB space and wake up seondary CPUs.

Typoes:

s/relocate/relocates
s/seondary/secondary

> Secondary CPUs run on unrelocated copy of Xen until turning on MMU.
> After turning on MMU secondary CPUs run on relocated copy of Xen.

The non-relocated copy of Xen is not marked as reserved. So Xen may
allocate the area for his own purpose before the secondary CPU has boot.

> 
> To add ability to relocate Xen in over 4GB space add following to
> compilation command: ARM32_RELOCATE_OVER_4GB=y

The virtualization extension requires LPAE. Any reason to make this
optional?

> Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
> Signed-off-by: Andrii Anisov <andrii.anisov@globallogic.com>
> ---
>  xen/Rules.mk                   |  1 +
>  xen/arch/arm/arm32/head.S      | 21 ++++++++++++++++++++-
>  xen/arch/arm/platforms/rcar2.c |  9 ++++++++-
>  xen/arch/arm/setup.c           | 17 ++++++++++++++++-
>  xen/arch/arm/smpboot.c         | 33 +++++++++++++++++++++++++++++----
>  5 files changed, 74 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index feb08d6..fbd34a5 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -64,6 +64,7 @@ CFLAGS-$(HAS_PCI)       += -DHAS_PCI
>  CFLAGS-$(HAS_IOPORTS)   += -DHAS_IOPORTS
>  CFLAGS-$(HAS_PDX)       += -DHAS_PDX
>  CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
> +CFLAGS-$(ARM32_RELOCATE_OVER_4GB) += -DARM32_RELOCATE_OVER_4GB
>  
>  ifneq ($(max_phys_cpus),)
>  CFLAGS-y                += -DMAX_PHYS_CPUS=$(max_phys_cpus)
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 5c0263e..26be1d0 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -262,8 +262,21 @@ cpu_init_done:
>          add   r4, r4, r10            /* r4 := paddr (boot_pagetable) */
>          mov   r5, #0                 /* r4:r5 is paddr (boot_pagetable) */
>          mcrr  CP64(r4, r5, HTTBR)

Newline here.

> +#ifdef ARM32_RELOCATE_OVER_4GB
> +        teq   r7, #0
> +        beq   1f                     /* construct pagetable if CPU0 */
>  
> -        /* Setup boot_pgtable: */
> +        /*Skip constructing TLBs for secondary CPUs, use constructed by CPU0*/

/* ... */

TLBs is confusing. I first though you were talking to Translation
Lookaside Buffer.

> +        PRINT("- Skip construction pagetable, using CPU0 table @")

This very fragile. The CPU0 may drop a part of the 1:1 mapping if it's
clashes with the fixmap and DTB mappings (see after the label "paging").

Therefore secondary CPUs may hang during boot.

> +        mov   r0, r5
> +        bl    putn
> +        mov   r0, r4
> +        bl    putn
> +        PRINT("  -\r\n")
> +        b   skip_constructing
> +#endif
> +
> +1:      /* Setup boot_pgtable: */
>          ldr   r1, =boot_second
>          add   r1, r1, r10            /* r1 := paddr (boot_second) */
>  
> @@ -346,6 +359,7 @@ virtphys_clash:
>          PRINT("- Unable to build boot page tables - virt and phys addresses clash. -\r\n")
>          b     fail
>  
> +skip_constructing:
>  1:
>          PRINT("- Turning on paging -\r\n")
>  
> @@ -427,6 +441,11 @@ paging:
>           * setup in init_secondary_pagetables. */
>  
>          ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by CPU 0 */
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +        ldr   r1, =_start
> +        sub r4, r1
> +        add r4, #BOOT_RELOC_VIRT_START
> +#endif

This chunk need some comment in order to explain what it's doing.

AFAIU the resulting value in r4, will be exactly the same as  "ldr r4,
=init_ttbr".

>          ldrd  r4, r5, [r4]           /* Actual value */
>          dsb
>          mcrr  CP64(r4, r5, HTTBR)
> diff --git a/xen/arch/arm/platforms/rcar2.c b/xen/arch/arm/platforms/rcar2.c
> index aef544c..4365166 100644
> --- a/xen/arch/arm/platforms/rcar2.c
> +++ b/xen/arch/arm/platforms/rcar2.c
> @@ -25,6 +25,9 @@
>  #define RCAR2_RAM_SIZE                         0x1000
>  #define RCAR2_SMP_START_OFFSET                 0xFFC
>  
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +extern paddr_t xen_relocation_offset;
> +#endif

Please define in an header.

>  static int __init rcar2_smp_init(void)
>  {
>      void __iomem *pram;
> @@ -38,7 +41,11 @@ static int __init rcar2_smp_init(void)
>      }
>  
>      /* setup reset vectors */
> -    writel(__pa(init_secondary), pram + RCAR2_SMP_START_OFFSET);
> +    writel(__pa(init_secondary)
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +            - xen_relocation_offset
> +#endif
> +            , pram + RCAR2_SMP_START_OFFSET);
>      iounmap(pram);
>  
>      sev();
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 4ec7c13..66e2834 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -407,7 +407,7 @@ static paddr_t __init get_xen_paddr(void)
>              if ( !e )
>                  continue;
>  
> -#ifdef CONFIG_ARM_32
> +#if defined (CONFIG_ARM_32) && !defined(ARM32_RELOCATE_OVER_4GB)
>              /* Xen must be under 4GB */
>              if ( e > 0x100000000ULL )
>                  e = 0x100000000ULL;
> @@ -699,6 +699,9 @@ void __init setup_cache(void)
>      cacheline_bytes = 1U << (4 + (ccsid & 0x7));
>  }
>  
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +paddr_t xen_relocation_offset;
> +#endif
>  /* C entry point for boot CPU */
>  void __init start_xen(unsigned long boot_phys_offset,
>                        unsigned long fdt_paddr,
> @@ -740,8 +743,20 @@ void __init start_xen(unsigned long boot_phys_offset,
>                               (paddr_t)(uintptr_t)(_end - _start + 1), NULL);
>      BUG_ON(!xen_bootmodule);
>  
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +    //save physical address of init_secondary

Xen is using /* ... */ for the comment.

> +    xen_relocation_offset = __pa(init_secondary);
> +#endif
>      xen_paddr = get_xen_paddr();
>      setup_pagetables(boot_phys_offset, xen_paddr);
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +    //Now Xen is relocated
> +    //Calculate offset of Xen relocation
> +    //It is difference between new physical address of init_secondary an old one
> +    //This offset is needed in several places when we have to write to old Xen location
> +    //(secondary CPUs run on old-located Xen and rely on some variables from CPU0)

Ditto

> +    xen_relocation_offset = __pa(init_secondary) - xen_relocation_offset;

The Xen may be relocated below the boot copy. So the final result may be
negative.

> +#endif
>  
>      /* Update Xen's address now that we have relocated. */
>      printk("Update BOOTMOD_XEN from %"PRIpaddr"-%"PRIpaddr" => %"PRIpaddr"-%"PRIpaddr"\n",
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index a96cda2..731144c 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -31,6 +31,9 @@
>  #include <xen/console.h>
>  #include <asm/gic.h>
>  #include <asm/psci.h>
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +#include <xen/vmap.h>
> +#endif
>  
>  cpumask_t cpu_online_map;
>  cpumask_t cpu_present_map;
> @@ -353,17 +356,33 @@ int __init cpu_up_send_sgi(int cpu)
>      return 0;
>  }
>  
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +extern paddr_t xen_relocation_offset;
> +#endif

This can be avoid by defining xen_relocation_offset in an header.

>  /* Bring up a remote CPU */
>  int __cpu_up(unsigned int cpu)
>  {
>      int rc;
>      s_time_t deadline;
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +    paddr_t p_info = __pa(&smp_up_cpu) - xen_relocation_offset;
> +    unsigned long* info = ioremap_nocache(p_info, sizeof(unsigned long));

unsigned long *info and sizeof(smp_up_cpu).

ioremap_nocache may fail and "info" doesn't have a real meaning.

Regards,

-- 
Julien Grall

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

* Re: [PATCH v1 2/2] arm: skip verifying memory continuity
  2015-04-08 12:36 ` [PATCH v1 2/2] arm: skip verifying memory continuity Iurii Konovalenko
@ 2015-04-08 16:20   ` Julien Grall
  0 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2015-04-08 16:20 UTC (permalink / raw)
  To: Iurii Konovalenko, xen-devel
  Cc: julien.grall, tim, ian.campbell, stefano.stabellini

Hi Iurii,

On 08/04/15 13:36, Iurii Konovalenko wrote:
> From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
> 
> Odd check.

Can you expand the commit explaining that Xen supports non-contiguous
bank since commit e01fa4e2 "xen: arm: Enable physical address space
compression (PDX) on arm"?

Regards,

-- 
Julien Grall

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 16:05   ` Julien Grall
@ 2015-04-08 17:24     ` Andrii Anisov
  2015-04-08 17:25       ` Andrii Anisov
  2015-05-08 16:02       ` Ian Campbell
  2015-04-10 13:58     ` Iurii Konovalenko
                       ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Andrii Anisov @ 2015-04-08 17:24 UTC (permalink / raw)
  To: Julien Grall
  Cc: Iurii Konovalenko, Ian Campbell, Stefano Stabellini,
	Julien Grall, Tim Deegan, xen-devel


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

> OOI, can you give more context why you need to relocate Xen over 4GB?

Julien,

The context here is pretty simple: just a workaround of the real system
limitations:

   1. Renesas R-Car H2 evaluation board lager has 4GB of RAM, 2GB mapped
   under 4GB boundary and 2GB over.
   2. The R-Car2 chip has IOMMU's for almost all active devices except GPU.
   3. Within our system we need one user domain with 2GB of RAM and
   dedicated GPU.
   4. While doing PoC we have no access to the GPU driver(and firmware)
   sources to introduce swiotlb here.

Having no option to handle GPU driver addressing with iommu (2) or
swiotlb(4), it was decided to place the user domain described in 3 within
RAM below 4GB with 1:1 mapping. Therefore we needed to move all the rest
(XEN, rest of domains) under 4GB.

Andrii Anisov | Team Lead
GlobalLogic
Kyiv, 03038, Protasov Business Park, M.Grinchenka, 2/1
P +38.044.492.9695x3664  M +380505738852  S andriyanisov
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt


On Wed, Apr 8, 2015 at 7:05 PM, Julien Grall <julien.grall@citrix.com>
wrote:
> Hi Iurii,
>
> OOI, can you give more context why you need to relocate Xen over 4GB?
>
> On 08/04/15 13:36, Iurii Konovalenko wrote:
>> From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
>>
>> Primary CPU relocate Xen in over 4GB space and wake up seondary CPUs.
>
> Typoes:
>
> s/relocate/relocates
> s/seondary/secondary
>
>> Secondary CPUs run on unrelocated copy of Xen until turning on MMU.
>> After turning on MMU secondary CPUs run on relocated copy of Xen.
>
> The non-relocated copy of Xen is not marked as reserved. So Xen may
> allocate the area for his own purpose before the secondary CPU has boot.
>
>>
>> To add ability to relocate Xen in over 4GB space add following to
>> compilation command: ARM32_RELOCATE_OVER_4GB=y
>
> The virtualization extension requires LPAE. Any reason to make this
> optional?
>
>> Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
>> Signed-off-by: Andrii Anisov <andrii.anisov@globallogic.com>
>> ---
>>  xen/Rules.mk                   |  1 +
>>  xen/arch/arm/arm32/head.S      | 21 ++++++++++++++++++++-
>>  xen/arch/arm/platforms/rcar2.c |  9 ++++++++-
>>  xen/arch/arm/setup.c           | 17 ++++++++++++++++-
>>  xen/arch/arm/smpboot.c         | 33 +++++++++++++++++++++++++++++----
>>  5 files changed, 74 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/Rules.mk b/xen/Rules.mk
>> index feb08d6..fbd34a5 100644
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -64,6 +64,7 @@ CFLAGS-$(HAS_PCI)       += -DHAS_PCI
>>  CFLAGS-$(HAS_IOPORTS)   += -DHAS_IOPORTS
>>  CFLAGS-$(HAS_PDX)       += -DHAS_PDX
>>  CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer
-DCONFIG_FRAME_POINTER
>> +CFLAGS-$(ARM32_RELOCATE_OVER_4GB) += -DARM32_RELOCATE_OVER_4GB
>>
>>  ifneq ($(max_phys_cpus),)
>>  CFLAGS-y                += -DMAX_PHYS_CPUS=$(max_phys_cpus)
>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>> index 5c0263e..26be1d0 100644
>> --- a/xen/arch/arm/arm32/head.S
>> +++ b/xen/arch/arm/arm32/head.S
>> @@ -262,8 +262,21 @@ cpu_init_done:
>>          add   r4, r4, r10            /* r4 := paddr (boot_pagetable) */
>>          mov   r5, #0                 /* r4:r5 is paddr (boot_pagetable)
*/
>>          mcrr  CP64(r4, r5, HTTBR)
>
> Newline here.
>
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +        teq   r7, #0
>> +        beq   1f                     /* construct pagetable if CPU0 */
>>
>> -        /* Setup boot_pgtable: */
>> +        /*Skip constructing TLBs for secondary CPUs, use constructed by
CPU0*/
>
> /* ... */
>
> TLBs is confusing. I first though you were talking to Translation
> Lookaside Buffer.
>
>> +        PRINT("- Skip construction pagetable, using CPU0 table @")
>
> This very fragile. The CPU0 may drop a part of the 1:1 mapping if it's
> clashes with the fixmap and DTB mappings (see after the label "paging").
>
> Therefore secondary CPUs may hang during boot.
>
>> +        mov   r0, r5
>> +        bl    putn
>> +        mov   r0, r4
>> +        bl    putn
>> +        PRINT("  -\r\n")
>> +        b   skip_constructing
>> +#endif
>> +
>> +1:      /* Setup boot_pgtable: */
>>          ldr   r1, =boot_second
>>          add   r1, r1, r10            /* r1 := paddr (boot_second) */
>>
>> @@ -346,6 +359,7 @@ virtphys_clash:
>>          PRINT("- Unable to build boot page tables - virt and phys
addresses clash. -\r\n")
>>          b     fail
>>
>> +skip_constructing:
>>  1:
>>          PRINT("- Turning on paging -\r\n")
>>
>> @@ -427,6 +441,11 @@ paging:
>>           * setup in init_secondary_pagetables. */
>>
>>          ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by
CPU 0 */
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +        ldr   r1, =_start
>> +        sub r4, r1
>> +        add r4, #BOOT_RELOC_VIRT_START
>> +#endif
>
> This chunk need some comment in order to explain what it's doing.
>
> AFAIU the resulting value in r4, will be exactly the same as  "ldr r4,
> =init_ttbr".
>
>>          ldrd  r4, r5, [r4]           /* Actual value */
>>          dsb
>>          mcrr  CP64(r4, r5, HTTBR)
>> diff --git a/xen/arch/arm/platforms/rcar2.c
b/xen/arch/arm/platforms/rcar2.c
>> index aef544c..4365166 100644
>> --- a/xen/arch/arm/platforms/rcar2.c
>> +++ b/xen/arch/arm/platforms/rcar2.c
>> @@ -25,6 +25,9 @@
>>  #define RCAR2_RAM_SIZE                         0x1000
>>  #define RCAR2_SMP_START_OFFSET                 0xFFC
>>
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +extern paddr_t xen_relocation_offset;
>> +#endif
>
> Please define in an header.
>
>>  static int __init rcar2_smp_init(void)
>>  {
>>      void __iomem *pram;
>> @@ -38,7 +41,11 @@ static int __init rcar2_smp_init(void)
>>      }
>>
>>      /* setup reset vectors */
>> -    writel(__pa(init_secondary), pram + RCAR2_SMP_START_OFFSET);
>> +    writel(__pa(init_secondary)
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +            - xen_relocation_offset
>> +#endif
>> +            , pram + RCAR2_SMP_START_OFFSET);
>>      iounmap(pram);
>>
>>      sev();
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 4ec7c13..66e2834 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -407,7 +407,7 @@ static paddr_t __init get_xen_paddr(void)
>>              if ( !e )
>>                  continue;
>>
>> -#ifdef CONFIG_ARM_32
>> +#if defined (CONFIG_ARM_32) && !defined(ARM32_RELOCATE_OVER_4GB)
>>              /* Xen must be under 4GB */
>>              if ( e > 0x100000000ULL )
>>                  e = 0x100000000ULL;
>> @@ -699,6 +699,9 @@ void __init setup_cache(void)
>>      cacheline_bytes = 1U << (4 + (ccsid & 0x7));
>>  }
>>
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +paddr_t xen_relocation_offset;
>> +#endif
>>  /* C entry point for boot CPU */
>>  void __init start_xen(unsigned long boot_phys_offset,
>>                        unsigned long fdt_paddr,
>> @@ -740,8 +743,20 @@ void __init start_xen(unsigned long
boot_phys_offset,
>>                               (paddr_t)(uintptr_t)(_end - _start + 1),
NULL);
>>      BUG_ON(!xen_bootmodule);
>>
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +    //save physical address of init_secondary
>
> Xen is using /* ... */ for the comment.
>
>> +    xen_relocation_offset = __pa(init_secondary);
>> +#endif
>>      xen_paddr = get_xen_paddr();
>>      setup_pagetables(boot_phys_offset, xen_paddr);
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +    //Now Xen is relocated
>> +    //Calculate offset of Xen relocation
>> +    //It is difference between new physical address of init_secondary
an old one
>> +    //This offset is needed in several places when we have to write to
old Xen location
>> +    //(secondary CPUs run on old-located Xen and rely on some variables
from CPU0)
>
> Ditto
>
>> +    xen_relocation_offset = __pa(init_secondary) -
xen_relocation_offset;
>
> The Xen may be relocated below the boot copy. So the final result may be
> negative.
>
>> +#endif
>>
>>      /* Update Xen's address now that we have relocated. */
>>      printk("Update BOOTMOD_XEN from %"PRIpaddr"-%"PRIpaddr" =>
%"PRIpaddr"-%"PRIpaddr"\n",
>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
>> index a96cda2..731144c 100644
>> --- a/xen/arch/arm/smpboot.c
>> +++ b/xen/arch/arm/smpboot.c
>> @@ -31,6 +31,9 @@
>>  #include <xen/console.h>
>>  #include <asm/gic.h>
>>  #include <asm/psci.h>
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +#include <xen/vmap.h>
>> +#endif
>>
>>  cpumask_t cpu_online_map;
>>  cpumask_t cpu_present_map;
>> @@ -353,17 +356,33 @@ int __init cpu_up_send_sgi(int cpu)
>>      return 0;
>>  }
>>
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +extern paddr_t xen_relocation_offset;
>> +#endif
>
> This can be avoid by defining xen_relocation_offset in an header.
>
>>  /* Bring up a remote CPU */
>>  int __cpu_up(unsigned int cpu)
>>  {
>>      int rc;
>>      s_time_t deadline;
>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +    paddr_t p_info = __pa(&smp_up_cpu) - xen_relocation_offset;
>> +    unsigned long* info = ioremap_nocache(p_info, sizeof(unsigned
long));
>
> unsigned long *info and sizeof(smp_up_cpu).
>
> ioremap_nocache may fail and "info" doesn't have a real meaning.
>
> Regards,
>
> --
> Julien Grall
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

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

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

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 17:24     ` Andrii Anisov
@ 2015-04-08 17:25       ` Andrii Anisov
  2015-05-08 16:02       ` Ian Campbell
  1 sibling, 0 replies; 16+ messages in thread
From: Andrii Anisov @ 2015-04-08 17:25 UTC (permalink / raw)
  To: Julien Grall
  Cc: Iurii Konovalenko, Ian Campbell, Stefano Stabellini,
	Julien Grall, Tim Deegan, xen-devel


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

>>Therefore we needed to move all the rest (XEN, rest of domains) OVER 4GB.


Andrii Anisov | Team Lead
GlobalLogic
Kyiv, 03038, Protasov Business Park, M.Grinchenka, 2/1
P +38.044.492.9695x3664  M +380505738852  S andriyanisov
www.globallogic.com
<http://www.globallogic.com/>
http://www.globallogic.com/email_disclaimer.txt

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

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

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

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 16:05   ` Julien Grall
  2015-04-08 17:24     ` Andrii Anisov
@ 2015-04-10 13:58     ` Iurii Konovalenko
  2015-04-10 14:25       ` Julien Grall
  2015-04-10 14:38     ` Andrii Anisov
  2015-04-15 16:29     ` Ian Campbell
  3 siblings, 1 reply; 16+ messages in thread
From: Iurii Konovalenko @ 2015-04-10 13:58 UTC (permalink / raw)
  To: Julien Grall
  Cc: Julien Grall, tim, Ian Campbell, Stefano Stabellini, xen-devel

Hi, Julien!

On Wed, Apr 8, 2015 at 7:05 PM, Julien Grall <julien.grall@citrix.com> wrote:

> The virtualization extension requires LPAE. Any reason to make this
> optional?
I agree with you - there is no real reason to make it optional. I will
rework patch
to include functionality without any flags by default.

>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +        ldr   r1, =_start
>> +        sub r4, r1
>> +        add r4, #BOOT_RELOC_VIRT_START
>> +#endif
>
> This chunk need some comment in order to explain what it's doing.
>
> AFAIU the resulting value in r4, will be exactly the same as  "ldr r4,
> =init_ttbr".
Not exactly. Virtual address of _start is 0x00200000, but
#BOOT_RELOC_VIRT_START is 0x00800000.
This part of code is needed to get value, that was "stashed by CPU 0" in
relocated copy of Xen when secondary CPUs run in non-relocated.

>> +    xen_relocation_offset = __pa(init_secondary) - xen_relocation_offset;
>
> The Xen may be relocated below the boot copy. So the final result may be
> negative.
This offset is used to get physical address of non-relocated variables
and write there values.
If it is negative, then calculating non-relocated address gives bigger
number (substraction of  negative value).
In addition, it's hard to imagine when it can be negative if we take
highest region of highest memory bank.

> ioremap_nocache may fail and "info" doesn't have a real meaning.
Ok, I will add verifying block and rename variable.

> Regards,
>
> --
> Julien Grall


Best regards.

Iurii Konovalenko | Senior Software Engineer
GlobalLogic

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-10 13:58     ` Iurii Konovalenko
@ 2015-04-10 14:25       ` Julien Grall
  2015-04-14 11:00         ` Ian Campbell
  0 siblings, 1 reply; 16+ messages in thread
From: Julien Grall @ 2015-04-10 14:25 UTC (permalink / raw)
  To: Iurii Konovalenko, Julien Grall
  Cc: xen-devel, Julien Grall, tim, Ian Campbell, Stefano Stabellini

On 10/04/15 14:58, Iurii Konovalenko wrote:
> Hi, Julien!

Hi Iurii,

> On Wed, Apr 8, 2015 at 7:05 PM, Julien Grall <julien.grall@citrix.com> wrote:
> 
>> The virtualization extension requires LPAE. Any reason to make this
>> optional?
> I agree with you - there is no real reason to make it optional. I will
> rework patch
> to include functionality without any flags by default.

Before doing a such change, I'd like the point of view of Stefano or Ian
on this patch.

>>> +#ifdef ARM32_RELOCATE_OVER_4GB
>>> +        ldr   r1, =_start
>>> +        sub r4, r1
>>> +        add r4, #BOOT_RELOC_VIRT_START
>>> +#endif
>>
>> This chunk need some comment in order to explain what it's doing.
>>
>> AFAIU the resulting value in r4, will be exactly the same as  "ldr r4,
>> =init_ttbr".
> Not exactly. Virtual address of _start is 0x00200000, but
> #BOOT_RELOC_VIRT_START is 0x00800000.
> This part of code is needed to get value, that was "stashed by CPU 0" in
> relocated copy of Xen when secondary CPUs run in non-relocated.

And the reloc Xen is mapped by setup_page_tables in mm.c, right?

If so, this definitely need a comment explaining why and how it works.
Because you are relying on multiple things in the code.
	- 1:1 mapping is not overlapped (see my comment on the previous mail)
	- reloc Xen will never overlap the boot Xen (for instance we could
decide that Xen doesn't need to be relocated).
	- After relocation, Xen is still modifying the boot page table (clean
them) but hopefully it's only the relocated version

This is making the code in setup_page_tables quite difficult to
understand now.

>>> +    xen_relocation_offset = __pa(init_secondary) - xen_relocation_offset;
>>
>> The Xen may be relocated below the boot copy. So the final result may be
>> negative.
> This offset is used to get physical address of non-relocated variables
> and write there values.
> If it is negative, then calculating non-relocated address gives bigger
> number (substraction of  negative value).
> In addition, it's hard to imagine when it can be negative if we take
> highest region of highest memory bank.

Simple, the board has all the memory banks below 4G and Xen has been
loaded by the bootloader at a very high address.

Regards,

-- 
Julien Grall

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 16:05   ` Julien Grall
  2015-04-08 17:24     ` Andrii Anisov
  2015-04-10 13:58     ` Iurii Konovalenko
@ 2015-04-10 14:38     ` Andrii Anisov
  2015-04-10 14:49       ` Julien Grall
  2015-04-15 16:29     ` Ian Campbell
  3 siblings, 1 reply; 16+ messages in thread
From: Andrii Anisov @ 2015-04-10 14:38 UTC (permalink / raw)
  To: Julien Grall
  Cc: Iurii Konovalenko, Ian Campbell, Stefano Stabellini,
	Julien Grall, Tim Deegan, xen-devel

Julien,

> > Secondary CPUs run on unrelocated copy of Xen until turning on MMU.
> > After turning on MMU secondary CPUs run on relocated copy of Xen.
>
> The non-relocated copy of Xen is not marked as reserved. So Xen may
> allocate the area for his own purpose before the secondary CPU has boot.

Could you please clarify how the same issue resolved with the piece of
bootloader code which implements secondary processors wfe in HYP mode
and jump to hypervisor?

Andrii Anisov | Team Lead
GlobalLogic
Kyiv, 03038, Protasov Business Park, M.Grinchenka, 2/1
P +38.044.492.9695x3664  M +380505738852  S andriyanisov
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-10 14:38     ` Andrii Anisov
@ 2015-04-10 14:49       ` Julien Grall
  2015-04-16 12:59         ` Ian Campbell
  0 siblings, 1 reply; 16+ messages in thread
From: Julien Grall @ 2015-04-10 14:49 UTC (permalink / raw)
  To: Andrii Anisov
  Cc: Iurii Konovalenko, Ian Campbell, Stefano Stabellini,
	Julien Grall, Tim Deegan, xen-devel

On 10/04/15 15:38, Andrii Anisov wrote:
> Julien,
> 
>>> Secondary CPUs run on unrelocated copy of Xen until turning on MMU.
>>> After turning on MMU secondary CPUs run on relocated copy of Xen.
>>
>> The non-relocated copy of Xen is not marked as reserved. So Xen may
>> allocate the area for his own purpose before the secondary CPU has boot.
> 
> Could you please clarify how the same issue resolved with the piece of
> bootloader code which implements secondary processors wfe in HYP mode
> and jump to hypervisor?

I'm not sure to understand the question.

Currently, Xen add a bootmodule  (see start_xen) to cover Xen. The base
address is updated once Xen is relocated.

You may want to add a bootmodule for the non-relocated Xen.

I hope it answers to your question.

Regards,

-- 
Julien Grall

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-10 14:25       ` Julien Grall
@ 2015-04-14 11:00         ` Ian Campbell
  0 siblings, 0 replies; 16+ messages in thread
From: Ian Campbell @ 2015-04-14 11:00 UTC (permalink / raw)
  To: Julien Grall
  Cc: Iurii Konovalenko, Julien Grall, tim, xen-devel, Stefano Stabellini

On Fri, 2015-04-10 at 15:25 +0100, Julien Grall wrote:
> On 10/04/15 14:58, Iurii Konovalenko wrote:
> > Hi, Julien!
> 
> Hi Iurii,
> 
> > On Wed, Apr 8, 2015 at 7:05 PM, Julien Grall <julien.grall@citrix.com> wrote:
> > 
> >> The virtualization extension requires LPAE. Any reason to make this
> >> optional?
> > I agree with you - there is no real reason to make it optional. I will
> > rework patch
> > to include functionality without any flags by default.
> 
> Before doing a such change, I'd like the point of view of Stefano or Ian
> on this patch.

Such things shouldn't be compile time optional. I don't think there's
any reason why it can't be done for every arm32 platform.

Normally I'd ask for a command line option which can overrides (i.e.
disable) this behaviour -- just in case. But I suspect that the relevant
code runs too early?

(I'll read the rest of this thread as I catch up with my email backlog).

Ian

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 16:05   ` Julien Grall
                       ` (2 preceding siblings ...)
  2015-04-10 14:38     ` Andrii Anisov
@ 2015-04-15 16:29     ` Ian Campbell
  3 siblings, 0 replies; 16+ messages in thread
From: Ian Campbell @ 2015-04-15 16:29 UTC (permalink / raw)
  To: Julien Grall
  Cc: Iurii Konovalenko, julien.grall, tim, stefano.stabellini, xen-devel

On Wed, 2015-04-08 at 17:05 +0100, Julien Grall wrote:
> > -        /* Setup boot_pgtable: */
> > +        /*Skip constructing TLBs for secondary CPUs, use constructed by CPU0*/
> 
> /* ... */
> 
> TLBs is confusing. I first though you were talking to Translation
> Lookaside Buffer.

Was "tbls" (i.e. shortened "tables") what was meant? In which case I'd
prefer spelling it out..

Ian.

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 12:36 ` [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space Iurii Konovalenko
  2015-04-08 16:05   ` Julien Grall
@ 2015-04-15 16:41   ` Ian Campbell
  1 sibling, 0 replies; 16+ messages in thread
From: Ian Campbell @ 2015-04-15 16:41 UTC (permalink / raw)
  To: Iurii Konovalenko; +Cc: julien.grall, tim, stefano.stabellini, xen-devel

On Wed, 2015-04-08 at 15:36 +0300, Iurii Konovalenko wrote:
> From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
> 
> Primary CPU relocate Xen in over 4GB space and wake up seondary CPUs.
> Secondary CPUs run on unrelocated copy of Xen until turning on MMU.

Which is a significant difference I think, and requires careful checking
that we do not clobber that copy while brining up the primary cpu.

Which would mean it needs to be reserved before we start allocating
heaps and relocating Xen. Ideally only the portion of head.S which we
need would be reserved, not the whole thing. And it should be freed once
everyone is up and running.

Alternatively perhaps the relevant portion of head.S could be copied to
a freshly allocated trampoline page.

> After turning on MMU secondary CPUs run on relocated copy of Xen.
> 
> To add ability to relocate Xen in over 4GB space add following to
> compilation command: ARM32_RELOCATE_OVER_4GB=y
> 
> Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
> Signed-off-by: Andrii Anisov <andrii.anisov@globallogic.com>
> ---
>  xen/Rules.mk                   |  1 +
>  xen/arch/arm/arm32/head.S      | 21 ++++++++++++++++++++-
>  xen/arch/arm/platforms/rcar2.c |  9 ++++++++-
>  xen/arch/arm/setup.c           | 17 ++++++++++++++++-
>  xen/arch/arm/smpboot.c         | 33 +++++++++++++++++++++++++++++----
>  5 files changed, 74 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index feb08d6..fbd34a5 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -64,6 +64,7 @@ CFLAGS-$(HAS_PCI)       += -DHAS_PCI
>  CFLAGS-$(HAS_IOPORTS)   += -DHAS_IOPORTS
>  CFLAGS-$(HAS_PDX)       += -DHAS_PDX
>  CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
> +CFLAGS-$(ARM32_RELOCATE_OVER_4GB) += -DARM32_RELOCATE_OVER_4GB
>  
>  ifneq ($(max_phys_cpus),)
>  CFLAGS-y                += -DMAX_PHYS_CPUS=$(max_phys_cpus)
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 5c0263e..26be1d0 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -262,8 +262,21 @@ cpu_init_done:
>          add   r4, r4, r10            /* r4 := paddr (boot_pagetable) */
>          mov   r5, #0                 /* r4:r5 is paddr (boot_pagetable) */
>          mcrr  CP64(r4, r5, HTTBR)
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +        teq   r7, #0
> +        beq   1f                     /* construct pagetable if CPU0 */
>  
> -        /* Setup boot_pgtable: */
> +        /*Skip constructing TLBs for secondary CPUs, use constructed by CPU0*/

Is this necessary due to the relocation for some reason?

Otherwise, iff its correct, then it should be done as a separate change
before this one.

Also please pay attention to the style used for comments in this file.

> @@ -427,6 +441,11 @@ paging:
>           * setup in init_secondary_pagetables. */
>  
>          ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by CPU 0 */
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +        ldr   r1, =_start
> +        sub r4, r1
> +        add r4, #BOOT_RELOC_VIRT_START

Please follow the existing convention of thoroughly documenting what
each line is doing. (i.e. the "r1 := foo" comments)

> +#endif
>          ldrd  r4, r5, [r4]           /* Actual value */
>          dsb
>          mcrr  CP64(r4, r5, HTTBR)
> diff --git a/xen/arch/arm/platforms/rcar2.c b/xen/arch/arm/platforms/rcar2.c
> index aef544c..4365166 100644
> --- a/xen/arch/arm/platforms/rcar2.c
> +++ b/xen/arch/arm/platforms/rcar2.c
> @@ -25,6 +25,9 @@
>  #define RCAR2_RAM_SIZE                         0x1000
>  #define RCAR2_SMP_START_OFFSET                 0xFFC
>  
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +extern paddr_t xen_relocation_offset;

This certainly doesn't belong here, such things belong in headers.


> @@ -740,8 +743,20 @@ void __init start_xen(unsigned long boot_phys_offset,
>                               (paddr_t)(uintptr_t)(_end - _start + 1), NULL);
>      BUG_ON(!xen_bootmodule);
>  
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +    //save physical address of init_secondary

This is not the Xen style for comments.

> +    xen_relocation_offset = __pa(init_secondary);

How does __pa(init_secondary) end up being an offset? Shouldn't this be
some calculation involving boot_phys_offset?

> +#endif
>      xen_paddr = get_xen_paddr();
>      setup_pagetables(boot_phys_offset, xen_paddr);
> +#ifdef ARM32_RELOCATE_OVER_4GB
> +    //Now Xen is relocated
> +    //Calculate offset of Xen relocation
> +    //It is difference between new physical address of init_secondary an old one
> +    //This offset is needed in several places when we have to write to old Xen location
> +    //(secondary CPUs run on old-located Xen and rely on some variables from CPU0)
> +    xen_relocation_offset = __pa(init_secondary) - xen_relocation_offset;

Oh I see, ewww. I think you should be able to calculate this directly
when you do the relocation, which is much nicer than relying on tricks
of __pa changing.

And rather than adding all of the uses of xen_relocatio_offset I think a
__boot_pa construct which does the adjustment should be used.

Ian.

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-10 14:49       ` Julien Grall
@ 2015-04-16 12:59         ` Ian Campbell
  0 siblings, 0 replies; 16+ messages in thread
From: Ian Campbell @ 2015-04-16 12:59 UTC (permalink / raw)
  To: Julien Grall
  Cc: Iurii Konovalenko, Stefano Stabellini, Julien Grall, Tim Deegan,
	Andrii Anisov, xen-devel

On Fri, 2015-04-10 at 15:49 +0100, Julien Grall wrote:
> On 10/04/15 15:38, Andrii Anisov wrote:
> > Julien,
> > 
> >>> Secondary CPUs run on unrelocated copy of Xen until turning on MMU.
> >>> After turning on MMU secondary CPUs run on relocated copy of Xen.
> >>
> >> The non-relocated copy of Xen is not marked as reserved. So Xen may
> >> allocate the area for his own purpose before the secondary CPU has boot.
> > 
> > Could you please clarify how the same issue resolved with the piece of
> > bootloader code which implements secondary processors wfe in HYP mode
> > and jump to hypervisor?
> 
> I'm not sure to understand the question.
> 
> Currently, Xen add a bootmodule  (see start_xen) to cover Xen. The base
> address is updated once Xen is relocated.
> 
> You may want to add a bootmodule for the non-relocated Xen.
> 
> I hope it answers to your question.

And if the question is about bootloader code which holds a spinning
secondary CPU then the answer is that this region had better be either
in Secure World only RAM or reserved in the firmware description (i.e.
FDT or ACPI memory map). In FDT that would usually be done via
the /memreserve/ directive, although one cold also imagine fudging the
memory nodes.

Ian.

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

* Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space
  2015-04-08 17:24     ` Andrii Anisov
  2015-04-08 17:25       ` Andrii Anisov
@ 2015-05-08 16:02       ` Ian Campbell
  1 sibling, 0 replies; 16+ messages in thread
From: Ian Campbell @ 2015-05-08 16:02 UTC (permalink / raw)
  To: Andrii Anisov
  Cc: Iurii Konovalenko, Stefano Stabellini, Julien Grall, Tim Deegan,
	xen-devel, Julien Grall

On Wed, 2015-04-08 at 20:24 +0300, Andrii Anisov wrote:

I was thinking about this a little more...

> The context here is pretty simple: just a workaround of the real
> system limitations:
>      1. Renesas R-Car H2 evaluation board lager has 4GB of RAM, 2GB
>         mapped under 4GB boundary and 2GB over. 
>      2. The R-Car2 chip has IOMMU's for almost all active devices
>         except GPU. 
>      3. Within our system we need one user domain with 2GB of RAM and
>         dedicated GPU.
>      4. While doing PoC we have no access to the GPU driver(and
>         firmware) sources to introduce swiotlb here. 
> Having no option to handle GPU driver addressing with iommu (2) or
> swiotlb(4), it was decided to place the user domain described in 3
> within RAM below 4GB with 1:1 mapping. Therefore we needed to move all
> the rest (XEN, rest of domains) under 4GB.

Xen itself (i.e. .text and .data, no the heap etc) is at most 2MB. Are
your constraints such that at a 2GB-2MB (i.e. 2046M) domain would not be
acceptable?

Ian.

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

end of thread, other threads:[~2015-05-08 16:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-08 12:36 [PATCH v1 0/2] relocate Xen in over 4GB space for arm32 Iurii Konovalenko
2015-04-08 12:36 ` [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space Iurii Konovalenko
2015-04-08 16:05   ` Julien Grall
2015-04-08 17:24     ` Andrii Anisov
2015-04-08 17:25       ` Andrii Anisov
2015-05-08 16:02       ` Ian Campbell
2015-04-10 13:58     ` Iurii Konovalenko
2015-04-10 14:25       ` Julien Grall
2015-04-14 11:00         ` Ian Campbell
2015-04-10 14:38     ` Andrii Anisov
2015-04-10 14:49       ` Julien Grall
2015-04-16 12:59         ` Ian Campbell
2015-04-15 16:29     ` Ian Campbell
2015-04-15 16:41   ` Ian Campbell
2015-04-08 12:36 ` [PATCH v1 2/2] arm: skip verifying memory continuity Iurii Konovalenko
2015-04-08 16:20   ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.