All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <marc.zyngier@arm.com>
To: AKASHI Takahiro <takahiro.akashi@linaro.org>,
	catalin.marinas@arm.com, will.deacon@arm.com,
	mark.rutland@arm.com
Cc: linaro-kernel@lists.linaro.org, christoffer.dall@linaro.org,
	geoff@infradead.org, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, broonie@kernel.org,
	david.griego@linaro.org, linux-arm-kernel@lists.infradead.org,
	freddy77@gmail.com
Subject: Re: [v4 2/4] arm64: kvm: add kvm cpu hotplug
Date: Tue, 26 May 2015 10:35:11 +0100	[thread overview]
Message-ID: <55643E4F.2000806@arm.com> (raw)
In-Reply-To: <1431047884-5637-3-git-send-email-takahiro.akashi@linaro.org>

On 08/05/15 02:18, AKASHI Takahiro wrote:
> This patch allows cpu cores to be up and down by adding
> kvm_arch_hardware_enable/isable(). This way, especially in kexec case,
> cores are reset to initial states and kexec can gracefully shutdown the
> system and reboot a new kernel from EL2.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h   |    1 -
>  arch/arm/kvm/arm.c                |   29 +++++++++++++++++++----------
>  arch/arm64/include/asm/kvm_host.h |    1 -
>  3 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 41008cd..ca97764 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -237,7 +237,6 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
> -static inline void kvm_arch_hardware_disable(void) {}
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 251ab9e..e989925 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -87,11 +87,6 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
>  	return &kvm_arm_running_vcpu;
>  }
>  
> -int kvm_arch_hardware_enable(void)
> -{
> -	return 0;
> -}
> -
>  int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>  {
>  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
> @@ -885,6 +880,9 @@ static void cpu_init_hyp_mode(void *dummy)

Since you removed the IPI, why do you keep the dummy argument?

>  	unsigned long stack_page;
>  	unsigned long vector_ptr;
>  
> +	if (__hyp_get_vectors() != hyp_default_vectors)
> +		return;
> +
>  	/* Switch from the HYP stub to our own HYP init vector */
>  	__hyp_set_vectors(kvm_get_idmap_vector());
>  
> @@ -921,6 +919,10 @@ static int hyp_init_cpu_notify(struct notifier_block *self,
>  		if (__hyp_get_vectors() == hyp_default_vectors)
>  			cpu_init_hyp_mode(NULL);
>  		break;
> +	case CPU_DYING:
> +	case CPU_DYING_FROZEN:
> +		kvm_cpu_reset(NULL);
> +		break;
>  	}
>  
>  	return NOTIFY_OK;
> @@ -936,6 +938,7 @@ static int hyp_init_cpu_pm_notifier(struct notifier_block *self,
>  				    void *v)
>  {
>  	if (cmd == CPU_PM_EXIT &&
> +	    kvm_arm_get_running_vcpu() &&
>  	    __hyp_get_vectors() == hyp_default_vectors) {
>  		cpu_init_hyp_mode(NULL);
>  		return NOTIFY_OK;
> @@ -1039,11 +1042,6 @@ static int init_hyp_mode(void)
>  	}
>  
>  	/*
> -	 * Execute the init code on each CPU.
> -	 */
> -	on_each_cpu(cpu_init_hyp_mode, NULL, 1);
> -
> -	/*
>  	 * Init HYP view of VGIC
>  	 */
>  	err = kvm_vgic_hyp_init();
> @@ -1144,6 +1142,17 @@ out_err:
>  	return err;
>  }
>  
> +int kvm_arch_hardware_enable(void)
> +{
> +	cpu_init_hyp_mode(NULL);
> +	return 0;
> +}
> +
> +void kvm_arch_hardware_disable(void)
> +{
> +	kvm_cpu_reset(NULL);
> +}
> +

Bahhh... Just rename cpu_init_hyp_mode to kvm_arch_hardware_enable, and
kvM_cpu_reset to kvm_arch_hardware_disable. I don't see the point of
keeping this indirection.

>  /* NOP: Compiling as a module not supported */
>  void kvm_arch_exit(void)
>  {
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 6a8da9c..831e6a4 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -262,7 +262,6 @@ static inline void vgic_arch_setup(const struct vgic_params *vgic)
>  	}
>  }
>  
> -static inline void kvm_arch_hardware_disable(void) {}
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> 


-- 
Jazz is not dead. It just smells funny...

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [v4 2/4] arm64: kvm: add kvm cpu hotplug
Date: Tue, 26 May 2015 10:35:11 +0100	[thread overview]
Message-ID: <55643E4F.2000806@arm.com> (raw)
In-Reply-To: <1431047884-5637-3-git-send-email-takahiro.akashi@linaro.org>

On 08/05/15 02:18, AKASHI Takahiro wrote:
> This patch allows cpu cores to be up and down by adding
> kvm_arch_hardware_enable/isable(). This way, especially in kexec case,
> cores are reset to initial states and kexec can gracefully shutdown the
> system and reboot a new kernel from EL2.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h   |    1 -
>  arch/arm/kvm/arm.c                |   29 +++++++++++++++++++----------
>  arch/arm64/include/asm/kvm_host.h |    1 -
>  3 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 41008cd..ca97764 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -237,7 +237,6 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
> -static inline void kvm_arch_hardware_disable(void) {}
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 251ab9e..e989925 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -87,11 +87,6 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
>  	return &kvm_arm_running_vcpu;
>  }
>  
> -int kvm_arch_hardware_enable(void)
> -{
> -	return 0;
> -}
> -
>  int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>  {
>  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
> @@ -885,6 +880,9 @@ static void cpu_init_hyp_mode(void *dummy)

Since you removed the IPI, why do you keep the dummy argument?

>  	unsigned long stack_page;
>  	unsigned long vector_ptr;
>  
> +	if (__hyp_get_vectors() != hyp_default_vectors)
> +		return;
> +
>  	/* Switch from the HYP stub to our own HYP init vector */
>  	__hyp_set_vectors(kvm_get_idmap_vector());
>  
> @@ -921,6 +919,10 @@ static int hyp_init_cpu_notify(struct notifier_block *self,
>  		if (__hyp_get_vectors() == hyp_default_vectors)
>  			cpu_init_hyp_mode(NULL);
>  		break;
> +	case CPU_DYING:
> +	case CPU_DYING_FROZEN:
> +		kvm_cpu_reset(NULL);
> +		break;
>  	}
>  
>  	return NOTIFY_OK;
> @@ -936,6 +938,7 @@ static int hyp_init_cpu_pm_notifier(struct notifier_block *self,
>  				    void *v)
>  {
>  	if (cmd == CPU_PM_EXIT &&
> +	    kvm_arm_get_running_vcpu() &&
>  	    __hyp_get_vectors() == hyp_default_vectors) {
>  		cpu_init_hyp_mode(NULL);
>  		return NOTIFY_OK;
> @@ -1039,11 +1042,6 @@ static int init_hyp_mode(void)
>  	}
>  
>  	/*
> -	 * Execute the init code on each CPU.
> -	 */
> -	on_each_cpu(cpu_init_hyp_mode, NULL, 1);
> -
> -	/*
>  	 * Init HYP view of VGIC
>  	 */
>  	err = kvm_vgic_hyp_init();
> @@ -1144,6 +1142,17 @@ out_err:
>  	return err;
>  }
>  
> +int kvm_arch_hardware_enable(void)
> +{
> +	cpu_init_hyp_mode(NULL);
> +	return 0;
> +}
> +
> +void kvm_arch_hardware_disable(void)
> +{
> +	kvm_cpu_reset(NULL);
> +}
> +

Bahhh... Just rename cpu_init_hyp_mode to kvm_arch_hardware_enable, and
kvM_cpu_reset to kvm_arch_hardware_disable. I don't see the point of
keeping this indirection.

>  /* NOP: Compiling as a module not supported */
>  void kvm_arch_exit(void)
>  {
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 6a8da9c..831e6a4 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -262,7 +262,6 @@ static inline void vgic_arch_setup(const struct vgic_params *vgic)
>  	}
>  }
>  
> -static inline void kvm_arch_hardware_disable(void) {}
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> 


-- 
Jazz is not dead. It just smells funny...

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <marc.zyngier@arm.com>
To: AKASHI Takahiro <takahiro.akashi@linaro.org>,
	catalin.marinas@arm.com, will.deacon@arm.com,
	mark.rutland@arm.com
Cc: linux-arm-kernel@lists.infradead.org,
	linaro-kernel@lists.linaro.org, geoff@infradead.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	broonie@kernel.org, david.griego@linaro.org,
	christoffer.dall@linaro.org, freddy77@gmail.com
Subject: Re: [v4 2/4] arm64: kvm: add kvm cpu hotplug
Date: Tue, 26 May 2015 10:35:11 +0100	[thread overview]
Message-ID: <55643E4F.2000806@arm.com> (raw)
In-Reply-To: <1431047884-5637-3-git-send-email-takahiro.akashi@linaro.org>

On 08/05/15 02:18, AKASHI Takahiro wrote:
> This patch allows cpu cores to be up and down by adding
> kvm_arch_hardware_enable/isable(). This way, especially in kexec case,
> cores are reset to initial states and kexec can gracefully shutdown the
> system and reboot a new kernel from EL2.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h   |    1 -
>  arch/arm/kvm/arm.c                |   29 +++++++++++++++++++----------
>  arch/arm64/include/asm/kvm_host.h |    1 -
>  3 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 41008cd..ca97764 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -237,7 +237,6 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
> -static inline void kvm_arch_hardware_disable(void) {}
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 251ab9e..e989925 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -87,11 +87,6 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
>  	return &kvm_arm_running_vcpu;
>  }
>  
> -int kvm_arch_hardware_enable(void)
> -{
> -	return 0;
> -}
> -
>  int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>  {
>  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
> @@ -885,6 +880,9 @@ static void cpu_init_hyp_mode(void *dummy)

Since you removed the IPI, why do you keep the dummy argument?

>  	unsigned long stack_page;
>  	unsigned long vector_ptr;
>  
> +	if (__hyp_get_vectors() != hyp_default_vectors)
> +		return;
> +
>  	/* Switch from the HYP stub to our own HYP init vector */
>  	__hyp_set_vectors(kvm_get_idmap_vector());
>  
> @@ -921,6 +919,10 @@ static int hyp_init_cpu_notify(struct notifier_block *self,
>  		if (__hyp_get_vectors() == hyp_default_vectors)
>  			cpu_init_hyp_mode(NULL);
>  		break;
> +	case CPU_DYING:
> +	case CPU_DYING_FROZEN:
> +		kvm_cpu_reset(NULL);
> +		break;
>  	}
>  
>  	return NOTIFY_OK;
> @@ -936,6 +938,7 @@ static int hyp_init_cpu_pm_notifier(struct notifier_block *self,
>  				    void *v)
>  {
>  	if (cmd == CPU_PM_EXIT &&
> +	    kvm_arm_get_running_vcpu() &&
>  	    __hyp_get_vectors() == hyp_default_vectors) {
>  		cpu_init_hyp_mode(NULL);
>  		return NOTIFY_OK;
> @@ -1039,11 +1042,6 @@ static int init_hyp_mode(void)
>  	}
>  
>  	/*
> -	 * Execute the init code on each CPU.
> -	 */
> -	on_each_cpu(cpu_init_hyp_mode, NULL, 1);
> -
> -	/*
>  	 * Init HYP view of VGIC
>  	 */
>  	err = kvm_vgic_hyp_init();
> @@ -1144,6 +1142,17 @@ out_err:
>  	return err;
>  }
>  
> +int kvm_arch_hardware_enable(void)
> +{
> +	cpu_init_hyp_mode(NULL);
> +	return 0;
> +}
> +
> +void kvm_arch_hardware_disable(void)
> +{
> +	kvm_cpu_reset(NULL);
> +}
> +

Bahhh... Just rename cpu_init_hyp_mode to kvm_arch_hardware_enable, and
kvM_cpu_reset to kvm_arch_hardware_disable. I don't see the point of
keeping this indirection.

>  /* NOP: Compiling as a module not supported */
>  void kvm_arch_exit(void)
>  {
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 6a8da9c..831e6a4 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -262,7 +262,6 @@ static inline void vgic_arch_setup(const struct vgic_params *vgic)
>  	}
>  }
>  
> -static inline void kvm_arch_hardware_disable(void) {}
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> 


-- 
Jazz is not dead. It just smells funny...

  reply	other threads:[~2015-05-26  9:35 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-08  1:18 [v4 0/4] arm64: kvm: reset hyp context for kexec AKASHI Takahiro
2015-05-08  1:18 ` AKASHI Takahiro
2015-05-08  1:18 ` AKASHI Takahiro
2015-05-08  1:18 ` [v4 1/4] arm64: kvm: add a cpu tear-down function AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-26  9:26   ` Marc Zyngier
2015-05-26  9:26     ` Marc Zyngier
2015-05-26  9:26     ` Marc Zyngier
2015-05-27  5:21     ` AKASHI Takahiro
2015-05-27  5:21       ` AKASHI Takahiro
2015-05-27  5:21       ` AKASHI Takahiro
2015-05-27  7:42       ` Marc Zyngier
2015-05-27  7:42         ` Marc Zyngier
2015-05-27  7:42         ` Marc Zyngier
2015-05-08  1:18 ` [v4 2/4] arm64: kvm: add kvm cpu hotplug AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-26  9:35   ` Marc Zyngier [this message]
2015-05-26  9:35     ` Marc Zyngier
2015-05-26  9:35     ` Marc Zyngier
2015-05-27  5:29     ` AKASHI Takahiro
2015-05-27  5:29       ` AKASHI Takahiro
2015-05-27  5:29       ` AKASHI Takahiro
2015-05-08  1:18 ` [v4 3/4] arm64: kvm: remove !KEXEC dependency AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-08  1:18 ` [v4 4/4] arm: kvm: add stub implementation for kvm_cpu_reset() AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-08  1:18   ` AKASHI Takahiro
2015-05-26  9:36   ` Marc Zyngier
2015-05-26  9:36     ` Marc Zyngier
2015-05-26  9:36     ` Marc Zyngier
2015-05-27  5:31     ` AKASHI Takahiro
2015-05-27  5:31       ` AKASHI Takahiro
2015-05-27  5:31       ` AKASHI Takahiro

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=55643E4F.2000806@arm.com \
    --to=marc.zyngier@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=christoffer.dall@linaro.org \
    --cc=david.griego@linaro.org \
    --cc=freddy77@gmail.com \
    --cc=geoff@infradead.org \
    --cc=kexec@lists.infradead.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=takahiro.akashi@linaro.org \
    --cc=will.deacon@arm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.