Re: [PATCH] SVM: forbid guest to execute monitor/mwait

Avi Kivity
2007-03-22T12:33:50Z

Joerg Roedel wrote:
> On Thu, Mar 22, 2007 at 02:01:18PM +0200, Avi Kivity wrote:
>
>> Joerg Roedel wrote:
>>
>>> On Thu, Mar 22, 2007 at 01:40:26PM +0200, Avi Kivity wrote:
>>>
>>>
>>>> Joerg Roedel wrote:
>>>>
>>>>
>>>>> Right. But it is possible for userspace to enable monitor bit for the
>>>>> guest. Without virtualization of these 2 instructions the guest would
>>>>> idle in the guest state after calling mwait and prevent other processes
>>>>> and guests from running in that time.
>>>>>
>>>> Won't an interrupt during mwait cause a vmexit?
>>>>
>>>>
>>> Yes, an interrupt would cause an vmexit. But depending on the HZ value
>>> of the host it may be a lot of time until the next interrupt. And over
>>> time this will sum up and slow down other tasks.
>>>
>>>
>> Well, a guest can cause exactly the same effect with
>>
>> cli
>> spin:
>> jmp spin
>>
>> without executing any instructions that are not reported by cpuid.
>>
>
> Intentionally, yes. In this case the guest knows what it is doing and we
> have no way to prevent such things anyway. If it executes mwait this
> delay loop is not intended by the guest.
>

Well, if the cpuid bit is off and the guest is executing the
instruction, then it probably isn't doing it intentionally...

> To summarize my point:
>
> We agree that we should hide the monitor cpuid bit to the guest from the
> kernel level. Allowing the guest to use mwait if its available on the
> processor without proper virtualization would cause the processor to
> idle in the guest mode.
>

I think it can be safely done from userspace, as long as host integrity
is not compromised.

> I would additionally forbid the guest to execute these instructions.
> Likely the guest won't use them anyway when the cpuid bit is not set,
> but if this bit is not set, the execution of these instructions should
> cause an #UD as in any real processor, imho.
>

I agree that it's better, but it's just marginally better. There are
zillions of instructions that are advertised by cpuid that won't be
trapped by svm (or vt) if the virtual cpuid bit is off.

Anyway, as the patch does improve things a bit, I'll apply it.

--
error compiling committee.c: too many arguments to function