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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 14205C433F5 for ; Tue, 11 Jan 2022 19:06:11 +0000 (UTC) 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=G3Q3/MqZcOvToKzXhRIBik+i54ovrZvVI2rjZQg+KGc=; b=ZoAS9RuCUoyC9Q QGbNQyrmtqoiEQwtaScueNse6I6s95ybobytuuvTPFzOHtO6+4wo7UZXx/r1XvQutVFxpIqHY8RwE 4q0J1aL2J0oEAeDy+RKZ/iLCGh7qVRa0+KmU10t7O5I0+FQza0x+N4IJXoAyMplwuZZhQv752tDAz PQHcCXCGKEjZ19fo3HCPNBiJAU0idVFgWfgzFlmd2byTFfJBLFJw2DJqrLufxvAtm1XUNPN+/MvX+ ag33hqhwVAKC4L3JpTwNxAmWD7xzrddMipy6C5F0WlicO4SSuZWs40JhgK2oJI5puogLI2yZ2ielz 9ARH/FYvu6Dc6EdKz4cg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7MRw-00HNWi-Rm; Tue, 11 Jan 2022 19:04:53 +0000 Received: from mail-pl1-x62e.google.com ([2607:f8b0:4864:20::62e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7MRs-00HNVj-LZ for linux-arm-kernel@lists.infradead.org; Tue, 11 Jan 2022 19:04:51 +0000 Received: by mail-pl1-x62e.google.com with SMTP id t18so190397plg.9 for ; Tue, 11 Jan 2022 11:04:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=K0voZAURqPJsMSPgWRWSbzQgUhtAjQkdnnmPm2jJT2w=; b=OBsot1EYclSvooVusPEFiHcqQS0tLiqutA1hLvc4Q8dJ1Ly3d4NacG3jUTcwT8b5B0 yAzAAMtSPazCmF1twB9t7p7OjthaBlEEVkPWyFLjoGrBIC4wlTb5/+rRXf0vM/6rqMCU Ragb7afH7stdPZ3KqbuRhWmkPsSooAAmEnsXlw63cnann8b5t+woTMjdUi9ErVLDZadL SBMqnGpNjCY4JuVHxg3BCfGS2f1fwERk57a6Mqygc7EBur2rRRWKrn5R3WR0uUengxin T+1Z9+vSV6mbJNk442DU5o4tAuCOGgn5t6ttzvE2XufqHuwrPcbXryB0l8VXzxOzfmXG EbOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=K0voZAURqPJsMSPgWRWSbzQgUhtAjQkdnnmPm2jJT2w=; b=DQjNZ86ShkSxg51uxARAocNOlujmzTJ/J151PrfjiEzBfLACvhoyjKUldHeYUXV3Rk y6bbZOVfnRxgRpN5+m69ieSZ/MyheWpQMl9BhhT+RRdYZhOlsGtJSEe656P2staKRW6O O1vP1OOq7S/JjgkVpd5xeg/0+QsmWj2rnUV+rlbgQngJOSqxzpvRnezcw0xV/E5c0mAu rczYl65nXTYGQ2F3Bllq5NpEi154eZQcZcQ6jpaTqP54L+KITdk9ylGH3oekSzZgAVja MWMvr684Jub9c6Iv0TjGhe7RZ17VSuQAHaHpMLWR/13VjuRUbOwv7T+cxFIY3cAvHZK3 3Fmw== X-Gm-Message-State: AOAM532WVDvdQ37XEcqA6EDylcf5xERekFs1XXlNjKBSlTXz0EZP4RoW IT326IKB8WROlJVdo7G+7XqWvA== X-Google-Smtp-Source: ABdhPJxSyV27y218zwiIgly8FGJNiKN5fGPK6Fudoin6YBhcWN0ZlQZF3CdSOjibQ9Gqz01uCEpKuA== X-Received: by 2002:a63:7c5e:: with SMTP id l30mr5246852pgn.297.1641927887737; Tue, 11 Jan 2022 11:04:47 -0800 (PST) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id l22sm11246197pfc.167.2022.01.11.11.04.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Jan 2022 11:04:47 -0800 (PST) Date: Tue, 11 Jan 2022 19:04:42 +0000 From: Sean Christopherson To: Raghavendra Rao Ananta Cc: Marc Zyngier , Andrew Jones , James Morse , Alexandru Elisei , Suzuki K Poulose , kvm@vger.kernel.org, Catalin Marinas , Peter Shier , linux-kernel@vger.kernel.org, Paolo Bonzini , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH v3 01/11] KVM: Capture VM start Message-ID: References: <20220104194918.373612-1-rananta@google.com> <20220104194918.373612-2-rananta@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220111_110448_734139_CE983A50 X-CRM114-Status: GOOD ( 33.13 ) 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 Tue, Jan 11, 2022, Raghavendra Rao Ananta wrote: > On Tue, Jan 11, 2022 at 9:36 AM Sean Christopherson wrote: > > In your proposed patch, KVM_RUN will take kvm->lock _every_ time. That introduces > > unnecessary contention as it will serialize this bit of code if multiple vCPUs > > are attempting KVM_RUN. By checking !vm_started, only the "first" KVM_RUN for a > > VM will acquire kvm->lock and thus avoid contention once the VM is up and running. > > There's still a possibility that multiple vCPUs will contend for kvm->lock on their > > first KVM_RUN, hence the quotes. I called it "naive" because it's possible there's > > a more elegant solution depending on the use case, e.g. a lockless approach might > > work (or it might not). > > > But is it safe to read kvm->vm_started without grabbing the lock in > the first place? Don't know, but that's my point. Without a consumer in generic KVM and due to my lack of arm64 knowledge, without a high-level description of how the flag will be used by arm64, it's really difficult to determine what's safe and what's not. For other architectures, it's an impossible question to answer because we don't know how the flag might be used. > use atomic_t maybe for this? No. An atomic_t is generally useful only if there are multiple writers that can possibly write different values. It's highly unlikely that simply switching to an atomic address the needs of arm64. > > > > > + kvm->vm_started = true; > > > > > + mutex_unlock(&kvm->lock); > > > > > > > > Lastly, why is this in generic KVM? > > > > > > > The v1 of the series originally had it in the arm specific code. > > > However, I was suggested to move it to the generic code since the book > > > keeping is not arch specific and could be helpful to others too [1]. > > > > I'm definitely in favor of moving/adding thing to generic KVM when it makes sense, > > but I'm skeptical in this particular case. The code _is_ arch specific in that > > arm64 apparently needs to acquire kvm->lock when checking if a vCPU has run, e.g. > > versus a hypothetical x86 use case that might be completely ok with a lockless > > implementation. And it's not obvious that there's a plausible, safe use case > > outside of arm64, e.g. on x86, there is very, very little that is truly shared > > across the entire VM/system, most things are per-thread/core/package in some way, > > shape, or form. In other words, I'm a wary of providing something like this for > > x86 because odds are good that any use will be functionally incorrect. > I've been going back and forth on this. I've seen a couple of > variables declared in the generic struct and used only in the arch > code. vcpu->valid_wakeup for instance, which is used only by s390 > arch. Maybe I'm looking at it the wrong way as to what can and can't > go in the generic kvm code. Ya, valid_wakeup is an oddball, I don't know why it's in kvm_vcpu instead of arch code that's wrapped with e.g. kvm_arch_vcpu_valid_wakeup(). That said, valid_wakeup is consumed by generic KVM, i.e. has well defined semantics for how it is used, so it's purely a "this code is rather odd" issue. vm_started on the other hand is only produced by generic KVM, and so its required semantics are unclear. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel