[PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER

[PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER

[Andrew Cooper]

Before the cpuid_policy logic came along, %cr4/EFER 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.

Similarly at the time, PVHv1 lacked the "CPUID passed through from hardware"
behaviour with PV guests had, and PVH dom0 had to be special-cased to be able
to boot.

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 in
the %cr4 case), but since Xen 4.9, all domains start with a suitable CPUID
policy, which is a more appropriate upper bound than host_cpuid_policy.

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

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

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.

v2:
 * Fix hvm_efer_valid() as well.
 * Clarify commit message somewhat.
---
 xen/arch/x86/hvm/hvm.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 401c4a9..21944e9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -886,12 +886,7 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
                            signed int cr0_pg)
 {
     const struct domain *d = v->domain;
-    const struct cpuid_policy *p;
-
-    if ( cr0_pg < 0 && !is_hardware_domain(d) )
-        p = d->arch.cpuid;
-    else
-        p = &host_cpuid_policy;
+    const struct cpuid_policy *p = d->arch.cpuid;
 
     if ( value & ~EFER_KNOWN_MASK )
         return "Unknown bits set";
@@ -928,14 +923,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 +940,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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER

[Jan Beulich]

>>> On 28.01.19 at 17:40, <andrew.cooper3@citrix.com> wrote:
> Before the cpuid_policy logic came along, %cr4/EFER 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.
> 
> Similarly at the time, PVHv1 lacked the "CPUID passed through from hardware"
> behaviour with PV guests had, and PVH dom0 had to be special-cased to be able
> to boot.
> 
> 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 in
> the %cr4 case), but since Xen 4.9, all domains start with a suitable CPUID
> policy, which is a more appropriate upper bound than host_cpuid_policy.
> 
> Finally, reposition the UMIP logic as it is the only row out of order.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER

[Wei Liu]

On Mon, Jan 28, 2019 at 04:40:59PM +0000, Andrew Cooper wrote:
> Before the cpuid_policy logic came along, %cr4/EFER 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.
> 
> Similarly at the time, PVHv1 lacked the "CPUID passed through from hardware"
> behaviour with PV guests had, and PVH dom0 had to be special-cased to be able
> to boot.
> 
> 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 in
> the %cr4 case), but since Xen 4.9, all domains start with a suitable CPUID
> policy, which is a more appropriate upper bound than host_cpuid_policy.
> 
> Finally, reposition the UMIP logic as it is the only row out of order.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER

[Juergen Gross]

On 28/01/2019 17:40, Andrew Cooper wrote:
> Before the cpuid_policy logic came along, %cr4/EFER 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.
> 
> Similarly at the time, PVHv1 lacked the "CPUID passed through from hardware"
> behaviour with PV guests had, and PVH dom0 had to be special-cased to be able
> to boot.
> 
> 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 in
> the %cr4 case), but since Xen 4.9, all domains start with a suitable CPUID
> policy, which is a more appropriate upper bound than host_cpuid_policy.
> 
> Finally, reposition the UMIP logic as it is the only row out of order.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

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