Re: [PATCH 2/2] ARM: KVM: add support for minimal host vs guest profiling

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

 



On Mon, Sep 10, 2012 at 5:27 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> On 09/09/12 22:18, Christoffer Dall wrote:
>> On Thu, Sep 6, 2012 at 1:16 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
>>> In order to be able to correctly profile what is happening on the
>>> host, we need to be able to identify when we're running on the guest,
>>> and log these events differently.
>>>
>>> Perf offers a simple way to register callbacks into KVM. Mimic what
>>> x86 does and enjoy being able to profile your KVM host.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>> ---
>>>  arch/arm/include/asm/kvm_host.h |  3 ++
>>>  arch/arm/kvm/Makefile           |  2 +-
>>>  arch/arm/kvm/arm.c              |  5 ++-
>>>  arch/arm/kvm/perf.c             | 70 +++++++++++++++++++++++++++++++++++++++++
>>>  4 files changed, 78 insertions(+), 2 deletions(-)
>>>  create mode 100644 arch/arm/kvm/perf.c
>>>
>>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>>> index 4302007..32299c1 100644
>>> --- a/arch/arm/include/asm/kvm_host.h
>>> +++ b/arch/arm/include/asm/kvm_host.h
>>> @@ -207,4 +207,7 @@ static inline int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)
>>>  struct kvm_vcpu *kvm_arm_get_running_vcpu(void);
>>>  struct kvm_vcpu __percpu **kvm_get_running_vcpus(void);
>>>
>>> +int kvm_perf_init(void);
>>> +int kvm_perf_teardown(void);
>>> +
>>>  #endif /* __ARM_KVM_HOST_H__ */
>>> diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
>>> index 4f27970..73c268a 100644
>>> --- a/arch/arm/kvm/Makefile
>>> +++ b/arch/arm/kvm/Makefile
>>> @@ -18,7 +18,7 @@ obj-$(CONFIG_KVM_ARM_HOST) += init.o interrupts.o exports.o
>>>
>>>  kvm-arm-y += $(addprefix ../../../virt/kvm/, kvm_main.o coalesced_mmio.o)
>>>
>>> -kvm-arm-y += arm.o guest.o mmu.o emulate.o reset.o coproc.o
>>> +kvm-arm-y += arm.o guest.o mmu.o emulate.o reset.o coproc.o perf.o
>>>
>>>  obj-$(CONFIG_KVM) += kvm-arm.o
>>>  obj-$(CONFIG_KVM_ARM_VGIC) += vgic.o
>>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>>> index 8d8bf72..743bdc8 100644
>>> --- a/arch/arm/kvm/arm.c
>>> +++ b/arch/arm/kvm/arm.c
>>> @@ -995,7 +995,9 @@ static int init_hyp_mode(void)
>>>         err = kvm_timer_hyp_init();
>>>         if (err)
>>>                 goto out_free_mappings;
>>> -
>>> +
>>> +       kvm_perf_init();
>>> +
>>>         return 0;
>>>  out_free_vfp:
>>>         free_percpu(kvm_host_vfp_state);
>>> @@ -1090,6 +1092,7 @@ void kvm_arch_exit(void)
>>>                 free_page(per_cpu(kvm_arm_hyp_stack_page, cpu));
>>>                 per_cpu(kvm_arm_hyp_stack_page, cpu) = 0;
>>>         }
>>> +       kvm_perf_teardown();
>>>  }
>>>
>>>  static int arm_init(void)
>>> diff --git a/arch/arm/kvm/perf.c b/arch/arm/kvm/perf.c
>>
>> seems a bit overkill with a separate file, I would prefer if this was
>> just in arm.c
>
> Frankly, I think arm.c contains too much stuff already - ioctls, various
> stubs, init code, the whole entry/exit logic. Yet you could argue that
> all this is intimately connected.
>
> But the perf code is completely standalone. No interaction with any of
> the above. Maybe it is just me being anal about that, but I like to have
> separate files for separate features. Keeps the clutter at a minimal
> level in my low-bandwidth brain... ;-)
>

I don't agree that it's completely stand-alone, the current vcpu is
set in arm.c which is what is being read directly. These are three
tiny functions (if you inline the register call in arch_init and
arch_exit), and I really think this belongs in there unless you plan
on writing that horrible guest trace thingy any time soon.

If anything should be factored out it should be the vcpu and exit
handling functionality, but overall arch-specific framework type stuff
is all in arm.c. Anyway, let's not block on this one right now.
_______________________________________________
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