Re: [PATCH v8 09/33] x86/cpu: add X86_CR4_FRED macro

[H. Peter Anvin]

On 6/5/23 05:01, Thomas Gleixner wrote:
> On Mon, Apr 10 2023 at 01:14, Xin Li wrote:
>
> > From: "H. Peter Anvin (Intel)" <hpa@xxxxxxxxx>
> >
> > Add X86_CR4_FRED macro for the FRED bit in %cr4. This bit should be a
>
> s/should/must/ no?
>
> > +/*
> > + * These bits should not change their value after CPU init is finished.
> > + * The explicit cast to unsigned long suppresses a warning on i386 for
> > + * x86-64 only feature bits >= 32.
> > + */
> >   static const unsigned long cr4_pinned_mask =
> > -	X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_UMIP |
> > -	X86_CR4_FSGSBASE | X86_CR4_CET;
> > +	(unsigned long)
>
> That type cast is required because:
>
> +#define X86_CR4_FRED		_BITULL(X86_CR4_FRED_BIT)
>
> Bah. Fred is 64 bit only. So why defining this as 1ULL << 32
> unconditionally and stripping the bit off on 32bit via the type cast?
>
> #ifdef CONFIG_X86_64
> #define X86_CR4_FRED		_BITUL(X86_CR4_FRED_BIT)
> #else
> #define X86_CR4_FRED		0
> #endif
>
> would be too obvious, right?

It also adds an #ifdef mess to avoid a simple typecast. Is that the 
right tradeoff?

I'm not saying it is or it isn't, it is an open question.

	-hpa