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

["Kang, Luwei" <luwei.kang@xxxxxxxxx>]

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

So, the option should be "### ipt (x86)" ?

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

Get it. Will fix it.

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

OK, will change it to bool. About __read_mostly attribute,  I will remove it if 
not read frequency.

Thanks,
Luwei Kang

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