| Message ID | 1374602713-716-3-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, 2013-07-23 at 19:05 +0100, Julien Grall wrote: > Unlike bx, eret will not update the instruction set (THUMB,ARM) according to > the return address. This will result to an unpredicable behaviour for the > processor if the address doesn't match the right instruction set. > > When the kernel is compiled with THUMB2, THUMB bit needs to be set in CPSR > for the secondary cpus. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/arch/arm/psci.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c > index 18feead..574c343 100644 > --- a/xen/arch/arm/psci.c > +++ b/xen/arch/arm/psci.c > @@ -43,6 +43,9 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > ctxt->ttbr1 = 0; > ctxt->ttbcr = 0; /* Defined Reset Value */ > ctxt->user_regs.cpsr = PSR_GUEST_INIT; > + /* Start the VCPU in THUMB mode if it's requested by the kernel */ > + if ( entry_point & 1 ) > + ctxt->user_regs.cpsr |= PSR_THUMB; Do we also need to clear bit 0 of the entry point, or does ERET get that right for us? > ctxt->flags = VGCF_online; > > domain_lock(d);
On 23 July 2013 19:12, Ian Campbell <ian.campbell@citrix.com> wrote: > On Tue, 2013-07-23 at 19:05 +0100, Julien Grall wrote: >> Unlike bx, eret will not update the instruction set (THUMB,ARM) according to >> the return address. This will result to an unpredicable behaviour for the >> processor if the address doesn't match the right instruction set. >> >> When the kernel is compiled with THUMB2, THUMB bit needs to be set in CPSR >> for the secondary cpus. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> --- >> xen/arch/arm/psci.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c >> index 18feead..574c343 100644 >> --- a/xen/arch/arm/psci.c >> +++ b/xen/arch/arm/psci.c >> @@ -43,6 +43,9 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) >> ctxt->ttbr1 = 0; >> ctxt->ttbcr = 0; /* Defined Reset Value */ >> ctxt->user_regs.cpsr = PSR_GUEST_INIT; >> + /* Start the VCPU in THUMB mode if it's requested by the kernel */ >> + if ( entry_point & 1 ) >> + ctxt->user_regs.cpsr |= PSR_THUMB; > > Do we also need to clear bit 0 of the entry point, or does ERET get that > right for us? ERET will clear bit 0. So no need to clear it. -- Julien Grall
On Tue, 2013-07-23 at 22:28 +0100, Julien Grall wrote: > On 23 July 2013 19:12, Ian Campbell <ian.campbell@citrix.com> wrote: > > On Tue, 2013-07-23 at 19:05 +0100, Julien Grall wrote: > >> Unlike bx, eret will not update the instruction set (THUMB,ARM) according to > >> the return address. This will result to an unpredicable behaviour for the > >> processor if the address doesn't match the right instruction set. > >> > >> When the kernel is compiled with THUMB2, THUMB bit needs to be set in CPSR > >> for the secondary cpus. > >> > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> --- > >> xen/arch/arm/psci.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c > >> index 18feead..574c343 100644 > >> --- a/xen/arch/arm/psci.c > >> +++ b/xen/arch/arm/psci.c > >> @@ -43,6 +43,9 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > >> ctxt->ttbr1 = 0; > >> ctxt->ttbcr = 0; /* Defined Reset Value */ > >> ctxt->user_regs.cpsr = PSR_GUEST_INIT; > >> + /* Start the VCPU in THUMB mode if it's requested by the kernel */ > >> + if ( entry_point & 1 ) > >> + ctxt->user_regs.cpsr |= PSR_THUMB; > > > > Do we also need to clear bit 0 of the entry point, or does ERET get that > > right for us? > > ERET will clear bit 0. So no need to clear it. Perfect! Acked-by: Ian Campbell <ian.campbell@citrix.com> Strictly speaking the PSCI spec (DEN 0022B.b) says that for aarch64 guests we should return INVALID_PARAMETERS (-2) if bit 1 is set. Ian
On 23 July 2013 23:07, Ian Campbell <ian.campbell@citrix.com> wrote: > On Tue, 2013-07-23 at 22:28 +0100, Julien Grall wrote: >> On 23 July 2013 19:12, Ian Campbell <ian.campbell@citrix.com> wrote: >> > On Tue, 2013-07-23 at 19:05 +0100, Julien Grall wrote: >> >> Unlike bx, eret will not update the instruction set (THUMB,ARM) according to >> >> the return address. This will result to an unpredicable behaviour for the >> >> processor if the address doesn't match the right instruction set. >> >> >> >> When the kernel is compiled with THUMB2, THUMB bit needs to be set in CPSR >> >> for the secondary cpus. >> >> >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> >> --- >> >> xen/arch/arm/psci.c | 3 +++ >> >> 1 file changed, 3 insertions(+) >> >> >> >> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c >> >> index 18feead..574c343 100644 >> >> --- a/xen/arch/arm/psci.c >> >> +++ b/xen/arch/arm/psci.c >> >> @@ -43,6 +43,9 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) >> >> ctxt->ttbr1 = 0; >> >> ctxt->ttbcr = 0; /* Defined Reset Value */ >> >> ctxt->user_regs.cpsr = PSR_GUEST_INIT; >> >> + /* Start the VCPU in THUMB mode if it's requested by the kernel */ >> >> + if ( entry_point & 1 ) >> >> + ctxt->user_regs.cpsr |= PSR_THUMB; >> > >> > Do we also need to clear bit 0 of the entry point, or does ERET get that >> > right for us? >> >> ERET will clear bit 0. So no need to clear it. > > Perfect! > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > Strictly speaking the PSCI spec (DEN 0022B.b) says that for aarch64 > guests we should return INVALID_PARAMETERS (-2) if bit 1 is set. I can update the patch to check if we are running an aarch64 guest. -- Julien Grall
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c index 18feead..574c343 100644 --- a/xen/arch/arm/psci.c +++ b/xen/arch/arm/psci.c @@ -43,6 +43,9 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) ctxt->ttbr1 = 0; ctxt->ttbcr = 0; /* Defined Reset Value */ ctxt->user_regs.cpsr = PSR_GUEST_INIT; + /* Start the VCPU in THUMB mode if it's requested by the kernel */ + if ( entry_point & 1 ) + ctxt->user_regs.cpsr |= PSR_THUMB; ctxt->flags = VGCF_online; domain_lock(d);
Unlike bx, eret will not update the instruction set (THUMB,ARM) according to the return address. This will result to an unpredicable behaviour for the processor if the address doesn't match the right instruction set. When the kernel is compiled with THUMB2, THUMB bit needs to be set in CPSR for the secondary cpus. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/psci.c | 3 +++ 1 file changed, 3 insertions(+)