mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH V3] perf: Add PERF_SAMPLE_PHYS_ADDR
@ 2017-08-09 20:42 kan.liang
  2017-08-17  7:45 ` Stephane Eranian
  0 siblings, 1 reply; 3+ messages in thread
From: kan.liang @ 2017-08-09 20:42 UTC (permalink / raw)
  To: peterz, mingo, linux-kernel; +Cc: acme, jolsa, tglx, eranian, ak, Kan Liang

From: Kan Liang <kan.liang@intel.com>

For understanding how the workload maps to memory channels and hardware
behavior, it's very important to collect address maps with physical
addresses. For example, 3D XPoint access can only be found by filtering
the physical address.
However, perf doesn't collect physical address information in sampling.

The load latency/DLA information in PEBS can be used to calculate the
physical address.
For kernel direct mapping addresses, virt_to_phys is used to convert the
virtual addresses from DLA to physical address.
For user virtual addresses, __get_user_pages_fast is used to walk the
pages tables for user physical address.
This does not work for vmalloc addresses. Right now these are not
resolved, but code to do that could be added.
For security, the physical address can only be exposed to root or
privileged user.
A new sample type PERF_SAMPLE_PHYS_ADDR is introduced to expose the
physical addresses.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---

The V2 version can be found here.
https://patchwork.kernel.org/patch/7979771/

Changes since V2
 - Only the kernel patch
 - Add example in changelog
 - Include a perf_paranoid_kernel() test (PeterZ)
 - Fix minor complier warning

 arch/x86/events/intel/ds.c      | 24 ++++++++++++++++++++++++
 arch/x86/events/perf_event.h    |  2 +-
 include/linux/perf_event.h      |  3 +++
 include/uapi/linux/perf_event.h |  4 +++-
 kernel/events/core.c            | 12 ++++++++++++
 5 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index a322fed..712c86b 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1083,6 +1083,7 @@ static void setup_pebs_sample_data(struct perf_event *event,
 	u64 sample_type;
 	int fll, fst, dsrc;
 	int fl = event->hw.flags;
+	struct page *p = NULL;
 
 	if (pebs == NULL)
 		return;
@@ -1179,6 +1180,29 @@ static void setup_pebs_sample_data(struct perf_event *event,
 	    x86_pmu.intel_cap.pebs_format >= 1)
 		data->addr = pebs->dla;
 
+	if ((sample_type & PERF_SAMPLE_PHYS_ADDR) && (data->addr != 0)) {
+		if (data->addr >= TASK_SIZE) {
+			/* If it's vmalloc()d memory, leave phys_addr as 0 */
+			if (virt_addr_valid(data->addr) &&
+			    !(data->addr >= VMALLOC_START && data->addr < VMALLOC_END))
+				data->phys_addr = (u64)virt_to_phys((void *)(uintptr_t)data->addr);
+		} else {
+			/*
+			 * Walking the pages tables for user address.
+			 * Interrupts are disabled, so it prevents any tear down
+			 * of the page tables.
+			 * Try IRQ-safe __get_user_pages_fast first.
+			 * If failed, leave phys_addr as 0.
+			 */
+			if ((current->mm != NULL) &&
+			    (__get_user_pages_fast(data->addr, 1, 0, &p) == 1))
+				data->phys_addr = page_to_phys(p) + data->addr % PAGE_SIZE;
+
+			if (p)
+				put_page(p);
+		}
+	}
+
 	if (x86_pmu.intel_cap.pebs_format >= 2) {
 		/* Only set the TSX weight when no memory weight. */
 		if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 476aec3..65bb91e 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -91,7 +91,7 @@ struct amd_nb {
 	(PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_ADDR | \
 	PERF_SAMPLE_ID | PERF_SAMPLE_CPU | PERF_SAMPLE_STREAM_ID | \
 	PERF_SAMPLE_DATA_SRC | PERF_SAMPLE_IDENTIFIER | \
-	PERF_SAMPLE_TRANSACTION)
+	PERF_SAMPLE_TRANSACTION | PERF_SAMPLE_PHYS_ADDR)
 
 /*
  * A debug store configuration.
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index a3b873f..6783c69 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -944,6 +944,8 @@ struct perf_sample_data {
 
 	struct perf_regs		regs_intr;
 	u64				stack_user_size;
+
+	u64				phys_addr;
 } ____cacheline_aligned;
 
 /* default value for data source */
@@ -964,6 +966,7 @@ static inline void perf_sample_data_init(struct perf_sample_data *data,
 	data->weight = 0;
 	data->data_src.val = PERF_MEM_NA;
 	data->txn = 0;
+	data->phys_addr = 0;
 }
 
 extern void perf_output_sample(struct perf_output_handle *handle,
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 642db5f..cbea02f 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -139,8 +139,9 @@ enum perf_event_sample_format {
 	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
 	PERF_SAMPLE_TRANSACTION			= 1U << 17,
 	PERF_SAMPLE_REGS_INTR			= 1U << 18,
+	PERF_SAMPLE_PHYS_ADDR			= 1U << 19,
 
-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
 };
 
 /*
@@ -814,6 +815,7 @@ enum perf_event_type {
 	 *	{ u64			transaction; } && PERF_SAMPLE_TRANSACTION
 	 *	{ u64			abi; # enum perf_sample_regs_abi
 	 *	  u64			regs[weight(mask)]; } && PERF_SAMPLE_REGS_INTR
+	 *	{ u64			phys_addr;} && PERF_SAMPLE_PHYS_ADDR
 	 * };
 	 */
 	PERF_RECORD_SAMPLE			= 9,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 426c2ff..07d4a74 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1570,6 +1570,9 @@ static void __perf_event_header_size(struct perf_event *event, u64 sample_type)
 	if (sample_type & PERF_SAMPLE_TRANSACTION)
 		size += sizeof(data->txn);
 
+	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+		size += sizeof(data->phys_addr);
+
 	event->header_size = size;
 }
 
@@ -5952,6 +5955,9 @@ void perf_output_sample(struct perf_output_handle *handle,
 	if (sample_type & PERF_SAMPLE_DATA_SRC)
 		perf_output_put(handle, data->data_src.val);
 
+	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+		perf_output_put(handle, data->phys_addr);
+
 	if (sample_type & PERF_SAMPLE_TRANSACTION)
 		perf_output_put(handle, data->txn);
 
@@ -9852,6 +9858,12 @@ SYSCALL_DEFINE5(perf_event_open,
 			return -EINVAL;
 	}
 
+	/* Only privileged users can get kernel addresses */
+	if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
+	    perf_paranoid_kernel() &&
+	    !capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
 	if (!attr.sample_max_stack)
 		attr.sample_max_stack = sysctl_perf_event_max_stack;
 
-- 
2.4.3

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH V3] perf: Add PERF_SAMPLE_PHYS_ADDR
  2017-08-09 20:42 [PATCH V3] perf: Add PERF_SAMPLE_PHYS_ADDR kan.liang
@ 2017-08-17  7:45 ` Stephane Eranian
  2017-08-17 13:42   ` Liang, Kan
  0 siblings, 1 reply; 3+ messages in thread
From: Stephane Eranian @ 2017-08-17  7:45 UTC (permalink / raw)
  To: Liang, Kan
  Cc: Peter Zijlstra, mingo, LKML, Arnaldo Carvalho de Melo, Jiri Olsa,
	Thomas Gleixner, ak

On Wed, Aug 9, 2017 at 1:42 PM,  <kan.liang@intel.com> wrote:
> From: Kan Liang <kan.liang@intel.com>
>
> For understanding how the workload maps to memory channels and hardware
> behavior, it's very important to collect address maps with physical
> addresses. For example, 3D XPoint access can only be found by filtering
> the physical address.
> However, perf doesn't collect physical address information in sampling.
>
> The load latency/DLA information in PEBS can be used to calculate the
> physical address.
> For kernel direct mapping addresses, virt_to_phys is used to convert the
> virtual addresses from DLA to physical address.
> For user virtual addresses, __get_user_pages_fast is used to walk the
> pages tables for user physical address.
> This does not work for vmalloc addresses. Right now these are not
> resolved, but code to do that could be added.
> For security, the physical address can only be exposed to root or
> privileged user.
> A new sample type PERF_SAMPLE_PHYS_ADDR is introduced to expose the
> physical addresses.
>
> Signed-off-by: Kan Liang <kan.liang@intel.com>

This is a useful feature that I would like to use for some of the
analysis we would like to do.
Please apply.
I am assuming you have the support in perf record/report?

> ---
>
> The V2 version can be found here.
> https://patchwork.kernel.org/patch/7979771/
>
> Changes since V2
>  - Only the kernel patch
>  - Add example in changelog
>  - Include a perf_paranoid_kernel() test (PeterZ)
>  - Fix minor complier warning
>
>  arch/x86/events/intel/ds.c      | 24 ++++++++++++++++++++++++
>  arch/x86/events/perf_event.h    |  2 +-
>  include/linux/perf_event.h      |  3 +++
>  include/uapi/linux/perf_event.h |  4 +++-
>  kernel/events/core.c            | 12 ++++++++++++
>  5 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index a322fed..712c86b 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c
> @@ -1083,6 +1083,7 @@ static void setup_pebs_sample_data(struct perf_event *event,
>         u64 sample_type;
>         int fll, fst, dsrc;
>         int fl = event->hw.flags;
> +       struct page *p = NULL;
>
>         if (pebs == NULL)
>                 return;
> @@ -1179,6 +1180,29 @@ static void setup_pebs_sample_data(struct perf_event *event,
>             x86_pmu.intel_cap.pebs_format >= 1)
>                 data->addr = pebs->dla;
>
> +       if ((sample_type & PERF_SAMPLE_PHYS_ADDR) && (data->addr != 0)) {
> +               if (data->addr >= TASK_SIZE) {
> +                       /* If it's vmalloc()d memory, leave phys_addr as 0 */
> +                       if (virt_addr_valid(data->addr) &&
> +                           !(data->addr >= VMALLOC_START && data->addr < VMALLOC_END))
> +                               data->phys_addr = (u64)virt_to_phys((void *)(uintptr_t)data->addr);
> +               } else {
> +                       /*
> +                        * Walking the pages tables for user address.
> +                        * Interrupts are disabled, so it prevents any tear down
> +                        * of the page tables.
> +                        * Try IRQ-safe __get_user_pages_fast first.
> +                        * If failed, leave phys_addr as 0.
> +                        */
> +                       if ((current->mm != NULL) &&
> +                           (__get_user_pages_fast(data->addr, 1, 0, &p) == 1))
> +                               data->phys_addr = page_to_phys(p) + data->addr % PAGE_SIZE;
> +
> +                       if (p)
> +                               put_page(p);
> +               }
> +       }
> +
This would be cleaner in a separate function in my opinion.

>         if (x86_pmu.intel_cap.pebs_format >= 2) {
>                 /* Only set the TSX weight when no memory weight. */
>                 if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)
> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index 476aec3..65bb91e 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -91,7 +91,7 @@ struct amd_nb {
>         (PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_ADDR | \
>         PERF_SAMPLE_ID | PERF_SAMPLE_CPU | PERF_SAMPLE_STREAM_ID | \
>         PERF_SAMPLE_DATA_SRC | PERF_SAMPLE_IDENTIFIER | \
> -       PERF_SAMPLE_TRANSACTION)
> +       PERF_SAMPLE_TRANSACTION | PERF_SAMPLE_PHYS_ADDR)
>
>  /*
>   * A debug store configuration.
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index a3b873f..6783c69 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -944,6 +944,8 @@ struct perf_sample_data {
>
>         struct perf_regs                regs_intr;
>         u64                             stack_user_size;
> +
> +       u64                             phys_addr;
>  } ____cacheline_aligned;
>
>  /* default value for data source */
> @@ -964,6 +966,7 @@ static inline void perf_sample_data_init(struct perf_sample_data *data,
>         data->weight = 0;
>         data->data_src.val = PERF_MEM_NA;
>         data->txn = 0;
> +       data->phys_addr = 0;
>  }
>
>  extern void perf_output_sample(struct perf_output_handle *handle,
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 642db5f..cbea02f 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -139,8 +139,9 @@ enum perf_event_sample_format {
>         PERF_SAMPLE_IDENTIFIER                  = 1U << 16,
>         PERF_SAMPLE_TRANSACTION                 = 1U << 17,
>         PERF_SAMPLE_REGS_INTR                   = 1U << 18,
> +       PERF_SAMPLE_PHYS_ADDR                   = 1U << 19,
>
> -       PERF_SAMPLE_MAX = 1U << 19,             /* non-ABI */
> +       PERF_SAMPLE_MAX = 1U << 20,             /* non-ABI */
>  };
>
>  /*
> @@ -814,6 +815,7 @@ enum perf_event_type {
>          *      { u64                   transaction; } && PERF_SAMPLE_TRANSACTION
>          *      { u64                   abi; # enum perf_sample_regs_abi
>          *        u64                   regs[weight(mask)]; } && PERF_SAMPLE_REGS_INTR
> +        *      { u64                   phys_addr;} && PERF_SAMPLE_PHYS_ADDR
>          * };
>          */
>         PERF_RECORD_SAMPLE                      = 9,
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 426c2ff..07d4a74 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -1570,6 +1570,9 @@ static void __perf_event_header_size(struct perf_event *event, u64 sample_type)
>         if (sample_type & PERF_SAMPLE_TRANSACTION)
>                 size += sizeof(data->txn);
>
> +       if (sample_type & PERF_SAMPLE_PHYS_ADDR)
> +               size += sizeof(data->phys_addr);
> +
>         event->header_size = size;
>  }
>
> @@ -5952,6 +5955,9 @@ void perf_output_sample(struct perf_output_handle *handle,
>         if (sample_type & PERF_SAMPLE_DATA_SRC)
>                 perf_output_put(handle, data->data_src.val);
>
> +       if (sample_type & PERF_SAMPLE_PHYS_ADDR)
> +               perf_output_put(handle, data->phys_addr);
> +

I think this is misplaced. You need to output in the order in which
you describe the record in the header file.
You have: DATA_SRC, SAMPLE_TRANSACTION, REGS_INTR, PHYS_ADDR. That
means you need to
output in that order. Otherwise your PHYS_ADDR may be interpreted as a
SAMPLE_TRANSACTION in case it is active.
At least this is my understanding of the ABI here.


>         if (sample_type & PERF_SAMPLE_TRANSACTION)
>                 perf_output_put(handle, data->txn);
>
> @@ -9852,6 +9858,12 @@ SYSCALL_DEFINE5(perf_event_open,
>                         return -EINVAL;
>         }
>
> +       /* Only privileged users can get kernel addresses */
> +       if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
> +           perf_paranoid_kernel() &&
> +           !capable(CAP_SYS_ADMIN))
> +               return -EACCES;
> +
>         if (!attr.sample_max_stack)
>                 attr.sample_max_stack = sysctl_perf_event_max_stack;
>
> --
> 2.4.3
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH V3] perf: Add PERF_SAMPLE_PHYS_ADDR
  2017-08-17  7:45 ` Stephane Eranian
@ 2017-08-17 13:42   ` Liang, Kan
  0 siblings, 0 replies; 3+ messages in thread
From: Liang, Kan @ 2017-08-17 13:42 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: Peter Zijlstra, mingo, LKML, Arnaldo Carvalho de Melo, Jiri Olsa,
	Thomas Gleixner, ak



> On Wed, Aug 9, 2017 at 1:42 PM,  <kan.liang@intel.com> wrote:
> > From: Kan Liang <kan.liang@intel.com>
> >
> > For understanding how the workload maps to memory channels and
> hardware
> > behavior, it's very important to collect address maps with physical
> > addresses. For example, 3D XPoint access can only be found by filtering
> > the physical address.
> > However, perf doesn't collect physical address information in sampling.
> >
> > The load latency/DLA information in PEBS can be used to calculate the
> > physical address.
> > For kernel direct mapping addresses, virt_to_phys is used to convert the
> > virtual addresses from DLA to physical address.
> > For user virtual addresses, __get_user_pages_fast is used to walk the
> > pages tables for user physical address.
> > This does not work for vmalloc addresses. Right now these are not
> > resolved, but code to do that could be added.
> > For security, the physical address can only be exposed to root or
> > privileged user.
> > A new sample type PERF_SAMPLE_PHYS_ADDR is introduced to expose the
> > physical addresses.
> >
> > Signed-off-by: Kan Liang <kan.liang@intel.com>
> 
> This is a useful feature that I would like to use for some of the
> analysis we would like to do.
> Please apply.
> I am assuming you have the support in perf record/report?
> 

Yes, the previous version includes both kernel and user space code.
https://lkml.org/lkml/2016/1/7/608
V3 only includes the kernel version.
Once it is accepted, I will send out the rest of patches for review.

> > ---
> >
> > The V2 version can be found here.
> > https://patchwork.kernel.org/patch/7979771/
> >
> > Changes since V2
> >  - Only the kernel patch
> >  - Add example in changelog
> >  - Include a perf_paranoid_kernel() test (PeterZ)
> >  - Fix minor complier warning
> >
> >  arch/x86/events/intel/ds.c      | 24 ++++++++++++++++++++++++
> >  arch/x86/events/perf_event.h    |  2 +-
> >  include/linux/perf_event.h      |  3 +++
> >  include/uapi/linux/perf_event.h |  4 +++-
> >  kernel/events/core.c            | 12 ++++++++++++
> >  5 files changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> > index a322fed..712c86b 100644
> > --- a/arch/x86/events/intel/ds.c
> > +++ b/arch/x86/events/intel/ds.c
> > @@ -1083,6 +1083,7 @@ static void setup_pebs_sample_data(struct
> perf_event *event,
> >         u64 sample_type;
> >         int fll, fst, dsrc;
> >         int fl = event->hw.flags;
> > +       struct page *p = NULL;
> >
> >         if (pebs == NULL)
> >                 return;
> > @@ -1179,6 +1180,29 @@ static void setup_pebs_sample_data(struct
> perf_event *event,
> >             x86_pmu.intel_cap.pebs_format >= 1)
> >                 data->addr = pebs->dla;
> >
> > +       if ((sample_type & PERF_SAMPLE_PHYS_ADDR) && (data->addr != 0)) {
> > +               if (data->addr >= TASK_SIZE) {
> > +                       /* If it's vmalloc()d memory, leave phys_addr as 0 */
> > +                       if (virt_addr_valid(data->addr) &&
> > +                           !(data->addr >= VMALLOC_START && data->addr <
> VMALLOC_END))
> > +                               data->phys_addr = (u64)virt_to_phys((void
> *)(uintptr_t)data->addr);
> > +               } else {
> > +                       /*
> > +                        * Walking the pages tables for user address.
> > +                        * Interrupts are disabled, so it prevents any tear down
> > +                        * of the page tables.
> > +                        * Try IRQ-safe __get_user_pages_fast first.
> > +                        * If failed, leave phys_addr as 0.
> > +                        */
> > +                       if ((current->mm != NULL) &&
> > +                           (__get_user_pages_fast(data->addr, 1, 0, &p) == 1))
> > +                               data->phys_addr = page_to_phys(p) + data->addr %
> PAGE_SIZE;
> > +
> > +                       if (p)
> > +                               put_page(p);
> > +               }
> > +       }
> > +
> This would be cleaner in a separate function in my opinion.

Will change it in next version.

> 
> >         if (x86_pmu.intel_cap.pebs_format >= 2) {
> >                 /* Only set the TSX weight when no memory weight. */
> >                 if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)
> > diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> > index 476aec3..65bb91e 100644
> > --- a/arch/x86/events/perf_event.h
> > +++ b/arch/x86/events/perf_event.h
> > @@ -91,7 +91,7 @@ struct amd_nb {
> >         (PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_ADDR | \
> >         PERF_SAMPLE_ID | PERF_SAMPLE_CPU | PERF_SAMPLE_STREAM_ID |
> \
> >         PERF_SAMPLE_DATA_SRC | PERF_SAMPLE_IDENTIFIER | \
> > -       PERF_SAMPLE_TRANSACTION)
> > +       PERF_SAMPLE_TRANSACTION | PERF_SAMPLE_PHYS_ADDR)
> >
> >  /*
> >   * A debug store configuration.
> > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> > index a3b873f..6783c69 100644
> > --- a/include/linux/perf_event.h
> > +++ b/include/linux/perf_event.h
> > @@ -944,6 +944,8 @@ struct perf_sample_data {
> >
> >         struct perf_regs                regs_intr;
> >         u64                             stack_user_size;
> > +
> > +       u64                             phys_addr;
> >  } ____cacheline_aligned;
> >
> >  /* default value for data source */
> > @@ -964,6 +966,7 @@ static inline void perf_sample_data_init(struct
> perf_sample_data *data,
> >         data->weight = 0;
> >         data->data_src.val = PERF_MEM_NA;
> >         data->txn = 0;
> > +       data->phys_addr = 0;
> >  }
> >
> >  extern void perf_output_sample(struct perf_output_handle *handle,
> > diff --git a/include/uapi/linux/perf_event.h
> b/include/uapi/linux/perf_event.h
> > index 642db5f..cbea02f 100644
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -139,8 +139,9 @@ enum perf_event_sample_format {
> >         PERF_SAMPLE_IDENTIFIER                  = 1U << 16,
> >         PERF_SAMPLE_TRANSACTION                 = 1U << 17,
> >         PERF_SAMPLE_REGS_INTR                   = 1U << 18,
> > +       PERF_SAMPLE_PHYS_ADDR                   = 1U << 19,
> >
> > -       PERF_SAMPLE_MAX = 1U << 19,             /* non-ABI */
> > +       PERF_SAMPLE_MAX = 1U << 20,             /* non-ABI */
> >  };
> >
> >  /*
> > @@ -814,6 +815,7 @@ enum perf_event_type {
> >          *      { u64                   transaction; } && PERF_SAMPLE_TRANSACTION
> >          *      { u64                   abi; # enum perf_sample_regs_abi
> >          *        u64                   regs[weight(mask)]; } &&
> PERF_SAMPLE_REGS_INTR
> > +        *      { u64                   phys_addr;} && PERF_SAMPLE_PHYS_ADDR
> >          * };
> >          */
> >         PERF_RECORD_SAMPLE                      = 9,
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 426c2ff..07d4a74 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -1570,6 +1570,9 @@ static void __perf_event_header_size(struct
> perf_event *event, u64 sample_type)
> >         if (sample_type & PERF_SAMPLE_TRANSACTION)
> >                 size += sizeof(data->txn);
> >
> > +       if (sample_type & PERF_SAMPLE_PHYS_ADDR)
> > +               size += sizeof(data->phys_addr);
> > +
> >         event->header_size = size;
> >  }
> >
> > @@ -5952,6 +5955,9 @@ void perf_output_sample(struct
> perf_output_handle *handle,
> >         if (sample_type & PERF_SAMPLE_DATA_SRC)
> >                 perf_output_put(handle, data->data_src.val);
> >
> > +       if (sample_type & PERF_SAMPLE_PHYS_ADDR)
> > +               perf_output_put(handle, data->phys_addr);
> > +
> 
> I think this is misplaced. You need to output in the order in which
> you describe the record in the header file.
> You have: DATA_SRC, SAMPLE_TRANSACTION, REGS_INTR, PHYS_ADDR. That
> means you need to
> output in that order. Otherwise your PHYS_ADDR may be interpreted as a
> SAMPLE_TRANSACTION in case it is active.
> At least this is my understanding of the ABI here.
> 

Will change it in next version.

Thanks,
Kan

> 
> >         if (sample_type & PERF_SAMPLE_TRANSACTION)
> >                 perf_output_put(handle, data->txn);
> >
> > @@ -9852,6 +9858,12 @@ SYSCALL_DEFINE5(perf_event_open,
> >                         return -EINVAL;
> >         }
> >
> > +       /* Only privileged users can get kernel addresses */
> > +       if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
> > +           perf_paranoid_kernel() &&
> > +           !capable(CAP_SYS_ADMIN))
> > +               return -EACCES;
> > +
> >         if (!attr.sample_max_stack)
> >                 attr.sample_max_stack = sysctl_perf_event_max_stack;
> >
> > --
> > 2.4.3
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-08-17 13:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-09 20:42 [PATCH V3] perf: Add PERF_SAMPLE_PHYS_ADDR kan.liang
2017-08-17  7:45 ` Stephane Eranian
2017-08-17 13:42   ` Liang, Kan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®