From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergey Senozhatsky Date: Fri, 04 Jun 2021 09:20:28 +0000 Subject: Re: [RFC][PATCH] kvm: add suspend pm-notifier Message-Id: List-Id: References: <20210603164315.682994-1-senozhatsky@chromium.org> <87v96uyq2v.wl-maz@kernel.org> In-Reply-To: <87v96uyq2v.wl-maz@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Marc Zyngier Cc: Sergey Senozhatsky , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Huacai Chen , Paul Mackerras , Christian Borntraeger , Suleiman Souhlal , x86@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-s390@vger.kernel.org On (21/06/04 09:46), Marc Zyngier wrote: [..] > > +void kvm_arch_pm_notifier(struct kvm *kvm) > > +{ > > +#ifdef CONFIG_PM > > + int c; > > + > > + mutex_lock(&kvm->lock); > > + for (c = 0; c < kvm->created_vcpus; c++) { > > + struct kvm_vcpu *vcpu = kvm->vcpus[c]; > > + int r; > > + > > + if (!vcpu) > > + continue; > > Wouldn't kvm_for_each_vcpu() avoid this kind of checks? Right, that's what I do in v2, which I haven't posted yet. [..] > > +#include > > + > > #ifndef KVM_MAX_VCPU_ID > > #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS > > #endif > > @@ -579,6 +581,10 @@ struct kvm { > > pid_t userspace_pid; > > unsigned int max_halt_poll_ns; > > u32 dirty_ring_size; > > + > > +#ifdef CONFIG_PM > > + struct notifier_block pm_notifier; > > +#endif > > I'd certainly like to be able to opt out from this on architectures > that do not implement anything useful in the PM callbacks. Well on the other hand PM-callbacks are harmless on those archs, they won't overload the __weak function. > Please consider making this an independent config option that individual > archs can buy into. Sure, I can take a look into this, but how is this better than __weak function? (that's a real question) [..] > > +#ifdef CONFIG_PM > > +static int kvm_pm_notifier_call(struct notifier_block *bl, > > + unsigned long state, > > + void *unused) > > +{ > > + struct kvm *kvm = container_of(bl, struct kvm, pm_notifier); > > + > > + switch (state) { > > + case PM_HIBERNATION_PREPARE: > > + case PM_SUSPEND_PREPARE: > > + kvm_arch_pm_notifier(kvm); > > How about passing the state to the notifier callback? I'd expect it to > be useful to do something on resume too. For different states we can have different kvm_arch functions instead. kvm_arch_pm_notifier() can be renamed to kvm_arch_suspend_notifier(), so that we don't need to have `switch (state)` in every arch-code. Then for resume/post resume states we can have kvm_arch_resume_notifier() arch functions. > > + break; > > + } > > + return NOTIFY_DONE; > > +} > > + > > +static void kvm_init_pm_notifier(struct kvm *kvm) > > +{ > > + kvm->pm_notifier.notifier_call = kvm_pm_notifier_call; > > + kvm->pm_notifier.priority = INT_MAX; > > How is this priority determined? Nothing magical here. I want this to be executed first, before we suspend ftrace, RCU and the like. Besides KVM is usually the last one to register its PM callbacks, so there can be something on the notifier list that returns NOTIFY_STOP_MASK in front of KVM PM-notifier list entry. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 72C5EC07E94 for ; Fri, 4 Jun 2021 15:29:14 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id D423D61404 for ; Fri, 4 Jun 2021 15:29:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D423D61404 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 4944A4B0FB; Fri, 4 Jun 2021 11:29:13 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@chromium.org Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CXdJb4xMkCR2; Fri, 4 Jun 2021 11:29:09 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 073B84B101; Fri, 4 Jun 2021 11:29:09 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id D618F4B0C4 for ; Fri, 4 Jun 2021 05:20:40 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bsa8BOfl3nOm for ; Fri, 4 Jun 2021 05:20:36 -0400 (EDT) Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id A68AB4B0C1 for ; Fri, 4 Jun 2021 05:20:36 -0400 (EDT) Received: by mail-pj1-f41.google.com with SMTP id fy24-20020a17090b0218b029016c5a59021fso562062pjb.0 for ; Fri, 04 Jun 2021 02:20:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=mBAEhjZZRv7Jcy7Zua0pi2TsECX3PFBDfNxkyFGsLO0=; b=GFzrB6mDdP8QDUkM5E2D3SpI1YuNW543kJ1Zx1kk0NuEJ9dY60dkreacgqq7B7HMEs uqcy1p2HaKtSeecuRPdMc+ZpGU/OEc3CdNONynbGrv/3vMYyT7bl4zUw0YUQ9/GOQcn/ p9MzmRmMsJwpomp+rOKp0w+JG2E9nloY+xf18= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=mBAEhjZZRv7Jcy7Zua0pi2TsECX3PFBDfNxkyFGsLO0=; b=ZQaDotc/iiIWYi83tzHYdM73jdcnFx34bQDRT/QsgoGl0iDClVD9LhR4625NJLFPId TZ6eclYDw/vCcXeOeainFFbKVw+29GRGDC2OqjJdAmhpr6vw7YnBE0/ojlS37ahH7lF8 Tnk1YbSqI+KBRhn4zitKxSb6Tuun6DWPFUtrRDElVqDMvT/mGtH8BrdQlKXeuCSEVk2F b7vVRMEzWfbOHQukhBFKfHem7tCBiTUS/xOelvBvCiTTc4NLC6oyBww5rkgeeX5RcAJ5 bRA+lW56ZmW63XKazQe/71FwrcNK1ALb02huxUEhAfQ1Wi4d4IVnmNMy1kk31pTqvR2x lIdg== X-Gm-Message-State: AOAM532ygLjJze74yTTOhlY3k4vBDUGJPV4boS/VjFTCf7kd3m+c0k2Z 48FZy4gRHO5vz5bBbX2Nj/vshw== X-Google-Smtp-Source: ABdhPJxqC/sFmH8NWFwbR+x7mytZyDKEx7TFmgf8/+UGv7fZox1XEZk3IYhTIOf3hSBOmUz/xertuQ== X-Received: by 2002:a17:90b:607:: with SMTP id gb7mr3845129pjb.5.1622798435671; Fri, 04 Jun 2021 02:20:35 -0700 (PDT) Received: from google.com ([2409:10:2e40:5100:36b:f5b6:c380:9ccf]) by smtp.gmail.com with ESMTPSA id p19sm4236799pjv.21.2021.06.04.02.20.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Jun 2021 02:20:34 -0700 (PDT) Date: Fri, 4 Jun 2021 18:20:28 +0900 From: Sergey Senozhatsky To: Marc Zyngier Subject: Re: [RFC][PATCH] kvm: add suspend pm-notifier Message-ID: References: <20210603164315.682994-1-senozhatsky@chromium.org> <87v96uyq2v.wl-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87v96uyq2v.wl-maz@kernel.org> X-Mailman-Approved-At: Fri, 04 Jun 2021 11:29:07 -0400 Cc: linux-s390@vger.kernel.org, x86@kernel.org, kvm@vger.kernel.org, Christian Borntraeger , Sean Christopherson , Huacai Chen , linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-mips@vger.kernel.org, Paul Mackerras , Sergey Senozhatsky , Suleiman Souhlal , Paolo Bonzini , Vitaly Kuznetsov , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Jim Mattson X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On (21/06/04 09:46), Marc Zyngier wrote: [..] > > +void kvm_arch_pm_notifier(struct kvm *kvm) > > +{ > > +#ifdef CONFIG_PM > > + int c; > > + > > + mutex_lock(&kvm->lock); > > + for (c = 0; c < kvm->created_vcpus; c++) { > > + struct kvm_vcpu *vcpu = kvm->vcpus[c]; > > + int r; > > + > > + if (!vcpu) > > + continue; > > Wouldn't kvm_for_each_vcpu() avoid this kind of checks? Right, that's what I do in v2, which I haven't posted yet. [..] > > +#include > > + > > #ifndef KVM_MAX_VCPU_ID > > #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS > > #endif > > @@ -579,6 +581,10 @@ struct kvm { > > pid_t userspace_pid; > > unsigned int max_halt_poll_ns; > > u32 dirty_ring_size; > > + > > +#ifdef CONFIG_PM > > + struct notifier_block pm_notifier; > > +#endif > > I'd certainly like to be able to opt out from this on architectures > that do not implement anything useful in the PM callbacks. Well on the other hand PM-callbacks are harmless on those archs, they won't overload the __weak function. > Please consider making this an independent config option that individual > archs can buy into. Sure, I can take a look into this, but how is this better than __weak function? (that's a real question) [..] > > +#ifdef CONFIG_PM > > +static int kvm_pm_notifier_call(struct notifier_block *bl, > > + unsigned long state, > > + void *unused) > > +{ > > + struct kvm *kvm = container_of(bl, struct kvm, pm_notifier); > > + > > + switch (state) { > > + case PM_HIBERNATION_PREPARE: > > + case PM_SUSPEND_PREPARE: > > + kvm_arch_pm_notifier(kvm); > > How about passing the state to the notifier callback? I'd expect it to > be useful to do something on resume too. For different states we can have different kvm_arch functions instead. kvm_arch_pm_notifier() can be renamed to kvm_arch_suspend_notifier(), so that we don't need to have `switch (state)` in every arch-code. Then for resume/post resume states we can have kvm_arch_resume_notifier() arch functions. > > + break; > > + } > > + return NOTIFY_DONE; > > +} > > + > > +static void kvm_init_pm_notifier(struct kvm *kvm) > > +{ > > + kvm->pm_notifier.notifier_call = kvm_pm_notifier_call; > > + kvm->pm_notifier.priority = INT_MAX; > > How is this priority determined? Nothing magical here. I want this to be executed first, before we suspend ftrace, RCU and the like. Besides KVM is usually the last one to register its PM callbacks, so there can be something on the notifier list that returns NOTIFY_STOP_MASK in front of KVM PM-notifier list entry. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 97D06C47083 for ; Fri, 4 Jun 2021 09:21:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8175E610A2 for ; Fri, 4 Jun 2021 09:21:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230136AbhFDJXV (ORCPT ); Fri, 4 Jun 2021 05:23:21 -0400 Received: from mail-pj1-f52.google.com ([209.85.216.52]:38530 "EHLO mail-pj1-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230124AbhFDJXV (ORCPT ); Fri, 4 Jun 2021 05:23:21 -0400 Received: by mail-pj1-f52.google.com with SMTP id m13-20020a17090b068db02901656cc93a75so7089239pjz.3 for ; Fri, 04 Jun 2021 02:21:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=mBAEhjZZRv7Jcy7Zua0pi2TsECX3PFBDfNxkyFGsLO0=; b=GFzrB6mDdP8QDUkM5E2D3SpI1YuNW543kJ1Zx1kk0NuEJ9dY60dkreacgqq7B7HMEs uqcy1p2HaKtSeecuRPdMc+ZpGU/OEc3CdNONynbGrv/3vMYyT7bl4zUw0YUQ9/GOQcn/ p9MzmRmMsJwpomp+rOKp0w+JG2E9nloY+xf18= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=mBAEhjZZRv7Jcy7Zua0pi2TsECX3PFBDfNxkyFGsLO0=; b=rbEqwcfkS9B7K51+fbI2jNMPezNRjPJAVebLZ0BTPiTcEiSOuq9qSn84BiyJP7jRSn bFnW1uBQfdM4SAOTAkq0Lp4XVgJaXud/4kHo90kTefwP8qD7LkIBogAxdWEjbNRi5908 5qiV0aV6uUzrF6qVQtLq40slKyUYu7a2aKgw3rTtL+CBkNkv7VyHcNciY2SPGV3VAFdi fny0VyBmKgaHdshytM42q6S7eMGBQ04ItlgI8u8y40SSaDSXVPjgkr7c0O3cf+x9AG75 bs0FIvHjpE+TZ/4n77odtV4ZkKoLQjmnDQM6KHayUsDIpEvzrnbbi050AJi2DbIGCa5a mPcw== X-Gm-Message-State: AOAM531Z1bCYmtXxWbczHosssVofx9y8DkGw+F7GNm5GVLuEYDJmjYAn rIgtjpV0LjKMQdNzWwx+zqCIEA== X-Google-Smtp-Source: ABdhPJxqC/sFmH8NWFwbR+x7mytZyDKEx7TFmgf8/+UGv7fZox1XEZk3IYhTIOf3hSBOmUz/xertuQ== X-Received: by 2002:a17:90b:607:: with SMTP id gb7mr3845129pjb.5.1622798435671; Fri, 04 Jun 2021 02:20:35 -0700 (PDT) Received: from google.com ([2409:10:2e40:5100:36b:f5b6:c380:9ccf]) by smtp.gmail.com with ESMTPSA id p19sm4236799pjv.21.2021.06.04.02.20.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Jun 2021 02:20:34 -0700 (PDT) Date: Fri, 4 Jun 2021 18:20:28 +0900 From: Sergey Senozhatsky To: Marc Zyngier Cc: Sergey Senozhatsky , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Huacai Chen , Paul Mackerras , Christian Borntraeger , Suleiman Souhlal , x86@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-s390@vger.kernel.org Subject: Re: [RFC][PATCH] kvm: add suspend pm-notifier Message-ID: References: <20210603164315.682994-1-senozhatsky@chromium.org> <87v96uyq2v.wl-maz@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87v96uyq2v.wl-maz@kernel.org> Precedence: bulk List-ID: X-Mailing-List: linux-mips@vger.kernel.org On (21/06/04 09:46), Marc Zyngier wrote: [..] > > +void kvm_arch_pm_notifier(struct kvm *kvm) > > +{ > > +#ifdef CONFIG_PM > > + int c; > > + > > + mutex_lock(&kvm->lock); > > + for (c = 0; c < kvm->created_vcpus; c++) { > > + struct kvm_vcpu *vcpu = kvm->vcpus[c]; > > + int r; > > + > > + if (!vcpu) > > + continue; > > Wouldn't kvm_for_each_vcpu() avoid this kind of checks? Right, that's what I do in v2, which I haven't posted yet. [..] > > +#include > > + > > #ifndef KVM_MAX_VCPU_ID > > #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS > > #endif > > @@ -579,6 +581,10 @@ struct kvm { > > pid_t userspace_pid; > > unsigned int max_halt_poll_ns; > > u32 dirty_ring_size; > > + > > +#ifdef CONFIG_PM > > + struct notifier_block pm_notifier; > > +#endif > > I'd certainly like to be able to opt out from this on architectures > that do not implement anything useful in the PM callbacks. Well on the other hand PM-callbacks are harmless on those archs, they won't overload the __weak function. > Please consider making this an independent config option that individual > archs can buy into. Sure, I can take a look into this, but how is this better than __weak function? (that's a real question) [..] > > +#ifdef CONFIG_PM > > +static int kvm_pm_notifier_call(struct notifier_block *bl, > > + unsigned long state, > > + void *unused) > > +{ > > + struct kvm *kvm = container_of(bl, struct kvm, pm_notifier); > > + > > + switch (state) { > > + case PM_HIBERNATION_PREPARE: > > + case PM_SUSPEND_PREPARE: > > + kvm_arch_pm_notifier(kvm); > > How about passing the state to the notifier callback? I'd expect it to > be useful to do something on resume too. For different states we can have different kvm_arch functions instead. kvm_arch_pm_notifier() can be renamed to kvm_arch_suspend_notifier(), so that we don't need to have `switch (state)` in every arch-code. Then for resume/post resume states we can have kvm_arch_resume_notifier() arch functions. > > + break; > > + } > > + return NOTIFY_DONE; > > +} > > + > > +static void kvm_init_pm_notifier(struct kvm *kvm) > > +{ > > + kvm->pm_notifier.notifier_call = kvm_pm_notifier_call; > > + kvm->pm_notifier.priority = INT_MAX; > > How is this priority determined? Nothing magical here. I want this to be executed first, before we suspend ftrace, RCU and the like. Besides KVM is usually the last one to register its PM callbacks, so there can be something on the notifier list that returns NOTIFY_STOP_MASK in front of KVM PM-notifier list entry. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DAAB7C07E94 for ; Fri, 4 Jun 2021 09:28:04 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A872061405 for ; Fri, 4 Jun 2021 09:28:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A872061405 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Jkcc98AN+2xOr1AquV4joAoUqLLinpmlAbCwjA4mf+k=; b=OlXXXDnSl9UvCv VIy+5gObaDkaYb1nWM/8eKO7FHnr/1iReJOv/MqAddOlihuEvMSsBGqicJUxeIzoBBeaPATwiZxvl jYDf6dSGHAM426hfygut+Gq5THbmPvCVzFoyzdgyQgmcyTF1AEqDhKQfbCm4gOVnJPXshPbib0BXl /abZQvdIU1j7ZoJPKP/qCNPgSpy6PxTzyIddk9POjoJrYpH7OqpOTVJ/72FLVrHQ2KwoIeZmbrJ2z NE/3sQ219sWhf/Jg/sV7/kbT5nkaDfCnKdov4iwrV8eCGBGx82YQj+sZWZAgfrY51tHjLYC0+eZod NO2Tfn6JMwZ4yVfyoY5A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lp65M-00CbK0-RA; Fri, 04 Jun 2021 09:25:51 +0000 Received: from mail-pl1-f179.google.com ([209.85.214.179]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lp61I-00CZiJ-It for linux-arm-kernel@lists.infradead.org; Fri, 04 Jun 2021 09:21:40 +0000 Received: by mail-pl1-f179.google.com with SMTP id 69so4331063plc.5 for ; Fri, 04 Jun 2021 02:21:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=mBAEhjZZRv7Jcy7Zua0pi2TsECX3PFBDfNxkyFGsLO0=; b=GFzrB6mDdP8QDUkM5E2D3SpI1YuNW543kJ1Zx1kk0NuEJ9dY60dkreacgqq7B7HMEs uqcy1p2HaKtSeecuRPdMc+ZpGU/OEc3CdNONynbGrv/3vMYyT7bl4zUw0YUQ9/GOQcn/ p9MzmRmMsJwpomp+rOKp0w+JG2E9nloY+xf18= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=mBAEhjZZRv7Jcy7Zua0pi2TsECX3PFBDfNxkyFGsLO0=; b=kHpY5kTRtdnwSEsqO8F2rweWTNeiKeA327CF+P+7u4k0dh7XFpHn0PB65F55dZAtep GjOsSw7bnbB0r1sVQ0kHc267cEwhUCqjOMiunVnZClrd9SRhFq4njB80hGplx1clhGIY l4PEZ3BOuoabyy5cdlqJy9oaPHjJh8hlYdM69GMz557bb+eEgq9/rBy8ct3tQaAwB+NO ovLBOXMQoFKVYChLTeEsy+M6EZOgby9rmmwjPGILdr8+R3a4IcRtnTPl8LxPxBkxexIq SnRSJpzlhEpIunofLAvH9e+Wdvrhy4qPttTzDCieO+MqBGJYYtdARg4DZU/nX0yx9mKE r0dA== X-Gm-Message-State: AOAM532YtXvBBbbTm3htMt3RIsGc/9cvA5YOxv9FGhW3KjZ8NqoJ4DZA K93y/rrEX1kgrnZ+qs5kDUzrmA== X-Google-Smtp-Source: ABdhPJxqC/sFmH8NWFwbR+x7mytZyDKEx7TFmgf8/+UGv7fZox1XEZk3IYhTIOf3hSBOmUz/xertuQ== X-Received: by 2002:a17:90b:607:: with SMTP id gb7mr3845129pjb.5.1622798435671; Fri, 04 Jun 2021 02:20:35 -0700 (PDT) Received: from google.com ([2409:10:2e40:5100:36b:f5b6:c380:9ccf]) by smtp.gmail.com with ESMTPSA id p19sm4236799pjv.21.2021.06.04.02.20.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Jun 2021 02:20:34 -0700 (PDT) Date: Fri, 4 Jun 2021 18:20:28 +0900 From: Sergey Senozhatsky To: Marc Zyngier Cc: Sergey Senozhatsky , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Huacai Chen , Paul Mackerras , Christian Borntraeger , Suleiman Souhlal , x86@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-s390@vger.kernel.org Subject: Re: [RFC][PATCH] kvm: add suspend pm-notifier Message-ID: References: <20210603164315.682994-1-senozhatsky@chromium.org> <87v96uyq2v.wl-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87v96uyq2v.wl-maz@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210604_022136_668881_FDA0872A X-CRM114-Status: GOOD ( 21.18 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On (21/06/04 09:46), Marc Zyngier wrote: [..] > > +void kvm_arch_pm_notifier(struct kvm *kvm) > > +{ > > +#ifdef CONFIG_PM > > + int c; > > + > > + mutex_lock(&kvm->lock); > > + for (c = 0; c < kvm->created_vcpus; c++) { > > + struct kvm_vcpu *vcpu = kvm->vcpus[c]; > > + int r; > > + > > + if (!vcpu) > > + continue; > > Wouldn't kvm_for_each_vcpu() avoid this kind of checks? Right, that's what I do in v2, which I haven't posted yet. [..] > > +#include > > + > > #ifndef KVM_MAX_VCPU_ID > > #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS > > #endif > > @@ -579,6 +581,10 @@ struct kvm { > > pid_t userspace_pid; > > unsigned int max_halt_poll_ns; > > u32 dirty_ring_size; > > + > > +#ifdef CONFIG_PM > > + struct notifier_block pm_notifier; > > +#endif > > I'd certainly like to be able to opt out from this on architectures > that do not implement anything useful in the PM callbacks. Well on the other hand PM-callbacks are harmless on those archs, they won't overload the __weak function. > Please consider making this an independent config option that individual > archs can buy into. Sure, I can take a look into this, but how is this better than __weak function? (that's a real question) [..] > > +#ifdef CONFIG_PM > > +static int kvm_pm_notifier_call(struct notifier_block *bl, > > + unsigned long state, > > + void *unused) > > +{ > > + struct kvm *kvm = container_of(bl, struct kvm, pm_notifier); > > + > > + switch (state) { > > + case PM_HIBERNATION_PREPARE: > > + case PM_SUSPEND_PREPARE: > > + kvm_arch_pm_notifier(kvm); > > How about passing the state to the notifier callback? I'd expect it to > be useful to do something on resume too. For different states we can have different kvm_arch functions instead. kvm_arch_pm_notifier() can be renamed to kvm_arch_suspend_notifier(), so that we don't need to have `switch (state)` in every arch-code. Then for resume/post resume states we can have kvm_arch_resume_notifier() arch functions. > > + break; > > + } > > + return NOTIFY_DONE; > > +} > > + > > +static void kvm_init_pm_notifier(struct kvm *kvm) > > +{ > > + kvm->pm_notifier.notifier_call = kvm_pm_notifier_call; > > + kvm->pm_notifier.priority = INT_MAX; > > How is this priority determined? Nothing magical here. I want this to be executed first, before we suspend ftrace, RCU and the like. Besides KVM is usually the last one to register its PM callbacks, so there can be something on the notifier list that returns NOTIFY_STOP_MASK in front of KVM PM-notifier list entry. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel