Re: [PATCH 1/2] ARM: perf: add guest handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
- 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...)
- A generalization of the above is that you can't ever trust the guest.

	M.
-- 
Jazz is not dead. It just smells funny...


_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm


[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux