Re: [Xen-devel] [PATCH RESEND v1 1/7] x86: add a flag to enable Intel processor trace

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.01.18 at 19:12, <luwei.kang@xxxxxxxxx> wrote:
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1009,6 +1009,13 @@ debug hypervisor only).
>  ### idle\_latency\_factor
>  > `= <integer>`
>  
> +### intel\_pt
> +> `= <boolean>`
> +
> +> Default: `true`
> +
> +Flag to enable Intel Processor Trace.

I agree with what Wei has said. In addition please use dashes in preference to
underscores for both command line options and file names (neither of the two
is constrained by C identifier naming restrictions). That said, I'm afraid "pt" 
is
an acronym we commonly associate with pass-through, so for all of file names,
command line option, and identifiers I'd like to ask for an alternative to be 
found.
"ptrace" may be an option, as - other than e.g. Linux - we don't associate any
meaning to it.

> --- /dev/null
> +++ b/xen/arch/x86/cpu/intel_pt.c
> @@ -0,0 +1,27 @@
> +/*
> + * intel_pt.c: Support Intel Processor Trace Virtualization.
> + *
> + * Copyright (c) 2018, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program; If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author: Luwei Kang <luwei.kang@xxxxxxxxx>
> + */
> +
> +#include <xen/types.h>
> +#include <xen/cache.h>
> +#include <xen/init.h>

Why the last one?

> +/* intel_pt: Flag to enable Intel Processor Trace (default on). */
> +bool_t __read_mostly opt_intel_pt = 1;

bool and true please. However, I'm not convinced we want this feature on
by default, even less so right from the first patch in the series.

Jan



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