All of lore.kernel.org
 help / color / mirror / Atom feed
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 5/6] arm/arm64: KVM: Turn off vcpus on PSCI shutdown/reboot
Date: Mon, 08 Dec 2014 12:04:53 +0000	[thread overview]
Message-ID: <548593E5.7070101@arm.com> (raw)
In-Reply-To: <1417641522-29056-6-git-send-email-christoffer.dall@linaro.org>

On 03/12/14 21:18, Christoffer Dall wrote:
> When a vcpu calls SYSTEM_OFF or SYSTEM_RESET with PSCI v0.2, the vcpus
> should really be turned off for the VM adhering to the suggestions in
> the PSCI spec, and it's the sane thing to do.
> 
> Also, clarify the behavior and expectations for exits to user space with
> the KVM_EXIT_SYSTEM_EVENT case.
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  Documentation/virtual/kvm/api.txt |  9 +++++++++
>  arch/arm/kvm/psci.c               | 19 +++++++++++++++++++
>  arch/arm64/include/asm/kvm_host.h |  1 +
>  3 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 81f1b97..228f9cf 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2957,6 +2957,15 @@ HVC instruction based PSCI call from the vcpu. The 'type' field describes
>  the system-level event type. The 'flags' field describes architecture
>  specific flags for the system-level event.
>  
> +Valid values for 'type' are:
> +  KVM_SYSTEM_EVENT_SHUTDOWN -- the guest has requested a shutdown of the
> +   VM. Userspace is not obliged to honour this, and if it does honour
> +   this does not need to destroy the VM synchronously (ie it may call
> +   KVM_RUN again before shutdown finally occurs).
> +  KVM_SYSTEM_EVENT_RESET -- the guest has requested a reset of the VM.
> +   As with SHUTDOWN, userspace can choose to ignore the request, or
> +   to schedule the reset to occur in the future and may call KVM_RUN again.
> +
>  		/* Fix the size of the union. */
>  		char padding[256];
>  	};
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 09cf377..ae0bb91 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -15,6 +15,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/preempt.h>
>  #include <linux/kvm_host.h>
>  #include <linux/wait.h>
>  
> @@ -166,6 +167,24 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
>  
>  static void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
>  {
> +	int i;
> +	struct kvm_vcpu *tmp;
> +
> +	/*
> +	 * The KVM ABI specifies that a system event exit may call KVM_RUN
> +	 * again and may perform shutdown/reboot at a later time that when the
> +	 * actual request is made.  Since we are implementing PSCI and a
> +	 * caller of PSCI reboot and shutdown expects that the system shuts
> +	 * down or reboots immediately, let's make sure that VCPUs are not run
> +	 * after this call is handled and before the VCPUs have been
> +	 * re-initialized.
> +	 */
> +	kvm_for_each_vcpu(i, tmp, vcpu->kvm)
> +		tmp->arch.pause = true;
> +	preempt_disable();
> +	force_vm_exit(cpu_all_mask);
> +	preempt_enable();
> +

I'm slightly uneasy about this force_vm_exit, as this is something that
is directly triggered by the guest. I suppose it is almost impossible to
find out which CPUs we're actually using...

>  	memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
>  	vcpu->run->system_event.type = type;
>  	vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 65c6152..0b7dfdb 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -198,6 +198,7 @@ struct kvm_vcpu *kvm_arm_get_running_vcpu(void);
>  struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void);
>  
>  u64 kvm_call_hyp(void *hypfn, ...);
> +void force_vm_exit(const cpumask_t *mask);
>  
>  int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		int exception_index);
> 

Other than that,

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <marc.zyngier@arm.com>
To: Christoffer Dall <christoffer.dall@linaro.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Peter Maydell <peter.maydell@linaro.org>,
	Laszlo Ersek <lersek@redhat.com>,
	Andrew Jones <drjones@redhat.com>
Subject: Re: [PATCH v2 5/6] arm/arm64: KVM: Turn off vcpus on PSCI shutdown/reboot
Date: Mon, 08 Dec 2014 12:04:53 +0000	[thread overview]
Message-ID: <548593E5.7070101@arm.com> (raw)
In-Reply-To: <1417641522-29056-6-git-send-email-christoffer.dall@linaro.org>

On 03/12/14 21:18, Christoffer Dall wrote:
> When a vcpu calls SYSTEM_OFF or SYSTEM_RESET with PSCI v0.2, the vcpus
> should really be turned off for the VM adhering to the suggestions in
> the PSCI spec, and it's the sane thing to do.
> 
> Also, clarify the behavior and expectations for exits to user space with
> the KVM_EXIT_SYSTEM_EVENT case.
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  Documentation/virtual/kvm/api.txt |  9 +++++++++
>  arch/arm/kvm/psci.c               | 19 +++++++++++++++++++
>  arch/arm64/include/asm/kvm_host.h |  1 +
>  3 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 81f1b97..228f9cf 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2957,6 +2957,15 @@ HVC instruction based PSCI call from the vcpu. The 'type' field describes
>  the system-level event type. The 'flags' field describes architecture
>  specific flags for the system-level event.
>  
> +Valid values for 'type' are:
> +  KVM_SYSTEM_EVENT_SHUTDOWN -- the guest has requested a shutdown of the
> +   VM. Userspace is not obliged to honour this, and if it does honour
> +   this does not need to destroy the VM synchronously (ie it may call
> +   KVM_RUN again before shutdown finally occurs).
> +  KVM_SYSTEM_EVENT_RESET -- the guest has requested a reset of the VM.
> +   As with SHUTDOWN, userspace can choose to ignore the request, or
> +   to schedule the reset to occur in the future and may call KVM_RUN again.
> +
>  		/* Fix the size of the union. */
>  		char padding[256];
>  	};
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 09cf377..ae0bb91 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -15,6 +15,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/preempt.h>
>  #include <linux/kvm_host.h>
>  #include <linux/wait.h>
>  
> @@ -166,6 +167,24 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
>  
>  static void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
>  {
> +	int i;
> +	struct kvm_vcpu *tmp;
> +
> +	/*
> +	 * The KVM ABI specifies that a system event exit may call KVM_RUN
> +	 * again and may perform shutdown/reboot at a later time that when the
> +	 * actual request is made.  Since we are implementing PSCI and a
> +	 * caller of PSCI reboot and shutdown expects that the system shuts
> +	 * down or reboots immediately, let's make sure that VCPUs are not run
> +	 * after this call is handled and before the VCPUs have been
> +	 * re-initialized.
> +	 */
> +	kvm_for_each_vcpu(i, tmp, vcpu->kvm)
> +		tmp->arch.pause = true;
> +	preempt_disable();
> +	force_vm_exit(cpu_all_mask);
> +	preempt_enable();
> +

I'm slightly uneasy about this force_vm_exit, as this is something that
is directly triggered by the guest. I suppose it is almost impossible to
find out which CPUs we're actually using...

>  	memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
>  	vcpu->run->system_event.type = type;
>  	vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 65c6152..0b7dfdb 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -198,6 +198,7 @@ struct kvm_vcpu *kvm_arm_get_running_vcpu(void);
>  struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void);
>  
>  u64 kvm_call_hyp(void *hypfn, ...);
> +void force_vm_exit(const cpumask_t *mask);
>  
>  int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		int exception_index);
> 

Other than that,

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

  reply	other threads:[~2014-12-08 12:04 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-03 21:18 [PATCH v2 0/6] Improve PSCI system events and fix reboot bugs Christoffer Dall
2014-12-03 21:18 ` Christoffer Dall
2014-12-03 21:18 ` [PATCH v2 1/6] arm/arm64: KVM: Don't clear the VCPU_POWER_OFF flag Christoffer Dall
2014-12-03 21:18   ` Christoffer Dall
2014-12-08 11:46   ` Marc Zyngier
2014-12-08 11:46     ` Marc Zyngier
2014-12-03 21:18 ` [PATCH v2 2/6] arm/arm64: KVM: Correct KVM_ARM_VCPU_INIT power off option Christoffer Dall
2014-12-03 21:18   ` Christoffer Dall
2014-12-08 11:47   ` Marc Zyngier
2014-12-08 11:47     ` Marc Zyngier
2014-12-03 21:18 ` [PATCH v2 3/6] arm/arm64: KVM: Reset the HCR on each vcpu when resetting the vcpu Christoffer Dall
2014-12-03 21:18   ` Christoffer Dall
2014-12-08 11:49   ` Marc Zyngier
2014-12-08 11:49     ` Marc Zyngier
2014-12-03 21:18 ` [PATCH v2 4/6] arm/arm64: KVM: Clarify KVM_ARM_VCPU_INIT ABI Christoffer Dall
2014-12-03 21:18   ` Christoffer Dall
2014-12-08 11:52   ` Marc Zyngier
2014-12-08 11:52     ` Marc Zyngier
2014-12-03 21:18 ` [PATCH v2 5/6] arm/arm64: KVM: Turn off vcpus on PSCI shutdown/reboot Christoffer Dall
2014-12-03 21:18   ` Christoffer Dall
2014-12-08 12:04   ` Marc Zyngier [this message]
2014-12-08 12:04     ` Marc Zyngier
2014-12-08 12:58     ` Christoffer Dall
2014-12-08 12:58       ` Christoffer Dall
2014-12-08 13:19       ` Marc Zyngier
2014-12-08 13:19         ` Marc Zyngier
2014-12-12 19:42         ` Christoffer Dall
2014-12-12 19:42           ` Christoffer Dall
2014-12-12 19:49         ` Christoffer Dall
2014-12-12 19:49           ` Christoffer Dall
2014-12-12 21:04           ` Marc Zyngier
2014-12-12 21:04             ` Marc Zyngier
2014-12-03 21:18 ` [PATCH v2 6/6] arm/arm64: KVM: Introduce stage2_unmap_vm Christoffer Dall
2014-12-03 21:18   ` Christoffer Dall
2014-12-08 12:08   ` Marc Zyngier
2014-12-08 12:08     ` Marc Zyngier
2014-12-05 17:24 ` [PATCH v2 0/6] Improve PSCI system events and fix reboot bugs Andrew Jones
2014-12-05 17:24   ` Andrew Jones
2014-12-08 11:24 ` Peter Maydell
2014-12-08 11:24   ` Peter Maydell

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=548593E5.7070101@arm.com \
    --to=marc.zyngier@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.