diff mbox series

[v2,05/39] x86/fpu/xstate: Introduce CET MSR and XSAVES supervisor states

Message ID 20220929222936.14584-6-rick.p.edgecombe@intel.com (mailing list archive)
State New
Headers show
Series Shadowstacks for userspace | expand

Commit Message

Rick Edgecombe Sept. 29, 2022, 10:29 p.m. UTC 
From: Yu-cheng Yu <yu-cheng.yu@intel.com>

Shadow stack register state can be managed with XSAVE. The registers
can logically be separated into two groups:
        * Registers controlling user-mode operation
        * Registers controlling kernel-mode operation

The architecture has two new XSAVE state components: one for each group
of those groups of registers. This lets an OS manage them separately if
it chooses. Future patches for host userspace and KVM guests will only
utilize the user-mode registers, so only configure XSAVE to save
user-mode registers. This state will add 16 bytes to the xsave buffer
size.

Future patches will use the user-mode XSAVE area to save guest user-mode
CET state. However, VMCS includes new fields for guest CET supervisor
states. KVM can use these to save and restore guest supervisor state, so
host supervisor XSAVE support is not required.

Adding this exacerbates the already unwieldy if statement in
check_xstate_against_struct() that handles warning about un-implemented
xfeatures. So refactor these check's by having XCHECK_SZ() set a bool when
it actually check's the xfeature. This ends up exceeding 80 chars, but was
better on balance than other options explored. Pass the bool as pointer to
make it clear that XCHECK_SZ() can change the variable.

While configuring user-mode XSAVE, clarify kernel-mode registers are not
managed by XSAVE by defining the xfeature in
XFEATURE_MASK_SUPERVISOR_UNSUPPORTED, like is done for XFEATURE_MASK_PT.
This serves more of a documentation as code purpose, and functionally,
only enables a few safety checks.

Both XSAVE state components are supervisor states, even the state
controlling user-mode operation. This is a departure from earlier features
like protection keys where the PKRU state a normal user (non-supervisor)
state. Having the user state be supervisor-managed ensures there is no
direct, unprivileged access to it, making it harder for an attacker to
subvert CET.

To facilitate this privileged access, define the two user-mode CET MSRs,
and the bits defined in those MSRs relevant to future shadow stack
enablement patches.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Kees Cook <keescook@chromium.org>

---

v2:
 - Change name to XFEATURE_CET_KERNEL_UNUSED (peterz)

KVM refresh:
 - Reword commit log using some verbiage posted by Dave Hansen
 - Remove unlikely to be used supervisor cet xsave struct
 - Clarify that supervisor cet state is not saved by xsave
 - Remove unused supervisor MSRs

v1:
 - Remove outdated reference to sigreturn checks on msr's.

Yu-cheng v29:
 - Move CET MSR definition up in msr-index.h.

 arch/x86/include/asm/fpu/types.h  | 14 ++++-
 arch/x86/include/asm/fpu/xstate.h |  6 +-
 arch/x86/kernel/fpu/xstate.c      | 93 ++++++++++++++++---------------
 3 files changed, 63 insertions(+), 50 deletions(-)

Comments

Kees Cook Oct. 3, 2022, 5:40 p.m. UTC | #1 
On Thu, Sep 29, 2022 at 03:29:02PM -0700, Rick Edgecombe wrote:
> [...]
> xfeatures. So refactor these check's by having XCHECK_SZ() set a bool when
> it actually check's the xfeature. This ends up exceeding 80 chars, but was

Spelling nit through-out all patches: possessive used for plurals. E.g.
the above "check's" instances should be "checks". Please review all the
documentation and commit logs; it shows up a lot. :)

> [...]
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index c8340156bfd2..5e6a4867fd05 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -39,26 +39,26 @@
>   */
>  static const char *xfeature_names[] =
>  {
> -	"x87 floating point registers"	,
> -	"SSE registers"			,
> -	"AVX registers"			,
> -	"MPX bounds registers"		,
> -	"MPX CSR"			,
> -	"AVX-512 opmask"		,
> -	"AVX-512 Hi256"			,
> -	"AVX-512 ZMM_Hi256"		,
> -	"Processor Trace (unused)"	,
> -	"Protection Keys User registers",
> -	"PASID state",
> -	"unknown xstate feature"	,
> -	"unknown xstate feature"	,
> -	"unknown xstate feature"	,
> -	"unknown xstate feature"	,
> -	"unknown xstate feature"	,
> -	"unknown xstate feature"	,
> -	"AMX Tile config"		,
> -	"AMX Tile data"			,
> -	"unknown xstate feature"	,
> +	"x87 floating point registers"			,
> +	"SSE registers"					,
> +	"AVX registers"					,
> +	"MPX bounds registers"				,
> +	"MPX CSR"					,
> +	"AVX-512 opmask"				,
> +	"AVX-512 Hi256"					,
> +	"AVX-512 ZMM_Hi256"				,
> +	"Processor Trace (unused)"			,
> +	"Protection Keys User registers"		,
> +	"PASID state"					,
> +	"Control-flow User registers"			,
> +	"Control-flow Kernel registers (unused)"	,
> +	"unknown xstate feature"			,
> +	"unknown xstate feature"			,
> +	"unknown xstate feature"			,
> +	"unknown xstate feature"			,
> +	"AMX Tile config"				,
> +	"AMX Tile data"					,
> +	"unknown xstate feature"			,

What a strange style. Why not just leave the commas after the " ? Then
these kinds of multi-line updates aren't needed in the future.

> [...]
> -	/*
> -	 * Make *SURE* to add any feature numbers in below if
> -	 * there are "holes" in the xsave state component
> -	 * numbers.
> -	 */
> -	if ((nr < XFEATURE_YMM) ||
> -	    (nr >= XFEATURE_MAX) ||
> -	    (nr == XFEATURE_PT_UNIMPLEMENTED_SO_FAR) ||
> -	    ((nr >= XFEATURE_RSRVD_COMP_11) && (nr <= XFEATURE_RSRVD_COMP_16))) {
> +	if (!chked) {
>  		WARN_ONCE(1, "no structure for xstate: %d\n", nr);
>  		XSTATE_WARN_ON(1);
>  		return false;

This clean-up feels like it should be part of a separate patch, but
okay. :)

Reviewed-by: Kees Cook <keescook@chromium.org>
Borislav Petkov Oct. 15, 2022, 9:46 a.m. UTC | #2 
On Thu, Sep 29, 2022 at 03:29:02PM -0700, Rick Edgecombe wrote:
> Both XSAVE state components are supervisor states, even the state
> controlling user-mode operation. This is a departure from earlier features
> like protection keys where the PKRU state a normal user (non-supervisor)
^^^^^

A verb is missing in that sentence.

> +	"x87 floating point registers"			,
> +	"SSE registers"					,
> +	"AVX registers"					,
> +	"MPX bounds registers"				,
> +	"MPX CSR"					,
> +	"AVX-512 opmask"				,
> +	"AVX-512 Hi256"					,
> +	"AVX-512 ZMM_Hi256"				,
> +	"Processor Trace (unused)"			,
> +	"Protection Keys User registers"		,
> +	"PASID state"					,
> +	"Control-flow User registers"			,
> +	"Control-flow Kernel registers (unused)"	,
> +	"unknown xstate feature"			,
> +	"unknown xstate feature"			,
> +	"unknown xstate feature"			,
> +	"unknown xstate feature"			,
> +	"AMX Tile config"				,
> +	"AMX Tile data"					,
> +	"unknown xstate feature"			,

What Kees said. :)

> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_YMM,       struct ymmh_struct);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_BNDREGS,   struct mpx_bndreg_state);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_BNDCSR,    struct mpx_bndcsr_state);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_OPMASK,    struct avx_512_opmask_state);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_ZMM_Hi256, struct avx_512_zmm_uppers_state);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_Hi16_ZMM,  struct avx_512_hi16_state);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_PKRU,      struct pkru_state);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_PASID,     struct ia32_pasid_state);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_XTILE_CFG, struct xtile_cfg);
> +	XCHECK_SZ(&chked, sz, nr, XFEATURE_CET_USER,  struct cet_user_state);

That looks silly. I wonder if you could do:

	switch (nr) {
	case XFEATURE_YMM:	XCHECK_SZ(sz, XFEATURE_YMM, struct ymmh_struct);	  return;
	case XFEATURE_BNDREGS:	XCHECK_SZ(sz, XFEATURE_BNDREGS, struct mpx_bndreg_state); return;
	case ...
	...
	default:
		/* that falls into the WARN etc */

and then you get rid of the if check in the macro itself and leave the
macro be a dumb, unconditional one.

Hmmm.
Rick Edgecombe Oct. 17, 2022, 6:57 p.m. UTC | #3 
On Sat, 2022-10-15 at 11:46 +0200, Borislav Petkov wrote:
> On Thu, Sep 29, 2022 at 03:29:02PM -0700, Rick Edgecombe wrote:
> > Both XSAVE state components are supervisor states, even the state
> > controlling user-mode operation. This is a departure from earlier
> > features
> > like protection keys where the PKRU state a normal user (non-
> > supervisor)
> 
> ^^^^^
> 
> A verb is missing in that sentence.

Oops yes.

> 
> > +	"x87 floating point registers"			,
> > +	"SSE registers"					,
> > +	"AVX registers"					,
> > +	"MPX bounds registers"				,
> > +	"MPX CSR"					,
> > +	"AVX-512 opmask"				,
> > +	"AVX-512 Hi256"					,
> > +	"AVX-512 ZMM_Hi256"				,
> > +	"Processor Trace (unused)"			,
> > +	"Protection Keys User registers"		,
> > +	"PASID state"					,
> > +	"Control-flow User registers"			,
> > +	"Control-flow Kernel registers (unused)"	,
> > +	"unknown xstate feature"			,
> > +	"unknown xstate feature"			,
> > +	"unknown xstate feature"			,
> > +	"unknown xstate feature"			,
> > +	"AMX Tile config"				,
> > +	"AMX Tile data"					,
> > +	"unknown xstate feature"			,
> 
> What Kees said. :)

Sure, I'll adjust the comma.

> 
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_YMM,       struct
> > ymmh_struct);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_BNDREGS,   struct
> > mpx_bndreg_state);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_BNDCSR,    struct
> > mpx_bndcsr_state);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_OPMASK,    struct
> > avx_512_opmask_state);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_ZMM_Hi256, struct
> > avx_512_zmm_uppers_state);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_Hi16_ZMM,  struct
> > avx_512_hi16_state);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_PKRU,      struct
> > pkru_state);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_PASID,     struct
> > ia32_pasid_state);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_XTILE_CFG, struct
> > xtile_cfg);
> > +	XCHECK_SZ(&chked, sz, nr, XFEATURE_CET_USER,  struct
> > cet_user_state);
> 
> That looks silly. I wonder if you could do:
> 
> 	switch (nr) {
> 	case XFEATURE_YMM:	XCHECK_SZ(sz, XFEATURE_YMM, struct
> ymmh_struct);	  return;
> 	case XFEATURE_BNDREGS:	XCHECK_SZ(sz, XFEATURE_BNDREGS,
> struct mpx_bndreg_state); return;
> 	case ...
> 	...
> 	default:
> 		/* that falls into the WARN etc */
> 
> and then you get rid of the if check in the macro itself and leave
> the
> macro be a dumb, unconditional one.
> 
> Hmmm.
> 

Hmm yea. Another reason the actual define is passed in is that the
macro want's to stringify the XFEATURE define in order to generate the 
message like this:
XFEATURE_YMM: struct is 123 bytes, cpu state is 456 bytes

The exact format of the message is probably not too critical though. If
instead it used xfeature_names[], it could be:
[AVX registers]: struct is 123 bytes, cpu state is 456 bytes

The full block looks like (like you had):
switch (nr) {
case XFEATURE_YMM:	  return XCHECK_SZ(sz, nr, struct ymmh_struct);
case XFEATURE_BNDREGS:	  return XCHECK_SZ(sz, nr, struct
mpx_bndreg_state);
case XFEATURE_BNDCSR:	  return XCHECK_SZ(sz, nr, struct
mpx_bndcsr_state);
case XFEATURE_OPMASK:	  return XCHECK_SZ(sz, nr, struct
avx_512_opmask_state);
case XFEATURE_ZMM_Hi256:  return XCHECK_SZ(sz, nr, struct
avx_512_zmm_uppers_state);
case XFEATURE_Hi16_ZMM:	  return XCHECK_SZ(sz, nr, struct
avx_512_hi16_state);
case XFEATURE_PKRU: 	  return XCHECK_SZ(sz, nr, struct pkru_state);
case XFEATURE_PASID: 	  return XCHECK_SZ(sz, nr, struct
ia32_pasid_state);
case XFEATURE_XTILE_CFG:  return XCHECK_SZ(sz, nr, struct xtile_cfg);
case XFEATURE_CET_USER:	  return XCHECK_SZ(sz, nr, struct
cet_user_state);
case XFEATURE_XTILE_DATA: check_xtile_data_against_struct(sz); return
true;
default:
	WARN_ONCE(1, "no structure for xstate: %d\n", nr);
	XSTATE_WARN_ON(1);
	return false;
}

I like how it fits the XFEATURE_XTILE_DATA check in with the rest.

Thanks!
Borislav Petkov Oct. 17, 2022, 7:33 p.m. UTC | #4 
On Mon, Oct 17, 2022 at 06:57:13PM +0000, Edgecombe, Rick P wrote:
> Hmm yea. Another reason the actual define is passed in is that the
> macro want's to stringify the XFEATURE define in order to generate the 
> message like this:
> XFEATURE_YMM: struct is 123 bytes, cpu state is 456 bytes
> 
> The exact format of the message is probably not too critical though. If
> instead it used xfeature_names[], it could be:
> [AVX registers]: struct is 123 bytes, cpu state is 456 bytes

Bah, "registers", that made me look at the thing. Yeah, not sure if all
those "registers" strings even matter there.

[AVX]: struct is 123 bytes, cpu state is 456 bytes

looks good enough to me too. But WTH do I know.

> The full block looks like (like you had):
> switch (nr) {
> case XFEATURE_YMM:	  return XCHECK_SZ(sz, nr, struct ymmh_struct);
> case XFEATURE_BNDREGS:	  return XCHECK_SZ(sz, nr, struct
> mpx_bndreg_state);
> case XFEATURE_BNDCSR:	  return XCHECK_SZ(sz, nr, struct
> mpx_bndcsr_state);
> case XFEATURE_OPMASK:	  return XCHECK_SZ(sz, nr, struct
> avx_512_opmask_state);
> case XFEATURE_ZMM_Hi256:  return XCHECK_SZ(sz, nr, struct
> avx_512_zmm_uppers_state);
> case XFEATURE_Hi16_ZMM:	  return XCHECK_SZ(sz, nr, struct
> avx_512_hi16_state);
> case XFEATURE_PKRU: 	  return XCHECK_SZ(sz, nr, struct pkru_state);
> case XFEATURE_PASID: 	  return XCHECK_SZ(sz, nr, struct
> ia32_pasid_state);
> case XFEATURE_XTILE_CFG:  return XCHECK_SZ(sz, nr, struct xtile_cfg);
> case XFEATURE_CET_USER:	  return XCHECK_SZ(sz, nr, struct
> cet_user_state);
> case XFEATURE_XTILE_DATA: check_xtile_data_against_struct(sz); return
> true;
> default:
> 	WARN_ONCE(1, "no structure for xstate: %d\n", nr);
> 	XSTATE_WARN_ON(1);
> 	return false;
> }
> 
> I like how it fits the XFEATURE_XTILE_DATA check in with the rest.

Yap, nice and straight-forward pattern. :)

Thx.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index eb7cd1139d97..344baad02b97 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -115,8 +115,8 @@  enum xfeature {
 	XFEATURE_PT_UNIMPLEMENTED_SO_FAR,
 	XFEATURE_PKRU,
 	XFEATURE_PASID,
-	XFEATURE_RSRVD_COMP_11,
-	XFEATURE_RSRVD_COMP_12,
+	XFEATURE_CET_USER,
+	XFEATURE_CET_KERNEL_UNUSED,
 	XFEATURE_RSRVD_COMP_13,
 	XFEATURE_RSRVD_COMP_14,
 	XFEATURE_LBR,
@@ -138,6 +138,8 @@  enum xfeature {
 #define XFEATURE_MASK_PT		(1 << XFEATURE_PT_UNIMPLEMENTED_SO_FAR)
 #define XFEATURE_MASK_PKRU		(1 << XFEATURE_PKRU)
 #define XFEATURE_MASK_PASID		(1 << XFEATURE_PASID)
+#define XFEATURE_MASK_CET_USER		(1 << XFEATURE_CET_USER)
+#define XFEATURE_MASK_CET_KERNEL	(1 << XFEATURE_CET_KERNEL_UNUSED)
 #define XFEATURE_MASK_LBR		(1 << XFEATURE_LBR)
 #define XFEATURE_MASK_XTILE_CFG		(1 << XFEATURE_XTILE_CFG)
 #define XFEATURE_MASK_XTILE_DATA	(1 << XFEATURE_XTILE_DATA)
@@ -252,6 +254,14 @@  struct pkru_state {
 	u32				pad;
 } __packed;
 
+/*
+ * State component 11 is Control-flow Enforcement user states
+ */
+struct cet_user_state {
+	u64 user_cet;			/* user control-flow settings */
+	u64 user_ssp;			/* user shadow stack pointer */
+};
+
 /*
  * State component 15: Architectural LBR configuration state.
  * The size of Arch LBR state depends on the number of LBRs (lbr_depth).
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index cd3dd170e23a..d4427b88ee12 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -50,7 +50,8 @@ 
 #define XFEATURE_MASK_USER_DYNAMIC	XFEATURE_MASK_XTILE_DATA
 
 /* All currently supported supervisor features */
-#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID)
+#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID | \
+					    XFEATURE_MASK_CET_USER)
 
 /*
  * A supervisor state component may not always contain valuable information,
@@ -77,7 +78,8 @@ 
  * Unsupported supervisor features. When a supervisor feature in this mask is
  * supported in the future, move it to the supported supervisor feature mask.
  */
-#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT)
+#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT | \
+					      XFEATURE_MASK_CET_KERNEL)
 
 /* All supervisor states including supported and unsupported states. */
 #define XFEATURE_MASK_SUPERVISOR_ALL (XFEATURE_MASK_SUPERVISOR_SUPPORTED | \
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c8340156bfd2..5e6a4867fd05 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -39,26 +39,26 @@ 
  */
 static const char *xfeature_names[] =
 {
-	"x87 floating point registers"	,
-	"SSE registers"			,
-	"AVX registers"			,
-	"MPX bounds registers"		,
-	"MPX CSR"			,
-	"AVX-512 opmask"		,
-	"AVX-512 Hi256"			,
-	"AVX-512 ZMM_Hi256"		,
-	"Processor Trace (unused)"	,
-	"Protection Keys User registers",
-	"PASID state",
-	"unknown xstate feature"	,
-	"unknown xstate feature"	,
-	"unknown xstate feature"	,
-	"unknown xstate feature"	,
-	"unknown xstate feature"	,
-	"unknown xstate feature"	,
-	"AMX Tile config"		,
-	"AMX Tile data"			,
-	"unknown xstate feature"	,
+	"x87 floating point registers"			,
+	"SSE registers"					,
+	"AVX registers"					,
+	"MPX bounds registers"				,
+	"MPX CSR"					,
+	"AVX-512 opmask"				,
+	"AVX-512 Hi256"					,
+	"AVX-512 ZMM_Hi256"				,
+	"Processor Trace (unused)"			,
+	"Protection Keys User registers"		,
+	"PASID state"					,
+	"Control-flow User registers"			,
+	"Control-flow Kernel registers (unused)"	,
+	"unknown xstate feature"			,
+	"unknown xstate feature"			,
+	"unknown xstate feature"			,
+	"unknown xstate feature"			,
+	"AMX Tile config"				,
+	"AMX Tile data"					,
+	"unknown xstate feature"			,
 };
 
 static unsigned short xsave_cpuid_features[] __initdata = {
@@ -73,6 +73,7 @@  static unsigned short xsave_cpuid_features[] __initdata = {
 	[XFEATURE_PT_UNIMPLEMENTED_SO_FAR]	= X86_FEATURE_INTEL_PT,
 	[XFEATURE_PKRU]				= X86_FEATURE_PKU,
 	[XFEATURE_PASID]			= X86_FEATURE_ENQCMD,
+	[XFEATURE_CET_USER]			= X86_FEATURE_SHSTK,
 	[XFEATURE_XTILE_CFG]			= X86_FEATURE_AMX_TILE,
 	[XFEATURE_XTILE_DATA]			= X86_FEATURE_AMX_TILE,
 };
@@ -276,6 +277,7 @@  static void __init print_xstate_features(void)
 	print_xstate_feature(XFEATURE_MASK_Hi16_ZMM);
 	print_xstate_feature(XFEATURE_MASK_PKRU);
 	print_xstate_feature(XFEATURE_MASK_PASID);
+	print_xstate_feature(XFEATURE_MASK_CET_USER);
 	print_xstate_feature(XFEATURE_MASK_XTILE_CFG);
 	print_xstate_feature(XFEATURE_MASK_XTILE_DATA);
 }
@@ -344,6 +346,7 @@  static __init void os_xrstor_booting(struct xregs_state *xstate)
 	 XFEATURE_MASK_BNDREGS |		\
 	 XFEATURE_MASK_BNDCSR |			\
 	 XFEATURE_MASK_PASID |			\
+	 XFEATURE_MASK_CET_USER |		\
 	 XFEATURE_MASK_XTILE)
 
 /*
@@ -446,13 +449,14 @@  static void __init __xstate_dump_leaves(void)
 	}									\
 } while (0)
 
-#define XCHECK_SZ(sz, nr, nr_macro, __struct) do {			\
-	if ((nr == nr_macro) &&						\
-	    WARN_ONCE(sz != sizeof(__struct),				\
-		"%s: struct is %zu bytes, cpu state %d bytes\n",	\
-		__stringify(nr_macro), sizeof(__struct), sz)) {		\
-		__xstate_dump_leaves();					\
-	}								\
+#define XCHECK_SZ(checked, sz, nr, nr_macro, __struct) do {			\
+	if (nr == nr_macro) {							\
+		*checked = true;						\
+		if (WARN_ONCE(sz != sizeof(__struct),				\
+			      "%s: struct is %zu bytes, cpu state %d bytes\n",	\
+			      __stringify(nr_macro), sizeof(__struct), sz))	\
+			__xstate_dump_leaves();					\
+	}									\
 } while (0)
 
 /**
@@ -527,33 +531,30 @@  static bool __init check_xstate_against_struct(int nr)
 	 * Ask the CPU for the size of the state.
 	 */
 	int sz = xfeature_size(nr);
+	bool chked = false;
+
 	/*
 	 * Match each CPU state with the corresponding software
 	 * structure.
 	 */
-	XCHECK_SZ(sz, nr, XFEATURE_YMM,       struct ymmh_struct);
-	XCHECK_SZ(sz, nr, XFEATURE_BNDREGS,   struct mpx_bndreg_state);
-	XCHECK_SZ(sz, nr, XFEATURE_BNDCSR,    struct mpx_bndcsr_state);
-	XCHECK_SZ(sz, nr, XFEATURE_OPMASK,    struct avx_512_opmask_state);
-	XCHECK_SZ(sz, nr, XFEATURE_ZMM_Hi256, struct avx_512_zmm_uppers_state);
-	XCHECK_SZ(sz, nr, XFEATURE_Hi16_ZMM,  struct avx_512_hi16_state);
-	XCHECK_SZ(sz, nr, XFEATURE_PKRU,      struct pkru_state);
-	XCHECK_SZ(sz, nr, XFEATURE_PASID,     struct ia32_pasid_state);
-	XCHECK_SZ(sz, nr, XFEATURE_XTILE_CFG, struct xtile_cfg);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_YMM,       struct ymmh_struct);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_BNDREGS,   struct mpx_bndreg_state);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_BNDCSR,    struct mpx_bndcsr_state);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_OPMASK,    struct avx_512_opmask_state);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_ZMM_Hi256, struct avx_512_zmm_uppers_state);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_Hi16_ZMM,  struct avx_512_hi16_state);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_PKRU,      struct pkru_state);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_PASID,     struct ia32_pasid_state);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_XTILE_CFG, struct xtile_cfg);
+	XCHECK_SZ(&chked, sz, nr, XFEATURE_CET_USER,  struct cet_user_state);
 
 	/* The tile data size varies between implementations. */
-	if (nr == XFEATURE_XTILE_DATA)
+	if (nr == XFEATURE_XTILE_DATA) {
 		check_xtile_data_against_struct(sz);
+		chked = true;
+	}
 
-	/*
-	 * Make *SURE* to add any feature numbers in below if
-	 * there are "holes" in the xsave state component
-	 * numbers.
-	 */
-	if ((nr < XFEATURE_YMM) ||
-	    (nr >= XFEATURE_MAX) ||
-	    (nr == XFEATURE_PT_UNIMPLEMENTED_SO_FAR) ||
-	    ((nr >= XFEATURE_RSRVD_COMP_11) && (nr <= XFEATURE_RSRVD_COMP_16))) {
+	if (!chked) {
 		WARN_ONCE(1, "no structure for xstate: %d\n", nr);
 		XSTATE_WARN_ON(1);
 		return false;