[Xen-devel] [PATCH for-4.12] x86/hvm: Fix hvm_cr4_guest_valid_bits() for PVH dom0

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Before the cpuid_policy logic came along, %cr4 auditing on migrate-in was
complicated, because at that point no CPUID information had been set for the
guest.  Auditing against the host CPUID was better than nothing, but not
ideal.

Order of information in the migration stream is still an issue (hence we still
need to keep the restore parameter to cope with a nested virt corner case),
but since Xen 4.9, the domain starts with the applicable max policy, which is
a more appropriate upper bound than the host cpuid policy.

This also makes the fix from c/s 9d2efbafb8 obsolete, as not even dom0 starts
without a policy.

Finally, reposition the UMIP logic as it is the only row out of order.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Juergen Gross <jgross@xxxxxxxx>

I suspect this isn't an issue in practice because there is no way to get
X86_CR4_SMXE set with the logic in this state, but it is a latent bug.

This ideally wants backporting to Xen 4.9 and later.  I don't think there is
anything we can reasonably do for 4.8 and earlier.

I should also note that the nested virt corner case probably isn't useful.
Noone ever bothered wiring the VMXON/VMCS pointer registers into the migration
stream, so migrating a VM will cause it to lose all configured state.
---
 xen/arch/x86/hvm/hvm.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 401c4a9..1966fd4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -928,14 +928,9 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t 
value,
 /* These bits in CR4 can be set by the guest. */
 unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore)
 {
-    const struct cpuid_policy *p;
+    const struct cpuid_policy *p = d->arch.cpuid;
     bool mce, vmxe;
 
-    if ( !restore && !is_hardware_domain(d) )
-        p = d->arch.cpuid;
-    else
-        p = &host_cpuid_policy;
-
     /* Logic broken out simply to aid readability below. */
     mce  = p->basic.mce || p->basic.mca;
     vmxe = p->basic.vmx && (restore || nestedhvm_enabled(d));
@@ -950,13 +945,13 @@ unsigned long hvm_cr4_guest_valid_bits(const struct 
domain *d, bool restore)
                                 X86_CR4_PCE                    |
             (p->basic.fxsr    ? X86_CR4_OSFXSR            : 0) |
             (p->basic.sse     ? X86_CR4_OSXMMEXCPT        : 0) |
+            (p->feat.umip     ? X86_CR4_UMIP              : 0) |
             (vmxe             ? X86_CR4_VMXE              : 0) |
             (p->feat.fsgsbase ? X86_CR4_FSGSBASE          : 0) |
             (p->basic.pcid    ? X86_CR4_PCIDE             : 0) |
             (p->basic.xsave   ? X86_CR4_OSXSAVE           : 0) |
             (p->feat.smep     ? X86_CR4_SMEP              : 0) |
             (p->feat.smap     ? X86_CR4_SMAP              : 0) |
-            (p->feat.umip     ? X86_CR4_UMIP              : 0) |
             (p->feat.pku      ? X86_CR4_PKE               : 0));
 }
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel