On 28/05/2020 13:03, Jan Beulich wrote: > On 27.05.2020 21:18, Andrew Cooper wrote: >> For now, any #CP exception or shadow stack #PF indicate a bug in Xen, but >> attempt to recover from #CP if taken in guest context. >> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > Reviewed-by: Jan Beulich <jbeul...@suse.com> > with one more question and a suggestion: > >> @@ -1445,8 +1447,10 @@ void do_page_fault(struct cpu_user_regs *regs) >> * >> * Anything remaining is an error, constituting corruption of the >> * pagetables and probably an L1TF vulnerable gadget. >> + * >> + * Any shadow stack access fault is a bug in Xen. >> */ >> - if ( error_code & PFEC_reserved_bit ) >> + if ( error_code & (PFEC_reserved_bit | PFEC_shstk) ) >> goto fatal; > Wouldn't you saying "any" imply putting this new check higher up, in > particular ahead of the call to fixup_page_fault()?
Can do. > >> @@ -940,7 +944,8 @@ autogen_stubs: /* Automatically generated stubs. */ >> entrypoint 1b >> >> /* Reserved exceptions, heading towards do_reserved_trap(). */ >> - .elseif vec == TRAP_copro_seg || vec == TRAP_spurious_int || (vec > >> TRAP_simd_error && vec < TRAP_nr) >> + .elseif vec == X86_EXC_CSO || vec == X86_EXC_SPV || \ >> + vec == X86_EXC_VE || (vec > X86_EXC_CP && vec < TRAP_nr) > Adding yet another || here adds to the fragility of the entire > construct. Wouldn't it be better to implement do_entry_VE at > this occasion, even its handling continues to end up in > do_reserved_trap()? This would have the benefit of avoiding the > pointless checking of %spl first thing in its handling. Feel > free to keep the R-b if you decide to go this route. I actually have a different plan, which deletes this entire clause, and simplifies our autogen sanity checking somewhat. For vectors which Xen has no implementation of (for whatever reason), use DPL0, non-present descriptors, and redirect #NP[IDT] into do_reserved_trap(). No need for any entry logic for the trivially fatal paths, or safety against being uncertain about error codes. However, its a little too close to 4.14 to clean this up now. ~Andrew
