| Message ID | 20230915150038.602577-5-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/spec-ctrl: AMD DIV fix, and VERW prerequisite bugfixes | expand |
On 15.09.2023 17:00, Andrew Cooper wrote: > ... to better explain how they're used. > > Doing so highlights that SPEC_CTRL_EXIT_TO_XEN is missing a VERW flush for the > corner case when e.g. an NMI hits late in an exit-to-guest path. > > Leave a TODO, which will be addressed in subsequent patches which arrange for > DO_COND_VERW to be safe within SPEC_CTRL_EXIT_TO_XEN. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Two nits though: > @@ -233,7 +236,11 @@ > X86_FEATURE_SC_MSR_PV > .endm > > -/* Use in interrupt/exception context. May interrupt Xen or PV context. */ > +/* > + * Used after an exception or maskable interrupt, hitting Xen or PV context. > + * There will either be a guest speculation context, or (baring fatal Isn't this "barring"? > @@ -260,7 +270,13 @@ > .endm > > /* > - * Use in IST interrupt/exception context. May interrupt Xen or PV context. > + * Used after an IST entry hitting Xen or PV context. Special care is needed, > + * because when hitting Xen context, there may not a well-formed speculation Missing "be"? Jan
On 18/09/2023 11:59 am, Jan Beulich wrote: > On 15.09.2023 17:00, Andrew Cooper wrote: >> ... to better explain how they're used. >> >> Doing so highlights that SPEC_CTRL_EXIT_TO_XEN is missing a VERW flush for the >> corner case when e.g. an NMI hits late in an exit-to-guest path. >> >> Leave a TODO, which will be addressed in subsequent patches which arrange for >> DO_COND_VERW to be safe within SPEC_CTRL_EXIT_TO_XEN. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Two nits though: > >> @@ -233,7 +236,11 @@ >> X86_FEATURE_SC_MSR_PV >> .endm >> >> -/* Use in interrupt/exception context. May interrupt Xen or PV context. */ >> +/* >> + * Used after an exception or maskable interrupt, hitting Xen or PV context. >> + * There will either be a guest speculation context, or (baring fatal > Isn't this "barring"? > >> @@ -260,7 +270,13 @@ >> .endm >> >> /* >> - * Use in IST interrupt/exception context. May interrupt Xen or PV context. >> + * Used after an IST entry hitting Xen or PV context. Special care is needed, >> + * because when hitting Xen context, there may not a well-formed speculation > Missing "be"? Both fixed, thanks. ~Andrew
diff --git a/xen/arch/x86/include/asm/spec_ctrl_asm.h b/xen/arch/x86/include/asm/spec_ctrl_asm.h index f768b0f48a0b..8996fe3fc0ef 100644 --- a/xen/arch/x86/include/asm/spec_ctrl_asm.h +++ b/xen/arch/x86/include/asm/spec_ctrl_asm.h @@ -218,7 +218,10 @@ wrmsr .endm -/* Use after an entry from PV context (syscall/sysenter/int80/int82/etc). */ +/* + * Used after an entry from PV context: SYSCALL, SYSENTER, INT, + * etc. There is always a guest speculation state in context. + */ .macro SPEC_CTRL_ENTRY_FROM_PV /* * Requires %rsp=regs/cpuinfo, %rdx=0 @@ -233,7 +236,11 @@ X86_FEATURE_SC_MSR_PV .endm -/* Use in interrupt/exception context. May interrupt Xen or PV context. */ +/* + * Used after an exception or maskable interrupt, hitting Xen or PV context. + * There will either be a guest speculation context, or (baring fatal + * exceptions) a well-formed Xen speculation context. + */ .macro SPEC_CTRL_ENTRY_FROM_INTR /* * Requires %rsp=regs, %r14=stack_end, %rdx=0 @@ -248,7 +255,10 @@ X86_FEATURE_SC_MSR_PV .endm -/* Use when exiting to PV guest context. */ +/* + * Used when exiting from any entry context, back to PV context. This + * includes from an IST entry which moved onto the primary stack. + */ .macro SPEC_CTRL_EXIT_TO_PV /* * Requires %rax=spec_ctrl, %rsp=regs/info @@ -260,7 +270,13 @@ .endm /* - * Use in IST interrupt/exception context. May interrupt Xen or PV context. + * Used after an IST entry hitting Xen or PV context. Special care is needed, + * because when hitting Xen context, there may not a well-formed speculation + * context. (i.e. it can hit in the middle of SPEC_CTRL_{ENTRY,EXIT}_* + * regions.) + * + * An IST entry which hits PV context moves onto the primary stack and leaves + * via SPEC_CTRL_EXIT_TO_PV, *not* SPEC_CTRL_EXIT_TO_XEN. */ .macro SPEC_CTRL_ENTRY_FROM_INTR_IST /* @@ -319,7 +335,14 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise): UNLIKELY_END(\@_serialise) .endm -/* Use when exiting to Xen context. */ +/* + * Use when exiting from any entry context, back to Xen context. This + * includes returning to other SPEC_CTRL_{ENTRY,EXIT}_* regions with an + * incomplete speculation context. + * + * Because we might have interrupted Xen beyond SPEC_CTRL_EXIT_TO_$GUEST, we + * need to treat this as if it were an EXIT_TO_$GUEST case too. + */ .macro SPEC_CTRL_EXIT_TO_XEN /* * Requires %rbx=stack_end @@ -344,6 +367,9 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise): wrmsr .L\@_skip_sc_msr: + + /* TODO VERW */ + .endm #endif /* __ASSEMBLY__ */
... to better explain how they're used. Doing so highlights that SPEC_CTRL_EXIT_TO_XEN is missing a VERW flush for the corner case when e.g. an NMI hits late in an exit-to-guest path. Leave a TODO, which will be addressed in subsequent patches which arrange for DO_COND_VERW to be safe within SPEC_CTRL_EXIT_TO_XEN. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> This was decided not to be XSA-worthy, as guests can't usefully control when IST events occur. v2: * Rewrite. --- xen/arch/x86/include/asm/spec_ctrl_asm.h | 36 ++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-)