Messages in this thread |  | | From | Thomas Gleixner <> | Subject | Re: [patch 1/3] x86/cpu: Remove segment load from switch_to_new_gdt() | Date | Mon, 18 Jul 2022 20:55:03 +0200 |
| |
On Mon, Jul 18 2022 at 11:43, Linus Torvalds wrote: > So I appreciate the added big comments in this code, but looking at this patch: > On Mon, Jul 18, 2022 at 10:52 AM Thomas Gleixner <tglx@linutronix.de> wrote: >> + * For secondary CPUs this is not a problem because they start >> + * already with the direct GDT and the real GSBASE. This invocation >> + * is pointless and will be removed in a subsequent step. >> + */ >> + if (IS_ENABLED(CONFIG_X86_64)) >> + wrmsrl(MSR_GS_BASE, cpu_kernelmode_gs_base(cpu)); >> } > > ... while those comments are nice and all, I do think this retains the > basic insanity of having "switch_to_new_gdt()" do magical things on > x86-64 that don't really match the name. > > So honestly, I'd be happier of that whole > > if (IS_ENABLED(CONFIG_X86_64)) > wrmsrl(MSR_GS_BASE, cpu_kernelmode_gs_base(cpu)); > > was migrated to the callers instead. There aren't *that* many callers. > > I expect that it is then quite possible that several of the call-sites > would go "GS_BASE is already correct here, I can remove this".
With the next patch we have only two left. The SMP and the UP case. Let me look whether the UP needs it at all.
> But even if every single caller keeps that wrmsrl() around, at least > it wouldn't be hidden behind a function call that has a name that > implies something completely different is happening. > > And no, I don't care *that* deeply, so this is just a suggestion. > > But wouldn't it be nice if this function was actually named by what it > does, rather than by what it used to do back in the i386 days when the > GDT affected the segment bases?
Yes. Let me come up with a sensible name.
Thanks,
tglx
|  |