On 5/22/2024 9:04 PM, Borislav Petkov
wrote:
As discussed here :https://lore.kernel.org/linux-mm/24f71d52-0891-4cfc-8dec-9f13ed618eee@xxxxxxxxx/Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. On Wed, May 22, 2024 at 06:42:55PM +0530, Balasubrmanian, Vignesh wrote:+enum custom_feature { + FEATURE_XSAVE_FP = 0, + FEATURE_XSAVE_SSE = 1, + FEATURE_XSAVE_YMM = 2, + FEATURE_XSAVE_BNDREGS = 3, + FEATURE_XSAVE_BNDCSR = 4, + FEATURE_XSAVE_OPMASK = 5, + FEATURE_XSAVE_ZMM_Hi256 = 6, + FEATURE_XSAVE_Hi16_ZMM = 7, + FEATURE_XSAVE_PT = 8, + FEATURE_XSAVE_PKRU = 9, + FEATURE_XSAVE_PASID = 10, + FEATURE_XSAVE_CET_USER = 11, + FEATURE_XSAVE_CET_SHADOW_STACK = 12, + FEATURE_XSAVE_HDC = 13, + FEATURE_XSAVE_UINTR = 14, + FEATURE_XSAVE_LBR = 15, + FEATURE_XSAVE_HWP = 16, + FEATURE_XSAVE_XTILE_CFG = 17, + FEATURE_XSAVE_XTILE_DATA = 18, + FEATURE_MAX, + FEATURE_XSAVE_EXTENDED_START = FEATURE_XSAVE_YMM, + FEATURE_XSAVE_EXTENDED_END = FEATURE_XSAVE_XTILE_DATA, +};Why can't this use the existing 'enum xfeature' which is providing exactly the same information already?First version of patch was similar to what you mentioned here and other review comments to use existing kernel definitions. https://lore.kernel.org/linux-mm/20240314112359.50713-1-vigbalas@xxxxxxx/T/ As per the review comment https://lore.kernel.org/linux-mm/20240314162954.GAZfMmAnYQoRjRbRzc@fat_crate.local/ , modified the patch to be a independent of kernel internal definitions. Though this enum and below function "get_sub_leaf" are not useful now, it will be required when we extend for a new/different features.No, Thomas' sugggestion is to use the existing xfeature enum - not define the same thing again. Why do you need that enum custom_feature thing if you can use /* * List of XSAVE features Linux knows about: */ enum xfeature { from arch/x86/include/asm/fpu/types.h ?
My understanding is that, if/when we extend this for other/new featured outside of XSAVE we might end up modifying the enum that is very specific to XSAVE.
Currently, this enum is the same as XSAVE, but when we add other features, this enum might have a different value of the XSAVE features and can be modified without disturbing the existing kernel code.
Please correct me if my understanding is wrong.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
