Re: [PATCH] prctl,x86 Add PR_[GET|SET]_CPUID for controlling the CPUID instruction.

On Mon, Sep 12, 2016 at 07:15:16AM -0700, Kyle Huey wrote:

Copied from PR_SET_TSC. Would you prefer something like
disable_cpuid/disable_cpuid_and_set_flag for
hard_disable_CPUID/disable_CPUID?

Maybe something like this:

switch_cpuid_faulting(bool on)
{
	if (on)
		msr_set_bit(MSR_MISC_FEATURES_ENABLES, 0);
	else
		msr_clear_bit(MSR_MISC_FEATURES_ENABLES, 0);
}

and call it with the respective argument.

quoted

quoted

@@ -210,6 +280,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
              update_debugctlmsr(debugctl);
      }

+     if (test_tsk_thread_flag(prev_p, TIF_NOCPUID) ^
+         test_tsk_thread_flag(next_p, TIF_NOCPUID)) {
+             /* prev and next are different */
+             if (test_tsk_thread_flag(next_p, TIF_NOCPUID))
+                     hard_disable_CPUID();
+             else
+                     hard_enable_CPUID();
+     }
+

Frankly, I can't say that I'm thrilled by this: if this is a niche
feature which has only a very narrow usage for debugging, I'd much
prefer if this whole thing were implemented with a static_key which was
false on the majority of the systems so that __switch_to() tests it much
cheaply.

Then and only then if your debugger runs arch_prctl(), it would enable
the key and then set_cpuid_mode() can query the MSR directly instead of
using another flag in the thread_info flags.

This would keep this niche feature out of the way of the hot paths.

My code is already in the slow path in __switch_to_xtra(), along with
other debugging features like TIF_BLOCKSTEP and TIF_NOTSC. Adding a
bit to the mask tested in __switch_to() shouldn't affect performance
of the hot path.

TIF_BLOCKSTEP maybe but TIF_NOTSC doesn't look like a debugging feature
to me, especially if it is called in seccomp.

And I know it is not on the hot path. But you're using precious TIF bits
for a niche feature. Practically, this is code which will be dead on the
majority of machines out there, because either the hw feature is not
there or because nobody is using it.

Maybe the virtualization aspect would gather more users of this but I
don't know what kvm guys are thinking about faulting CPUID. Let me add
them to CC.

In any case, I'd do the static_key approach because it is simpler and
less obtrusive for your purpose.

But there are plenty more people on CC, this is just me.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--