On 03/20/18 17:36, Andy Lutomirski wrote:
>>
>> But we also don't need swapgs when we have rdgsbase/wrgsbase available.
>> We can indeed just unconditionally save it (via rdgsbase) into the stack
>> frame and wrgsbase the correct percpu value. In that case it might be
>> necessary in order to avoid insane complexity to also save/restore the
>> gs selector.
>
> This is exactly what the old code did. I liked the old code better.
>
>>
>> Is it going to be faster? *Probably* not as swapgs is designed to be
>> fast; it does, however, eliminate the need to RDMSR/WRMSR inside the
>> kernel task switch as the user space gsbase will simply live on the
>> stack. (This is assuming we do this unconditionally on every method of
>> kernel entry, including non-paranoid. I'm not sure if we ever care
>> about the userspace GS/GSBASE inside a paranoid handler, but if we do it
>> would be rather messy to find if we do this conditionally.
>>
>> Now...
>>
>> + ALTERNATIVE "jmp .Lparanoid_entry_no_fsgsbase", \
>> + "RDGSBASE %rdx", X86_FEATURE_FSGSBASE
>> + READ_KERNEL_GSBASE %rax
>>
>> READ_KERNEL_GSBASE here seems like a Really Bad Name[TM] for this macro,
>> since it seems to imply reading MSR_KERNEL_GS_BASE, rather than finding
>> the current percpu offset. I would prefer calling it something like
>> FIND_PERCPU_BASE or something like that.
>
> I think we should revert to what the old patches did here.
>
I don't really understand why you want to do it this way.
1. It means that the location of the user space gs_base is ill-defined,
whereas with the SWAPGS it is *always* in the same place.
2. It is most likely slower, although I obviously haven't benchmarked
it.
-hpa
