> /*
> + * Temporary hack: zero gs now that we've saved it so that Xen
> + * doesn't try to reload the old value after changing the GDT
> + * during the context switch. This can go away once Xen has
> + * been taught to only reload %gs when it absolutely must.
> + */
> + loadsegment(gs, 0);
Sorry, but i don't really want that unconditionally in the context switch.
Adding a paravirt ops for it would be also ugly. Can Xen be fixed?
-Andi
Andi Kleen wrote:
> Sorry, but i don't really want that unconditionally in the context switch.
> Adding a paravirt ops for it would be also ugly. Can Xen be fixed?
>
Yes. I'm happy to drop this one.
J
On 21/2/07 22:10, "Andi Kleen" <[email protected]> wrote:
>> /*
>> + * Temporary hack: zero gs now that we've saved it so that Xen
>> + * doesn't try to reload the old value after changing the GDT
>> + * during the context switch. This can go away once Xen has
>> + * been taught to only reload %gs when it absolutely must.
>> + */
>> + loadsegment(gs, 0);
>
> Sorry, but i don't really want that unconditionally in the context switch.
> Adding a paravirt ops for it would be also ugly. Can Xen be fixed?
This is not an issue for correctness so this can safely be removed and still
work with current Xen.
-- Keir
Andi Kleen wrote:
>> /*
>> + * Temporary hack: zero gs now that we've saved it so that Xen
>> + * doesn't try to reload the old value after changing the GDT
>> + * during the context switch. This can go away once Xen has
>> + * been taught to only reload %gs when it absolutely must.
>> + */
>> + loadsegment(gs, 0);
>>
>
> Sorry, but i don't really want that unconditionally in the context switch.
> Adding a paravirt ops for it would be also ugly. Can Xen be fixed?
>
I agree with that, but especially because this is not even the right
place to save and clear gs; when userspace uses an LDT based %gs, you
need to do this all the way back in mmu_context.h before you switch the
LDT out. We have the same bug, but I was loathe to try to fix it yet,
since it could mean some strange code movement. The awkward bit is that
switch_mm doesn't have the pointer to the previous task, only it's mm,
so there is no place to save gs. Perhaps it is time to move the LDT
switch into switch_to instead of switch_mm? It's not like there are
different per-process LDTs in the fixmap or something, which I could see
as a valid reason to associate this with the mm switch. But all process
LDTs are global, so there isn't an issue.
And I noticed this martian hanging out in mmu_context.h:
#define deactivate_mm(tsk, mm) \
asm("movl %0,%%gs": :"r" (0));
Zach
Zachary Amsden wrote:
> I agree with that, but especially because this is not even the right
> place to save and clear gs; when userspace uses an LDT based %gs, you
> need to do this all the way back in mmu_context.h before you switch
> the LDT out.
Yeah. This patch was really just to shut my debug Xen build up.
There's no correctness issue one way or the other, so it doesn't really
matter that it doesn't catch LDT-using processes.
> And I noticed this martian hanging out in mmu_context.h:
>
> #define deactivate_mm(tsk, mm) \
> asm("movl %0,%%gs": :"r" (0));
Yeah, I've always wondered what this is for. Its a remnant of a
remnant, I think.
J