[PATCH] x86/vmx: Add command line option to enable EPT without PAT

[PATCH] x86/vmx: Add command line option to enable EPT without PAT

[Aravindh Puthiyaparambil]

The fix for XSA-60 disables EPT if PAT is not available. This patch
adds a command line option called "ept_without_pat", that allows EPT to
be enabled even when PAT is not present. This is to enable Xen to run as
a nested guest with EPT on hypervisors that have nested EPT but not
nested PAT.

Signed-off-by: Aravindh Puthiyaparambil <aravindp@cisco.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Eddie Dong <eddie.dong@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
 docs/misc/xen-command-line.markdown | 11 +++++++++++
 xen/arch/x86/hvm/vmx/vmx.c          |  5 ++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 87de2dc..9dc501b 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -523,6 +523,17 @@ Either force retrieval of monitor EDID information via VESA DDC, or
 disable it (edid=no). This option should not normally be required
 except for debugging purposes.
 
+### ept_without_pat
+> `= <boolean>`
+
+Allow EPT to be enabled when PAT is not present. 
+
+*Warning:*
+This is an unsupported option and should be used only to allow Xen to run with
+EPT as a nested guest on hypervisors that do not have nested PAT.
+
+> Default: `false`
+
 ### extra\_guest\_irqs
 > `= [<domU number>][,<dom0 number>]`
 
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 180cf6c..a308a93 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -58,6 +58,9 @@
 #include <asm/hvm/nestedhvm.h>
 #include <asm/event.h>
 
+static bool_t __initdata opt_ept_without_pat= 0;
+boolean_param("ept_without_pat", opt_ept_without_pat);
+
 enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
 
 static void vmx_ctxt_switch_from(struct vcpu *v);
@@ -1724,7 +1727,7 @@ const struct hvm_function_table * __init start_vmx(void)
      * Do not enable EPT when (!cpu_has_vmx_pat), to prevent security hole
      * (refer to http://xenbits.xen.org/xsa/advisory-60.html).
      */
-    if ( cpu_has_vmx_ept && cpu_has_vmx_pat )
+    if ( cpu_has_vmx_ept && (cpu_has_vmx_pat || opt_ept_without_pat) )
     {
         vmx_function_table.hap_supported = 1;
 
-- 
1.8.3.2

Re: [PATCH] x86/vmx: Add command line option to enable EPT without PAT

[Andrew Cooper]

On 16/04/2014 22:15, Aravindh Puthiyaparambil wrote:
> The fix for XSA-60 disables EPT if PAT is not available. This patch
> adds a command line option called "ept_without_pat", that allows EPT to
> be enabled even when PAT is not present. This is to enable Xen to run as
> a nested guest with EPT on hypervisors that have nested EPT but not
> nested PAT.
>
> Signed-off-by: Aravindh Puthiyaparambil <aravindp@cisco.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Eddie Dong <eddie.dong@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> ---
>  docs/misc/xen-command-line.markdown | 11 +++++++++++
>  xen/arch/x86/hvm/vmx/vmx.c          |  5 ++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 87de2dc..9dc501b 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -523,6 +523,17 @@ Either force retrieval of monitor EDID information via VESA DDC, or
>  disable it (edid=no). This option should not normally be required
>  except for debugging purposes.
>  
> +### ept_without_pat

Need to escape underscores with a backslash so markdown doesn't try to
italicise 'without'

Also, this in an Intel-specific option so should be annotated.  See the
documentation for 'vpid' as an example.

> +> `= <boolean>`
> +
> +Allow EPT to be enabled when PAT is not present. 
> +
> +*Warning:*
> +This is an unsupported option and should be used only to allow Xen to run with
> +EPT as a nested guest on hypervisors that do not have nested PAT.

I would not necessarily describe it as an unsupported option.  The
reason for the PAT requirement is because XSA-60 was a DoS attack with
HVM guests switching CR0.CD in combination with PCIPassthrough.

In the case that the administrator has weighed the risks, it need not be
unsupported.  In an environment without PCIPassthrough then it should be
unconditionally safe as flipping CR0.CD should turn into a noop, and the
benefit is the addition of nested EPT.  As a result, I might word the
paragraph a little more like this:

*Warning:*
Due to CVE-2013-2212, PAT is by default required as a prerequisite for
using EPT.  If you are not PCI Passthrough, or trust the guest
administrator who would be using passthrough, then the PAT requirement
can be relaxed.  This option is useful for nested virtualisation cases
where the outer hypervisor does not expose PAT functionality to Xen.

Or words to that effect, subject to taste.

> +
> +> Default: `false`

Default statement should be ahead of the description.

> +
>  ### extra\_guest\_irqs
>  > `= [<domU number>][,<dom0 number>]`
>  
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 180cf6c..a308a93 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -58,6 +58,9 @@
>  #include <asm/hvm/nestedhvm.h>
>  #include <asm/event.h>
>  
> +static bool_t __initdata opt_ept_without_pat= 0;

space before =, but the assignment of 0 is redundant and can be dropped.

~Andrew

> +boolean_param("ept_without_pat", opt_ept_without_pat);
> +
>  enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
>  
>  static void vmx_ctxt_switch_from(struct vcpu *v);
> @@ -1724,7 +1727,7 @@ const struct hvm_function_table * __init start_vmx(void)
>       * Do not enable EPT when (!cpu_has_vmx_pat), to prevent security hole
>       * (refer to http://xenbits.xen.org/xsa/advisory-60.html).
>       */
> -    if ( cpu_has_vmx_ept && cpu_has_vmx_pat )
> +    if ( cpu_has_vmx_ept && (cpu_has_vmx_pat || opt_ept_without_pat) )
>      {
>          vmx_function_table.hap_supported = 1;
>  

Re: [PATCH] x86/vmx: Add command line option to enable EPT without PAT

[Aravindh Puthiyaparambil (aravindp)]

>>  docs/misc/xen-command-line.markdown | 11 +++++++++++
>>  xen/arch/x86/hvm/vmx/vmx.c          |  5 ++++-
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/misc/xen-command-line.markdown
>> b/docs/misc/xen-command-line.markdown
>> index 87de2dc..9dc501b 100644
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -523,6 +523,17 @@ Either force retrieval of monitor EDID
>> information via VESA DDC, or  disable it (edid=no). This option should
>> not normally be required  except for debugging purposes.
>>
>> +### ept_without_pat
>
>Need to escape underscores with a backslash so markdown doesn't try to
>italicise 'without'
>
>Also, this in an Intel-specific option so should be annotated.  See the
>documentation for 'vpid' as an example.
>
>> +> `= <boolean>`
>> +
>> +Allow EPT to be enabled when PAT is not present.
>> +
>> +*Warning:*
>> +This is an unsupported option and should be used only to allow Xen to
>> +run with EPT as a nested guest on hypervisors that do not have nested
>PAT.
>
>I would not necessarily describe it as an unsupported option.  The reason for
>the PAT requirement is because XSA-60 was a DoS attack with HVM guests
>switching CR0.CD in combination with PCIPassthrough.
>
>In the case that the administrator has weighed the risks, it need not be
>unsupported.  In an environment without PCIPassthrough then it should be
>unconditionally safe as flipping CR0.CD should turn into a noop, and the
>benefit is the addition of nested EPT.  As a result, I might word the paragraph
>a little more like this:
>
>*Warning:*
>Due to CVE-2013-2212, PAT is by default required as a prerequisite for using
>EPT.  If you are not PCI Passthrough, or trust the guest administrator who
>would be using passthrough, then the PAT requirement can be relaxed.  This
>option is useful for nested virtualisation cases where the outer hypervisor
>does not expose PAT functionality to Xen.
>
>Or words to that effect, subject to taste.
>
>> +
>> +> Default: `false`
>
>Default statement should be ahead of the description.
>
>> +
>>  ### extra\_guest\_irqs
>>  > `= [<domU number>][,<dom0 number>]`
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 180cf6c..a308a93 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -58,6 +58,9 @@
>>  #include <asm/hvm/nestedhvm.h>
>>  #include <asm/event.h>
>>
>> +static bool_t __initdata opt_ept_without_pat= 0;
>
>space before =, but the assignment of 0 is redundant and can be dropped.

Thanks for the feedback. I will send out a patch with the changes you asked for.

Thanks,
Aravindh