Re: [PATCH v25 14/20] i386: separate fpu_helper into user and sysemu parts

[Claudio Fontana <cfontana@suse.de>]

On 2/26/21 7:42 PM, Richard Henderson wrote:
> On 2/26/21 9:51 AM, Claudio Fontana wrote:
>> +/* fpu_helper.c */
>> +
>> +void do_fsave(CPUX86State *env, target_ulong ptr, int data32, uintptr_t ra);
>> +void do_frstor(CPUX86State *env, target_ulong ptr, int data32, uintptr_t ra);
>> +void do_fxsave(CPUX86State *env, target_ulong ptr, uintptr_t ra);
>> +void do_fxrstor(CPUX86State *env, target_ulong ptr, uintptr_t ra);
> 
> Is this really worth it?

No, probably not. Will respin without these parts,
and leaving the small ifdefs in the global tcg/fpu_helper.c

Thanks,

Claudio


> It means that we unnecessarily expose these functions
> in system mode, where they are still not safe.
> 
> I had thought about doing something like
> 
> #ifdef CONFIG_USER_ONLY
> #define SYSEMU_STATIC
> #else
> #define SYSEMU_STATIC static
> #endif
> 
> SYSEMU_STATIC void
> cpu_x86_fsave(...)
> {
>   ...
> }
> 
> void helper_fsave(...)
> {
>     cpu_x86_fsave(..., GETPC());
> }
> 
> but I thought the existing example of "do_" functions within that file to be
> compelling.  I think small sections of this CONFIG within a file should be
> fine, especially where it interacts with other functions like this.
> 
> I guess either way,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
>