From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kai Huang Subject: Re: [PATCH 02/10] VMX: New parameter to control PML enabling Date: Thu, 02 Apr 2015 13:46:10 +0800 Message-ID: <551CD7A2.9020409@linux.intel.com> References: <1427423754-11841-1-git-send-email-kai.huang@linux.intel.com> <1427423754-11841-3-git-send-email-kai.huang@linux.intel.com> <5515C09A.9040303@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <5515C09A.9040303@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , jbeulich@suse.com, tim@xen.org, kevin.tian@intel.com, yang.z.zhang@intel.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 03/28/2015 04:42 AM, Andrew Cooper wrote: > On 27/03/15 02:35, Kai Huang wrote: >> A top level EPT parameter "ept=3D" and a sub boolean "pml_enabl= e" are >> added to control PML. Other booleans can be further added for any other = EPT >> related features. >> >> Signed-off-by: Kai Huang > Please patch docs/misc/xen-command-line.markdown as well. See the > existing "psr" option as a similar example. > > Also, as indicated in patch 1, I think patches 1 and 2 need swapping in > the series. > >> --- >> xen/arch/x86/hvm/vmx/vmcs.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c >> index 2f645fe..9b20a4b 100644 >> --- a/xen/arch/x86/hvm/vmx/vmcs.c >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >> @@ -50,6 +50,16 @@ boolean_param("unrestricted_guest", opt_unrestricted_= guest_enabled); >> static bool_t __read_mostly opt_apicv_enabled =3D 1; >> boolean_param("apicv", opt_apicv_enabled); >> = >> +static void parse_ept_param(char *s); >> +/* >> + * The 'ept' parameter controls functionalities that depend on, or impa= ct the >> + * EPT mechanism. Optional comma separated value may contain: >> + * >> + * pml Enable PML >> + */ >> +custom_param("ept", parse_ept_param); > It is common to put the custom_param() call below parse_ept_param() so > you don't need to forward-declare the function. The comment can happily > live at the top of parse_ept_param(). Hi Andrew, Looks it's better to keep parse_ept_param() below custom_param(), as = simply moving parse_ept_param() above custom_param() results in below = error (I also changed pml_enable to opt_pml_enabled), as it references = opt_pml_enabled variable, which is defined below custom_param(). = Actually for "iommu=3D" parameter, parse_iommu_param() was also = placed below custom_param(). What do you think? vmcs.c: In function =91parse_ept_param=92: vmcs.c:74:13: error: =91opt_pml_enabled=92 undeclared (first use in this = function) opt_pml_enabled =3D val; ^ vmcs.c:74:13: note: each undeclared identifier is reported only once for = each function it appears in vmcs.c: At top level: vmcs.c:81:29: error: =91opt_pml_enabled=92 defined but not used = [-Werror=3Dunused-variable] static bool_t __read_mostly opt_pml_enabled =3D 0; Thanks, -Kai > >> +static bool_t __read_mostly pml_enable =3D 0; >> + >> /* >> * These two parameters are used to config the controls for Pause-Loop= Exiting: >> * ple_gap: upper bound on the amount of time between two successive >> @@ -92,6 +102,28 @@ DEFINE_PER_CPU(bool_t, vmxon); >> static u32 vmcs_revision_id __read_mostly; >> u64 __read_mostly vmx_basic_msr; >> = >> +/* Copied from parse_iommu_param */ > Not a useful comment, as it is likely to diverge in the future. > >> +static void parse_ept_param(char *s) > __init > > ~Andrew > >> +{ >> + char *ss; >> + int val; >> + >> + do { >> + val =3D !!strncmp(s, "no-", 3); >> + if ( !val ) >> + s +=3D 3; >> + >> + ss =3D strchr(s, ','); >> + if ( ss ) >> + *ss =3D '\0'; >> + >> + if ( !strcmp(s, "pml") ) >> + pml_enable =3D val; >> + >> + s =3D ss + 1; >> + } while ( ss ); >> +} >> + >> static void __init vmx_display_features(void) >> { >> int printed =3D 0; > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel