linux-kernel - Re: [PATCH 04/13] perf/core: Extend perf_output_sample_regs() to include perf_arch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <2e315e8f-bb95-b083-cc6c-c7aafd3d0efb@linux.vnet.ibm.com>
Date:   Thu, 1 Sep 2016 09:12:22 +0530
From:   Madhavan Srinivasan <maddy@...ux.vnet.ibm.com>
To:     Nilay Vaish <nilayvaish@...il.com>
Cc:     Linux Kernel list <linux-kernel@...r.kernel.org>,
        linuxppc-dev@...ts.ozlabs.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Stephane Eranian <eranian@...il.com>,
        Russell King <linux@....linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
Subject: Re: [PATCH 04/13] perf/core: Extend perf_output_sample_regs() to
 include perf_arch_regs



On Tuesday 30 August 2016 09:41 PM, Nilay Vaish wrote:
> On 28 August 2016 at 16:00, Madhavan Srinivasan
> <maddy@...ux.vnet.ibm.com> wrote:
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index 274288819829..e16bf4d057d1 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -5371,16 +5371,24 @@ u64 __attribute__((weak)) perf_arch_reg_value(struct perf_arch_regs *regs,
>>
>>   static void
>>   perf_output_sample_regs(struct perf_output_handle *handle,
>> -                       struct pt_regs *regs, u64 mask)
>> +                               struct perf_regs *regs, u64 mask)
>>   {
>>          int bit;
>>          DECLARE_BITMAP(_mask, 64);
>> +       u64 arch_regs_mask = regs->arch_regs_mask;
>>
>>          bitmap_from_u64(_mask, mask);
>>          for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) {
>>                  u64 val;
>>
>> -               val = perf_reg_value(regs, bit);
>> +               val = perf_reg_value(regs->regs, bit);
>> +               perf_output_put(handle, val);
>> +       }
>> +
>> +       bitmap_from_u64(_mask, arch_regs_mask);
>> +       for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) {
>> +               u64 val;
>> +               val = perf_arch_reg_value(regs->arch_regs, bit);
>>                  perf_output_put(handle, val);
>>          }
>>   }
>> @@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle,
>>                  if (abi) {
>>                          u64 mask = event->attr.sample_regs_user;
>>                          perf_output_sample_regs(handle,
>> -                                               data->regs_user.regs,
>> +                                               &data->regs_user,
>>                                                  mask);
>>                  }
>>          }
>> @@ -5827,7 +5835,7 @@ void perf_output_sample(struct perf_output_handle *handle,
>>                          u64 mask = event->attr.sample_regs_intr;
>>
>>                          perf_output_sample_regs(handle,
>> -                                               data->regs_intr.regs,
>> +                                               &data->regs_intr,
>>                                                  mask);
>>                  }
>>          }
>> --
>> 2.7.4
>>
> I would like to suggest a slightly different version.  Would it make
> more sense to have something like following:

I agree we are outputting two different structures, but since we use the
INTR_REG infrastructure to dump the arch pmu registers, I preferred to
extend perf_output_sample_regs. But I guess I can break it up.

Maddy

>
> @@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle,
>                   if (abi) {
>                          u64 mask = event->attr.sample_regs_user;
>                          perf_output_sample_regs(handle,
>                                                  data->regs_user.regs,
>                                                  mask);
>                  }
> +
> +              if (arch_regs_mask) {
> +                   perf_output_pmu_regs(handle,
> data->regs_users.arch_regs, arch_regs_mask);
> +              }
>          }
>
>
> Somehow I don't like outputting the two sets of registers through the
> same function call.
>
> --
> Nilay
>