On Tue, Mar 04, 2025 at 02:50:48PM -0800, Dave Hansen wrote: >On 11/26/24 02:17, Chao Gao wrote: >> From: Yang Weijiang <weijiang.yang@xxxxxxxxx> >> >> Define new fpu_guest_cfg to hold all guest FPU settings so that it can >> differ from generic kernel FPU settings, e.g., enabling CET supervisor >> xstate by default for guest fpstate while it's remained disabled in >> kernel FPU config. >> >> The kernel dynamic xfeatures are specifically used by guest fpstate now, >> add the mask for guest fpstate so that guest_perm.__state_perm == >> (fpu_kernel_cfg.default_xfeature | XFEATURE_MASK_KERNEL_DYNAMIC). And >> if guest fpstate is re-allocated to hold user dynamic xfeatures, the >> resulting permissions are consumed before calculate new guest fpstate. > >This kinda restates what the code does, but I don't think it matches the >code. Actually, it does (see more below) > >> With new guest FPU config added, there're 3 categories of FPU configs in >> kernel, the usages and key fields are recapped as below. > >This changelog is pretty rough. It's got a lot of words but not much >substance. Will revise the changelog. [...] > > >> >> @fpu_guest_cfg.default_size >> - size of compacted buffer with 'fpu_guest_cfg.default_features' > >This looks like kerneldoc, not changelog. Are you sure you want it _here_? No, I agree this should be a comment above fpu_guest/kernel/user_cfg. > > >> @@ -829,6 +835,10 @@ void __init fpu__init_system_xstate(unsigned int legacy_size) >> fpu_user_cfg.default_features = fpu_user_cfg.max_features; >> fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC; >> >> + fpu_guest_cfg.max_features = fpu_kernel_cfg.max_features; >> + fpu_guest_cfg.default_features = fpu_guest_cfg.max_features; >> + fpu_guest_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC; > >I thought this was saying above that it was _setting_ dynamic features. > >Why _not_ set them by default? At first glance, I had the same question. XFEATURE_MASK_*KERNEL*_DYNAMIC is not excluded here, so it is enabled by default. I believe the confusion arises partly from the order of the patches. I will reorder the patches as you suggested in patch 5.
