[PATCH for-4.8 1/2] x86/vmx: Correct the long mode check in vmx_cpuid_intercept()

[PATCH for-4.8 1/2] x86/vmx: Correct the long mode check in vmx_cpuid_intercept()

[Andrew Cooper]

%cs.L may be set in a legacy mode segment, or clear in a compatibility mode
segment; it is not the correct way to check for long mode being active.

Both of these situations result in incorrect visibility of the SYSCALL feature
in CPUID, and by extension, incorrect behaviour in hvm_efer_valid().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/vmx/vmx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9a8f694..a18db28 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2407,7 +2407,6 @@ static void vmx_cpuid_intercept(
     unsigned int *ecx, unsigned int *edx)
 {
     unsigned int input = *eax;
-    struct segment_register cs;
     struct vcpu *v = current;
 
     hvm_cpuid(input, eax, ebx, ecx, edx);
@@ -2416,8 +2415,7 @@ static void vmx_cpuid_intercept(
     {
         case 0x80000001:
             /* SYSCALL is visible iff running in long mode. */
-            vmx_get_segment_register(v, x86_seg_cs, &cs);
-            if ( cs.attr.fields.l )
+            if ( hvm_long_mode_enabled(v) )
                 *edx |= cpufeat_mask(X86_FEATURE_SYSCALL);
             else
                 *edx &= ~(cpufeat_mask(X86_FEATURE_SYSCALL));
-- 
2.1.4


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

[PATCH for-4.8 2/2] x86/traps: Don't call hvm_hypervisor_cpuid_leaf() for PV guests

[Andrew Cooper]

Luckily, hvm_hypervisor_cpuid_leaf() and vmx_hypervisor_cpuid_leaf() are safe
to execute in the context of a PV guest, but HVM-specific feature flags
shouldn't be visible to PV guests.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/traps.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 14abb62..d56d76e 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -928,6 +928,11 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
         break;
 
     case 4:
+        if ( !has_hvm_container_domain(currd) )
+        {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        }
         hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
         break;
 
-- 
2.1.4


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

Re: [PATCH for-4.8 1/2] x86/vmx: Correct the long mode check in vmx_cpuid_intercept()

[Jan Beulich]

>>> On 14.11.16 at 12:01, <andrew.cooper3@citrix.com> wrote:
> %cs.L may be set in a legacy mode segment, or clear in a compatibility mode
> segment; it is not the correct way to check for long mode being active.
> 
> Both of these situations result in incorrect visibility of the SYSCALL feature
> in CPUID, and by extension, incorrect behaviour in hvm_efer_valid().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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


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

Re: [PATCH for-4.8 2/2] x86/traps: Don't call hvm_hypervisor_cpuid_leaf() for PV guests

[Jan Beulich]

>>> On 14.11.16 at 12:01, <andrew.cooper3@citrix.com> wrote:
> Luckily, hvm_hypervisor_cpuid_leaf() and vmx_hypervisor_cpuid_leaf() are safe
> to execute in the context of a PV guest, but HVM-specific feature flags
> shouldn't be visible to PV guests.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -928,6 +928,11 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
>          break;
>  
>      case 4:
> +        if ( !has_hvm_container_domain(currd) )
> +        {
> +            *eax = *ebx = *ecx = *edx = 0;
> +            break;
> +        }
>          hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
>          break;

... this being the last leaf, wouldn't we better limit the number of
leaves (reported in leaf 0) to 3 for PV?

Jan


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

Re: [PATCH for-4.8 2/2] x86/traps: Don't call hvm_hypervisor_cpuid_leaf() for PV guests

[Andrew Cooper]

On 14/11/16 11:38, Jan Beulich wrote:
>>>> On 14.11.16 at 12:01, <andrew.cooper3@citrix.com> wrote:
>> Luckily, hvm_hypervisor_cpuid_leaf() and vmx_hypervisor_cpuid_leaf() are safe
>> to execute in the context of a PV guest, but HVM-specific feature flags
>> shouldn't be visible to PV guests.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> albeit ...
>
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -928,6 +928,11 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
>>          break;
>>  
>>      case 4:
>> +        if ( !has_hvm_container_domain(currd) )
>> +        {
>> +            *eax = *ebx = *ecx = *edx = 0;
>> +            break;
>> +        }
>>          hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
>>          break;
> ... this being the last leaf, wouldn't we better limit the number of
> leaves (reported in leaf 0) to 3 for PV?

I considered this, but decided not to.

The current max leaf handling is fragile, owing to some dubious control
from the toolstack, and the existence of XEN_CPUID_MAX_NUM_LEAVES in the
public API is absolutely broken.

I am going to need to rework this all anyway, and its not clear whether
we can/should report less than 4 leaves to PV guests.

~Andrew

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

Re: [PATCH for-4.8 1/2] x86/vmx: Correct the long mode check in vmx_cpuid_intercept()

[Wei Liu]

On Mon, Nov 14, 2016 at 11:01:25AM +0000, Andrew Cooper wrote:
> %cs.L may be set in a legacy mode segment, or clear in a compatibility mode
> segment; it is not the correct way to check for long mode being active.
> 
> Both of these situations result in incorrect visibility of the SYSCALL feature
> in CPUID, and by extension, incorrect behaviour in hvm_efer_valid().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Both patches:

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


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

Re: [PATCH for-4.8 1/2] x86/vmx: Correct the long mode check in vmx_cpuid_intercept()

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Monday, November 14, 2016 7:01 PM
> 
> %cs.L may be set in a legacy mode segment, or clear in a compatibility mode
> segment; it is not the correct way to check for long mode being active.
> 
> Both of these situations result in incorrect visibility of the SYSCALL feature
> in CPUID, and by extension, incorrect behaviour in hvm_efer_valid().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

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