From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Borntraeger Date: Mon, 27 Sep 2021 06:58:17 +0000 Subject: Re: [PATCH 05/14] KVM: s390: Clear valid_wakeup in kvm_s390_handle_wait(), not in arch hook Message-Id: List-Id: References: <20210925005528.1145584-1-seanjc@google.com> <20210925005528.1145584-6-seanjc@google.com> In-Reply-To: <20210925005528.1145584-6-seanjc@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Sean Christopherson , Marc Zyngier , Huacai Chen , Aleksandar Markovic , Paul Mackerras , Janosch Frank , Paolo Bonzini Cc: James Morse , Alexandru Elisei , Suzuki K Poulose , David Hildenbrand , Cornelia Huck , Claudio Imbrenda , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-kernel@vger.kernel.org, David Matlack , Jing Zhang Am 25.09.21 um 02:55 schrieb Sean Christopherson: > Move the clearing of valid_wakeup out of kvm_arch_vcpu_block_finish() so > that a future patch can drop said arch hook. Unlike the other blocking- > related arch hooks (vcpu_blocking/unblocking()), vcpu_block_finish() needs > to be called even if the KVM doesn't actually block the vCPU. This will > allow future patches to differentiate between truly blocking the vCPU and > emulating a halt condition without introducing a contradiction. > > Alternatively, the hook could be renamed to kvm_arch_vcpu_halt_finish(), > but there's literally one call site in s390, and future cleanup can also > be done to handle valid_wakeup fully within kvm_s390_handle_wait() and > allow generic KVM to drop vcpu_valid_wakeup(). > > No functional change intended. > > Signed-off-by: Sean Christopherson Reviewed-by: Christian Borntraeger > --- > arch/s390/kvm/interrupt.c | 1 + > arch/s390/kvm/kvm-s390.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > index 10722455fd02..520450a7956f 100644 > --- a/arch/s390/kvm/interrupt.c > +++ b/arch/s390/kvm/interrupt.c > @@ -1336,6 +1336,7 @@ int kvm_s390_handle_wait(struct kvm_vcpu *vcpu) > no_timer: > srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); > kvm_vcpu_block(vcpu); > + vcpu->valid_wakeup = false; > __unset_cpu_idle(vcpu); > vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 7cabe6778b1b..08ed68639a21 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -5082,7 +5082,7 @@ static inline unsigned long nonhyp_mask(int i) > > void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) > { > - vcpu->valid_wakeup = false; > + maybe just remove the line instead of adding an empty one? > } > > static int __init kvm_s390_init(void) >