[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+EHjTzSSVbJGyWbR98eyshZbNsaf6uSu8zqJs7kHkwGYZf2yQ@mail.gmail.com>
Date: Thu, 1 Jul 2021 10:45:47 +0100
From: Fuad Tabba <tabba@...gle.com>
To: Jean-Philippe Brucker <jean-philippe@...aro.org>
Cc: maz@...nel.org, salil.mehta@...wei.com, lorenzo.pieralisi@....com,
kvm@...r.kernel.org, corbet@....net, catalin.marinas@....com,
linux-kernel@...r.kernel.org, will@...nel.org,
jonathan.cameron@...wei.com, pbonzini@...hat.com,
kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org
Subject: Re: [RFC PATCH 3/5] KVM: arm64: Allow userspace to request WFI
Hi Jean-Philippe,
On Tue, Jun 8, 2021 at 4:54 PM Jean-Philippe Brucker
<jean-philippe@...aro.org> wrote:
>
> To help userspace implement PSCI CPU_SUSPEND, allow setting the "HALTED"
> MP state to request a WFI before returning to the guest.
>
> Userspace won't obtain a HALTED mp_state from a KVM_GET_MP_STATE call
> unless they set it themselves. When set by KVM, to handle wfi or
> CPU_SUSPEND, it is consumed before returning to userspace.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@...aro.org>
> ---
> Documentation/virt/kvm/api.rst | 15 +++++++++------
> include/uapi/linux/kvm.h | 1 +
> arch/arm64/kvm/arm.c | 11 ++++++++++-
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 7fcb2fd38f42..e4fe7fb60d5d 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -1416,8 +1416,8 @@ Possible values are:
> which has not yet received an INIT signal [x86]
> KVM_MP_STATE_INIT_RECEIVED the vcpu has received an INIT signal, and is
> now ready for a SIPI [x86]
> - KVM_MP_STATE_HALTED the vcpu has executed a HLT instruction and
> - is waiting for an interrupt [x86]
> + KVM_MP_STATE_HALTED the vcpu has executed a HLT/WFI instruction
> + and is waiting for an interrupt [x86,arm64]
Considering that arm64 has a HLT instruction (for debugging), which is
very different from the x86 one, would it be good to clarify that in
the comment. e.g.,
"the vcpu has executed a HLT(x86)/WFI(arm64) instruction"?
Thanks,
/fuad
> KVM_MP_STATE_SIPI_RECEIVED the vcpu has just received a SIPI (vector
> accessible via KVM_GET_VCPU_EVENTS) [x86]
> KVM_MP_STATE_STOPPED the vcpu is stopped [s390,arm/arm64]
> @@ -1435,8 +1435,9 @@ these architectures.
> For arm/arm64:
> ^^^^^^^^^^^^^^
>
> -The only states that are valid are KVM_MP_STATE_STOPPED and
> -KVM_MP_STATE_RUNNABLE which reflect if the vcpu is paused or not.
> +Valid states are KVM_MP_STATE_STOPPED and KVM_MP_STATE_RUNNABLE which reflect
> +if the vcpu is paused or not. If KVM_CAP_ARM_MP_HALTED is present, state
> +KVM_MP_STATE_HALTED is also valid.
>
> 4.39 KVM_SET_MP_STATE
> ---------------------
> @@ -1457,8 +1458,10 @@ these architectures.
> For arm/arm64:
> ^^^^^^^^^^^^^^
>
> -The only states that are valid are KVM_MP_STATE_STOPPED and
> -KVM_MP_STATE_RUNNABLE which reflect if the vcpu should be paused or not.
> +Valid states are KVM_MP_STATE_STOPPED and KVM_MP_STATE_RUNNABLE which reflect
> +if the vcpu should be paused or not. If KVM_CAP_ARM_MP_HALTED is present,
> +KVM_MP_STATE_HALTED can be set, to wait for interrupts targeted at the vcpu
> +before running it.
>
> 4.40 KVM_SET_IDENTITY_MAP_ADDR
> ------------------------------
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 79d9c44d1ad7..06ba64c49737 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1083,6 +1083,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_SGX_ATTRIBUTE 196
> #define KVM_CAP_VM_COPY_ENC_CONTEXT_FROM 197
> #define KVM_CAP_PTP_KVM 198
> +#define KVM_CAP_ARM_MP_HALTED 199
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index d8cbaa0373c7..d6ad977fea5f 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -207,6 +207,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_SET_GUEST_DEBUG:
> case KVM_CAP_VCPU_ATTRIBUTES:
> case KVM_CAP_PTP_KVM:
> + case KVM_CAP_ARM_MP_HALTED:
> r = 1;
> break;
> case KVM_CAP_SET_GUEST_DEBUG2:
> @@ -469,6 +470,9 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
> case KVM_MP_STATE_RUNNABLE:
> vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
> break;
> + case KVM_MP_STATE_HALTED:
> + kvm_arm_vcpu_suspend(vcpu);
> + break;
> case KVM_MP_STATE_STOPPED:
> kvm_arm_vcpu_power_off(vcpu);
> break;
> @@ -699,7 +703,12 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
> preempt_enable();
> }
>
> - if (kvm_check_request(KVM_REQ_SUSPEND, vcpu)) {
> + /*
> + * Check mp_state again in case userspace changed their mind
> + * after requesting suspend.
> + */
> + if (kvm_check_request(KVM_REQ_SUSPEND, vcpu) &&
> + vcpu->arch.mp_state == KVM_MP_STATE_HALTED) {
> if (!irq_pending) {
> kvm_vcpu_block(vcpu);
> kvm_clear_request(KVM_REQ_UNHALT, vcpu);
> --
> 2.31.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@...ts.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Powered by blists - more mailing lists