| Message ID | fbf8a9fcca05a1b554ac0d01b0c46fbb6263c435.1721271251.git.zhouquan@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add perf support to collect KVM guest statistics from host side | expand |
On Thu, Jul 18, 2024 at 07:23:51PM GMT, zhouquan@iscas.ac.cn wrote: > From: Quan Zhou <zhouquan@iscas.ac.cn> > > For the information collected on the host side, we need to > identify which data originates from the guest and record > these events separately. This can be achieved by having > KVM register perf callbacks. > > Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn> > --- > arch/riscv/include/asm/kvm_host.h | 6 ++++++ > arch/riscv/kvm/Kconfig | 1 + > arch/riscv/kvm/main.c | 12 ++++++++++-- > arch/riscv/kvm/vcpu.c | 7 +++++++ > 4 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h > index d96281278586..b7bbe1c0c5dd 100644 > --- a/arch/riscv/include/asm/kvm_host.h > +++ b/arch/riscv/include/asm/kvm_host.h > @@ -285,6 +285,12 @@ struct kvm_vcpu_arch { > } sta; > }; > > +/* TODO: A more explicit approach might be needed here than this simple one */ Can you elaborate on this concern? > +static inline bool kvm_arch_pmi_in_guest(struct kvm_vcpu *vcpu) > +{ > + return IS_ENABLED(CONFIG_GUEST_PERF_EVENTS) && !!vcpu; > +} > + > static inline void kvm_arch_sync_events(struct kvm *kvm) {} > static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} > > diff --git a/arch/riscv/kvm/Kconfig b/arch/riscv/kvm/Kconfig > index 26d1727f0550..0c3cbb0915ff 100644 > --- a/arch/riscv/kvm/Kconfig > +++ b/arch/riscv/kvm/Kconfig > @@ -32,6 +32,7 @@ config KVM > select KVM_XFER_TO_GUEST_WORK > select KVM_GENERIC_MMU_NOTIFIER > select SCHED_INFO > + select GUEST_PERF_EVENTS if PERF_EVENTS > help > Support hosting virtualized guest machines. > > diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c > index bab2ec34cd87..734b48d8f6dd 100644 > --- a/arch/riscv/kvm/main.c > +++ b/arch/riscv/kvm/main.c > @@ -51,6 +51,12 @@ void kvm_arch_hardware_disable(void) > csr_write(CSR_HIDELEG, 0); > } > > +static void kvm_riscv_teardown(void) > +{ > + kvm_riscv_aia_exit(); > + kvm_unregister_perf_callbacks(); > +} > + > static int __init riscv_kvm_init(void) > { > int rc; > @@ -105,9 +111,11 @@ static int __init riscv_kvm_init(void) > kvm_info("AIA available with %d guest external interrupts\n", > kvm_riscv_aia_nr_hgei); > > + kvm_register_perf_callbacks(NULL); > + > rc = kvm_init(sizeof(struct kvm_vcpu), 0, THIS_MODULE); > if (rc) { > - kvm_riscv_aia_exit(); > + kvm_riscv_teardown(); > return rc; > } > > @@ -117,7 +125,7 @@ module_init(riscv_kvm_init); > > static void __exit riscv_kvm_exit(void) > { > - kvm_riscv_aia_exit(); > + kvm_riscv_teardown(); > > kvm_exit(); > } > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c > index 17e21df36cc1..c9d291865141 100644 > --- a/arch/riscv/kvm/vcpu.c > +++ b/arch/riscv/kvm/vcpu.c > @@ -222,6 +222,13 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu) > return (vcpu->arch.guest_context.sstatus & SR_SPP) ? true : false; > } > > +#ifdef CONFIG_GUEST_PERF_EVENTS > +unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu) > +{ > + return vcpu->arch.guest_context.sepc; > +} > +#endif > + > vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) > { > return VM_FAULT_SIGBUS; > -- > 2.34.1 > Otherwise, Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
On 2024/7/19 01:09, Andrew Jones wrote: > On Thu, Jul 18, 2024 at 07:23:51PM GMT, zhouquan@iscas.ac.cn wrote: >> From: Quan Zhou <zhouquan@iscas.ac.cn> >> >> For the information collected on the host side, we need to >> identify which data originates from the guest and record >> these events separately. This can be achieved by having >> KVM register perf callbacks. >> >> Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn> >> --- >> arch/riscv/include/asm/kvm_host.h | 6 ++++++ >> arch/riscv/kvm/Kconfig | 1 + >> arch/riscv/kvm/main.c | 12 ++++++++++-- >> arch/riscv/kvm/vcpu.c | 7 +++++++ >> 4 files changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h >> index d96281278586..b7bbe1c0c5dd 100644 >> --- a/arch/riscv/include/asm/kvm_host.h >> +++ b/arch/riscv/include/asm/kvm_host.h >> @@ -285,6 +285,12 @@ struct kvm_vcpu_arch { >> } sta; >> }; >> >> +/* TODO: A more explicit approach might be needed here than this simple one */ > > Can you elaborate on this concern? > Can we determine if a PMU interrupt occurs in the guest solely based on whether the vcpu is loaded? In the kvm_arch_vcpu_ioctl_run while loop, when the guest exits, host-side interrupts should be handled in local_irq_{enable/disable}. Should we add a flag here (referencing x86) to further determine if "this is a host interrupt triggered while the guest was running, rather than when the host is running". Doing so might further reduce the likelihood of perf incorrectly identifying "PMI in host" as "PMI in guest," but it may not completely eliminate it. Have I misunderstood any points here? Thanks, Quan >> +static inline bool kvm_arch_pmi_in_guest(struct kvm_vcpu *vcpu) >> +{ >> + return IS_ENABLED(CONFIG_GUEST_PERF_EVENTS) && !!vcpu; >> +} >> + >> static inline void kvm_arch_sync_events(struct kvm *kvm) {} >> static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} >> >> diff --git a/arch/riscv/kvm/Kconfig b/arch/riscv/kvm/Kconfig >> index 26d1727f0550..0c3cbb0915ff 100644 >> --- a/arch/riscv/kvm/Kconfig >> +++ b/arch/riscv/kvm/Kconfig >> @@ -32,6 +32,7 @@ config KVM >> select KVM_XFER_TO_GUEST_WORK >> select KVM_GENERIC_MMU_NOTIFIER >> select SCHED_INFO >> + select GUEST_PERF_EVENTS if PERF_EVENTS >> help >> Support hosting virtualized guest machines. >> >> diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c >> index bab2ec34cd87..734b48d8f6dd 100644 >> --- a/arch/riscv/kvm/main.c >> +++ b/arch/riscv/kvm/main.c >> @@ -51,6 +51,12 @@ void kvm_arch_hardware_disable(void) >> csr_write(CSR_HIDELEG, 0); >> } >> >> +static void kvm_riscv_teardown(void) >> +{ >> + kvm_riscv_aia_exit(); >> + kvm_unregister_perf_callbacks(); >> +} >> + >> static int __init riscv_kvm_init(void) >> { >> int rc; >> @@ -105,9 +111,11 @@ static int __init riscv_kvm_init(void) >> kvm_info("AIA available with %d guest external interrupts\n", >> kvm_riscv_aia_nr_hgei); >> >> + kvm_register_perf_callbacks(NULL); >> + >> rc = kvm_init(sizeof(struct kvm_vcpu), 0, THIS_MODULE); >> if (rc) { >> - kvm_riscv_aia_exit(); >> + kvm_riscv_teardown(); >> return rc; >> } >> >> @@ -117,7 +125,7 @@ module_init(riscv_kvm_init); >> >> static void __exit riscv_kvm_exit(void) >> { >> - kvm_riscv_aia_exit(); >> + kvm_riscv_teardown(); >> >> kvm_exit(); >> } >> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c >> index 17e21df36cc1..c9d291865141 100644 >> --- a/arch/riscv/kvm/vcpu.c >> +++ b/arch/riscv/kvm/vcpu.c >> @@ -222,6 +222,13 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu) >> return (vcpu->arch.guest_context.sstatus & SR_SPP) ? true : false; >> } >> >> +#ifdef CONFIG_GUEST_PERF_EVENTS >> +unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu) >> +{ >> + return vcpu->arch.guest_context.sepc; >> +} >> +#endif >> + >> vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) >> { >> return VM_FAULT_SIGBUS; >> -- >> 2.34.1 >> > > Otherwise, > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h index d96281278586..b7bbe1c0c5dd 100644 --- a/arch/riscv/include/asm/kvm_host.h +++ b/arch/riscv/include/asm/kvm_host.h @@ -285,6 +285,12 @@ struct kvm_vcpu_arch { } sta; }; +/* TODO: A more explicit approach might be needed here than this simple one */ +static inline bool kvm_arch_pmi_in_guest(struct kvm_vcpu *vcpu) +{ + return IS_ENABLED(CONFIG_GUEST_PERF_EVENTS) && !!vcpu; +} + static inline void kvm_arch_sync_events(struct kvm *kvm) {} static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} diff --git a/arch/riscv/kvm/Kconfig b/arch/riscv/kvm/Kconfig index 26d1727f0550..0c3cbb0915ff 100644 --- a/arch/riscv/kvm/Kconfig +++ b/arch/riscv/kvm/Kconfig @@ -32,6 +32,7 @@ config KVM select KVM_XFER_TO_GUEST_WORK select KVM_GENERIC_MMU_NOTIFIER select SCHED_INFO + select GUEST_PERF_EVENTS if PERF_EVENTS help Support hosting virtualized guest machines. diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c index bab2ec34cd87..734b48d8f6dd 100644 --- a/arch/riscv/kvm/main.c +++ b/arch/riscv/kvm/main.c @@ -51,6 +51,12 @@ void kvm_arch_hardware_disable(void) csr_write(CSR_HIDELEG, 0); } +static void kvm_riscv_teardown(void) +{ + kvm_riscv_aia_exit(); + kvm_unregister_perf_callbacks(); +} + static int __init riscv_kvm_init(void) { int rc; @@ -105,9 +111,11 @@ static int __init riscv_kvm_init(void) kvm_info("AIA available with %d guest external interrupts\n", kvm_riscv_aia_nr_hgei); + kvm_register_perf_callbacks(NULL); + rc = kvm_init(sizeof(struct kvm_vcpu), 0, THIS_MODULE); if (rc) { - kvm_riscv_aia_exit(); + kvm_riscv_teardown(); return rc; } @@ -117,7 +125,7 @@ module_init(riscv_kvm_init); static void __exit riscv_kvm_exit(void) { - kvm_riscv_aia_exit(); + kvm_riscv_teardown(); kvm_exit(); } diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c index 17e21df36cc1..c9d291865141 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -222,6 +222,13 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu) return (vcpu->arch.guest_context.sstatus & SR_SPP) ? true : false; } +#ifdef CONFIG_GUEST_PERF_EVENTS +unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu) +{ + return vcpu->arch.guest_context.sepc; +} +#endif + vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf) { return VM_FAULT_SIGBUS;