SVM: forbid guest to execute monitor/mwait
PATCH 1/1 SVM: forbid guest to execute monitor/mwait
From: Joerg Roedel <[email protected]>
This patch forbids the guest to execute monitor/mwait instructions on SVM. This is necessary because the guest can execute these instructions if they are available even if the kvm cpuid doesn't report its existence.
Signature · 3 lines
Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG
drivers/kvm/svm.c +5 -1
@@ -511,7 +511,9 @@ static void init_vmcb(struct vmcb *vmcb) (1ULL <<INTERCEPT_VMSAVE ) | (1ULL <<INTERCEPT_STGI ) | (1ULL <<INTERCEPT_CLGI ) | - (1ULL <<INTERCEPT_SKINIT ); + (1ULL <<INTERCEPT_SKINIT ) | + (1ULL <<INTERCEPT_MONITOR ) | + (1ULL <<INTERCEPT_MWAIT ); control->iopm_base_pa = iopm_base; control->msrpm_base_pa = msrpm_base;
@@ -1292,6 +1294,8 @@ static int (*svm_exit_handlers[])(struct kvm_vcpu *vcpu, [SVM_EXIT_STGI] = invalid_op_interception, [SVM_EXIT_CLGI] = invalid_op_interception, [SVM_EXIT_SKINIT] = invalid_op_interception, + [SVM_EXIT_MONITOR] = invalid_op_interception, + [SVM_EXIT_MWAIT] = invalid_op_interception, };
drivers/kvm/svm.h +6 -0
@@ -44,6 +44,9 @@ enum {INTERCEPT_RDTSCP ,INTERCEPT_ICEBP ,INTERCEPT_WBINVD , +INTERCEPT_MONITOR , +INTERCEPT_MWAIT , + INTERCEPT_MWAIT_COND, };
@@ -298,6 +301,9 @@ struct __attribute__ ((__packed__)) vmcb {#define SVM_EXIT_RDTSCP 0x087#define SVM_EXIT_ICEBP 0x088#define SVM_EXIT_WBINVD 0x089 +#define SVM_EXIT_MONITOR 0x08a +#define SVM_EXIT_MWAIT 0x08b +#define SVM_EXIT_MWAIT_COND 0x08c#define SVM_EXIT_NPF 0x400#define SVM_EXIT_ERR -1
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 8 lines · Joerg Roedel
From: Joerg Roedel <[email protected]>
This patch forbids the guest to execute monitor/mwait instructions on SVM. This is necessary because the guest can execute these instructions if they are available even if the kvm cpuid doesn't report its existence.
You're intercepting the instructions unconditionally. What about the case where cpuid does report monitor/mwait support? The guest can legitimately use them then.
Signature · 8 lines
Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): 46023264.4010105-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 10 lines · Avi Kivity
Quoted 8 lines · Joerg Roedel
From: Joerg Roedel <[email protected]>
This patch forbids the guest to execute monitor/mwait instructions on SVM. This is necessary because the guest can execute these instructions if they are available even if the kvm cpuid doesn't report its existence.
Quoted 2 lines
You're intercepting the instructions unconditionally. What about the case where cpuid does report monitor/mwait support? The guest can legitimately use them then.
I prepared a patch for QEmu to handle this but finally not submitted it because QEmu does not set the Monitor bit anyway. But you are right. It is possible for userspace to set the Montior bit and the guest won't be able to use it.
I think an architecture dependent cpuid mask feature in the kernel would solve this problem. I'll prepare a patch for that.
Joerg
Signature · 11 lines
Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 17 lines · Joerg Roedel
Quoted 11 lines · Avi Kivity
Quoted 9 lines · Joerg Roedel
From: Joerg Roedel <[email protected]>
This patch forbids the guest to execute monitor/mwait instructions on SVM. This is necessary because the guest can execute these instructions if they are available even if the kvm cpuid doesn't report its existence.
Quoted 3 lines
You're intercepting the instructions unconditionally. What about the case where cpuid does report monitor/mwait support? The guest can legitimately use them then.
Quoted 8 lines
I prepared a patch for QEmu to handle this but finally not submitted it because QEmu does not set the Monitor bit anyway. But you are right. It is possible for userspace to set the Montior bit and the guest won't be able to use it.
I think an architecture dependent cpuid mask feature in the kernel would solve this problem. I'll prepare a patch for that.
I think I'm misunderstanding something.
Is there actually an AMD cpu that supports monitor/mwait? If not (understandably, having no hyperthreading), then wouldn't executing monitor or mwait generate #UD anyway? I don't think there's need for the cpuid mask as long as setting the monitor bit cannot endanger the kernel.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): 46025528.5010707-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 26 lines · Avi Kivity
Quoted 16 lines · Joerg Roedel
Quoted 4 lines · Avi Kivity
Quoted 2 lines · Joerg Roedel
From: Joerg Roedel <[email protected]>
Quoted 9 lines
Quoted 6 lines
This patch forbids the guest to execute monitor/mwait instructions on SVM. This is necessary because the guest can execute these instructions if they are available even if the kvm cpuid doesn't report its existence.
You're intercepting the instructions unconditionally. What about the case where cpuid does report monitor/mwait support? The guest can legitimately use them then.
Quoted 8 lines
I prepared a patch for QEmu to handle this but finally not submitted it because QEmu does not set the Monitor bit anyway. But you are right. It is possible for userspace to set the Montior bit and the guest won't be able to use it.
I think an architecture dependent cpuid mask feature in the kernel would solve this problem. I'll prepare a patch for that.
Quoted 3 lines
I think I'm misunderstanding something.
Is there actually an AMD cpu that supports monitor/mwait? If not (understandably, having no
Yes, the family 10h processors have support for that.
Quoted 3 lines
hyperthreading), then wouldn't executing monitor or mwait generate #UD anyway? I don't think there's need for the cpuid mask as long as setting the monitor bit cannot endanger the kernel.
There is no danger for the host kernel but for the guest. If the userspace sets the monitor bit the guest will receive an #UD when trying to use it. And we don't want the guest to use it because it is not virtualized yet.
Joerg
Signature · 11 lines
Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 5 lines · Joerg Roedel
There is no danger for the host kernel but for the guest. If the userspace sets the monitor bit the guest will receive an #UD when trying to use it. And we don't want the guest to use it because it is not virtualized yet.
So, isn't a simpler fix not to set the monitor bit in cpuid?
The patch is correct, but I don't see why it's needed as any guest will check the cpuid bit before using monitor.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): 46025D94.8050606-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 11 lines · Avi Kivity
Quoted 5 lines · Joerg Roedel
There is no danger for the host kernel but for the guest. If the userspace sets the monitor bit the guest will receive an #UD when trying to use it. And we don't want the guest to use it because it is not virtualized yet.
So, isn't a simpler fix not to set the monitor bit in cpuid?
The patch is correct, but I don't see why it's needed as any guest will check the cpuid bit before using monitor.
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. I don't think this is acceptable. And if we disable this bit in cpuid we should also prevent the guest from executing it to emulate the behavior of a real cpu in host mode.
So I agree we should mask the cpuid monitor bit. The savest way to do so is in the kvm-amd module imho. And, also imho, we should the guest really forbid to execute it.
Joerg
Signature · 11 lines
Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 12 lines · Joerg Roedel
Quoted 10 lines · Avi Kivity
Quoted 6 lines · Joerg Roedel
There is no danger for the host kernel but for the guest. If the userspace sets the monitor bit the guest will receive an #UD when trying to use it. And we don't want the guest to use it because it is not virtualized yet.
So, isn't a simpler fix not to set the monitor bit in cpuid?
Quoted 8 lines
Quoted 3 lines
The patch is correct, but I don't see why it's needed as any guest will check the cpuid bit before using monitor.
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?
Quoted 8 lines
I don't think this is acceptable.
And if we disable this bit in cpuid we should also prevent the guest from executing it to emulate the behavior of a real cpu in host mode.
So I agree we should mask the cpuid monitor bit. The savest way to do so is in the kvm-amd module imho. And, also imho, we should the guest really forbid to execute it.
I want to understand it exactly first. I don't see any barrier to applying the patch, I just want a full understanding.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): 46026B2A.2080501-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 7 lines · Avi Kivity
Quoted 4 lines · Joerg Roedel
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.
Joerg
Signature · 11 lines
Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 9 lines · Joerg Roedel
Quoted 7 lines · Avi Kivity
Quoted 5 lines · Joerg Roedel
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.
Quoted 7 lines
Quoted 2 lines
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.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): 4602700E.1030500-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 17 lines · Avi Kivity
Quoted 8 lines · Joerg Roedel
Quoted 6 lines · Avi Kivity
Quoted 4 lines · Joerg Roedel
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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 7 lines
Well, a guest can cause exactly the same effect with
cli
spin:
jmp spinwithout 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.
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 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.
Joerg
Signature · 11 lines
Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 29 lines · Joerg Roedel
Quoted 27 lines · Avi Kivity
Quoted 11 lines · Joerg Roedel
Quoted 8 lines · Avi Kivity
Quoted 5 lines · Joerg Roedel
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.
Quoted 8 lines
Quoted 3 lines
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
Quoted 7 lines
Quoted 2 lines
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...
Quoted 7 lines
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.
Quoted 5 lines
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.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
A message that is not in the archive (sent off-list or not yet received): 460277AE.9040403-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 8 lines · Avi Kivity
Quoted 5 lines · Joerg Roedel
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.
Ok. This way is also most efficient because it does not require any further patching :) QEmu currently does not set this bit.
Signature · 11 lines
Joerg Roedel Operating System Research Center AMD Saxony LLC & Co. KG ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV