All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michał Leszczyński" <michal.leszczynski@cert.pl>
To: Tamas K Lengyel <tamas.k.lengyel@gmail.com>
Cc: "Tamas K Lengyel" <tamas.lengyel@intel.com>,
	"Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	Xen-devel <xen-devel@lists.xenproject.org>,
	"Kang, Luwei" <luwei.kang@intel.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH v4 01/10] x86/vmx: add Intel PT MSR definitions
Date: Tue, 30 Jun 2020 20:27:26 +0200 (CEST)	[thread overview]
Message-ID: <1131260497.16365560.1593541646453.JavaMail.zimbra@cert.pl> (raw)
In-Reply-To: <CABfawh=N-PVmxDWa=QR5ttt=rZ7gmh148ZsjRV+EX7Td525Wuw@mail.gmail.com>

----- 30 cze 2020 o 20:03, Tamas K Lengyel tamas.k.lengyel@gmail.com napisał(a):

> On Tue, Jun 30, 2020 at 11:39 AM Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>>
>> On 30/06/2020 13:33, Michał Leszczyński wrote:
>> > diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
>> > index b328a47ed8..0203029be9 100644
>> > --- a/xen/include/asm-x86/msr-index.h
>> > +++ b/xen/include/asm-x86/msr-index.h
>> > @@ -69,6 +69,43 @@
>> >  #define MSR_MCU_OPT_CTRL                    0x00000123
>> >  #define  MCU_OPT_CTRL_RNGDS_MITG_DIS        (_AC(1, ULL) <<  0)
>> >
>> > +/* Intel PT MSRs */
>> > +#define MSR_RTIT_OUTPUT_BASE                0x00000560
>> > +
>> > +#define MSR_RTIT_OUTPUT_MASK                0x00000561
>> > +
>> > +#define MSR_RTIT_CTL                        0x00000570
>> > +#define  RTIT_CTL_TRACEEN                    (_AC(1, ULL) <<  0)
>> > +#define  RTIT_CTL_CYCEN                      (_AC(1, ULL) <<  1)
>>
>> In addition to what Jan has said, please can we be consistent with an
>> underscore (or not) before EN.  Preferably with, so these would become
>> TRACE_EN and CYC_EN.
>>
>> That said, there are a lot of bit definitions which aren't used at all.
>> IMO, it would be better to introduce defines when you use them.
> 
> In the past I found it very valuable when this type of plumbing was
> already present in Xen instead of me having to go into the SDM to digg
> out the magic numbers. So while some of the bits might not be used
> right now I also don't see any downside in having them, just in case.
> 
> Tamas


+1 for keeping the unused #defines, this is a helpful piece of knowledge
which speeds up further patch development. It doesn't affect the compilation
nor runtime time and it doesn't occupy too much space in the code so I would
opt for keep it.

I will rebase this series onto latest master within patch v5. The remaining
patches in this series are not affected and still could be reviewed,
so I will wait a few days before posting the new version.


Best regards,
Michał Leszczyński
CERT Polska


  reply	other threads:[~2020-06-30 18:28 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30 12:33 [PATCH v4 00/10] Implement support for external IPT monitoring Michał Leszczyński
2020-06-30 12:33 ` [PATCH v4 01/10] x86/vmx: add Intel PT MSR definitions Michał Leszczyński
2020-06-30 16:23   ` Jan Beulich
2020-06-30 17:37   ` Andrew Cooper
2020-06-30 18:03     ` Tamas K Lengyel
2020-06-30 18:27       ` Michał Leszczyński [this message]
2020-07-01 17:52   ` Andrew Cooper
2020-06-30 12:33 ` [PATCH v4 02/10] x86/vmx: add IPT cpu feature Michał Leszczyński
2020-07-01  9:49   ` Roger Pau Monné
2020-07-01 15:12   ` Julien Grall
2020-07-01 16:06     ` Andrew Cooper
2020-07-01 16:17       ` Julien Grall
2020-07-01 16:18         ` Julien Grall
2020-07-01 17:26           ` Andrew Cooper
2020-07-01 18:02             ` Julien Grall
2020-07-01 18:06               ` Andrew Cooper
2020-07-01 18:09                 ` Julien Grall
2020-07-02  8:29                   ` Jan Beulich
2020-07-02  8:42                     ` Julien Grall
2020-07-02  8:50                       ` Jan Beulich
2020-07-02  8:54                         ` Julien Grall
2020-07-02  9:18                           ` Jan Beulich
2020-07-02  9:57                             ` Julien Grall
2020-07-02 13:30                               ` Jan Beulich
2020-07-02 14:14                                 ` Julien Grall
2020-07-02 14:17                                   ` Jan Beulich
2020-07-02 14:31                                     ` Julien Grall
2020-07-02 20:28                                       ` Michał Leszczyński
2020-07-03  7:58                                         ` Julien Grall
2020-07-04 19:16                                           ` Michał Leszczyński
2020-07-01 21:42   ` Andrew Cooper
2020-07-02  8:10     ` Roger Pau Monné
2020-07-02  8:34       ` Jan Beulich
2020-07-02 20:29         ` Michał Leszczyński
2020-06-30 12:33 ` [PATCH v4 03/10] tools/libxl: add vmtrace_pt_size parameter Michał Leszczyński
2020-07-01 10:05   ` Roger Pau Monné
2020-07-02  9:00   ` Roger Pau Monné
2020-07-02 16:23     ` Michał Leszczyński
2020-07-03  9:44       ` Roger Pau Monné
2020-07-03  9:56         ` Jan Beulich
2020-07-03 10:11           ` Roger Pau Monné
2020-07-04 17:23             ` Julien Grall
2020-07-06  8:46               ` Jan Beulich
2020-07-07  8:44                 ` Julien Grall
2020-07-07  9:10                   ` Jan Beulich
2020-07-07  9:16                     ` Julien Grall
2020-07-07 11:17                       ` Michał Leszczyński
2020-07-07 11:21                         ` Jan Beulich
2020-07-07 11:35                           ` Michał Leszczyński
2020-07-02 10:24   ` Anthony PERARD
2020-07-04 17:48   ` Julien Grall
2020-06-30 12:33 ` [PATCH v4 04/10] x86/vmx: implement processor tracing for VMX Michał Leszczyński
2020-07-01 10:30   ` Roger Pau Monné
2020-06-30 12:33 ` [PATCH v4 05/10] common/domain: allocate vmtrace_pt_buffer Michał Leszczyński
2020-07-01 10:38   ` Roger Pau Monné
2020-07-01 15:35   ` Julien Grall
2020-06-30 12:33 ` [PATCH v4 06/10] memory: batch processing in acquire_resource() Michał Leszczyński
2020-07-01 10:46   ` Roger Pau Monné
2020-07-03 10:35   ` Julien Grall
2020-07-03 10:52     ` Paul Durrant
2020-07-03 11:17       ` Julien Grall
2020-07-03 11:22         ` Jan Beulich
2020-07-03 11:36           ` Julien Grall
2020-07-03 12:50             ` Jan Beulich
2020-07-03 11:40         ` Paul Durrant
2020-06-30 12:33 ` [PATCH v4 07/10] x86/mm: add vmtrace_buf resource type Michał Leszczyński
2020-07-01 10:52   ` Roger Pau Monné
2020-06-30 12:33 ` [PATCH v4 08/10] x86/domctl: add XEN_DOMCTL_vmtrace_op Michał Leszczyński
2020-07-01 11:00   ` Roger Pau Monné
2020-06-30 12:33 ` [PATCH v4 09/10] tools/libxc: add xc_vmtrace_* functions Michał Leszczyński
2020-07-21 10:52   ` Wei Liu
2020-06-30 12:33 ` [PATCH v4 10/10] tools/proctrace: add proctrace tool Michał Leszczyński
2020-07-02 15:10   ` Andrew Cooper
2020-07-21 10:52     ` Wei Liu
2020-06-30 12:48 ` [PATCH v4 00/10] Implement support for external IPT monitoring Hubert Jasudowicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1131260497.16365560.1593541646453.JavaMail.zimbra@cert.pl \
    --to=michal.leszczynski@cert.pl \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=luwei.kang@intel.com \
    --cc=roger.pau@citrix.com \
    --cc=tamas.k.lengyel@gmail.com \
    --cc=tamas.lengyel@intel.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.