2014-07-15 15:26:54

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

Xen doesn't call start_secondary.

Cc: [email protected]
Signed-off-by: Andy Lutomirski <[email protected]>
---

espfix still doesn't seem to work on Xen (it goes boom in some way that
I don't understand right now), but initializing all CPUs instead of just
one of them seems like a good start.

ISTM the right fix is probably to shove the espfix logic into
native_iret and to tweak the paravirt logic so that native_iret always
gets invoked. I suspect that Xen will need its own implementation of
espfix64 in the hypervisor and that, ultimately, someone may want to
stop initializing espfix64 at all on Xen guests.

arch/x86/xen/smp.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 7005974..eea9bcc 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -23,6 +23,7 @@
#include <asm/desc.h>
#include <asm/pgtable.h>
#include <asm/cpu.h>
+#include <asm/espfix.h>

#include <xen/interface/xen.h>
#include <xen/interface/vcpu.h>
@@ -85,6 +86,13 @@ static void cpu_bringup(void)

xen_setup_cpu_clockevents();

+ /*
+ * Enable the espfix hack for this CPU
+ */
+#ifdef CONFIG_X86_ESPFIX64
+ init_espfix_ap();
+#endif
+
notify_cpu_starting(cpu);

set_cpu_online(cpu, true);
--
1.9.3


2014-07-15 15:38:10

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
> Xen doesn't call start_secondary.

Duh!
>
> Cc: [email protected]
> Signed-off-by: Andy Lutomirski <[email protected]>
> ---
>
> espfix still doesn't seem to work on Xen (it goes boom in some way that
> I don't understand right now), but initializing all CPUs instead of just
> one of them seems like a good start.
>
> ISTM the right fix is probably to shove the espfix logic into
> native_iret and to tweak the paravirt logic so that native_iret always
> gets invoked. I suspect that Xen will need its own implementation of
> espfix64 in the hypervisor and that, ultimately, someone may want to
> stop initializing espfix64 at all on Xen guests.

I think just disallowing would be preferrable.

Thanks for posting this patch!
>
> arch/x86/xen/smp.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 7005974..eea9bcc 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -23,6 +23,7 @@
> #include <asm/desc.h>
> #include <asm/pgtable.h>
> #include <asm/cpu.h>
> +#include <asm/espfix.h>
>
> #include <xen/interface/xen.h>
> #include <xen/interface/vcpu.h>
> @@ -85,6 +86,13 @@ static void cpu_bringup(void)
>
> xen_setup_cpu_clockevents();
>
> + /*
> + * Enable the espfix hack for this CPU
> + */
> +#ifdef CONFIG_X86_ESPFIX64
> + init_espfix_ap();
> +#endif
> +
> notify_cpu_starting(cpu);
>
> set_cpu_online(cpu, true);
> --
> 1.9.3
>

2014-07-15 15:43:19

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>> Xen doesn't call start_secondary.
> Duh!
>> Cc: [email protected]
>> Signed-off-by: Andy Lutomirski <[email protected]>
>> ---
>>
>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>> I don't understand right now), but initializing all CPUs instead of just
>> one of them seems like a good start.
>>
>> ISTM the right fix is probably to shove the espfix logic into
>> native_iret and to tweak the paravirt logic so that native_iret always
>> gets invoked. I suspect that Xen will need its own implementation of
>> espfix64 in the hypervisor and that, ultimately, someone may want to
>> stop initializing espfix64 at all on Xen guests.
> I think just disallowing would be preferrable.

I've been looking at sigreturn_64 and it seems to be crashing dom0 (with
both mine and your patches). In kprobe_int3_handler().


-boris

>
> Thanks for posting this patch!
>> arch/x86/xen/smp.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
>> index 7005974..eea9bcc 100644
>> --- a/arch/x86/xen/smp.c
>> +++ b/arch/x86/xen/smp.c
>> @@ -23,6 +23,7 @@
>> #include <asm/desc.h>
>> #include <asm/pgtable.h>
>> #include <asm/cpu.h>
>> +#include <asm/espfix.h>
>>
>> #include <xen/interface/xen.h>
>> #include <xen/interface/vcpu.h>
>> @@ -85,6 +86,13 @@ static void cpu_bringup(void)
>>
>> xen_setup_cpu_clockevents();
>>
>> + /*
>> + * Enable the espfix hack for this CPU
>> + */
>> +#ifdef CONFIG_X86_ESPFIX64
>> + init_espfix_ap();
>> +#endif
>> +
>> notify_cpu_starting(cpu);
>>
>> set_cpu_online(cpu, true);
>> --
>> 1.9.3
>>

2014-07-15 15:45:04

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On Tue, Jul 15, 2014 at 8:38 AM, Konrad Rzeszutek Wilk
<[email protected]> wrote:
> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>> Xen doesn't call start_secondary.
>
> Duh!
>>
>> Cc: [email protected]
>> Signed-off-by: Andy Lutomirski <[email protected]>
>> ---
>>
>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>> I don't understand right now), but initializing all CPUs instead of just
>> one of them seems like a good start.
>>
>> ISTM the right fix is probably to shove the espfix logic into
>> native_iret and to tweak the paravirt logic so that native_iret always
>> gets invoked. I suspect that Xen will need its own implementation of
>> espfix64 in the hypervisor and that, ultimately, someone may want to
>> stop initializing espfix64 at all on Xen guests.
>
> I think just disallowing would be preferrable.

Disabling what?

Sorry, my flu-addled brain needs more clarity. I'm currently working
on a patch on top of this one to move all of the espfix64 invocation
logic into native_iret, which will have the effect of preventing it
from being used on Xen.

Is that what you mean?

--Andy

>
> Thanks for posting this patch!
>>
>> arch/x86/xen/smp.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
>> index 7005974..eea9bcc 100644
>> --- a/arch/x86/xen/smp.c
>> +++ b/arch/x86/xen/smp.c
>> @@ -23,6 +23,7 @@
>> #include <asm/desc.h>
>> #include <asm/pgtable.h>
>> #include <asm/cpu.h>
>> +#include <asm/espfix.h>
>>
>> #include <xen/interface/xen.h>
>> #include <xen/interface/vcpu.h>
>> @@ -85,6 +86,13 @@ static void cpu_bringup(void)
>>
>> xen_setup_cpu_clockevents();
>>
>> + /*
>> + * Enable the espfix hack for this CPU
>> + */
>> +#ifdef CONFIG_X86_ESPFIX64
>> + init_espfix_ap();
>> +#endif
>> +
>> notify_cpu_starting(cpu);
>>
>> set_cpu_online(cpu, true);
>> --
>> 1.9.3
>>



--
Andy Lutomirski
AMA Capital Management, LLC

2014-07-15 15:55:16

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On Tue, Jul 15, 2014 at 8:45 AM, Boris Ostrovsky
<[email protected]> wrote:
> On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
>>
>> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>>>
>>> Xen doesn't call start_secondary.
>>
>> Duh!
>>>
>>> Cc: [email protected]
>>> Signed-off-by: Andy Lutomirski <[email protected]>
>>> ---
>>>
>>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>>> I don't understand right now), but initializing all CPUs instead of just
>>> one of them seems like a good start.
>>>
>>> ISTM the right fix is probably to shove the espfix logic into
>>> native_iret and to tweak the paravirt logic so that native_iret always
>>> gets invoked. I suspect that Xen will need its own implementation of
>>> espfix64 in the hypervisor and that, ultimately, someone may want to
>>> stop initializing espfix64 at all on Xen guests.
>>
>> I think just disallowing would be preferrable.
>
>
> I've been looking at sigreturn_64 and it seems to be crashing dom0 (with
> both mine and your patches). In kprobe_int3_handler().

You need:

http://lkml.kernel.org/g/c4e339882c121aa76254f2adde3fcbdf502faec2.1405099506.git.luto@amacapital.net

The newer version of sigreturn_32 that I pushed is a much better test
-- it tests the 64-bit cases (yay thunks!) and works on kernels
without my SS sigcontext fix.

--Andy

2014-07-15 15:55:19

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On 07/15/2014 08:38 AM, Konrad Rzeszutek Wilk wrote:
>
> I think just disallowing would be preferrable.
>

So I have yet to hear anyone talk about what the Xen PV IRET does for
16-bit stack segments. Can we get the answer for that, please?

-hpa

2014-07-15 15:56:30

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On Tue, Jul 15, 2014 at 08:44:39AM -0700, Andy Lutomirski wrote:
> On Tue, Jul 15, 2014 at 8:38 AM, Konrad Rzeszutek Wilk
> <[email protected]> wrote:
> > On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
> >> Xen doesn't call start_secondary.
> >
> > Duh!
> >>
> >> Cc: [email protected]
> >> Signed-off-by: Andy Lutomirski <[email protected]>
> >> ---
> >>
> >> espfix still doesn't seem to work on Xen (it goes boom in some way that
> >> I don't understand right now), but initializing all CPUs instead of just
> >> one of them seems like a good start.
> >>
> >> ISTM the right fix is probably to shove the espfix logic into
> >> native_iret and to tweak the paravirt logic so that native_iret always
> >> gets invoked. I suspect that Xen will need its own implementation of
> >> espfix64 in the hypervisor and that, ultimately, someone may want to
> >> stop initializing espfix64 at all on Xen guests.
> >
> > I think just disallowing would be preferrable.
>
> Disabling what?
>
> Sorry, my flu-addled brain needs more clarity. I'm currently working
> on a patch on top of this one to move all of the espfix64 invocation
> logic into native_iret, which will have the effect of preventing it
> from being used on Xen.
>
> Is that what you mean?

Yes. I presume the logic to deal with the bits losing information
has to be dealt in the Xen case somehow. Peter asked whether the
Xen IRET handles a 16-bit stack segment - and if it restores all of the
RSP then we are OK.

I don't have yet that information and my brain is a in low-power right
now (-ENOSLEEP).

2014-07-15 16:00:43

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On Tue, Jul 15, 2014 at 8:50 AM, H. Peter Anvin <[email protected]> wrote:
> On 07/15/2014 08:38 AM, Konrad Rzeszutek Wilk wrote:
>>
>> I think just disallowing would be preferrable.
>>
>
> So I have yet to hear anyone talk about what the Xen PV IRET does for
> 16-bit stack segments. Can we get the answer for that, please?
>

If I'm reading the right thing, it does:

59 .Lforce_iret:
60 /* Mimic SYSRET behavior. */
61 movq 8(%rsp),%rcx # RIP
62 movq 24(%rsp),%r11 # RFLAGS
63 ALIGN
64 /* No special register assumptions. */
65 iret_exit_to_guest:
66 addq $8,%rsp
67 .Lft0: iretq

while running on Xen's stack. IOW, it leaks bits 31:16 of the Xen
stack, and there's nothing the guest can do about it short of using a
user-mode trampoline or disabling 16-bit stack segments entirely.

See:

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/x86_64/entry.S;h=a3ed216b390c2e87a21ff377850ee34ee7f2bc74;hb=HEAD

and (search for do_iret):

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/traps.c;h=677074b4e628ed99d407b1045d859355e590d604;hb=HEAD

> -hpa
>
>



--
Andy Lutomirski
AMA Capital Management, LLC

2014-07-15 16:03:03

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On 07/15/2014 11:54 AM, Andy Lutomirski wrote:
> On Tue, Jul 15, 2014 at 8:45 AM, Boris Ostrovsky
> <[email protected]> wrote:
>> On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
>>> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>>>> Xen doesn't call start_secondary.
>>> Duh!
>>>> Cc: [email protected]
>>>> Signed-off-by: Andy Lutomirski <[email protected]>
>>>> ---
>>>>
>>>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>>>> I don't understand right now), but initializing all CPUs instead of just
>>>> one of them seems like a good start.
>>>>
>>>> ISTM the right fix is probably to shove the espfix logic into
>>>> native_iret and to tweak the paravirt logic so that native_iret always
>>>> gets invoked. I suspect that Xen will need its own implementation of
>>>> espfix64 in the hypervisor and that, ultimately, someone may want to
>>>> stop initializing espfix64 at all on Xen guests.
>>> I think just disallowing would be preferrable.
>>
>> I've been looking at sigreturn_64 and it seems to be crashing dom0 (with
>> both mine and your patches). In kprobe_int3_handler().
> You need:
>
> http://lkml.kernel.org/g/c4e339882c121aa76254f2adde3fcbdf502faec2.1405099506.git.luto@amacapital.net
>
> The newer version of sigreturn_32 that I pushed is a much better test
> -- it tests the 64-bit cases (yay thunks!) and works on kernels
> without my SS sigcontext fix.

Yes, that does it. At least we don't have yet another failure mode with
this, which was the biggest concern.

Thanks.

-boris

2014-07-15 16:17:34

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

On Tue, Jul 15, 2014 at 9:05 AM, Boris Ostrovsky
<[email protected]> wrote:
> On 07/15/2014 11:54 AM, Andy Lutomirski wrote:
>>
>> On Tue, Jul 15, 2014 at 8:45 AM, Boris Ostrovsky
>> <[email protected]> wrote:
>>>
>>> On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
>>>>
>>>> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>>>>>
>>>>> Xen doesn't call start_secondary.
>>>>
>>>> Duh!
>>>>>
>>>>> Cc: [email protected]
>>>>> Signed-off-by: Andy Lutomirski <[email protected]>
>>>>> ---
>>>>>
>>>>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>>>>> I don't understand right now), but initializing all CPUs instead of
>>>>> just
>>>>> one of them seems like a good start.
>>>>>
>>>>> ISTM the right fix is probably to shove the espfix logic into
>>>>> native_iret and to tweak the paravirt logic so that native_iret always
>>>>> gets invoked. I suspect that Xen will need its own implementation of
>>>>> espfix64 in the hypervisor and that, ultimately, someone may want to
>>>>> stop initializing espfix64 at all on Xen guests.
>>>>
>>>> I think just disallowing would be preferrable.
>>>
>>>
>>> I've been looking at sigreturn_64 and it seems to be crashing dom0 (with
>>> both mine and your patches). In kprobe_int3_handler().
>>
>> You need:
>>
>>
>> http://lkml.kernel.org/g/c4e339882c121aa76254f2adde3fcbdf502faec2.1405099506.git.luto@amacapital.net
>>
>> The newer version of sigreturn_32 that I pushed is a much better test
>> -- it tests the 64-bit cases (yay thunks!) and works on kernels
>> without my SS sigcontext fix.
>
>
> Yes, that does it. At least we don't have yet another failure mode with
> this, which was the biggest concern.

I updated the sigreturn test again -- the new version exercises the
double-fault case, too. It passes on a non-Xen kernel with the
krpobes fix.

--Andy

>
> Thanks.
>
> -boris
>



--
Andy Lutomirski
AMA Capital Management, LLC