[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/4] x86: reduce CET-SS related #ifdef-ary

  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Tue, 28 Jul 2020 15:29:46 +0100
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Tue, 28 Jul 2020 14:30:05 +0000
  • Ironport-sdr: 4qN49BupJCa7Xl6rlLMnQJWTmnds5IOcajG79uEVeOJwNtrb85pQZt0FQmHndZnOCafupHIn2S y9zh08NWGema4O6Aizuf6zFaT56IzgF+LFJJ4erpkuFr2DD+aJ2kP0J2Ok73fCNLrSBa6O6Ziw 9bDii9LF1UfE71u2ykVRwjcbat1NuT7LFoDV2xWpupn4+3jeK/GQw30c3Byd+OF8G5D00DWD8+ vqLUCjmmjyqQqE7AvfWP0iVGLIKybZp9TYNEhwXhthh0R1rvlQBLGOwL01T8trixc7SXYyqJ2A OdI=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 15/07/2020 11:48, Jan Beulich wrote:
> Commit b586a81b7a90 ("x86/CET: Fix build following c/s 43b98e7190") had
> to introduce a number of #ifdef-s to make the build work with older tool
> chains. Introduce an assembler macro covering for tool chains not
> knowing of CET-SS, allowing some conditionals where just SETSSBSY is the
> problem to be dropped again.
> No change to generated code.

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Now that I've done this I'm not longer sure which direction is better to
> follow: On one hand this introduces dead code (even if just NOPs) into
> CET-SS-disabled builds. Otoh this is a step towards breaking the tool
> chain version dependency of the feature.

The toolchain dependency can't be broken, because of incssp and wrss in C.

There is 0 value and added complexity to trying to partially support
legacy toolchains.  Furthermore, this adds a pile of nops into builds
which have specifically opted out of CONFIG_XEN_SHSTK, which isn't ideal
for embedded usecases.

As a consequence, I think its better to keep things consistent with how
they are now.

One thing I already considered was to make cpu_has_xen_shstk return
false for !CONFIG_XEN_SHSTK, which subsumes at least one hunk in this

> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -198,9 +198,7 @@ ENTRY(cr4_pv32_restore)
>  /* See lstar_enter for entry register state. */
>  ENTRY(cstar_enter)
>          ALTERNATIVE "", "setssbsy", X86_FEATURE_XEN_SHSTK
> -#endif

I can't currently think of any option better than leaving these ifdef's
in place, other than perhaps


and I don't like it much.

The think is that everything present there is semantically relevant
information, and dropping it makes the code worse rather than better.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.