x86/amd: Avoid cpu_has_hypervisor evaluating true on native hardware

Currently when booting native on AMD hardware, cpuidmask_defaults._1cd gets
configured with the HYPERVISOR bit before native CPUID is scanned for feature
bits.

This results in cpu_has_hypervisor becoming set as part of identify_cpu(), and
ends up appearing in the raw and host CPU policies.

A combination of this bug, and c/s bb502a8ca59 "x86: check feature flags after
resume" which checks that feature bits don't go missing, results in broken S3
on AMD hardware.

Alter amd_init_levelling() to exclude the HYPERVISOR bit from
cpumask_defaults, and update domain_cpu_policy_changed() to allow it to be
explicitly forwarded.

This also fixes a bug on kexec, where the hypervisor bit is left enabled for
the new kernel to find.

These changes highlight a further but - dom0 construction is asymetric with
domU construction, by not having any calls to domain_cpu_policy_changed().
Extend arch_domain_create() to always call domain_cpu_policy_changed().

Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index e351dd227ff61df64e2ee7d50f33e2990bf33a34..f95a8e0fd32e9d8ad1360b69b3af9d566081e22e 100644 (file)
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -298,9 +298,6 @@ static void __init noinline amd_init_levelling(void)
                        ecx |= cpufeat_mask(X86_FEATURE_OSXSAVE);
                edx |= cpufeat_mask(X86_FEATURE_APIC);
 
-               /* Allow the HYPERVISOR bit to be set via guest policy. */
-               ecx |= cpufeat_mask(X86_FEATURE_HYPERVISOR);
-
                cpuidmask_defaults._1cd = ((uint64_t)ecx << 32) | edx;
        }
 

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index b3ec9e115b91ea3cae47612cd1755482c3beabb4..66150abf4c6086b7644061d0f6591f0163d400e3 100644 (file)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -656,6 +656,8 @@ int arch_domain_create(struct domain *d,
      */
     d->arch.x87_fip_width = cpu_has_fpu_sel ? 0 : 8;
 
+    domain_cpu_policy_changed(d);
+
     return 0;
 
  fail:

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 4fa9c91140c269a931025c8905459b99a2354359..ce76d6d77641f9c6a3a64dd4620c29d212bc88ef 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -48,7 +48,7 @@ static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
 }
 #endif
 
-static void domain_cpu_policy_changed(struct domain *d)
+void domain_cpu_policy_changed(struct domain *d)
 {
     const struct cpuid_policy *p = d->arch.cpuid;
     struct vcpu *v;
@@ -106,6 +106,13 @@ static void domain_cpu_policy_changed(struct domain *d)
                     ecx = 0;
                 edx = cpufeat_mask(X86_FEATURE_APIC);
 
+                /*
+                 * If the Hypervisor bit is set in the policy, we can also
+                 * forward it into real CPUID.
+                 */
+                if ( p->basic.hypervisor )
+                    ecx |= cpufeat_mask(X86_FEATURE_HYPERVISOR);
+
                 mask |= ((uint64_t)ecx << 32) | edx;
                 break;
             }

diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index f0c25ffec0c284ad1c37990bfe920f9119bc7c87..1843c76d1a0b9030d0325997adc345b3c5e4b46d 100644 (file)
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -624,6 +624,8 @@ struct guest_memory_policy
 void update_guest_memory_policy(struct vcpu *v,
                                 struct guest_memory_policy *policy);
 
+void domain_cpu_policy_changed(struct domain *d);
+
 bool update_runstate_area(struct vcpu *);
 bool update_secondary_system_time(struct vcpu *,
                                   struct vcpu_time_info *);