On Mon, Sep 10, 2012 at 5:03 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> On 09/09/12 21:33, Christoffer Dall wrote:
>> On Fri, Sep 7, 2012 at 10:49 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
>>> On 07/09/12 15:28, Will Deacon wrote:
>>>> On Thu, Sep 06, 2012 at 06:16:03PM +0100, Marc Zyngier wrote:
>>>>> Add minimal guest support to perf, so it can distinguish whether
>>>>> the PMU interrupt was in the host or the guest, as well as collecting
>>>>> some very basic information (guest PC, user vs kernel mode).
>>>>>
>>>>> This is not feature complete though, as it doesn't support backtracing
>>>>> in the guest.
>>>>>
>>>>> Based on the x86 implementation, tested with KVM/ARM.
>>>>>
>>>>> Cc: Will Deacon <will.deacon@xxxxxxx>
>>>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>>>> ---
>>>>> arch/arm/include/asm/perf_event.h | 7 ++++++-
>>>>> arch/arm/kernel/perf_event.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 42 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/include/asm/perf_event.h b/arch/arm/include/asm/perf_event.h
>>>>> index e074948..77537bb 100644
>>>>> --- a/arch/arm/include/asm/perf_event.h
>>>>> +++ b/arch/arm/include/asm/perf_event.h
>>>>> @@ -12,6 +12,11 @@
>>>>> #ifndef __ARM_PERF_EVENT_H__
>>>>> #define __ARM_PERF_EVENT_H__
>>>>>
>>>>> -/* Nothing to see here... */
>>>>> +/* Not much to see here... */
>>>>
>>>> You can just remove this silly comment :)
>>>
>>> Now you're taking away the little fun I had hacking this. Life is soooo
>>> cruel... ;-)
>>>
>>>>> +struct pt_regs;
>>>>> +extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
>>>>> +extern unsigned long perf_misc_flags(struct pt_regs *regs);
>>>>> +#define perf_misc_flags(regs) perf_misc_flags(regs)
>>>>>
>>>>> #endif /* __ARM_PERF_EVENT_H__ */
>>>>> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
>>>>> index ab243b8..8836425 100644
>>>>> --- a/arch/arm/kernel/perf_event.c
>>>>> +++ b/arch/arm/kernel/perf_event.c
>>>>> @@ -817,6 +817,10 @@ perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
>>>>> {
>>>>> struct frame_tail __user *tail;
>>>>>
>>>>> + if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
>>>>> + /* TODO: We don't support guest os callchain now */
>>>>> + return;
>>>>> + }
>>>>>
>>>>> tail = (struct frame_tail __user *)regs->ARM_fp - 1;
>>>>>
>>>>> @@ -844,9 +848,41 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
>>>>> {
>>>>> struct stackframe fr;
>>>>>
>>>>> + if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
>>>>> + /* TODO: We don't support guest os callchain now */
>>>>> + return;
>>>>> + }
>>>>> +
>>>>
>>>> Will we ever support this? It sounds incredibly difficult -- how can you
>>>> walk the guest stack without things disappearing under your feet?
>>>
>>> It's hard, but not impossible:
>>> - Stop the world (all vcpus)
>>> - Repeatedly call into HYP mode to resolve all the VAs needed by the
>>> backtrace (cringe...)
>>> - Restart the world
>>> - Witness your guest running at the speed of a snail lost in the Sahara.
>>>
>>> I reckon it is a perfect project for a summer student! :-)
>>>
>>>>> +unsigned long perf_instruction_pointer(struct pt_regs *regs)
>>>>> +{
>>>>> + if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
>>>>> + return perf_guest_cbs->get_guest_ip();
>>>>> +
>>>>> + return instruction_pointer(regs);
>>>>> +}
>>>>> +
>>>>> +unsigned long perf_misc_flags(struct pt_regs *regs)
>>>>> +{
>>>>> + int misc = 0;
>>>>> +
>>>>> + if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
>>>>> + if (perf_guest_cbs->is_user_mode())
>>>>> + misc |= PERF_RECORD_MISC_GUEST_USER;
>>>>> + else
>>>>> + misc |= PERF_RECORD_MISC_GUEST_KERNEL;
>>>>> + } else {
>>>>> + if (user_mode(regs))
>>>>> + misc |= PERF_RECORD_MISC_USER;
>>>>> + else
>>>>> + misc |= PERF_RECORD_MISC_KERNEL;
>>>>> + }
>>>>> +
>>>>> + return misc;
>>>>> +}
>>>>
>>>> Hmm, both of these look like they should be made generic but I see that x86
>>>> and ppc do have some differences, so I guess we should have our own copies
>>>> for the time being...
>>>
>>> We can probably factor in the common code and let ARM use the default.
>>> I'll have a look if I get a chance.
>>>
>> or, maybe we can just ask the guest nicely using some kind of PV api
>> and see if it does it for us...
>
> Are you thinking of some reverse PV to get the backtrace? I can see
> quite a few problems with this approach:
>
> - The guest might not be Linux, and the idea of implementing this code
> more than once seem a waste of energy
I was thinking there is existing perf probes that could be reused, and
for now we don't care about anything else than Linux as guests.
> - You don't want the guest to be able to lie - think of sample based
> tracing for security purposes (I know it's a long shot...)
it would depend on your use case, if it's used only to profile host
kvm when you're in control of the guest kernel you run, it might be a
use case.
> - A generalization of the above is that you can't ever trust the guest.
>
this I am very much aware of, thanks.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
