From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Aravindh Puthiyaparambil (aravindp)" <aravindp@cisco.com>
Subject: Re: [PATCH] x86/vmx: Add command line option to enable
 EPT without PAT
Date: Wed, 16 Apr 2014 22:29:23 +0000
Message-ID: <97A500D504438F4ABC02EBA81613CC63317E0027@xmb-aln-x02.cisco.com>
References: <1397682915-9084-1-git-send-email-aravindp@cisco.com>
	<534F003A.2060505@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <aravindp@cisco.com>) id 1WaYKe-000326-G3
	for xen-devel@lists.xenproject.org; Wed, 16 Apr 2014 22:29:28 +0000
In-Reply-To: <534F003A.2060505@citrix.com>
Content-Language: en-US
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>, "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Kevin Tian <kevin.tian@intel.com>, Eddie Dong <eddie.dong@intel.com>, Jun Nakajima <jun.nakajima@intel.com>
List-Id: xen-devel@lists.xenproject.org

>>  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