From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH RFC 5/9] xen: arm: Defer setting of VTCR_EL2 until after CPUs are up Date: Wed, 30 Jul 2014 18:11:27 +0100 Message-ID: <53D9273F.2060404@linaro.org> References: <3ef2b68c511f3e31de409b76757b95c78b99d750.1406728037.git.ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , xen-devel@lists.xen.org Cc: tim@xen.org, vijay.kilari@gmail.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 07/30/2014 02:47 PM, Ian Campbell wrote: > Currently we retain the hardcoded values but soon we will want to > calculate the correct values based upon the CPU properties common to all processors, which are only available once they are all up. NIT: I think it misses a newline on the second line. It looks strange to see the first line short and not the second. > Signed-off-by: Ian Campbell > --- > xen/arch/arm/p2m.c | 16 +++++++++++++--- > xen/arch/arm/setup.c | 4 ++-- > xen/arch/arm/smpboot.c | 2 -- > 3 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 705b29b..225d125 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1059,8 +1059,17 @@ err: > return page; > } > > +static void setup_virt_paging_one(void *data) I would add __init. > +{ > + unsigned long val = (unsigned long)data; VTCR_EL2 is a 32 bit register. I would use uint32_t for the variable type. > + WRITE_SYSREG32(val, VTCR_EL2); > + isb(); > +} > + > void __cpuinit setup_virt_paging(void) This can become __init now. > { > + unsigned long val; > + > /* Setup Stage 2 address translation */ > /* SH0=11 (Inner-shareable) > * ORGN0=IRGN0=01 (Normal memory, Write-Back Write-Allocate Cacheable) > @@ -1070,11 +1079,12 @@ void __cpuinit setup_virt_paging(void) > * PS=010 == 40 bits > */ > #ifdef CONFIG_ARM_32 > - WRITE_SYSREG32(0x80003558, VTCR_EL2); > + val = 0x80003558; > #else > - WRITE_SYSREG32(0x80023558, VTCR_EL2); > + val = 0x80023558; > #endif > - isb(); > + setup_virt_paging_one((void *)val); > + smp_call_function(setup_virt_paging_one, (void *)val, 1); > } > /* > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 446b4dc..ddaef7e 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -788,8 +788,6 @@ void __init start_xen(unsigned long boot_phys_offset, > > gic_init(); > > - setup_virt_paging(); > - > p2m_vmid_allocator_init(); > > softirq_init(); > @@ -838,6 +836,8 @@ void __init start_xen(unsigned long boot_phys_offset, > > do_initcalls(); > > + setup_virt_paging(); > + Assuming there is no CPU hotplug, which IIRC is not yet support, this is only depends on the SMP bring up. Can we move this before do_initcalls? So if we need to initialize some code that is relying on VTCR_EL2, it will be possible. I was mostly thinking about IOMMU drivers where a similar trick will be necessary sooner or later. Regards, -- Julien Grall