Re: [Xen-devel] [PATCH v2 01/10] x86: add an flag to enable Intel Processor Trace in guest

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.05.18 at 15:27, <luwei.kang@xxxxxxxxx> wrote:
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1215,6 +1215,16 @@ Rather than only mapping RAM pages for IOMMU accesses 
> for Dom0, with this
>  option all pages not marked as unusable in the E820 table will get a mapping
>  established.
>  
> +### ipt
> +> `= guest`
> +
> +> Default: `off`
> +
> +This option is use for switch on the Intel Processor Trace feature
> +in HVM guest when 'ipt=guest'. By default, this feature is disabled
> +in guest. Intel Processor Trace virtualization depend on
> +EPT, so it can only enabled in HVM guest at present.
> +
>  ### irq\_ratelimit (x86)

Did you not notice the (x86) here when re-basing?

> --- /dev/null
> +++ b/xen/arch/x86/cpu/ipt.c
> @@ -0,0 +1,42 @@
> +/*
> + * ipt.c: Support for 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/errno.h>
> +#include <xen/init.h>
> +#include <xen/lib.h>
> +#include <xen/string.h>
> +#include <asm/ipt.h>
> +
> +/* ipt: Flag to enable Intel Processor Trace (default off). */
> +unsigned int __read_mostly ipt_mode = IPT_MODE_OFF;
> +static int parse_ipt_params(const char *str);

I think it was pointed out before that the forward declaration can be
avoided if you move ...

> +custom_param("ipt", parse_ipt_params);

... this line ...

> +static int __init parse_ipt_params(const char *str)
> +{
> +    if ( !strcmp("guest", str) )
> +        ipt_mode = IPT_MODE_GUEST;
> +    else if ( str )
> +    {
> +        printk("Unknown Intel Processor Trace mode specified: '%s'\n", str);
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}

... here.

> --- /dev/null
> +++ b/xen/include/asm-x86/ipt.h
> @@ -0,0 +1,29 @@
> +/*
> + * ipt.h: Intel Processor Trace virtualization for HVM domain.
> + *
> + * 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>
> + */
> +
> +#ifndef __ASM_X86_HVM_IPT_H_
> +#define __ASM_X86_HVM_IPT_H_
> +
> +#define IPT_MODE_OFF        0
> +#define IPT_MODE_GUEST      (1<<0)
> +
> +extern unsigned int ipt_mode;

At this point I can't see why the variable can't be bool. With the
patch being placed first in the series it is also impossible (without
peeking into later patches) to judge whether its __read_mostly
attribute is actually appropriate.

Jan



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