Received-SPF: pass (google.com: domain of linux-kernel+bounces-44776-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <9a26e279-0d48-4b3a-a459-874bc32f6a35@intel.com>
Date: Tue, 30 Jan 2024 22:54:20 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v9 06/27] x86/fpu/xstate: Create guest fpstate with guest
 specific config
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
CC: "Hansen, Dave" <dave.hansen@intel.com>, "seanjc@google.com"
	<seanjc@google.com>, "x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, "pbonzini@redhat.com"
	<pbonzini@redhat.com>, "yuan.yao@linux.intel.com" <yuan.yao@linux.intel.com>,
	"john.allen@amd.com" <john.allen@amd.com>, "peterz@infradead.org"
	<peterz@infradead.org>, "Gao, Chao" <chao.gao@intel.com>,
	"mlevitsk@redhat.com" <mlevitsk@redhat.com>
References: <20240124024200.102792-1-weijiang.yang@intel.com>
 <20240124024200.102792-7-weijiang.yang@intel.com>
 <c179a5f4fba9086f9d35e56ec16423558cea5b8b.camel@intel.com>
Content-Language: en-US
From: "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <c179a5f4fba9086f9d35e56ec16423558cea5b8b.camel@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V1g0S1JMejVXbnR6RmFtcm9ibDNYR20rRGw0ZW0rcjJyTS9vdDRMZ1FnV1lv?=
 =?utf-8?B?YklhaVhYZkRFVDkrcnkzWEF0RlZxUHZKeDR6MGh4SmVtRCtzUUZ1TGRtZWNG?=
 =?utf-8?B?eHM3QStuTGUzcDJuc29jbUhjU0hMNjFZS1dhaUJOY0t1aCtpRUdtZzZqVjJw?=
 =?utf-8?B?NHc0dDdKN3l3Tk0vSkp1Y09TZ3U3UTd2ZzlMZUFaT1R1cXdJVVRRV2ZDclRH?=
 =?utf-8?B?Ym9yYVpzMlhadEFPY3FYOEgrMTBzSytoS1o3L1huQjRpalNObVpMQTh1amx3?=
 =?utf-8?B?dXJhVWRFNFZMMTN6NzVmUVhoU3Z3d0JLMDZNWXhXTDBnUS9PU1hLcVNxOXNn?=
 =?utf-8?B?bWdxMXhybkRzWmRiZkV5UHQ4VTR4dWFmUmpVdmppY2RTZlpyY2JVWmRqck1R?=
 =?utf-8?B?cnpEOEdwNGI3ZFlrVGtFOCsvZVNZUGdvZEpTaXJEZTNxYS9uM1ByOVZNT2tI?=
 =?utf-8?B?elRxU2F1SVhKUkRuUTZBOXhtQ1BiVm51VE5mTXE0VXJMR2ZDTUNFZU9Ib1l2?=
 =?utf-8?B?alJabGJubHhhcndsUnRqcjA1QkM2Q3ozRUZ0VHdCZnhkSmh2TlY3SnZYMXU1?=
 =?utf-8?B?QXpUNkdCQTNNRGNKZ3hwOTB2dUprREd5ZWNWRXhsODNTeldMZUE5WUREZU5I?=
 =?utf-8?B?Zk9tbFJNNHZGUjByMEFsMHdKM1BkRFlDV3Z1MzdaTHhHK0JJNVdpM2wycjJu?=
 =?utf-8?B?Wm9tUjNKRlZmT292ZndtM2s3NWdYdlpzVmtKVlg4SktUQlBLQVFFeCswM094?=
 =?utf-8?B?SUdvNjdITm5PMXVNKzFEdmxROTYxdnNBbjkyRXo4bXVrRDVlWkllQ0ZDOGsw?=
 =?utf-8?B?MDdSdURsV0ZXTXU2ck5ha3Q2NHNPa1VOQzZhOUJ5VkcxVjhPckwzN1J0ZWww?=
 =?utf-8?B?dEVpTmFxbStBSHp5ZEtDU0VvN3dqYkhmSGg3b0NUVzEzYy92Mk01d2lZbFVS?=
 =?utf-8?B?cVpVd0JnVWJCdW0rZEpMRmVVMXBTZjIwc0g1MVVKL0I1UjB0TXhudFlISUlk?=
 =?utf-8?B?N2lwZUFDNTgzeHptYjZLYnZkZU5yYWlLQlY4eDQwSk04K003SG5nT2dveldu?=
 =?utf-8?B?Mmlwa1R3ZFhUL3RLQUl0dS9uOXpWR2pMVkZVUGVmTExxcktmWVhFRnpQbkdm?=
 =?utf-8?B?OFRiMU85Ny9sQzhVMnhyVEIxYUIzNDN0d0d2bDlSWUpkNGtmaFZnOVl6ZTc5?=
 =?utf-8?B?Nzg2Z2dNWmkxdk5BbjhrS0szSW92dXNGZUMyczR3cUxibjJsdEFkVDlDOTV1?=
 =?utf-8?B?ZldGNkVRS0FKVGNvYThzSitkMGFTNko3QUUvVi9paUM2QmdOaGdSaFBOM1k0?=
 =?utf-8?B?UVFWMmVLYnVtOHIzNHlIWFN5TU1FcElVOG1SS3hYVmtWY0YrTVYzT1dYcXlF?=
 =?utf-8?B?ckdvUlJMeFpMNU1xMC90Uk5tZTlnWXFuY3NsQ0M2ZVRPOHNFYXppVE1wWURE?=
 =?utf-8?B?N29JOUtud1dNeGFBaVBTZHhJMGpTNnJjZEtPdTBBQlZSQkZqTlZKZGEvWGlz?=
 =?utf-8?B?VU1LL1MwMk45cHNBWkt4bE9ONTg1OG5KSnBXUDRaSVh3NFRQQi81Z3pudzZB?=
 =?utf-8?B?WXQvMjRZRHNxbkViaG5GQkNDaFJMZFcvOGU1KzI0ejNQREYycjd2MGlVTVd1?=
 =?utf-8?B?SDRKRkpwRy80NjBTL0M5Y2xsK0xHSnRXQ0k1b3IrN25qY1M0SXRZMmw4VFNn?=
 =?utf-8?B?cWVLL29OUU13V25jc0dkd1pEY2kwaWhxaEJqd3NMOWVLOXBlL01BUTlpY3lv?=
 =?utf-8?B?VXExT09EeENDc054KzZWMTZFSGpkN2xjSjdlZFkwMm5aY0JhNHRMd1hWemZG?=
 =?utf-8?B?SUFwOVR6aHlleWtQQzFjRTRGSXVCYUV2WDhEZ0NrVjc0RS9ZT3RaRHlnOFlX?=
 =?utf-8?B?MFYzV3kvd2dLSlZteCtxQ0FucGl0d0RhOCtLUzZMRzI3Z0FuTkxVUEhFREpq?=
 =?utf-8?B?SkQ1V2U4K3JHT1JvVzN3VlBKYkd0V3gxZWZGZVE3bndlOXJiall1Ri9FQWRO?=
 =?utf-8?B?QmpnZjhTU1pOVnVzb0luZXV3V0g2STFDUzRuUzQ5WGZzUFNtN1hwcmNHQ3RK?=
 =?utf-8?B?SzFOZGRiT3dDald0bUhsTTkxeVprRDNKbXlpZlZTdm95SHRaSjBkODVZck80?=
 =?utf-8?B?dS9FVDhqNlRGY256Z2ZhMG54aG1VSWlCOHdISFM4U0g5SjZNbHpqT0daamF5?=
 =?utf-8?B?REE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e139e2e4-71a6-4022-99a1-08dc21a3608c
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2024 14:54:36.5844
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: AoYa/u5NwdZtxceRRB48hQLosh14Pxda8CIKevd2zNMPtapOtNBGMiZwEiNKN3FFcivv5ykIwDJGn1I7cpG6Sg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5862
X-OriginatorOrg: intel.com

On 1/30/2024 9:38 AM, Edgecombe, Rick P wrote:
> On Tue, 2024-01-23 at 18:41 -0800, Yang Weijiang wrote:
>> -bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu)
>> +static struct fpstate *__fpu_alloc_init_guest_fpstate(struct
>> fpu_guest *gfpu)
>>   {
>>          struct fpstate *fpstate;
>>          unsigned int size;
>>   
>> -       size = fpu_user_cfg.default_size + ALIGN(offsetof(struct
>> fpstate, regs), 64);
>> +       /*
>> +        * fpu_guest_cfg.default_size is initialized to hold all
>> enabled
>> +        * xfeatures except the user dynamic xfeatures. If the user
>> dynamic
>> +        * xfeatures are enabled, the guest fpstate will be re-
>> allocated to
>> +        * hold all guest enabled xfeatures, so omit user dynamic
>> xfeatures
>> +        * here.
>> +        */
>> +       size = fpu_guest_cfg.default_size +
>> +              ALIGN(offsetof(struct fpstate, regs), 64);
> Minor, I'm not sure the extra char warrants changing it to a wrapped
> line, but that's just my personal opinion.
>
>> +
>>          fpstate = vzalloc(size);
>>          if (!fpstate)
>> -               return false;
>> +               return NULL;
>> +       /*
>> +        * Initialize sizes and feature masks, use fpu_user_cfg.*
>> +        * for user_* settings for compatibility of exiting uAPIs.
>> +        */
>> +       fpstate->size           = fpu_guest_cfg.default_size;
>> +       fpstate->xfeatures      = fpu_guest_cfg.default_features;
>> +       fpstate->user_size      = fpu_user_cfg.default_size;
>> +       fpstate->user_xfeatures = fpu_user_cfg.default_features;
>> +       fpstate->xfd            = 0;
>>   
>> -       /* Leave xfd to 0 (the reset value defined by spec) */
>> -       __fpstate_reset(fpstate, 0);
>>          fpstate_init_user(fpstate);
>>          fpstate->is_valloc      = true;
>>          fpstate->is_guest       = true;
>>   
>>          gfpu->fpstate           = fpstate;
>> -       gfpu->xfeatures         = fpu_user_cfg.default_features;
>> -       gfpu->perm              = fpu_user_cfg.default_features;
>> +       gfpu->xfeatures         = fpu_guest_cfg.default_features;
>> +       gfpu->perm              = fpu_guest_cfg.default_features;
>> +
>> +       return fpstate;
>> +}
>> +
>> +bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu)
>> +{
>> +       struct fpstate *fpstate;
>> +
>> +       fpstate = __fpu_alloc_init_guest_fpstate(gfpu);
> The above two statements could be just one line and still even fit
> under 80 chars.

Indeed, the variable is redundant, I'll remove it, thanks!

>
> All the same,
>
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>
>> +
>> +       if (!fpstate)
>> +               return false;
>>