On 08/01/15 15:23, Jan Beulich wrote:
Following the earlier similar change validating CR4 modifications.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1672,20 +1672,53 @@ static int hvm_save_cpu_ctxt(struct doma
return 0;
}
-static bool_t hvm_efer_valid(struct domain *d,
- uint64_t value, uint64_t efer_validbits)
+static bool_t hvm_efer_valid(const struct vcpu *v, uint64_t value,
+ bool_t restore)
{
- if ( nestedhvm_enabled(d) && cpu_has_svm )
- efer_validbits |= EFER_SVME;
+ unsigned int ext1_ecx = 0, ext1_edx = 0;
- return !((value & ~efer_validbits) ||
- ((sizeof(long) != 8) && (value & EFER_LME)) ||
- (!cpu_has_svm && (value & EFER_SVME)) ||
- (!cpu_has_nx && (value & EFER_NX)) ||
- (!cpu_has_syscall && (value & EFER_SCE)) ||
- (!cpu_has_lmsl && (value & EFER_LMSLE)) ||
- (!cpu_has_ffxsr && (value & EFER_FFXSE)) ||
- ((value & (EFER_LME|EFER_LMA)) == EFER_LMA));
+ if ( !restore && !is_hardware_domain(v->domain) )
+ {
+ unsigned int level;
+
+ ASSERT(v == current);
+ hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
+ if ( level >= 0x80000001 )
+ hvm_cpuid(0x80000001, NULL, NULL, &ext1_ecx, &ext1_edx);
+ }
+ else
+ {
+ ext1_edx = boot_cpu_data.x86_capability[X86_FEATURE_LM / 32];
+ ext1_ecx = boot_cpu_data.x86_capability[X86_FEATURE_SVM / 32];
+ }
+
+ if ( (value & EFER_SCE) &&
+ !(ext1_edx & cpufeat_mask(X86_FEATURE_SYSCALL)) )
+ return 0;
+
+ if ( (value & (EFER_LME | EFER_LMA)) &&
+ !(ext1_edx & cpufeat_mask(X86_FEATURE_LM)) )
+ return 0;
+
+ if ( (value & EFER_LMA) && !(value & EFER_LME) )
+ return 0;
The LME/LMA handling more complicated than this.
LMA is read-only on Intel, but specified as read-write on AMD, with
the requirement that if it doesn't match the value generated by
hardware, a #GP fault will occur. I believe this actually means it
is read-only on AMD as well.
LMA only gets set by hardware after paging is enabled and the
processor switches properly into long mode, which means that there
is a window between setting LME and setting CR0.PG where LMA should
read as 0.
I think hvm_efer_valid() also needs the current EFER and CR0 to work
out what the current LMA should be, and reject any attempt to change
it.
+
+ if ( (value & EFER_NX) && !(ext1_edx & cpufeat_mask(X86_FEATURE_NX)) )
+ return 0;
+
+ if ( (value & EFER_SVME) &&
+ (!(ext1_ecx & cpufeat_mask(X86_FEATURE_SVM)) ||
+ !nestedhvm_enabled(v->domain)) )
This is going to cause an issue for the restore case, as the HVM
PARAMs follow the architectural state.
I don't believe it is reasonable for nestedhvm_enabled() to disagree
with cpufeat_mask(X86_FEATURE_{SVM,VMX}), or for the toolstack to be
able to yank nestedhvm while the VM is active.
Looking at the checks when setting HVM_PARAM_NESTEDHVM, neither of
the guest feature flags are actually checked before setting up the
nested infrastructure.
Having said all of this, don't appear to be any migration records
associated with nested state (hardware-loaded VMCS/VMCB pointers,
currently nested?) which leads me to suspect that a domain actually
using nested virt is not going to survive a migration, so it might
be acceptable to fudge the checking of SVME for now.
~Andrew
+ return 0;
+
+ if ( (value & EFER_LMSLE) && !cpu_has_lmsl )
+ return 0;
+
+ if ( (value & EFER_FFXSE) &&
+ !(ext1_edx & cpufeat_mask(X86_FEATURE_FFXSR)) )
+ return 0;
+
+ return 1;
}
/* These reserved bits in lower 32 remain 0 after any load of CR0 */
@@ -1763,7 +1796,6 @@ static int hvm_load_cpu_ctxt(struct doma
struct vcpu *v;
struct hvm_hw_cpu ctxt;
struct segment_register seg;
- uint64_t efer_validbits;
/* Which vcpu is this? */
vcpuid = hvm_load_instance(h);
@@ -1794,9 +1826,7 @@ static int hvm_load_cpu_ctxt(struct doma
return -EINVAL;
}
- efer_validbits = EFER_FFXSE | EFER_LMSLE | EFER_LME | EFER_LMA
- | EFER_NX | EFER_SCE;
- if ( !hvm_efer_valid(d, ctxt.msr_efer, efer_validbits) )
+ if ( !hvm_efer_valid(v, ctxt.msr_efer, 1) )
{
printk(XENLOG_G_ERR "HVM%d restore: bad EFER %#" PRIx64 "\n",
d->domain_id, ctxt.msr_efer);
@@ -2936,12 +2966,10 @@ err:
int hvm_set_efer(uint64_t value)
{
struct vcpu *v = current;
- uint64_t efer_validbits;
value &= ~EFER_LMA;
- efer_validbits = EFER_FFXSE | EFER_LMSLE | EFER_LME | EFER_NX | EFER_SCE;
- if ( !hvm_efer_valid(v->domain, value, efer_validbits) )
+ if ( !hvm_efer_valid(v, value, 0) )
{
gdprintk(XENLOG_WARNING, "Trying to set reserved bit in "
"EFER: %#"PRIx64"\n", value);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|