Re: [PATCH 3/4] x86/umip: Identify the str and sldt instructions

[Ricardo Neri]

On Mon, Nov 13, 2017 at 09:12:03AM +0100, Ingo Molnar wrote:
> 
> * Ricardo Neri <ricardo.neri-calderon@xxxxxxxxxxxxxxx> wrote:
> 
> > The instructions str and sldt are not emulated in any case. Thus, it made
> > sense to not implement functionality to identify them. However, a
> > subsequent commit will introduce functionality to warn about the use of
> > all the instructions that UMIP protect, not only those that are emulated.
> > A first step for that is the ability to identify them.
> > 
> > Plus, now that str and sldt are identified, we need to explicitly avoid
> > their emulation (i.e., not rely on unsuccessful identification). Group
> > togehter all the cases that we do not want to emulate: str, sldt and user
> > long mode processes.
> 
> Did you notice how in all your previous patches (both in the code and in the 
> changelogs) I have manually fixed up the capitalization of these instruction 
> mnenonics?

I am sorry, I tried to see where you made these changes but I could not find
any. I did a git diff of arch/x86/kernel/umip.c between the branch rneri/umip_v11
of my repository [1] and the master branch of the tip tree and I did not find
any differences.

Also, looking at the log of the master branch of the tip tree I see, for
instance:

commit 1e5db223696afa55e6a038fac638f759e1fdcc01
Author: Ricardo Neri <ricardo.neri-calderon@xxxxxxxxxxxxxxx>
Date:   Sun Nov 5 18:27:52 2017 -0800

    x86/umip: Add emulation code for UMIP instructions
    
    The feature User-Mode Instruction Prevention present in recent Intel
    processor prevents a group of instructions (sgdt, sidt, sldt, smsw, and
    str) from being executed with CPL > 0. Otherwise, a general protection
    fault is issued.
    ...

The instruction mnemonics were not capitalized. Is the master branch the one where
I can look at your fixes?

> 
> The capitalized form is much more readable, especially with seriously overloaded 
> acronyms such as 'str' ...

I see.
> 
> You now repeat the same bad pattern, in fact you regress existing code:
> 
> > -	/* SLDT AND STR are not emulated */
> 
> > +	/* Do not emulate sldt, str or user long mode processes. */
> 
> Please be more careful with such details, and please fix & resend this series.

Sure, I will submit a v2 with capitalized mnemonics in both the code and the
patch descriptions. I will be more careful in the future.

Thanks and BR,
Ricardo

[1]. https://github.com/ricardon/tip/commits/rneri/umip_v11