Re: [Xen-devel] [PATCH RFC] x86/traps: Make the main trap handlers safe for use early during Xen boot

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.05.14 at 16:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> Most of this patch is an analysis of the safety of the trap handlers.
> 
> Traps 0, 4, 5, 9-12, 16, 17 and 19 all end up in do_trap().  do_trap() is
> mostly safe, performing an exception table search and possibly panic()s.
> 
> There is one complication with traps 16 and 19 which will see about calling
> the fpu_exception_callback.  This involves following current which is not
> valid early on boot.  The has_hvm_container_vcpu(curr) check is preceeded 
> with
> a system_state check, so in the exceedingly unlikely case that Xen takes an
> x87/SIMP trap while booting, it will panic() instead of following a bogus
> current vcpu.
> 
> Traps 1, 3, 6-8, 13 and 15 are completely safe with respect to running during
> early boot.  They all have well formed and obvious differences between faults
> in Xen and faults in guests, with the Xen faults doing little more than
> exception table walks or panic()s.

I think 15 should be moved into the undefined / reserved category too.
Nothing should be generating this.

I would similarly question 9 (TRAP_copro_seg), which supposedly is
valid only for old 32-bit CPUs.

> I would appreciate some careful review of this.  As part of developing the
> other early init fixes, I will test what I can of this, but doubt it will be
> comprehensive testing.

The main thing I wonder is why you submit this separately - this is
going to become useful only once these get wired up earlier than
they are right now.

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -561,7 +561,8 @@ static void do_trap(struct cpu_user_regs *regs, int 
> use_error_code)
>      }
>  
>      if ( ((trapnr == TRAP_copro_error) || (trapnr == TRAP_simd_error)) &&
> -         has_hvm_container_vcpu(curr) && 
> curr->arch.hvm_vcpu.fpu_exception_callback )
> +         system_state == SYS_STATE_active && has_hvm_container_vcpu(curr) &&

This seems too specific a check - I think this ought to be "system_state >=
SYS_STATE_active".

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel