| 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
| ||
|
Message-ID: <CAOQ_QsgfnUMJD9XVCBbbA-dZryA2a2yBksmM6mGLZE-8ux_Wsg@mail.gmail.com>
Date: Thu, 17 Mar 2022 10:36:27 -0700
From: Oliver Upton <oupton@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: seanjc@...gle.com, maz@...nel.org, kvm@...r.kernel.org,
pbonzini@...hat.com, kevin.tian@...el.com, tglx@...utronix.de,
James Morse <james.morse@....com>,
Alexandru Elisei <Alexandru.Elisei@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Eric Auger <eric.auger@...hat.com>, Jia He <justin.he@....com>,
John Garry <john.garry@...wei.com>,
Shaokun Zhang <zhangshaokun@...ilicon.com>,
Qi Liu <liuqi115@...wei.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Richter <tmricht@...ux.ibm.com>,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/6] KVM: arm64: Simplify the CPUHP logic
On Tue, Feb 15, 2022 at 7:16 PM Chao Gao <chao.gao@...el.com> wrote:
>
> From: Marc Zyngier <maz@...nel.org>
>
> For a number of historical reasons, the KVM/arm64 hotplug setup is pretty
> complicated, and we have two extra CPUHP notifiers for vGIC and timers.
>
> It looks pretty pointless, and gets in the way of further changes.
> So let's just expose some helpers that can be called from the core
> CPUHP callback, and get rid of everything else.
>
> This gives us the opportunity to drop a useless notifier entry,
> as well as tidy-up the timer enable/disable, which was a bit odd.
>
> Signed-off-by: Marc Zyngier <maz@...nel.org>
> Signed-off-by: Chao Gao <chao.gao@...el.com>
Reviewed-by: Oliver Upton <oupton@...gle.com>
> ---
> arch/arm64/kvm/arch_timer.c | 27 ++++++++++-----------------
> arch/arm64/kvm/arm.c | 4 ++++
> arch/arm64/kvm/vgic/vgic-init.c | 19 ++-----------------
> include/kvm/arm_arch_timer.h | 4 ++++
> include/kvm/arm_vgic.h | 4 ++++
> include/linux/cpuhotplug.h | 3 ---
> 6 files changed, 24 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 6e542e2eae32..f9d14c6dc0b4 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -796,10 +796,18 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
> ptimer->host_timer_irq_flags = host_ptimer_irq_flags;
> }
>
> -static void kvm_timer_init_interrupt(void *info)
> +void kvm_timer_cpu_up(void)
> {
> enable_percpu_irq(host_vtimer_irq, host_vtimer_irq_flags);
> - enable_percpu_irq(host_ptimer_irq, host_ptimer_irq_flags);
> + if (host_ptimer_irq)
> + enable_percpu_irq(host_ptimer_irq, host_ptimer_irq_flags);
> +}
> +
> +void kvm_timer_cpu_down(void)
> +{
> + disable_percpu_irq(host_vtimer_irq);
> + if (host_ptimer_irq)
> + disable_percpu_irq(host_ptimer_irq);
> }
>
> int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value)
> @@ -961,18 +969,6 @@ void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
> preempt_enable();
> }
>
> -static int kvm_timer_starting_cpu(unsigned int cpu)
> -{
> - kvm_timer_init_interrupt(NULL);
> - return 0;
> -}
> -
> -static int kvm_timer_dying_cpu(unsigned int cpu)
> -{
> - disable_percpu_irq(host_vtimer_irq);
> - return 0;
> -}
> -
> static int timer_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
> {
> if (vcpu)
> @@ -1170,9 +1166,6 @@ int kvm_timer_hyp_init(bool has_gic)
> goto out_free_irq;
> }
>
> - cpuhp_setup_state(CPUHP_AP_KVM_ARM_TIMER_STARTING,
> - "kvm/arm/timer:starting", kvm_timer_starting_cpu,
> - kvm_timer_dying_cpu);
> return 0;
> out_free_irq:
> free_percpu_irq(host_vtimer_irq, kvm_get_running_vcpus());
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 0165cf3aac3a..31b049e48b19 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1658,6 +1658,8 @@ static void _kvm_arch_hardware_enable(void *discard)
> {
> if (!__this_cpu_read(kvm_arm_hardware_enabled)) {
> cpu_hyp_reinit();
> + kvm_vgic_cpu_up();
> + kvm_timer_cpu_up();
> __this_cpu_write(kvm_arm_hardware_enabled, 1);
> }
> }
> @@ -1671,6 +1673,8 @@ int kvm_arch_hardware_enable(void)
> static void _kvm_arch_hardware_disable(void *discard)
> {
> if (__this_cpu_read(kvm_arm_hardware_enabled)) {
> + kvm_timer_cpu_down();
> + kvm_vgic_cpu_down();
> cpu_hyp_reset();
> __this_cpu_write(kvm_arm_hardware_enabled, 0);
> }
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index fc00304fe7d8..60038a8516de 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -460,17 +460,15 @@ int kvm_vgic_map_resources(struct kvm *kvm)
>
> /* GENERIC PROBE */
>
> -static int vgic_init_cpu_starting(unsigned int cpu)
> +void kvm_vgic_cpu_up(void)
> {
> enable_percpu_irq(kvm_vgic_global_state.maint_irq, 0);
> - return 0;
> }
>
>
> -static int vgic_init_cpu_dying(unsigned int cpu)
> +void kvm_vgic_cpu_down(void)
> {
> disable_percpu_irq(kvm_vgic_global_state.maint_irq);
> - return 0;
> }
>
> static irqreturn_t vgic_maintenance_handler(int irq, void *data)
> @@ -579,19 +577,6 @@ int kvm_vgic_hyp_init(void)
> return ret;
> }
>
> - ret = cpuhp_setup_state(CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
> - "kvm/arm/vgic:starting",
> - vgic_init_cpu_starting, vgic_init_cpu_dying);
> - if (ret) {
> - kvm_err("Cannot register vgic CPU notifier\n");
> - goto out_free_irq;
> - }
> -
> kvm_info("vgic interrupt IRQ%d\n", kvm_vgic_global_state.maint_irq);
> return 0;
> -
> -out_free_irq:
> - free_percpu_irq(kvm_vgic_global_state.maint_irq,
> - kvm_get_running_vcpus());
> - return ret;
> }
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index 51c19381108c..16a2f65fcfb4 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -106,4 +106,8 @@ void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
> u32 timer_get_ctl(struct arch_timer_context *ctxt);
> u64 timer_get_cval(struct arch_timer_context *ctxt);
>
> +/* CPU HP callbacks */
> +void kvm_timer_cpu_up(void);
> +void kvm_timer_cpu_down(void);
> +
> #endif
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index bb30a6803d9f..a2a0cca05a73 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -427,4 +427,8 @@ int vgic_v4_load(struct kvm_vcpu *vcpu);
> void vgic_v4_commit(struct kvm_vcpu *vcpu);
> int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db);
>
> +/* CPU HP callbacks */
> +void kvm_vgic_cpu_up(void);
> +void kvm_vgic_cpu_down(void);
> +
> #endif /* __KVM_ARM_VGIC_H */
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 411a428ace4d..4345b8eafc03 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -183,9 +183,6 @@ enum cpuhp_state {
> CPUHP_AP_TI_GP_TIMER_STARTING,
> CPUHP_AP_HYPERV_TIMER_STARTING,
> CPUHP_AP_KVM_STARTING,
> - CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
> - CPUHP_AP_KVM_ARM_VGIC_STARTING,
> - CPUHP_AP_KVM_ARM_TIMER_STARTING,
> /* Must be the last timer callback */
> CPUHP_AP_DUMMY_TIMER_STARTING,
> CPUHP_AP_ARM_XEN_STARTING,
> --
> 2.25.1
>
Powered by blists - more mailing lists