On 01/09/2018 04:43 PM, Liran Alon wrote:
----- wei.w.wang@xxxxxxxxx wrote:
This patch shows an alternative approach to the one posted here:
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mail-2Darchive.com_linux-2Dkernel-40vger.kernel.org_msg1580364.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Jk6Q8nNzkQ6LJ6g42qARkg6ryIDGQr-yKXPNGZbpTx0&m=o_uh0-IfLdLMwf2MaOOQVKtZbPlJ-fDVmglRj277cwk&s=j1oGOoYysZhUuGuuvuwdTU_OO7bv1ysIyvDtAlc6C-c&e=
The advantages are
1) Simpler;
2) More reasonable because this is used to fill the hardware security
hole, for all the x86 cpus that physically support the two CPUIDs,
which means the hole already exists physically. All the VMs should
use this feature no matter what CPU model they are using. So,
I'm not sure I 100% agree with this.
There should be a way for the userspace agent to disable these CPUIDs if wanted.
You don't want to lose the ability to expose a mimic of a real physical CPU-model of core2duo that
doesn't have these CPUIDs. A good solution can be that these features will be exposed by default to guests
if available on hardware but can still be explicitly not-exposed if userspace agent wishes so.
I think the case we are handling here is different:
It shouldn't be treated as a regular feature (e.g. xsaves) that a user
can choose to use or not. It is a security hole (or say a bug). When we
fixed a bug, we don't give users an option to select to trigger the bug,
right?
The only weird side-effect of this is that live-migration between different physical hosts running with
exact same QEMU cmdline will result in different CPUID values exposed to guest.
I think live migration itself doesn't do the CPUID check, so adding the
QEMU side hardcode part doesn't help. That is, even using that patch
7/7, I think it wouldn't make a difference.
This is the same case when we migrate a VM from a skylake physical
machine to an older machine, the difference of supported CPUID won't
even generate a warning when the destination side QEMU gets booted, right?
exposing
the two CPUIDs as long as they are physically supported by the
hardware,
and this doesn't require the QEMU side hardcode as usual.
When the related feature bits are added to the kernel, and we can
simply
change it to:
best->edx |= F(SPEC_CTRL) | F(PRED_CMD);
Signed-off-by: Wei Wang <wei.w.wang@xxxxxxxxx>
---
arch/x86/kvm/cpuid.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0099e10..c33d3d4 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -70,6 +70,8 @@ u64 kvm_supported_xcr0(void)
/* These are scattered features in cpufeatures.h. */
#define KVM_CPUID_BIT_AVX512_4VNNIW 2
#define KVM_CPUID_BIT_AVX512_4FMAPS 3
+#define KVM_CPUID_BIT_SPEC_CTRL 26
+#define KVM_CPUID_BIT_STIBP 27
#define KF(x) bit(KVM_CPUID_BIT_##x)
int kvm_update_cpuid(struct kvm_vcpu *vcpu)
@@ -109,6 +111,9 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
}
}
+ if (cpuid_edx(0x7) & (KF(SPEC_CTRL) | KF(STIBP)))
You should put this inside the "if (best) {...}" block above.
Right, thanks.
Best,
Wei
