All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Upton <oupton@google.com>
To: kvm-riscv@lists.infradead.org
Subject: [PATCH v5 07/13] KVM: arm64: Add support for userspace to suspend a vCPU
Date: Fri, 29 Apr 2022 03:42:28 +0000	[thread overview]
Message-ID: <YmtepGWYckmUKln+@google.com> (raw)
In-Reply-To: <CAAeT=Fx5Nb0EJ+6825fYxAxF9bK5DHOXNmJiSVGP=JVSbWuCrQ@mail.gmail.com>

On Thu, Apr 21, 2022 at 11:28:42PM -0700, Reiji Watanabe wrote:

[...]

> > > > +static void kvm_arm_vcpu_suspend(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +       vcpu->arch.mp_state.mp_state = KVM_MP_STATE_SUSPENDED;
> > > > +       kvm_make_request(KVM_REQ_SUSPEND, vcpu);
> > > > +       kvm_vcpu_kick(vcpu);
> > >
> > > Considering the patch 8 will remove the call to kvm_vcpu_kick()
> > > (BTW, I wonder why you wanted to make that change in the patch-8
> > > instead of the patch-7),
> >
> > Squashed the diff into the wrong patch! Marc pointed out this is of
> > course cargo-culted as I was following the pattern laid down by
> > KVM_REQ_SLEEP :)
> 
> I see. Thanks for the clarification !
> 
> > > it looks like we could use the mp_state
> > > KVM_MP_STATE_SUSPENDED instead of using KVM_REQ_SUSPEND.
> > > What is the reason why you prefer to introduce KVM_REQ_SUSPEND
> > > rather than simply using KVM_MP_STATE_SUSPENDED ?
> >
> > I was trying to avoid any heavy refactoring in adding new
> > functionality here, as we handle KVM_MP_STATE_STOPPED similarly (make
> > a request). ARM is definitely a bit different than x86 in the way that
> > we handle the MP states, as x86 doesn't bounce through vCPU requests
> > to do it and instead directly checks the mp_state value.
> 
> The difference from KVM_MP_STATE_STOPPED is that kvm_arm_vcpu_power_off()
> calls kvm_vcpu_kick(), which made me think having KVM_REQ_SLEEP was
> reasonable (it appears kvm_vcpu_kick() won't be needed there due to
> the same reason as kvm_arm_vcpu_suspend).

Just to finish the thought on this before mailing out what I hope is the
last take on all of this. I'm going to leave the pointless call to
kvm_vcpu_kick() in place, if only to follow the pattern of other MP
states.

That will all get cleaned up later on, as discussed :)

--
Thanks,
Oliver


WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oupton@google.com>
To: Reiji Watanabe <reijiw@google.com>
Cc: Marc Zyngier <maz@kernel.org>, Wanpeng Li <wanpengli@tencent.com>,
	kvm@vger.kernel.org, Joerg Roedel <joro@8bytes.org>,
	Peter Shier <pshier@google.com>,
	kvm-riscv@lists.infradead.org,
	Atish Patra <atishp@atishpatra.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	kvmarm@lists.cs.columbia.edu, Jim Mattson <jmattson@google.com>
Subject: Re: [PATCH v5 07/13] KVM: arm64: Add support for userspace to suspend a vCPU
Date: Fri, 29 Apr 2022 03:42:28 +0000	[thread overview]
Message-ID: <YmtepGWYckmUKln+@google.com> (raw)
In-Reply-To: <CAAeT=Fx5Nb0EJ+6825fYxAxF9bK5DHOXNmJiSVGP=JVSbWuCrQ@mail.gmail.com>

On Thu, Apr 21, 2022 at 11:28:42PM -0700, Reiji Watanabe wrote:

[...]

> > > > +static void kvm_arm_vcpu_suspend(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +       vcpu->arch.mp_state.mp_state = KVM_MP_STATE_SUSPENDED;
> > > > +       kvm_make_request(KVM_REQ_SUSPEND, vcpu);
> > > > +       kvm_vcpu_kick(vcpu);
> > >
> > > Considering the patch 8 will remove the call to kvm_vcpu_kick()
> > > (BTW, I wonder why you wanted to make that change in the patch-8
> > > instead of the patch-7),
> >
> > Squashed the diff into the wrong patch! Marc pointed out this is of
> > course cargo-culted as I was following the pattern laid down by
> > KVM_REQ_SLEEP :)
> 
> I see. Thanks for the clarification !
> 
> > > it looks like we could use the mp_state
> > > KVM_MP_STATE_SUSPENDED instead of using KVM_REQ_SUSPEND.
> > > What is the reason why you prefer to introduce KVM_REQ_SUSPEND
> > > rather than simply using KVM_MP_STATE_SUSPENDED ?
> >
> > I was trying to avoid any heavy refactoring in adding new
> > functionality here, as we handle KVM_MP_STATE_STOPPED similarly (make
> > a request). ARM is definitely a bit different than x86 in the way that
> > we handle the MP states, as x86 doesn't bounce through vCPU requests
> > to do it and instead directly checks the mp_state value.
> 
> The difference from KVM_MP_STATE_STOPPED is that kvm_arm_vcpu_power_off()
> calls kvm_vcpu_kick(), which made me think having KVM_REQ_SLEEP was
> reasonable (it appears kvm_vcpu_kick() won't be needed there due to
> the same reason as kvm_arm_vcpu_suspend).

Just to finish the thought on this before mailing out what I hope is the
last take on all of this. I'm going to leave the pointless call to
kvm_vcpu_kick() in place, if only to follow the pattern of other MP
states.

That will all get cleaned up later on, as discussed :)

--
Thanks,
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oupton@google.com>
To: Reiji Watanabe <reijiw@google.com>
Cc: kvmarm@lists.cs.columbia.edu,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Anup Patel <anup@brainfault.org>,
	Atish Patra <atishp@atishpatra.org>,
	James Morse <james.morse@arm.com>,
	Jing Zhang <jingzhangos@google.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm-riscv@lists.infradead.org, kvm@vger.kernel.org,
	Marc Zyngier <maz@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Peter Shier <pshier@google.com>,
	Raghavendra Rao Ananta <rananta@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Sean Christopherson <seanjc@google.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>
Subject: Re: [PATCH v5 07/13] KVM: arm64: Add support for userspace to suspend a vCPU
Date: Fri, 29 Apr 2022 03:42:28 +0000	[thread overview]
Message-ID: <YmtepGWYckmUKln+@google.com> (raw)
In-Reply-To: <CAAeT=Fx5Nb0EJ+6825fYxAxF9bK5DHOXNmJiSVGP=JVSbWuCrQ@mail.gmail.com>

On Thu, Apr 21, 2022 at 11:28:42PM -0700, Reiji Watanabe wrote:

[...]

> > > > +static void kvm_arm_vcpu_suspend(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +       vcpu->arch.mp_state.mp_state = KVM_MP_STATE_SUSPENDED;
> > > > +       kvm_make_request(KVM_REQ_SUSPEND, vcpu);
> > > > +       kvm_vcpu_kick(vcpu);
> > >
> > > Considering the patch 8 will remove the call to kvm_vcpu_kick()
> > > (BTW, I wonder why you wanted to make that change in the patch-8
> > > instead of the patch-7),
> >
> > Squashed the diff into the wrong patch! Marc pointed out this is of
> > course cargo-culted as I was following the pattern laid down by
> > KVM_REQ_SLEEP :)
> 
> I see. Thanks for the clarification !
> 
> > > it looks like we could use the mp_state
> > > KVM_MP_STATE_SUSPENDED instead of using KVM_REQ_SUSPEND.
> > > What is the reason why you prefer to introduce KVM_REQ_SUSPEND
> > > rather than simply using KVM_MP_STATE_SUSPENDED ?
> >
> > I was trying to avoid any heavy refactoring in adding new
> > functionality here, as we handle KVM_MP_STATE_STOPPED similarly (make
> > a request). ARM is definitely a bit different than x86 in the way that
> > we handle the MP states, as x86 doesn't bounce through vCPU requests
> > to do it and instead directly checks the mp_state value.
> 
> The difference from KVM_MP_STATE_STOPPED is that kvm_arm_vcpu_power_off()
> calls kvm_vcpu_kick(), which made me think having KVM_REQ_SLEEP was
> reasonable (it appears kvm_vcpu_kick() won't be needed there due to
> the same reason as kvm_arm_vcpu_suspend).

Just to finish the thought on this before mailing out what I hope is the
last take on all of this. I'm going to leave the pointless call to
kvm_vcpu_kick() in place, if only to follow the pattern of other MP
states.

That will all get cleaned up later on, as discussed :)

--
Thanks,
Oliver

  reply	other threads:[~2022-04-29  3:42 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-09 18:45 [PATCH v5 00/13] KVM: arm64: PSCI SYSTEM_SUSPEND support Oliver Upton
2022-04-09 18:45 ` Oliver Upton
2022-04-09 18:45 ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 01/13] KVM: arm64: Don't depend on fallthrough to hide SYSTEM_RESET2 Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-14  5:00   ` Reiji Watanabe
2022-04-14  5:00     ` Reiji Watanabe
2022-04-14  5:00     ` Reiji Watanabe
2022-04-09 18:45 ` [PATCH v5 02/13] KVM: arm64: Dedupe vCPU power off helpers Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 03/13] KVM: arm64: Track vCPU power state using MP state values Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-14  5:26   ` Reiji Watanabe
2022-04-14  5:26     ` Reiji Watanabe
2022-04-14  5:26     ` Reiji Watanabe
2022-04-21  3:31     ` Oliver Upton
2022-04-21  3:31       ` Oliver Upton
2022-04-21  3:31       ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 04/13] KVM: arm64: Rename the KVM_REQ_SLEEP handler Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 05/13] KVM: Create helper for setting a system event exit Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-14  5:40   ` Reiji Watanabe
2022-04-14  5:40     ` Reiji Watanabe
2022-04-14  5:40     ` Reiji Watanabe
2022-04-09 18:45 ` [PATCH v5 06/13] KVM: arm64: Return a value from check_vcpu_requests() Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-22  6:37   ` Reiji Watanabe
2022-04-22  6:37     ` Reiji Watanabe
2022-04-22  6:37     ` Reiji Watanabe
2022-04-09 18:45 ` [PATCH v5 07/13] KVM: arm64: Add support for userspace to suspend a vCPU Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-21  3:12   ` Reiji Watanabe
2022-04-21  3:12     ` Reiji Watanabe
2022-04-21  3:12     ` Reiji Watanabe
2022-04-21  3:23     ` Oliver Upton
2022-04-21  3:23       ` Oliver Upton
2022-04-21  3:23       ` Oliver Upton
2022-04-22  6:28       ` Reiji Watanabe
2022-04-22  6:28         ` Reiji Watanabe
2022-04-22  6:28         ` Reiji Watanabe
2022-04-29  3:42         ` Oliver Upton [this message]
2022-04-29  3:42           ` Oliver Upton
2022-04-29  3:42           ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 08/13] KVM: arm64: Implement PSCI SYSTEM_SUSPEND Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-22  7:02   ` Reiji Watanabe
2022-04-22  7:02     ` Reiji Watanabe
2022-04-22  7:02     ` Reiji Watanabe
2022-04-09 18:45 ` [PATCH v5 09/13] selftests: KVM: Rename psci_cpu_on_test to psci_test Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 10/13] selftests: KVM: Create helper for making SMCCC calls Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 11/13] selftests: KVM: Use KVM_SET_MP_STATE to power off vCPU in psci_test Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 12/13] selftests: KVM: Refactor psci_test to make it amenable to new tests Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45 ` [PATCH v5 13/13] selftests: KVM: Test SYSTEM_SUSPEND PSCI call Oliver Upton
2022-04-09 18:45   ` Oliver Upton
2022-04-09 18:45   ` Oliver Upton

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=YmtepGWYckmUKln+@google.com \
    --to=oupton@google.com \
    --cc=kvm-riscv@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.