[PATCH v7] um: Enable preemption in UML

[Anton Ivanov]

On 28/03/2024 09:27, Johannes Berg wrote:
> 
>> @@ -23,7 +23,7 @@ struct thread_info {
>>   	int			preempt_count;  /* 0 => preemptable,
>>   						   <0 => BUG */
>>   	struct thread_info	*real_thread;    /* Points to non-IRQ stack */
>> -	unsigned long aux_fp_regs[FP_SIZE];	/* auxiliary fp_regs to save/restore
>> +	unsigned long aux_fp_regs[FP_SIZE] __aligned(64);	/* auxiliary fp_regs to save/restore
>>   						   them out-of-band */
> 
> nit: that comment looks strange now, maybe pull it up before the member?
> 
>   /* auxiliary ... out-of-band */
>   unsigned long aux_fp_regs[...] __aligned(64);
> 

Ack.

> 
>> +#ifdef CONFIG_64BIT
>> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
>> +		__builtin_ia32_xsaveopt64(&current_thread_info()->aux_fp_regs, KNOWN_387_FEATURES);
>> +	else {
>> +		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
>> +			__builtin_ia32_xsave64(&current_thread_info()->aux_fp_regs, KNOWN_387_FEATURES);
>> +		else
>> +			__builtin_ia32_fxsave64(&current_thread_info()->aux_fp_regs);
>> +	}
> 
> Why not write this as a chain?
> 
> 	if (likely(cpu_has(...))
> 		__builtin_ia32_xsaveopt64(...);
> 	else if (likely(cpu_has(...)))
> 		__builtin_ia32_xave64(...);
> 	else
> 		__builtin_ia32_fxsave64(...);
> 
> 
> and IMHO pulling the "&current_thread_info()->aux_fp_regs" that appears
> on all three of them into a local variable would make that more readable
> too.

Ack.

> 
>> +#else
>> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
>> +		__builtin_ia32_xsaveopt(&current->aux_fp_regs, KNOWN_387_FEATURES);
>> +	else {
>> +		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
>> +			__builtin_ia32_xsave(&current->aux_fp_regs, KNOWN_387_FEATURES);
>> +		else
>> +			__builtin_ia32_fxsave(&current->aux_fp_regs);
>> +	}
>> +#endif
> 
> And all of the above also applies for 32-bit,

Ack.

> 
>> +void kernel_fpu_end(void)
> 
> and this as well.
> 
> johannes
> 
> 

Will fix, rebase, retest and resubmit.

-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/