Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1214718 > unrolled thread

[PATCH 02/11] x86, fpu: rename xfeature_bit

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 02/11] x86, fpu: rename xfeature_bit Dave Hansen <dave@sr71.net> - 2015-08-27 19:20 +0200
    Re: [PATCH 02/11] x86, fpu: rename xfeature_bit Ingo Molnar <mingo@kernel.org> - 2015-08-28 07:20 +0200

#1214718 — [PATCH 02/11] x86, fpu: rename xfeature_bit

From: Dave Hansen <dave.hansen@linux.intel.com>

The 'xfeature_bit's are at best xfeature bit _numbers_.  Calling
them bits is at best inconsistent with ending the enum list with
'XFEATURES_NR_MAX'.

This patch renames the enum to be 'xfeature_nr'.  These also
happen to be what the Intel documentation calls a "state
component".

We also want to differentiate these from the "XSTATE_*" macros.
The "XSTATE_*" macros are a mask, and should probably be
renamed, but that's another patch.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: x86@kernel.org
Cc: Borislav Petkov <bp@alien8.de>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
---

 b/arch/x86/include/asm/fpu/types.h |   38 ++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff -puN arch/x86/include/asm/fpu/types.h~x86-fpu-rename-xfeature_bit arch/x86/include/asm/fpu/types.h
--- a/arch/x86/include/asm/fpu/types.h~x86-fpu-rename-xfeature_bit	2015-08-27 10:08:01.569634174 -0700
+++ b/arch/x86/include/asm/fpu/types.h	2015-08-27 10:08:01.572634311 -0700
@@ -95,27 +95,31 @@ struct swregs_state {
 /*
  * List of XSAVE features Linux knows about:
  */
-enum xfeature_bit {
-	XSTATE_BIT_FP,
-	XSTATE_BIT_SSE,
-	XSTATE_BIT_YMM,
-	XSTATE_BIT_BNDREGS,
-	XSTATE_BIT_BNDCSR,
-	XSTATE_BIT_OPMASK,
-	XSTATE_BIT_ZMM_Hi256,
-	XSTATE_BIT_Hi16_ZMM,
+enum xfeature_nr {
+	XFEATURE_NR_FP,
+	XFEATURE_NR_SSE,
+	/*
+	 * Values above here are "legacy states".
+	 * Those below are "extended states".
+	 */
+	XFEATURE_NR_YMM,
+	XFEATURE_NR_BNDREGS,
+	XFEATURE_NR_BNDCSR,
+	XFEATURE_NR_OPMASK,
+	XFEATURE_NR_ZMM_Hi256,
+	XFEATURE_NR_Hi16_ZMM,
 
 	XFEATURES_NR_MAX,
 };
 
-#define XSTATE_FP		(1 << XSTATE_BIT_FP)
-#define XSTATE_SSE		(1 << XSTATE_BIT_SSE)
-#define XSTATE_YMM		(1 << XSTATE_BIT_YMM)
-#define XSTATE_BNDREGS		(1 << XSTATE_BIT_BNDREGS)
-#define XSTATE_BNDCSR		(1 << XSTATE_BIT_BNDCSR)
-#define XSTATE_OPMASK		(1 << XSTATE_BIT_OPMASK)
-#define XSTATE_ZMM_Hi256	(1 << XSTATE_BIT_ZMM_Hi256)
-#define XSTATE_Hi16_ZMM		(1 << XSTATE_BIT_Hi16_ZMM)
+#define XSTATE_FP		(1 << XFEATURE_NR_FP)
+#define XSTATE_SSE		(1 << XFEATURE_NR_SSE)
+#define XSTATE_YMM		(1 << XFEATURE_NR_YMM)
+#define XSTATE_BNDREGS		(1 << XFEATURE_NR_BNDREGS)
+#define XSTATE_BNDCSR		(1 << XFEATURE_NR_BNDCSR)
+#define XSTATE_OPMASK		(1 << XFEATURE_NR_OPMASK)
+#define XSTATE_ZMM_Hi256	(1 << XFEATURE_NR_ZMM_Hi256)
+#define XSTATE_Hi16_ZMM		(1 << XFEATURE_NR_Hi16_ZMM)
 
 #define XSTATE_FPSSE		(XSTATE_FP | XSTATE_SSE)
 #define XSTATE_AVX512		(XSTATE_OPMASK | XSTATE_ZMM_Hi256 | XSTATE_Hi16_ZMM)
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1215094

* Dave Hansen <dave@sr71.net> wrote:

> +++ b/arch/x86/include/asm/fpu/types.h	2015-08-27 10:08:01.572634311 -0700
> @@ -95,27 +95,31 @@ struct swregs_state {
>  /*
>   * List of XSAVE features Linux knows about:
>   */
> -enum xfeature_bit {
> -	XSTATE_BIT_FP,
> -	XSTATE_BIT_SSE,
> -	XSTATE_BIT_YMM,
> -	XSTATE_BIT_BNDREGS,
> -	XSTATE_BIT_BNDCSR,
> -	XSTATE_BIT_OPMASK,
> -	XSTATE_BIT_ZMM_Hi256,
> -	XSTATE_BIT_Hi16_ZMM,
> +enum xfeature_nr {
> +	XFEATURE_NR_FP,
> +	XFEATURE_NR_SSE,
> +	/*
> +	 * Values above here are "legacy states".
> +	 * Those below are "extended states".
> +	 */
> +	XFEATURE_NR_YMM,
> +	XFEATURE_NR_BNDREGS,
> +	XFEATURE_NR_BNDCSR,
> +	XFEATURE_NR_OPMASK,
> +	XFEATURE_NR_ZMM_Hi256,
> +	XFEATURE_NR_Hi16_ZMM,
>  
>  	XFEATURES_NR_MAX,
>  };
> +#define XSTATE_FP		(1 << XFEATURE_NR_FP)
> +#define XSTATE_SSE		(1 << XFEATURE_NR_SSE)
> +#define XSTATE_YMM		(1 << XFEATURE_NR_YMM)
> +#define XSTATE_BNDREGS		(1 << XFEATURE_NR_BNDREGS)
> +#define XSTATE_BNDCSR		(1 << XFEATURE_NR_BNDCSR)
> +#define XSTATE_OPMASK		(1 << XFEATURE_NR_OPMASK)
> +#define XSTATE_ZMM_Hi256	(1 << XFEATURE_NR_ZMM_Hi256)
> +#define XSTATE_Hi16_ZMM		(1 << XFEATURE_NR_Hi16_ZMM)

So I think this is still somewhat confusing.

'NR' is often used as a maximum kind of thing, not as a bit index.

So I think we should instead take up the existing conventions of the cpufeatures.h 
definitions which are pretty sane, and simply name the bit indices XFEATURE_XYZ:

enum xfeatures {
	XFEATURE_FP,
	XFEATURE_SSE,
	...
	XFEATURE_MAX
};

this way we ensure that bitmasks are visibly masks, i.e.:

	#define XFEATURE_MASK_FP		(1 << XFEATURE_FP)
	#define XFEATURE_MASK_SSE		(1 << XFEATURE_SSE)

it's slightly longer to write but unambiguous, and it also matches what we use for 
the x86 CPU ID feature definitions.

Similarly we could rename other mask fields from 'xstate' to 'xfeature_mask', to 
make it all consistent throughout.

What do you think?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web