Re: [PATCH] x86/xen: Fix 32-bit PV guests's usage of kernel_stack

[Boris Ostrovsky]

On 04/09/2014 11:25 AM, Steven Rostedt wrote:
> On Wed, 09 Apr 2014 11:12:33 -0400
> Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
>
> > Commit 198d208df4371734ac4728f69cb585c284d20a15 made 32-bit kernels use
> > kernel_stack to point to thread_info. That change missed a couple of
> > updates needed by Xen's PV guests:
> >
> > 1. kernel_stack needs to be initialized for secondary CPUs
> > 2. GET_THREAD_INFO() now uses %fs register which may not be the kernel's
> > version when executing xen_iret()
> >
> > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > ---
> >   arch/x86/xen/smp.c        |    3 ++-
> >   arch/x86/xen/xen-asm_32.S |    4 ++++
> >   2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> > index a18eadd..7005974 100644
> > --- a/arch/x86/xen/smp.c
> > +++ b/arch/x86/xen/smp.c
> > @@ -441,10 +441,11 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
> >   	irq_ctx_init(cpu);
> >   #else
> >   	clear_tsk_thread_flag(idle, TIF_FORK);
> > +#endif
> >   	per_cpu(kernel_stack, cpu) =
> >   		(unsigned long)task_stack_page(idle) -
> >   		KERNEL_STACK_OFFSET + THREAD_SIZE;
> > -#endif
> > +
> >   	xen_setup_runstate_info(cpu);
> >   	xen_setup_timer(cpu);
> >   	xen_init_lock_cpu(cpu);
> > diff --git a/arch/x86/xen/xen-asm_32.S b/arch/x86/xen/xen-asm_32.S
> > index 33ca6e4..d433637 100644
> > --- a/arch/x86/xen/xen-asm_32.S
> > +++ b/arch/x86/xen/xen-asm_32.S
> > @@ -88,7 +88,11 @@ ENTRY(xen_iret)
> >   	 * avoid having to reload %fs
> Looks like this patch makes the above comment incorrect.


I didn't notice this (it was full 3 lines above the change), it will 
obviously have to be updated.


> >   	 */
> >   #ifdef CONFIG_SMP
> > +	pushw %fs
> > +	movl $(__KERNEL_PERCPU), %eax
> > +	movl %eax, %fs
> >   	GET_THREAD_INFO(%eax)
> > +	popw %fs
> >   	movl %ss:TI_cpu(%eax), %eax
> Hmm, you're using GET_THREAD_INFO() to get the thread info to find the
> current CPU for later use to avoid using %fs. ??
>
> Is it possible to just push the cpu on the kernel irq stack and be able
> to retrieve it manually? Looks like the only reason to use the
> thread_info here is to get to the CPU index for below.

Or vcpu_info, which is what we are really after.

However, it looks like using %fs/GET_THREAD_INFO() should be OK as well, 
provided I deal with a potential faulting.

-boris

> >   	movl %ss:__per_cpu_offset(,%eax,4), %eax
> >   	mov %ss:xen_vcpu(%eax), %eax
> -- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/