From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space Date: Wed, 15 Apr 2015 17:41:38 +0100 Message-ID: <1429116098.25195.22.camel@citrix.com> References: <1428496597-18981-1-git-send-email-oiurii.konovalenko@globallogic.com> <1428496597-18981-2-git-send-email-oiurii.konovalenko@globallogic.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1428496597-18981-2-git-send-email-oiurii.konovalenko@globallogic.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Iurii Konovalenko Cc: julien.grall@linaro.org, tim@xen.org, stefano.stabellini@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Wed, 2015-04-08 at 15:36 +0300, Iurii Konovalenko wrote: > From: Iurii Konovalenko > > 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 > Signed-off-by: Andrii Anisov > --- > 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.