Re: [RFC 1/4] perf kvm: Enable 'record' on powerpc

[Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>]

Thanks Arnaldo for putting the effort.

I've tested this patch on powerpc and it looks fine to me. Please find 
my below comments.

On Friday 25 March 2016 02:45 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 22, 2016 at 11:19:21PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Mar 22, 2016 at 04:12:11PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Wed, Feb 24, 2016 at 02:37:42PM +0530, Ravi Bangoria escreveu:
>>>> 'perf kvm record' is not available on powerpc because 'perf' relies on
>>>> the 'cycles' event (a PMU event) to profile the guest. However, for
>>>> powerpc, this can't be used from the host because the PMUs are controlled
>>>> by the guest rather than the host.
>>>>
>>>> There exists a tracepoint 'kvm_hv:kvm_guest_exit' in powerpc which is
>>>> hit whenever any of the threads exit the guest context. The guest
>>>> instruction pointer dumped along with this tracepoint data in the field
>>>> 'pc', can be used as guest instruction pointer.
>>>>
>>>> This patch changes default event as kvm_hv:kvm_guest_exit for recording
>>>> guest data in host on powerpc. As we are using host event to record guest
>>>> data, this approach will enable only --guest option of 'perf kvm'. Still
>>>> --host --guest together won't work.
>>> It should, i.e. --host --guest should translate to:
>>>
>>>     -e cycles:H,kvm_hv:kvm_guest_exit
>>>
>>> I.e. both collect cycles only in the host, and also the tracepoint that
>>> will allow us to get the guest approximation for the unavailable cycles
>>> event, no?
>>>
>>> I'm putting the infrastructure work needed for this the perf/cpumode
>>> branch. More work will be put there soon.
>> So I took a different path and made perf_evsel__parse_sample set a new
>> perf_sample.cpumode field, this way we'll end up having just to set a
>> per-evsel ->post_parse_sample() callback for the event that replaces
>> "cycles" for PPC guests where we'll just set data->ip and data->cpumode,
>> the rest of the code remains unchanged.
>>
>> The changes I made looks useful in itself, as, IIRC more code was
>> removed than added.
>>
>> I'll continue tomorrow and will test with the kvm:kvm_exit on x86_64 for
>> testing, that has:
> Ok, so the infrastructure got merged already and from there the next
> steps are in running with:
>
>   perf kvm --guest record -a -e cycles:H,kvm:kvm_exit
>
> And then, with the patch below applied, try:
>
> perf kvm --guestkallsyms kallsyms.guest --guestmodules modules.guest report -i perf.data.guest --munge-ppc-guest-sample kvm:kvm_exit


The initial proposal was to change the default event as "kvm_guest_exit" 
for kvm recording/reporting
on ppc. If I understand it correctly, your patch creates a handler for 
reporting kvm events
based on "munge_ppc_guest_event" and the required tracepoint i.e., we 
need to mention the
required tracepoint event name for recording and reporting.

There might be a little bit of an issue here. For scripts which depend 
on generic perf kvm record/report,
we need to change those appropriately to prevent those from failing on 
powerpc. Otherwise, (just a
thought) can we create some kind of an alias to map the ppc specific 
perf kvm commands with the
generic perf kvm.
For e.g :
perf kvm record -e "kvm_hv:kvm_guest_exit"  mapped to  perf kvm record
&
perf kvm report --munge-ppc-guest-sample kvm_hv:kvm_guest_exit mapped 
to  perf kvm report.


Regards,
Ravi