linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 4/6] KVM: arm64: Simplify the CPUHP logic
       [not found] <20220216031528.92558-1-chao.gao@intel.com>
@ 2022-02-16  3:15 ` Chao Gao
  2022-03-17 17:36   ` Oliver Upton
  0 siblings, 1 reply; 2+ messages in thread
From: Chao Gao @ 2022-02-16  3:15 UTC (permalink / raw)
  To: seanjc, maz, kvm, pbonzini, kevin.tian, tglx
  Cc: Chao Gao, James Morse, Alexandru Elisei, Suzuki K Poulose,
	Catalin Marinas, Will Deacon, Eric Auger, Oliver Upton, Jia He,
	John Garry, Shaokun Zhang, Qi Liu, Daniel Lezcano,
	Thomas Richter, linux-arm-kernel, kvmarm, linux-kernel

From: Marc Zyngier <maz@kernel.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@kernel.org>
Signed-off-by: Chao Gao <chao.gao@intel.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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v4 4/6] KVM: arm64: Simplify the CPUHP logic
  2022-02-16  3:15 ` [PATCH v4 4/6] KVM: arm64: Simplify the CPUHP logic Chao Gao
@ 2022-03-17 17:36   ` Oliver Upton
  0 siblings, 0 replies; 2+ messages in thread
From: Oliver Upton @ 2022-03-17 17:36 UTC (permalink / raw)
  To: Chao Gao
  Cc: seanjc, maz, kvm, pbonzini, kevin.tian, tglx, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Catalin Marinas, Will Deacon,
	Eric Auger, Jia He, John Garry, Shaokun Zhang, Qi Liu,
	Daniel Lezcano, Thomas Richter, linux-arm-kernel, kvmarm,
	linux-kernel

On Tue, Feb 15, 2022 at 7:16 PM Chao Gao <chao.gao@intel.com> wrote:
>
> From: Marc Zyngier <maz@kernel.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@kernel.org>
> Signed-off-by: Chao Gao <chao.gao@intel.com>

Reviewed-by: Oliver Upton <oupton@google.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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-03-17 17:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220216031528.92558-1-chao.gao@intel.com>
2022-02-16  3:15 ` [PATCH v4 4/6] KVM: arm64: Simplify the CPUHP logic Chao Gao
2022-03-17 17:36   ` Oliver Upton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).