| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 22 Sep 2021 08:25:44 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Marc Zyngier <maz@...nel.org>, Guo Ren <guoren@...nel.org>,
Nick Hu <nickhu@...estech.com>,
Greentime Hu <green.hu@...il.com>,
Vincent Chen <deanbo422@...il.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Juergen Gross <jgross@...e.com>
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
James Morse <james.morse@....com>,
Alexandru Elisei <alexandru.elisei@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>,
Stefano Stabellini <sstabellini@...nel.org>,
linux-arm-kernel@...ts.infradead.org,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
kvmarm@...ts.cs.columbia.edu, linux-csky@...r.kernel.org,
linux-riscv@...ts.infradead.org, kvm@...r.kernel.org,
xen-devel@...ts.xenproject.org,
Artem Kashkanov <artem.kashkanov@...el.com>,
Like Xu <like.xu.linux@...il.com>,
Zhu Lingshan <lingshan.zhu@...el.com>
Subject: Re: [PATCH v3 04/16] perf: Stop pretending that perf can handle
multiple guest callbacks
On 22/09/21 02:05, Sean Christopherson wrote:
> Drop the 'int' return value from the perf (un)register callbacks helpers
> and stop pretending perf can support multiple callbacks. The 'int'
> returns are not future proofing anything as none of the callers take
> action on an error. It's also not obvious that there will ever be
> co-tenant hypervisors, and if there are, that allowing multiple callbacks
> to be registered is desirable or even correct.
>
> Opportunistically rename callbacks=>cbs in the affected declarations to
> match their definitions.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> arch/arm64/include/asm/kvm_host.h | 4 ++--
> arch/arm64/kvm/perf.c | 8 ++++----
> include/linux/perf_event.h | 12 ++++++------
> kernel/events/core.c | 16 ++++------------
> 4 files changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 41911585ae0c..ed940aec89e0 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -670,8 +670,8 @@ unsigned long kvm_mmio_read_buf(const void *buf, unsigned int len);
> int kvm_handle_mmio_return(struct kvm_vcpu *vcpu);
> int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa);
>
> -int kvm_perf_init(void);
> -int kvm_perf_teardown(void);
> +void kvm_perf_init(void);
> +void kvm_perf_teardown(void);
>
> long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
> gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu);
> diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
> index 151c31fb9860..c37c0cf1bfe9 100644
> --- a/arch/arm64/kvm/perf.c
> +++ b/arch/arm64/kvm/perf.c
> @@ -48,15 +48,15 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
> .get_guest_ip = kvm_get_guest_ip,
> };
>
> -int kvm_perf_init(void)
> +void kvm_perf_init(void)
> {
> if (kvm_pmu_probe_pmuver() != 0xf && !is_protected_kvm_enabled())
> static_branch_enable(&kvm_arm_pmu_available);
>
> - return perf_register_guest_info_callbacks(&kvm_guest_cbs);
> + perf_register_guest_info_callbacks(&kvm_guest_cbs);
> }
>
> -int kvm_perf_teardown(void)
> +void kvm_perf_teardown(void)
> {
> - return perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
> + perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
> }
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 6b0405e578c1..317d4658afe9 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1245,8 +1245,8 @@ static inline struct perf_guest_info_callbacks *perf_get_guest_cbs(void)
> /* Prevent reloading between a !NULL check and dereferences. */
> return READ_ONCE(perf_guest_cbs);
> }
> -extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
> -extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
> +extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs);
> +extern void perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs);
>
> extern void perf_event_exec(void);
> extern void perf_event_comm(struct task_struct *tsk, bool exec);
> @@ -1489,10 +1489,10 @@ perf_sw_event(u32 event_id, u64 nr, struct pt_regs *regs, u64 addr) { }
> static inline void
> perf_bp_event(struct perf_event *event, void *data) { }
>
> -static inline int perf_register_guest_info_callbacks
> -(struct perf_guest_info_callbacks *callbacks) { return 0; }
> -static inline int perf_unregister_guest_info_callbacks
> -(struct perf_guest_info_callbacks *callbacks) { return 0; }
> +static inline void perf_register_guest_info_callbacks
> +(struct perf_guest_info_callbacks *cbs) { }
> +static inline void perf_unregister_guest_info_callbacks
> +(struct perf_guest_info_callbacks *cbs) { }
>
> static inline void perf_event_mmap(struct vm_area_struct *vma) { }
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 80ff050a7b55..d90a43572400 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6482,31 +6482,23 @@ static void perf_pending_event(struct irq_work *entry)
> perf_swevent_put_recursion_context(rctx);
> }
>
> -/*
> - * We assume there is only KVM supporting the callbacks.
> - * Later on, we might change it to a list if there is
> - * another virtualization implementation supporting the callbacks.
> - */
> struct perf_guest_info_callbacks *perf_guest_cbs;
> -
> -int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
> +void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
> {
> if (WARN_ON_ONCE(perf_guest_cbs))
> - return -EBUSY;
> + return;
>
> WRITE_ONCE(perf_guest_cbs, cbs);
Maybe you want a smp_store_release or rcu_assign_pointer here?
> - return 0;
> }
> EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
>
> -int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
> +void perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
> {
> if (WARN_ON_ONCE(perf_guest_cbs != cbs))
> - return -EINVAL;
> + return;
>
> WRITE_ONCE(perf_guest_cbs, NULL);
> synchronize_rcu();
> - return 0;
> }
> EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
>
>
Apart from the above,
Reviewed-by: Paolo Bonzini <pbonzini@...hat.com>
Powered by blists - more mailing lists