Re: [PATCH 14/15] x86/fsgsbase/64: Support legacy behavior when FS/GS updated by ptracer

[Bae, Chang Seok]

On 3/20/18, 08:05, "Andy Lutomirski" <luto@xxxxxxxxxx> wrote:
> I've also suggested something like this myself, but this approach is
> far more complicated than the older approach.  Was there something
> that the old approach would break?  If so, what?
Sorry, I don't know your suggestion. Can you elaborate your suggestion?
  
>> +               /*
>> +                * %fs setting goes to reload its base, when tracee
>> +                * resumes without FSGSBASE (legacy). Here with FSGSBASE
>> +                * FS base is (manually) fetched from GDT/LDT when needed.
>> +                */
>> +               else if (static_cpu_has(X86_FEATURE_FSGSBASE) &&
>> +                        (value != 0) && (task->thread.fsindex != value))
>> +                       task->thread.fsbase = task_seg_base(task, value);

> The comment above should explain why you're checking this particular
> condition.  I find the fsindex != value check to be *very* surprising.
>  On a real CPU, writing some nonzero value to %fs does the same thing
>  regardless of what the old value of %fs was.

With FSGSBASE, when both index and base are not changed, base will
be (always) fetched from GDT/LDT. This is not thought as legacy behavior 
we need to support, AFAIK.

> This is_fully_covered thing is IMO overcomplicated.  Why not just make
> a separate helper set_fsgs_index_and_base() and have putregs() call it
> when both are set at once?

Using helper function here is exactly what I did at first. I thought this
tag is simple enough and straightforward at the end. But I'm open to
factor it out.