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 7B0D0ECAAD1 for ; Tue, 30 Aug 2022 14:43:36 +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=3PS4JIGG8QTaPkBvRip24APw29f8/yFhQYLNjhdbBw0=; b=cRSLjpw6E59+Tg /6fq1YbL88a2NqAdbYxJBYArEDBcWjyf66H0+3TMBFs/TrX/QLGllK7EPw1sC+/hUsB7PzPLL0JT+ 0I6gTuNFGVHY4LU838joZagC0QqvcKyoxhMocKiBJtPKjtRpQPYDGkYoYlqw4nWU5Mc69EahWuoNf goCXb7xKFYullyZfOQYxTTnah8ntKpBdknkBZ24gDzLrX6yIlPge2/xBQJdmB4pg7juM7e9abvoW0 yhgNLmA6y+ijex4c9u7uB9d26TVSZblqopqXvnHZAajVaFxgjdASQZ9b75/r52Av0BgpKPhSu2JwR eMrgrgDQnuzGDp8Rpt0g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oT2Rg-0003un-S9; Tue, 30 Aug 2022 14:42:29 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oT2Rd-0003se-PS for linux-arm-kernel@lists.infradead.org; Tue, 30 Aug 2022 14:42:27 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661870541; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+te87sYtTGczNabgaJy1Pj2NfLsGoso4jkdFkum7FlU=; b=b2/XSxWPaY3A0uZ0jqYq/654nxllL8+IPfWSjjRUkhh0kd9fpGJnD+Ysg7Qa3NlunJbFhY QDwi+iF5qO2XuOa4YVnMJMut8fsI3PMhG1vQp/wb7olTLQhB4n/GkKOL6khzVRgZq3yw3E 3A23S9+0I3SosTABacwwk6lhfhniQtE= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-78-f53RKcEgOIaa1g_V6Q_www-1; Tue, 30 Aug 2022 10:42:20 -0400 X-MC-Unique: f53RKcEgOIaa1g_V6Q_www-1 Received: by mail-qk1-f200.google.com with SMTP id s9-20020a05620a254900b006b54dd4d6deso9268483qko.3 for ; Tue, 30 Aug 2022 07:42:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc; bh=+te87sYtTGczNabgaJy1Pj2NfLsGoso4jkdFkum7FlU=; b=BSBSTwZG+3iRiHVJHTRgfRsAB25cM41jrCuVIWmsjlCPOOsfohkgenovH4DYoF1HNy xu1ThnhZxn/4X1BrrtC6dbu3QFSKU41a5GqADyLZxGd1ycgjqGQT00A+8m2MvQ2B73S1 pnXEEav5X1xtCG12dV3e+scMjk6hspRoToLmc1oIhZ1C+mQC68CtDpKk8/C/jMhf708R IUydtDzGqFYAJyNimaOia0DauS0uDYPwSePu5/1m4S5QKLOiZKUpiWIReAuIpFsK7MEp Jck17bfaqdCglo/MlXPu45gP40zlKtACTmpPgW0iI1lUIINUXWyB0gxf+w5lX+GtGvkD QYcg== X-Gm-Message-State: ACgBeo1R+nGLDz2GymG1yfI7/hiZOXS+Ng3Ozq1DoPYgnpKMzhWFWt7Z o3zp71HeSntnsIBbx4ICd7qHsWRrcqW+lJxrnd3ADQqQnbmeUGKLyZAkXHR8wtRNZXEQnA8e8iQ SIN2pH1efaf49vyGfKqUMUHgfrm5ENWACBFw= X-Received: by 2002:a05:622a:1a0d:b0:343:6284:cbc8 with SMTP id f13-20020a05622a1a0d00b003436284cbc8mr15163370qtb.341.1661870540165; Tue, 30 Aug 2022 07:42:20 -0700 (PDT) X-Google-Smtp-Source: AA6agR6D8op/R0BZbn270yGLNeyiqE7221aPAIVz7ZsIkOvVsCOIjnQ86kAB8hellaaWH315TLq1vg== X-Received: by 2002:a05:622a:1a0d:b0:343:6284:cbc8 with SMTP id f13-20020a05622a1a0d00b003436284cbc8mr15163320qtb.341.1661870539902; Tue, 30 Aug 2022 07:42:19 -0700 (PDT) Received: from xz-m1.local (bras-base-aurron9127w-grc-35-70-27-3-10.dsl.bell.ca. [70.27.3.10]) by smtp.gmail.com with ESMTPSA id x6-20020ac86b46000000b00339b8a5639csm7064707qts.95.2022.08.30.07.42.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Aug 2022 07:42:19 -0700 (PDT) Date: Tue, 30 Aug 2022 10:42:16 -0400 From: Peter Xu To: Marc Zyngier Cc: Paolo Bonzini , Oliver Upton , Gavin Shan , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org, corbet@lwn.net, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, shuah@kernel.org, seanjc@google.com, drjones@redhat.com, dmatlack@google.com, bgardon@google.com, ricarkol@google.com, zhenyzha@redhat.com, shan.gavin@gmail.com Subject: Re: [PATCH v1 1/5] KVM: arm64: Enable ring-based dirty memory tracking Message-ID: References: <20220819005601.198436-1-gshan@redhat.com> <20220819005601.198436-2-gshan@redhat.com> <87lerkwtm5.wl-maz@kernel.org> <41fb5a1f-29a9-e6bb-9fab-4c83a2a8fce5@redhat.com> <87fshovtu0.wl-maz@kernel.org> <87a67uwve8.wl-maz@kernel.org> <99364855-b4e9-8a69-e1ca-ed09d103e4c8@redhat.com> <874jxzvxak.wl-maz@kernel.org> MIME-Version: 1.0 In-Reply-To: <874jxzvxak.wl-maz@kernel.org> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220830_074225_941414_5A89E795 X-CRM114-Status: GOOD ( 23.80 ) 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 Fri, Aug 26, 2022 at 04:28:51PM +0100, Marc Zyngier wrote: > On Fri, 26 Aug 2022 11:58:08 +0100, > Paolo Bonzini wrote: > > > > On 8/23/22 22:35, Marc Zyngier wrote: > > >> Heh, yeah I need to get that out the door. I'll also note that Gavin's > > >> changes are still relevant without that series, as we do write unprotect > > >> in parallel at PTE granularity after commit f783ef1c0e82 ("KVM: arm64: > > >> Add fast path to handle permission relaxation during dirty logging"). > > > > > > Ah, true. Now if only someone could explain how the whole > > > producer-consumer thing works without a trace of a barrier, that'd be > > > great... > > > > Do you mean this? > > > > void kvm_dirty_ring_push(struct kvm_dirty_ring *ring, u32 slot, u64 offset) > > Of course not. I mean this: > > static int kvm_vm_ioctl_reset_dirty_pages(struct kvm *kvm) > { > unsigned long i; > struct kvm_vcpu *vcpu; > int cleared = 0; > > if (!kvm->dirty_ring_size) > return -EINVAL; > > mutex_lock(&kvm->slots_lock); > > kvm_for_each_vcpu(i, vcpu, kvm) > cleared += kvm_dirty_ring_reset(vcpu->kvm, &vcpu->dirty_ring); > [...] > } > > and this > > int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring) > { > u32 cur_slot, next_slot; > u64 cur_offset, next_offset; > unsigned long mask; > int count = 0; > struct kvm_dirty_gfn *entry; > bool first_round = true; > > /* This is only needed to make compilers happy */ > cur_slot = cur_offset = mask = 0; > > while (true) { > entry = &ring->dirty_gfns[ring->reset_index & (ring->size - 1)]; > > if (!kvm_dirty_gfn_harvested(entry)) > break; > [...] > > } > > which provides no ordering whatsoever when a ring is updated from one > CPU and reset from another. Marc, I thought we won't hit this as long as we properly take care of other orderings of (a) gfn push, and (b) gfn collect, but after a second thought I think it's indeed logically possible that with a reversed ordering here we can be reading some garbage gfn before (a) happens butt also read the valid flag after (b). It seems we must have all the barriers correctly applied always. If that's correct, do you perhaps mean something like this to just add the last piece of barrier? ===8<=== diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c index f4c2a6eb1666..ea620bfb012d 100644 --- a/virt/kvm/dirty_ring.c +++ b/virt/kvm/dirty_ring.c @@ -84,7 +84,7 @@ static inline void kvm_dirty_gfn_set_dirtied(struct kvm_dirty_gfn *gfn) static inline bool kvm_dirty_gfn_harvested(struct kvm_dirty_gfn *gfn) { - return gfn->flags & KVM_DIRTY_GFN_F_RESET; + return smp_load_acquire(&gfn->flags) & KVM_DIRTY_GFN_F_RESET; } int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring) ===8<=== Thanks, -- Peter Xu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel