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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4DCA8C4332F for ; Thu, 28 Oct 2021 15:34:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 315A5610C7 for ; Thu, 28 Oct 2021 15:34:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230281AbhJ1Pgy (ORCPT ); Thu, 28 Oct 2021 11:36:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46930 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230183AbhJ1Pgx (ORCPT ); Thu, 28 Oct 2021 11:36:53 -0400 Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0FC32C061767 for ; Thu, 28 Oct 2021 08:34:26 -0700 (PDT) Received: by mail-pg1-x529.google.com with SMTP id g184so6734399pgc.6 for ; Thu, 28 Oct 2021 08:34:26 -0700 (PDT) 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=Dh2oRVWq7FNB4VxeeBx+DY+B7J6W36/D0YFmfA4JinPWULNj2JAyVIdGN5W95l+d90 4Uu/ZwJdWUivmqUAxVTynE14t1JCgr4B8VIlvCnfQRBjV/9sZBN9FbtVQViBrcsIkfKk FKX6ULIrvBdeMPobrWIP7p55k83rg/EH2wU4GD4W9e2j7rIISQUbKASJH6T77kFM7m0Z Qtlmy8wjfeMDEMJrWpV2K5PFqGDN6VxYLPZtUxrtHEs4QyogxcvUebO7UVfiBHutG04k U0NkfKu6Je8Pt6shzy+xbyjZteg84KZAIk3v/fR91ZfKR/MAff+MA9k1aMrDIiCOqEqs s/bw== 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=cGatOcopayXA43Uhhddtl8SJsQNveDBfdGh7JeI7z8quJHl3vPYL8Fy+Hu6/ZYsRU0 qdJJe59gxgqclXQPAydhpFiG5VQUMn5dCVfcswMCGSoulXfbwIR5vWHliBTlRxYn4SlR RZLi5n9gStI9zpmKO6f+ja7MPsWY2O1TUzby/Wh4frBWRCeNajdil4jEHh1hNqYA03LM wOdDcxQKaolH365unGTLUUJPEPgIjtYuYv7aFrEdqhIw8/luCTYrDwRDLsOoPxUS5UZJ 9ikhSdaZxBSjjKdlwt2BG+NO4/x09pt2SSLFpHvSa4c2+ya8HK2v1Qz3L8Y/SYkiZNdE RL2w== X-Gm-Message-State: AOAM530RR9DBIrw2jqyywwVe2o5v3UA+OBbhkwo6M1aOHJqWpZFJ5BQ2 89HLNCwt//r5fvF5jnWdjsEggw== X-Google-Smtp-Source: ABdhPJz6jdrYVfnN/6E/mwaWkSEy+zhYc5xKvC8pKY4AVCJGv9Uh9d4/rKPGgqsum0tyEq6Ye9Z/qw== X-Received: by 2002:aa7:9427:0:b0:47c:3b8e:5253 with SMTP id y7-20020aa79427000000b0047c3b8e5253mr5166508pfo.69.1635435265242; Thu, 28 Oct 2021 08:34:25 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id p12sm4586055pfh.52.2021.10.28.08.34.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Oct 2021 08:34:24 -0700 (PDT) Date: Thu, 28 Oct 2021 15:34:21 +0000 From: Sean Christopherson To: Maxim Levitsky Cc: Marc Zyngier , Huacai Chen , Aleksandar Markovic , Paul Mackerras , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Paolo Bonzini , James Morse , Alexandru Elisei , Suzuki K Poulose , Atish Patra , 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, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, David Matlack , Oliver Upton , Jing Zhang Subject: Re: [PATCH v2 21/43] KVM: VMX: Clean up PI pre/post-block WARNs Message-ID: References: <20211009021236.4122790-1-seanjc@google.com> <20211009021236.4122790-22-seanjc@google.com> <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 28, 2021, Maxim Levitsky wrote: > On Fri, 2021-10-08 at 19:12 -0700, Sean Christopherson wrote: > > Move the WARN sanity checks out of the PI descriptor update loop so as > > not to spam the kernel log if the condition is violated and the update > > takes multiple attempts due to another writer. This also eliminates a > > few extra uops from the retry path. > > > > Technically not checking every attempt could mean KVM will now fail to > > WARN in a scenario that would have failed before, but any such failure > > would be inherently racy as some other agent (CPU or device) would have > > to concurrent modify the PI descriptor. ... > Don't know for sure if this is desired. I'll would just use WARN_ON_ONCE instead > if the warning spams the log. > > If there is a race I would rather want to catch it even if rare. Paolo had similar concerns[*]. I copied the most relevant part of the discussion below, let me know if you object to the outcome. Thanks for the reviews! [*] https://lore.kernel.org/all/YXllGfrjPX1pVUx6@google.com/T/#u On Wed, Oct 27, 2021 at 8:38 AM Paolo Bonzini wrote: > On 27/10/21 17:28, Sean Christopherson wrote: > > On Wed, Oct 27, 2021, Paolo Bonzini wrote: > > > On 27/10/21 16:41, Sean Christopherson wrote: > > > > The other thing I don't like about having the WARN in the loop is that it suggests > > > > that something other than the vCPU can modify the NDST and SN fields, which is > > > > wrong and confusing (for me). > > > > > > Yeah, I can agree with that. Can you add it in a comment above the cmpxchg > > > loop, it can be as simple as > > > > > > /* The processor can set ON concurrently. */ > > > > > > when you respin patch 21 and the rest of the series? > > > > I can definitely add a comment, but I think that comment is incorrect. > > It's completely backwards indeed. I first had "the hardware" and then > shut down my brain for a second to replace it. > > > So something like this? > > > > /* ON can be set concurrently by a different vCPU or by hardware. */ > > Yes, of course. 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67583C433EF for ; Thu, 28 Oct 2021 15:34:40 +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 1FFB0610C7 for ; Thu, 28 Oct 2021 15:34:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 1FFB0610C7 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=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=mzANhvjhvG7QML8GTePBmqS6RqebL1gHVKU5VGAm2ak=; b=y+XjgKU0kqKtgz nqKqkm4UtQgETmyNhEkYna3+vTPxP0zDGT2ZNs1PEXn6Ha5gqW2A81Pxc8JfBs04TXMX+wXSFwupU F2r5Uh1S51tCrcOFS81gICRpxFDrC0qJUb69braQnhlsORQAg8PW/TFbkdWlGl4GMiVgLYcuJEPkS QSRieyDFpmpIlkUGzGRhDGbwq0Rnzu9GgNGEy6XgdYH5wF0VsHb/KpgpXitlLyMW7Tfzl8K95l51q rMAu7ckHykkQLOV57JRbY+5I6ZxuazmTOyoDZn7tmbaEVlWYN9npzF5R/3OAtwgQ5sdEiDvFWXzg/ nU0thxV6iu+rtwVwwTzw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mg7QD-008PsP-IT; Thu, 28 Oct 2021 15:34:29 +0000 Received: from mail-pf1-x42e.google.com ([2607:f8b0:4864:20::42e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mg7QA-008Pq8-OJ for linux-riscv@lists.infradead.org; Thu, 28 Oct 2021 15:34:28 +0000 Received: by mail-pf1-x42e.google.com with SMTP id t184so6385227pfd.0 for ; Thu, 28 Oct 2021 08:34:26 -0700 (PDT) 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=Dh2oRVWq7FNB4VxeeBx+DY+B7J6W36/D0YFmfA4JinPWULNj2JAyVIdGN5W95l+d90 4Uu/ZwJdWUivmqUAxVTynE14t1JCgr4B8VIlvCnfQRBjV/9sZBN9FbtVQViBrcsIkfKk FKX6ULIrvBdeMPobrWIP7p55k83rg/EH2wU4GD4W9e2j7rIISQUbKASJH6T77kFM7m0Z Qtlmy8wjfeMDEMJrWpV2K5PFqGDN6VxYLPZtUxrtHEs4QyogxcvUebO7UVfiBHutG04k U0NkfKu6Je8Pt6shzy+xbyjZteg84KZAIk3v/fR91ZfKR/MAff+MA9k1aMrDIiCOqEqs s/bw== 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=21rXaHqLUOD0MxojpaQV32Fvxfd9/vQjLa5GsajKfkVALJNdYlS4/o6RrypEZMRqcg NqvjwRBe5fbKmvhW5qot6e9HSTLpvrooFTIgtlXXpgSLIEKNOaoyhwSZoyfjCFtfiEyu /fMFfMK7QAdpyXXcs505IkUEi364KIRflFaYpGdDANZXnQmSgptRvV+TrMpLH4iEbPiM FzTuBmOapk6hJ4C3CUQE1xax2ElnzCVOOZ8GSgtRvAIjWkLiAdbOxGMAsUpDf0BVvR1Q RN7L2Zy7fA3kexL9EQnaFpLss7LOHaV4NbprccJOt0/HTr37KVIICAfB2VrQm9lOSn7c E5oQ== X-Gm-Message-State: AOAM530MDijonYGTFFlW1lkTj9RXmcl/JezWsz/aDzBMH7HT1FhMNqso HV2D7GAkTmVpbMm3xdEBKan1rg== X-Google-Smtp-Source: ABdhPJz6jdrYVfnN/6E/mwaWkSEy+zhYc5xKvC8pKY4AVCJGv9Uh9d4/rKPGgqsum0tyEq6Ye9Z/qw== X-Received: by 2002:aa7:9427:0:b0:47c:3b8e:5253 with SMTP id y7-20020aa79427000000b0047c3b8e5253mr5166508pfo.69.1635435265242; Thu, 28 Oct 2021 08:34:25 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id p12sm4586055pfh.52.2021.10.28.08.34.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Oct 2021 08:34:24 -0700 (PDT) Date: Thu, 28 Oct 2021 15:34:21 +0000 From: Sean Christopherson To: Maxim Levitsky Cc: Marc Zyngier , Huacai Chen , Aleksandar Markovic , Paul Mackerras , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Paolo Bonzini , James Morse , Alexandru Elisei , Suzuki K Poulose , Atish Patra , 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, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, David Matlack , Oliver Upton , Jing Zhang Subject: Re: [PATCH v2 21/43] KVM: VMX: Clean up PI pre/post-block WARNs Message-ID: References: <20211009021236.4122790-1-seanjc@google.com> <20211009021236.4122790-22-seanjc@google.com> <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211028_083426_826774_298EC9DD X-CRM114-Status: GOOD ( 29.11 ) X-BeenThere: linux-riscv@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-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Thu, Oct 28, 2021, Maxim Levitsky wrote: > On Fri, 2021-10-08 at 19:12 -0700, Sean Christopherson wrote: > > Move the WARN sanity checks out of the PI descriptor update loop so as > > not to spam the kernel log if the condition is violated and the update > > takes multiple attempts due to another writer. This also eliminates a > > few extra uops from the retry path. > > > > Technically not checking every attempt could mean KVM will now fail to > > WARN in a scenario that would have failed before, but any such failure > > would be inherently racy as some other agent (CPU or device) would have > > to concurrent modify the PI descriptor. ... > Don't know for sure if this is desired. I'll would just use WARN_ON_ONCE instead > if the warning spams the log. > > If there is a race I would rather want to catch it even if rare. Paolo had similar concerns[*]. I copied the most relevant part of the discussion below, let me know if you object to the outcome. Thanks for the reviews! [*] https://lore.kernel.org/all/YXllGfrjPX1pVUx6@google.com/T/#u On Wed, Oct 27, 2021 at 8:38 AM Paolo Bonzini wrote: > On 27/10/21 17:28, Sean Christopherson wrote: > > On Wed, Oct 27, 2021, Paolo Bonzini wrote: > > > On 27/10/21 16:41, Sean Christopherson wrote: > > > > The other thing I don't like about having the WARN in the loop is that it suggests > > > > that something other than the vCPU can modify the NDST and SN fields, which is > > > > wrong and confusing (for me). > > > > > > Yeah, I can agree with that. Can you add it in a comment above the cmpxchg > > > loop, it can be as simple as > > > > > > /* The processor can set ON concurrently. */ > > > > > > when you respin patch 21 and the rest of the series? > > > > I can definitely add a comment, but I think that comment is incorrect. > > It's completely backwards indeed. I first had "the hardware" and then > shut down my brain for a second to replace it. > > > So something like this? > > > > /* ON can be set concurrently by a different vCPU or by hardware. */ > > Yes, of course. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0CB96C433F5 for ; Thu, 28 Oct 2021 15:34:31 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 7F803610CF for ; Thu, 28 Oct 2021 15:34:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 7F803610CF Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id EFBB24B175; Thu, 28 Oct 2021 11:34:29 -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=@google.com 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 s1hy9ClNC5VK; Thu, 28 Oct 2021 11:34:28 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id E16794B1E3; Thu, 28 Oct 2021 11:34:28 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 8B1584B1DF for ; Thu, 28 Oct 2021 11:34:27 -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 we-iu9lbz2Ry for ; Thu, 28 Oct 2021 11:34:26 -0400 (EDT) Received: from mail-pg1-f169.google.com (mail-pg1-f169.google.com [209.85.215.169]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 5E4364B1DA for ; Thu, 28 Oct 2021 11:34:26 -0400 (EDT) Received: by mail-pg1-f169.google.com with SMTP id c4so6717144pgv.11 for ; Thu, 28 Oct 2021 08:34:26 -0700 (PDT) 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=Dh2oRVWq7FNB4VxeeBx+DY+B7J6W36/D0YFmfA4JinPWULNj2JAyVIdGN5W95l+d90 4Uu/ZwJdWUivmqUAxVTynE14t1JCgr4B8VIlvCnfQRBjV/9sZBN9FbtVQViBrcsIkfKk FKX6ULIrvBdeMPobrWIP7p55k83rg/EH2wU4GD4W9e2j7rIISQUbKASJH6T77kFM7m0Z Qtlmy8wjfeMDEMJrWpV2K5PFqGDN6VxYLPZtUxrtHEs4QyogxcvUebO7UVfiBHutG04k U0NkfKu6Je8Pt6shzy+xbyjZteg84KZAIk3v/fR91ZfKR/MAff+MA9k1aMrDIiCOqEqs s/bw== 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=L5BXbm46P2lrcEGqgPHhh0biyE/rQlVIo+SjIWsCEpReX1LLfQsF5KA3TRicEkujox vEcJXds8TgZ0CSKBDLoMcRkNVWUY9cRwgBpVEhTC5rbQ1iJEXPpGR5q0MJUligEwML+P NAivVGZy2gQtYICPQLTYs/3AFd6xbGZXr8uHyqxEIzktIGXm+xMqa6OdBcZSYSfiKo42 2/9VK/xnRBaeC6uYKF3thvcdRRFTHxynbBuPZ8iHFNdCIuB0UTOht8MYhm5gSxZHUevR CijcM9qO6it7Nk3E5zNlfFTzYXEaymPeGYuJTaGH53CcBkTi0BTsdz2HEfpAb/qqwNra GI3g== X-Gm-Message-State: AOAM533sCTmGK+Eb/Ciu/yf4P5NsrM23oH50zkZbG745sCDcyc29n1td H+eMISQfXvOjrihUwrYxAQ9J3g== X-Google-Smtp-Source: ABdhPJz6jdrYVfnN/6E/mwaWkSEy+zhYc5xKvC8pKY4AVCJGv9Uh9d4/rKPGgqsum0tyEq6Ye9Z/qw== X-Received: by 2002:aa7:9427:0:b0:47c:3b8e:5253 with SMTP id y7-20020aa79427000000b0047c3b8e5253mr5166508pfo.69.1635435265242; Thu, 28 Oct 2021 08:34:25 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id p12sm4586055pfh.52.2021.10.28.08.34.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Oct 2021 08:34:24 -0700 (PDT) Date: Thu, 28 Oct 2021 15:34:21 +0000 From: Sean Christopherson To: Maxim Levitsky Subject: Re: [PATCH v2 21/43] KVM: VMX: Clean up PI pre/post-block WARNs Message-ID: References: <20211009021236.4122790-1-seanjc@google.com> <20211009021236.4122790-22-seanjc@google.com> <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> Cc: Cornelia Huck , Wanpeng Li , kvm@vger.kernel.org, David Hildenbrand , linux-kernel@vger.kernel.org, Paul Mackerras , Atish Patra , linux-riscv@lists.infradead.org, Claudio Imbrenda , kvmarm@lists.cs.columbia.edu, Janosch Frank , Marc Zyngier , Joerg Roedel , Huacai Chen , Christian Borntraeger , Aleksandar Markovic , Albert Ou , kvm-ppc@vger.kernel.org, Paul Walmsley , David Matlack , linux-arm-kernel@lists.infradead.org, Jim Mattson , Anup Patel , linux-mips@vger.kernel.org, Palmer Dabbelt , kvm-riscv@lists.infradead.org, Paolo Bonzini , Vitaly Kuznetsov 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 Thu, Oct 28, 2021, Maxim Levitsky wrote: > On Fri, 2021-10-08 at 19:12 -0700, Sean Christopherson wrote: > > Move the WARN sanity checks out of the PI descriptor update loop so as > > not to spam the kernel log if the condition is violated and the update > > takes multiple attempts due to another writer. This also eliminates a > > few extra uops from the retry path. > > > > Technically not checking every attempt could mean KVM will now fail to > > WARN in a scenario that would have failed before, but any such failure > > would be inherently racy as some other agent (CPU or device) would have > > to concurrent modify the PI descriptor. ... > Don't know for sure if this is desired. I'll would just use WARN_ON_ONCE instead > if the warning spams the log. > > If there is a race I would rather want to catch it even if rare. Paolo had similar concerns[*]. I copied the most relevant part of the discussion below, let me know if you object to the outcome. Thanks for the reviews! [*] https://lore.kernel.org/all/YXllGfrjPX1pVUx6@google.com/T/#u On Wed, Oct 27, 2021 at 8:38 AM Paolo Bonzini wrote: > On 27/10/21 17:28, Sean Christopherson wrote: > > On Wed, Oct 27, 2021, Paolo Bonzini wrote: > > > On 27/10/21 16:41, Sean Christopherson wrote: > > > > The other thing I don't like about having the WARN in the loop is that it suggests > > > > that something other than the vCPU can modify the NDST and SN fields, which is > > > > wrong and confusing (for me). > > > > > > Yeah, I can agree with that. Can you add it in a comment above the cmpxchg > > > loop, it can be as simple as > > > > > > /* The processor can set ON concurrently. */ > > > > > > when you respin patch 21 and the rest of the series? > > > > I can definitely add a comment, but I think that comment is incorrect. > > It's completely backwards indeed. I first had "the hardware" and then > shut down my brain for a second to replace it. > > > So something like this? > > > > /* ON can be set concurrently by a different vCPU or by hardware. */ > > Yes, of course. _______________________________________________ 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CFD88C433EF for ; Thu, 28 Oct 2021 15:35:49 +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 90C6B61073 for ; Thu, 28 Oct 2021 15:35:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 90C6B61073 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=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=54uV7vPrjmRc4cQ76LEyso6nCMk70qYiS6B5jOr2/40=; b=2z1dcBIfyMCbLw z3VqFmOdmkz9jrXhPEFHuRDevkleG+jbA4TAp7gHUuDpewObIDJhZbROWo582/udEqDNFNE0IUCki hmtVvf25jn25BVhty2XesUQKH2W5I+DqTWoyN7SgUKuoTFRqtaHwZATJA1XTxtzad62XpzAUEMOPM NrMiWoLhAshN8iD6V8c7hI+EQTfKzXG9FrcTEX41P0DkdA8s5V67MYpDcRTZKtHEZK6XlakVLKe+7 Znsb9m9O18i/x3t9F2tGK7YKzOjU9AFD+P0gK48Tdglnj4g/ZiNY9S7TsvetIBxkiglsZrWUkm2YG 35KZ5BRFlhLgLgEibHjA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mg7QG-008Psw-9x; Thu, 28 Oct 2021 15:34:32 +0000 Received: from mail-pf1-x42e.google.com ([2607:f8b0:4864:20::42e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mg7QA-008Pq9-OI for linux-arm-kernel@lists.infradead.org; Thu, 28 Oct 2021 15:34:28 +0000 Received: by mail-pf1-x42e.google.com with SMTP id 187so6305316pfc.10 for ; Thu, 28 Oct 2021 08:34:26 -0700 (PDT) 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=Dh2oRVWq7FNB4VxeeBx+DY+B7J6W36/D0YFmfA4JinPWULNj2JAyVIdGN5W95l+d90 4Uu/ZwJdWUivmqUAxVTynE14t1JCgr4B8VIlvCnfQRBjV/9sZBN9FbtVQViBrcsIkfKk FKX6ULIrvBdeMPobrWIP7p55k83rg/EH2wU4GD4W9e2j7rIISQUbKASJH6T77kFM7m0Z Qtlmy8wjfeMDEMJrWpV2K5PFqGDN6VxYLPZtUxrtHEs4QyogxcvUebO7UVfiBHutG04k U0NkfKu6Je8Pt6shzy+xbyjZteg84KZAIk3v/fR91ZfKR/MAff+MA9k1aMrDIiCOqEqs s/bw== 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=ObU0mLn8cgLGt35bHM9Ve7PBUotjGGEWKnNtAqnV8Y0=; b=jTxcKIwzL/1XzdZzjLQB8XIET/TZE1HhZbkSuedqS/NrPiRYfT5W21URZ/5ky8W85x aarObbI5cMNWfuYBZPPRewbs4tmgx3acX6qBlFk5Qe6bLx7NiRqMFCF5VhmH5FuVUKZO 0OcSG6sATRghRQBf2w4yFcwJInvLP2l/xjOOCWeZbdNmeYfC8CxXmcsV0ekSwQsCjQuj usd497Ha+GYiwu4gVCb8E+VEgLgkYtlVi0+nAtBiH9NC4TLRAYQx7hPwSozhHAyGo5fK 9JUKL/i19iKE1eJBekVbdfP/3DFDeHJSOmJgBP4aBzZCzRd+9ZmivgbkueVd3/5fn4QT zv+g== X-Gm-Message-State: AOAM531wCDqqZ1yqobtvJdIxoMHJkanpUhEvicoWLxzXOsm6LjVXuLdH 5HkABO+NvmdDTwiClWBRVDmmtg== X-Google-Smtp-Source: ABdhPJz6jdrYVfnN/6E/mwaWkSEy+zhYc5xKvC8pKY4AVCJGv9Uh9d4/rKPGgqsum0tyEq6Ye9Z/qw== X-Received: by 2002:aa7:9427:0:b0:47c:3b8e:5253 with SMTP id y7-20020aa79427000000b0047c3b8e5253mr5166508pfo.69.1635435265242; Thu, 28 Oct 2021 08:34:25 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id p12sm4586055pfh.52.2021.10.28.08.34.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Oct 2021 08:34:24 -0700 (PDT) Date: Thu, 28 Oct 2021 15:34:21 +0000 From: Sean Christopherson To: Maxim Levitsky Cc: Marc Zyngier , Huacai Chen , Aleksandar Markovic , Paul Mackerras , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Paolo Bonzini , James Morse , Alexandru Elisei , Suzuki K Poulose , Atish Patra , 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, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, David Matlack , Oliver Upton , Jing Zhang Subject: Re: [PATCH v2 21/43] KVM: VMX: Clean up PI pre/post-block WARNs Message-ID: References: <20211009021236.4122790-1-seanjc@google.com> <20211009021236.4122790-22-seanjc@google.com> <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6b2bdfad87e268e861b6cc331d25790dade8e27b.camel@redhat.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211028_083426_829123_9E448CBB X-CRM114-Status: GOOD ( 30.72 ) 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 Thu, Oct 28, 2021, Maxim Levitsky wrote: > On Fri, 2021-10-08 at 19:12 -0700, Sean Christopherson wrote: > > Move the WARN sanity checks out of the PI descriptor update loop so as > > not to spam the kernel log if the condition is violated and the update > > takes multiple attempts due to another writer. This also eliminates a > > few extra uops from the retry path. > > > > Technically not checking every attempt could mean KVM will now fail to > > WARN in a scenario that would have failed before, but any such failure > > would be inherently racy as some other agent (CPU or device) would have > > to concurrent modify the PI descriptor. ... > Don't know for sure if this is desired. I'll would just use WARN_ON_ONCE instead > if the warning spams the log. > > If there is a race I would rather want to catch it even if rare. Paolo had similar concerns[*]. I copied the most relevant part of the discussion below, let me know if you object to the outcome. Thanks for the reviews! [*] https://lore.kernel.org/all/YXllGfrjPX1pVUx6@google.com/T/#u On Wed, Oct 27, 2021 at 8:38 AM Paolo Bonzini wrote: > On 27/10/21 17:28, Sean Christopherson wrote: > > On Wed, Oct 27, 2021, Paolo Bonzini wrote: > > > On 27/10/21 16:41, Sean Christopherson wrote: > > > > The other thing I don't like about having the WARN in the loop is that it suggests > > > > that something other than the vCPU can modify the NDST and SN fields, which is > > > > wrong and confusing (for me). > > > > > > Yeah, I can agree with that. Can you add it in a comment above the cmpxchg > > > loop, it can be as simple as > > > > > > /* The processor can set ON concurrently. */ > > > > > > when you respin patch 21 and the rest of the series? > > > > I can definitely add a comment, but I think that comment is incorrect. > > It's completely backwards indeed. I first had "the hardware" and then > shut down my brain for a second to replace it. > > > So something like this? > > > > /* ON can be set concurrently by a different vCPU or by hardware. */ > > Yes, of course. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel