On 13.09.2023 22:27, Andrew Cooper wrote:
> --- 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 a synchronous entry from PV context. SYSCALL, SYSENTER, INT,
> + * etc. Will always interrupt a guest speculation context.
> + */
> .macro SPEC_CTRL_ENTRY_FROM_PV
> /*
> * Requires %rsp=regs/cpuinfo, %rdx=0
For the entry point comments - not being a native speaker -, the use of
"{will,may} interrupt" reads odd. You're describing the macros here,
not the the events that led to their invocation. Therefore it would seem
to me that "{will,may} have interrupted" would be more appropriate.
> @@ -233,7 +236,11 @@
> X86_FEATURE_SC_MSR_PV
> .endm
>
> -/* Use in interrupt/exception context. May interrupt Xen or PV context. */
> +/*
> + * Used after a synchronous interrupt or exception. May interrupt Xen or PV
> + * context, but will not interrupt Xen with a guest speculation context,
> + * outside of fatal error cases.
> + */
> .macro SPEC_CTRL_ENTRY_FROM_INTR
> /*
> * Requires %rsp=regs, %r14=stack_end, %rdx=0
I find "synchronous" here odd, when this includes interrupts. Below you
at least qualify things further, by saying "fully asynchronous with
finding sane state"; I think further qualification is warranted here as
well, to avoid any ambiguity.
> @@ -319,7 +334,14 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise):
> UNLIKELY_END(\@_serialise)
> .endm
>
> -/* Use when exiting to Xen in IST context. */
> +/*
> + * Use when exiting from any entry context, back to Xen context. This
> + * includes returning to other SPEC_CTRL_{ENTRY,EXIT}_* regions with
> + * unsanitised state.
> + *
> + * Because we might have interrupted Xen beyond SPEC_CTRL_EXIT_TO_$GUEST, we
> + * must treat this as if it were an EXIT_TO_$GUEST case too.
> + */
> .macro SPEC_CTRL_EXIT_TO_XEN
> /*
> * Requires %rbx=stack_end
Is it really "must"? At least in theory there are ways to recognize that
exit is back to Xen context outside of interrupted entry/exit regions
(simply by evaluating how far below stack top %rsp is).
> @@ -344,6 +366,9 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise):
> wrmsr
>
> .L\@_skip_sc_msr:
> +
> + /* TODO VERW */
> +
> .endm
I don't think this comment is strictly necessary to add here, when the
omission is addressed in a later patch. But I also don't mind its
addition.
Jan
