linux-kernel - Re: [PATCH v4 4/8] x86/fpu/xstate: Differentiate default features for host and guest FPUs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aaac7044-5c65-4d96-9e00-815b90be56e6@intel.com>
Date: Tue, 1 Apr 2025 10:18:07 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Chao Gao <chao.gao@...el.com>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>, <tglx@...utronix.de>,
	<dave.hansen@...el.com>, <seanjc@...gle.com>, <pbonzini@...hat.com>
CC: <peterz@...radead.org>, <rick.p.edgecombe@...el.com>,
	<weijiang.yang@...el.com>, <john.allen@....com>, <bp@...en8.de>,
	<xin3.li@...el.com>, Ingo Molnar <mingo@...hat.com>, Dave Hansen
	<dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>, "Maxim
 Levitsky" <mlevitsk@...hat.com>, Mitchell Levy <levymitchell0@...il.com>,
	Samuel Holland <samuel.holland@...ive.com>, Vignesh Balasubramanian
	<vigbalas@....com>, Aruna Ramakrishna <aruna.ramakrishna@...cle.com>
Subject: Re: [PATCH v4 4/8] x86/fpu/xstate: Differentiate default features for
 host and guest FPUs

On 3/18/2025 8:31 AM, Chao Gao wrote:
> 
> @@ -807,9 +811,11 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
>   	/* Clean out dynamic features from default */
>   	fpu_kernel_cfg.default_features = fpu_kernel_cfg.max_features;
>   	fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
> +	fpu_kernel_cfg.guest_default_features = fpu_kernel_cfg.default_features;
>   
>   	fpu_user_cfg.default_features = fpu_user_cfg.max_features;
>   	fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
> +	fpu_user_cfg.guest_default_features = fpu_user_cfg.default_features;

And you'll add up this on patch7:

   + /* Clean out guest-only features from default */
   + fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_SUPERVISOR_GUEST;

I'm not sure this overall hunk is entirely clear.

Taking a step back, we currently define three types of xfeature sets:

   1. 'default_features'     in a task-inlined buffer
   2. 'max_features'         in an extended buffer
   3. 'independent_features' in a separate buffer (only for LBR)

The VCPU fpstate has so far followed (1) and (2). Now, since we're 
introducing divergence at (1), you've named it guest_default_features:

   'default_features' < 'guest_default_features' < 'max_features'

I don’t see a strong reason against introducing this new field, as 
'guest' already implies the VCPU state. However, rather than directly 
modifying or extending struct fpu_state_config — which may not align 
well with VCPU FPU properties — a dedicated struct could provide a 
cleaner and more structured alternative:

   struct vcpu_fpu_config {
     unsigned int size;
     unsigned int user_size;
     u64 features;
     u64 user_features;
   } guest_default_cfg;

This struct would make VCPU-specific state handling clearer:

   (1) Guest permission setup:

        /* Set the guest default permission */
        fpu->guest_perm.__state_perm      = guest_default_cfg.features;
        fpu->guest_perm.__state_size      = guest_default_cfg.size;
        fpu->guest_perm.__user_state_size = guest_default_cfg.user_size;

   (2) VCPU allocation time:

        fpstate->size           = guest_default_cfg.size;
        fpstate->user_size      = guest_default_cfg.user_size;
        fpstate->xfeatures      = guest_default_cfg.features;
        fpstate->user_xfeatures = guest_default_cfg.user_features;

These assignments considerably make the code more readable.

With that, going back to the default settings, perhaps refactoring it 
could be an option to improve clarity in distinguishing guest vs. host 
settings.

See the attached diff file. I thought this restructuring could make the 
logic more explicit and highlight the differences between guest and host 
settings.

Thanks,
Chang
View attachment "guest_default.patch" of type "text/plain" (2350 bytes)