Re: [PATCH 3/4] x86: drop ASM_{CL,ST}AC

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/07/2020 11:49, Jan Beulich wrote:
> Use ALTERNATIVE directly, such that at the use sites it is visible that
> alternative code patching is in use. Similarly avoid hiding the fact in
> SAVE_ALL.
>
> No change to generated code.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Definitely +1 to not hiding the STAC/CLAC in SAVE_ALL.  I've been
meaning to undo that mistake for ages.

OOI, what made you change your mind?  I'm pleased that you have.

>
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -2165,9 +2165,9 @@ void activate_debugregs(const struct vcp
>  void asm_domain_crash_synchronous(unsigned long addr)
>  {
>      /*
> -     * We need clear AC bit here because in entry.S AC is set
> -     * by ASM_STAC to temporarily allow accesses to user pages
> -     * which is prevented by SMAP by default.
> +     * We need to clear AC bit here because in entry.S it gets set to
> +     * temporarily allow accesses to user pages which is prevented by
> +     * SMAP by default.

As you're adjusting the text, It should read "We need to clear the AC
bit ..."

But I also think it would be clearer to say that exception fixup may
leave user access enabled, which we fix up here by unconditionally
disabling user access.

Preferably with this rewritten, Reviewed-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>