[PATCH 2/5] xen/x86-64: clean up warnings about IST-using traps

[PATCH 2/5] xen/x86-64: clean up warnings about IST-using traps

[Jeremy Fitzhardinge]

Ignore known IST-using traps.  Aside from the debugger traps, they're
low-level faults which Xen will handle for us, so the kernel needn't
worry about them.  Keep warning in case unknown trap starts using IST.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/enlighten.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index cb49f57..88f3aa4 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -439,12 +439,30 @@ static int cvt_gate_to_trap(int vector, const gate_desc *val,
 
 	addr = gate_offset(*val);
 #ifdef CONFIG_X86_64
+	/*
+	 * Look for known traps using IST, and substitute them
+	 * appropriately.  The debugger ones are the only ones we care
+	 * about.  Xen will handle faults like double_fault and
+	 * machine_check, so we should never see them.  Warn if
+	 * there's an unexpected IST-using fault handler.
+	 */
 	if (addr == (unsigned long)debug)
 		addr = (unsigned long)xen_debug;
 	else if (addr == (unsigned long)int3)
 		addr = (unsigned long)xen_int3;
-	else
-		WARN_ON(val->ist != 0);
+	else if (addr == (unsigned long)double_fault ||
+		 addr == (unsigned long)stack_segment) {
+		/* Don't need to handle these */
+		return 0;
+#ifdef CONFIG_X86_MCE
+	} else if (addr == (unsigned long)machine_check) {
+		return 0;
+#endif
+	} else {
+		/* Some other trap using IST? */
+		if (WARN_ON(val->ist != 0))
+			return 0;
+	}
 #endif	/* CONFIG_X86_64 */
 	info->address = addr;
 
-- 
1.6.0.6

Re: [Xen-devel] [PATCH 2/5] xen/x86-64: clean up warnings aboutIST-using traps

[Jan Beulich]

>>> Jeremy Fitzhardinge <jeremy@goop.org> 07.05.09 20:56 >>>
>Ignore known IST-using traps.  Aside from the debugger traps, they're
>low-level faults which Xen will handle for us, so the kernel needn't
>worry about them.  Keep warning in case unknown trap starts using IST.
>
>Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>---
> arch/x86/xen/enlighten.c |   22 ++++++++++++++++++++--
> 1 files changed, 20 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>index cb49f57..88f3aa4 100644
>--- a/arch/x86/xen/enlighten.c
>+++ b/arch/x86/xen/enlighten.c
>@@ -439,12 +439,30 @@ static int cvt_gate_to_trap(int vector, const gate_desc *val,
> 
> 	addr = gate_offset(*val);
> #ifdef CONFIG_X86_64
>+	/*
>+	 * Look for known traps using IST, and substitute them
>+	 * appropriately.  The debugger ones are the only ones we care
>+	 * about.  Xen will handle faults like double_fault and
>+	 * machine_check, so we should never see them.  Warn if
>+	 * there's an unexpected IST-using fault handler.
>+	 */
> 	if (addr == (unsigned long)debug)
> 		addr = (unsigned long)xen_debug;
> 	else if (addr == (unsigned long)int3)
> 		addr = (unsigned long)xen_int3;
>-	else
>-		WARN_ON(val->ist != 0);
>+	else if (addr == (unsigned long)double_fault ||
>+		 addr == (unsigned long)stack_segment) {

I don't think you want to exclude handling stack faults: Ordinary memory
references using rsp or rbp as the base register will cause these instead
of general protection faults when the resulting effective address is non-
canonical.

>+		/* Don't need to handle these */
>+		return 0;
>+#ifdef CONFIG_X86_MCE
>+	} else if (addr == (unsigned long)machine_check) {
>+		return 0;
>+#endif
>+	} else {
>+		/* Some other trap using IST? */
>+		if (WARN_ON(val->ist != 0))
>+			return 0;
>+	}
> #endif	/* CONFIG_X86_64 */
> 	info->address = addr;
> 
>-- 
>1.6.0.6

Jan

Re: [Xen-devel] [PATCH 2/5] xen/x86-64: clean up warnings aboutIST-using traps

[Ingo Molnar]

* Jan Beulich <jbeulich@novell.com> wrote:

> >>> Jeremy Fitzhardinge <jeremy@goop.org> 07.05.09 20:56 >>>
> >Ignore known IST-using traps.  Aside from the debugger traps, they're
> >low-level faults which Xen will handle for us, so the kernel needn't
> >worry about them.  Keep warning in case unknown trap starts using IST.
> >
> >Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> >---
> > arch/x86/xen/enlighten.c |   22 ++++++++++++++++++++--
> > 1 files changed, 20 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> >index cb49f57..88f3aa4 100644
> >--- a/arch/x86/xen/enlighten.c
> >+++ b/arch/x86/xen/enlighten.c
> >@@ -439,12 +439,30 @@ static int cvt_gate_to_trap(int vector, const gate_desc *val,
> > 
> > 	addr = gate_offset(*val);
> > #ifdef CONFIG_X86_64
> >+	/*
> >+	 * Look for known traps using IST, and substitute them
> >+	 * appropriately.  The debugger ones are the only ones we care
> >+	 * about.  Xen will handle faults like double_fault and
> >+	 * machine_check, so we should never see them.  Warn if
> >+	 * there's an unexpected IST-using fault handler.
> >+	 */
> > 	if (addr == (unsigned long)debug)
> > 		addr = (unsigned long)xen_debug;
> > 	else if (addr == (unsigned long)int3)
> > 		addr = (unsigned long)xen_int3;
> >-	else
> >-		WARN_ON(val->ist != 0);
> >+	else if (addr == (unsigned long)double_fault ||
> >+		 addr == (unsigned long)stack_segment) {
> 
> I don't think you want to exclude handling stack faults: Ordinary 
> memory references using rsp or rbp as the base register will cause 
> these instead of general protection faults when the resulting 
> effective address is non- canonical.

Yes. Also, patches 1-2-3 really just do the same thing, one fixes 
the other one. Would be nice to have a single patch for that whole 
ugly topic of 64-bit Xen not handling ISTs properly.

If this was a CPU we'd say "sorry, dont run Linux on it then" ...

I've applied the reservation fix to x86/urgent, and the #5 patch to 
x86/xen (it's more of a cleanup, not a fix for .30, right?).

	Ingo

Re: [Xen-devel] [PATCH 2/5] xen/x86-64: clean up warnings aboutIST-using traps

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> Yes. Also, patches 1-2-3 really just do the same thing, one fixes 
> the other one. Would be nice to have a single patch for that whole 
> ugly topic of 64-bit Xen not handling ISTs properly.
>   

Well, 1 is sufficient on its own to fix the breakpoint/watchpoint 
problem.  Looks like I need to do something similar with stack.  The 2,3 
are more general cleanups that aren't essential to the functional fix, 
and should definitely be folded together.

> If this was a CPU we'd say "sorry, dont run Linux on it then" ...
>   

??  Guests don't really need IST because all the tricky traps that 
really require it (NMI, MCE, etc) are handled by Xen.  The stack 
exception was an oversight on my part, because I thought it was 
something that would only happen with a bad kernel stack (but I guess 
that's doublefault).

> I've applied the reservation fix to x86/urgent, and the #5 patch to 
> x86/xen (it's more of a cleanup, not a fix for .30, right?).
>   

Right.

    J

Re: [Xen-devel] [PATCH 2/5] xen/x86-64: clean up warnings aboutIST-using traps

[Jeremy Fitzhardinge]

Jan Beulich wrote:
>> else if (addr == (unsigned long)int3)
>> 		addr = (unsigned long)xen_int3;
>> -	else
>> -		WARN_ON(val->ist != 0);
>> +	else if (addr == (unsigned long)double_fault ||
>> +		 addr == (unsigned long)stack_segment) {
>>     
>
> I don't think you want to exclude handling stack faults: Ordinary memory
> references using rsp or rbp as the base register will cause these instead
> of general protection faults when the resulting effective address is non-
> canonical.
>   

So even usermode memory accesses?  Ew.  Well, we can run the trap 
handler on the normal kernel stack in that case.  I'll cook something up.

    J

Re: [Xen-devel] [PATCH 2/5] xen/x86-64: clean up warnings aboutIST-usingtraps

[Jan Beulich]

>>> Jeremy Fitzhardinge <jeremy@goop.org> 08.05.09 17:23 >>>
>Jan Beulich wrote:
>>> else if (addr == (unsigned long)int3)
>>> 		addr = (unsigned long)xen_int3;
>>> -	else
>>> -		WARN_ON(val->ist != 0);
>>> +	else if (addr == (unsigned long)double_fault ||
>>> +		 addr == (unsigned long)stack_segment) {
>>>     
>>
>> I don't think you want to exclude handling stack faults: Ordinary memory
>> references using rsp or rbp as the base register will cause these instead
>> of general protection faults when the resulting effective address is non-
>> canonical.
>>   
>
>So even usermode memory accesses?  Ew.

Yes, just try it out - you'll observe them generate SIGBUS instead of SIGSEGV.

Jan