From: Yuan Yao <yuan.yao@linux.intel.com>
To: isaku.yamahata@intel.com
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
Paolo Bonzini <pbonzini@redhat.com>,
Sean Christopherson <seanjc@google.com>,
Thomas Gleixner <tglx@linutronix.de>,
Marc Zyngier <maz@kernel.org>, Will Deacon <will@kernel.org>,
isaku.yamahata@gmail.com, Kai Huang <kai.huang@intel.com>,
Chao Gao <chao.gao@intel.com>,
Atish Patra <atishp@atishpatra.org>,
Shaokun Zhang <zhangshaokun@hisilicon.com>,
Qi Liu <liuqi115@huawei.com>, John Garry <john.garry@huawei.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Huang Ying <ying.huang@intel.com>,
Huacai Chen <chenhuacai@kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Borislav Petkov <bp@alien8.de>, Oliver Upton <oupton@google.com>
Subject: Re: [PATCH v3 06/22] KVM: arm64: Simplify the CPUHP logic
Date: Mon, 5 Sep 2022 15:05:09 +0800 [thread overview]
Message-ID: <20220905070509.f5neutyqgvbklefi@yy-desk-7060> (raw)
In-Reply-To: <72481a7bc0ff08093f4f0f04cece877ee82de0cf.1662084396.git.isaku.yamahata@intel.com>
On Thu, Sep 01, 2022 at 07:17:41PM -0700, isaku.yamahata@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>
> Link: https://lore.kernel.org/r/20220216031528.92558-5-chao.gao@intel.com
> Signed-off-by: Isaku Yamahata <isaku.yamahata@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 bb24a76b4224..33fca1a691a5 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -811,10 +811,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);
> }
Should "host_vtimer_irq" be checked yet as host_ptimer_irq ? Because
the host_{v,p}timer_irq is set in same function kvm_irq_init() which
called AFTER the on_each_cpu(_kvm_arch_hardware_enable, NULL, 1) from
init_subsystems():
kvm_init()
kvm_arch_init()
init_subsystems()
on_each_cpu(_kvm_arch_hardware_enable, NULL, 1);
kvm_timer_hyp_init()
kvm_irq_init()
host_vtimer_irq = info->virtual_irq;
host_ptimer_irq = info->physical_irq;
hardware_enable_all()
>
> int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value)
> @@ -976,18 +984,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)
> @@ -1185,9 +1181,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 3385fb57c11a..0a2f616c4d63 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1670,6 +1670,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);
> }
> }
> @@ -1683,6 +1685,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 f6d4f4052555..6c7f6ae21ec0 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -465,17 +465,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)
> @@ -584,19 +582,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 cd6d8f260eab..1638418f72dd 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -104,4 +104,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 4df9e73a8bb5..fc4acc91ba06 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -431,4 +431,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 f61447913db9..7337414e4947 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -186,9 +186,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
>
next prev parent reply other threads:[~2022-09-05 7:05 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-02 2:17 [PATCH v3 00/22] KVM: hardware enable/disable reorganize isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 01/22] KVM: x86: Drop kvm_user_return_msr_cpu_online() isaku.yamahata
2022-09-05 1:59 ` Chao Gao
2022-09-05 5:30 ` Yuan Yao
2022-09-02 2:17 ` [PATCH v3 02/22] KVM: x86: Use this_cpu_ptr() instead of per_cpu_ptr(smp_processor_id()) isaku.yamahata
2022-09-05 5:35 ` Yuan Yao
2022-09-02 2:17 ` [PATCH v3 03/22] KVM: x86: Move check_processor_compatibility from init ops to runtime ops isaku.yamahata
2022-09-05 5:42 ` Yuan Yao
2022-09-02 2:17 ` [PATCH v3 04/22] Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs" isaku.yamahata
2022-09-05 5:48 ` Yuan Yao
2022-09-02 2:17 ` [PATCH v3 05/22] KVM: Provide more information in kernel log if hardware enabling fails isaku.yamahata
2022-09-05 5:56 ` Yuan Yao
2022-09-02 2:17 ` [PATCH v3 06/22] KVM: arm64: Simplify the CPUHP logic isaku.yamahata
2022-09-05 7:05 ` Yuan Yao [this message]
2022-09-05 9:29 ` Marc Zyngier
2022-09-05 12:39 ` Marc Zyngier
2022-09-07 15:12 ` Isaku Yamahata
2022-09-02 2:17 ` [PATCH v3 07/22] KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section isaku.yamahata
2022-09-05 7:49 ` Yuan Yao
2022-09-02 2:17 ` [PATCH v3 08/22] KVM: Do compatibility checks on hotplugged CPUs isaku.yamahata
2022-09-06 1:25 ` Yuan Yao
2022-09-02 2:17 ` [PATCH v3 09/22] KVM: Do processor compatibility check on resume isaku.yamahata
2022-09-05 8:40 ` Yuan Yao
2022-09-05 9:27 ` Yuan Yao
2022-09-08 18:21 ` Isaku Yamahata
2022-09-02 2:17 ` [PATCH v3 10/22] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock isaku.yamahata
2022-09-06 2:46 ` Yuan Yao
2022-09-06 6:32 ` Marc Zyngier
2022-09-06 21:44 ` Isaku Yamahata
2022-09-08 18:24 ` Isaku Yamahata
2022-09-02 2:17 ` [PATCH v3 11/22] KVM: Add arch hooks for PM events with empty stub isaku.yamahata
2022-09-06 6:25 ` Yuan Yao
2022-09-08 19:11 ` Isaku Yamahata
2022-09-02 2:17 ` [PATCH v3 12/22] KVM: x86: Move TSC fixup logic to KVM arch resume callback isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 13/22] KVM: Add arch hook when VM is added/deleted isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 14/22] KVM: Move out KVM arch PM hooks and hardware enable/disable logic isaku.yamahata
2022-09-06 7:43 ` Yuan Yao
2022-09-08 19:15 ` Isaku Yamahata
2022-09-02 2:17 ` [PATCH v3 15/22] KVM: kvm_arch.c: Remove _nolock post fix isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 16/22] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed isaku.yamahata
2022-09-07 5:56 ` Yuan Yao
2022-09-08 22:51 ` Isaku Yamahata
2022-09-02 2:17 ` [PATCH v3 17/22] KVM: x86: Duplicate arch callbacks related to pm events isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 18/22] KVM: Eliminate kvm_arch_post_init_vm() isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 19/22] KVM: x86: Delete kvm_arch_hardware_enable/disable() isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 20/22] KVM: Add config to not compile kvm_arch.c isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 21/22] RFC: KVM: x86: Remove cpus_hardware_enabled and related sanity check isaku.yamahata
2022-09-02 2:17 ` [PATCH v3 22/22] RFC: KVM: " isaku.yamahata
2022-09-05 15:38 ` [PATCH v3 00/22] KVM: hardware enable/disable reorganize Marc Zyngier
2022-09-06 22:25 ` Isaku Yamahata
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220905070509.f5neutyqgvbklefi@yy-desk-7060 \
--to=yuan.yao@linux.intel.com \
--cc=atishp@atishpatra.org \
--cc=bp@alien8.de \
--cc=chao.gao@intel.com \
--cc=chenhuacai@kernel.org \
--cc=daniel.lezcano@linaro.org \
--cc=dave.hansen@linux.intel.com \
--cc=isaku.yamahata@gmail.com \
--cc=isaku.yamahata@intel.com \
--cc=john.garry@huawei.com \
--cc=kai.huang@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=liuqi115@huawei.com \
--cc=maz@kernel.org \
--cc=oupton@google.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
--cc=ying.huang@intel.com \
--cc=zhangshaokun@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).