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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E225FC433EF for ; Mon, 11 Apr 2022 20:37:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1349923AbiDKUjo (ORCPT ); Mon, 11 Apr 2022 16:39:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36374 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349875AbiDKUjm (ORCPT ); Mon, 11 Apr 2022 16:39:42 -0400 Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2799A1BEA0 for ; Mon, 11 Apr 2022 13:37:27 -0700 (PDT) Received: by mail-pf1-x436.google.com with SMTP id bo5so15610197pfb.4 for ; Mon, 11 Apr 2022 13:37:27 -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=AYKc8ZeDRNtBCWJwQpHr7JcieMpfMfOkAMNDb3v8MjY=; b=hWfvTE+5RKFSbpiLlmi4b5/iTRF1eQULyL8xZLRHFm/u7l3Rwd4oaMM4vBtwvH50/k Rfbpsu+k4nTnFXBMH8k1khmANVJhl97UDy89rTpe+kU+G8brFoXwN2GDFh66yJIEa7ck YIySXZvImVcfXLw6QOukQllDrCv0GF9RZ3uOrD9fFcE4AqJvZ4Z2ow7aIeEeDfCl6ZJe 0/F+KgoA7GWxPW4Of9ImDmNwgX4rvnLAquVYP2o4DKwx8UHlS89VOeCAjdPlb1TXUSSd 0p3CLTXgdOmzPrXX38+m5G5kLFldgONBUfWxwC6td0eDBUq+BccnCu/0m4WPZ+GAL4Tw u/NQ== 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=AYKc8ZeDRNtBCWJwQpHr7JcieMpfMfOkAMNDb3v8MjY=; b=P0FYOEa4xY4G59iaIijgaJ0oWQhWRUnlGRegbMbN6C06Z5Qge+zy4ip02eKihaz6/n tztbINMcwkU4dRkly/wOcNd6ZLmPdj2DDWl4PQGkfSBUD1HWkGDU+QX+0rOy/kpgh2+y cTT8pVhesPZ1nDp7fz648M9WI9KeVvz50LWO2DUQLVNTv4WGe67AYVLDlSFHGnBTL7qj ZRwWIMCfYghPXli0ylczDg6TW6WQ3jHio4mvz9lCvhvey4Z32re1p5ERn/M8NKBinHA5 5BuM2BcHojnhjrBgkE106/WvFMIOPejT78mIOaneY/GM0lZLh6s59vgSSqRHd+zpRv2J Z16w== X-Gm-Message-State: AOAM533Tg+CFPv8wWjyazZt3iT3H+2phXSDCadwOFYlLs+OCR8cbOzo6 /iU/HfQHir3+r1e0yHGPdnYcZA== X-Google-Smtp-Source: ABdhPJzZ64CwOXnrawiJRiNmvAMK3O1eLGBgCgolpctZJ4DZKuhTsSiJwt0g7J+Rh+QifwXJbj/jXQ== X-Received: by 2002:a63:2263:0:b0:399:561e:810b with SMTP id t35-20020a632263000000b00399561e810bmr27472837pgm.615.1649709446436; Mon, 11 Apr 2022 13:37:26 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id gt14-20020a17090af2ce00b001c701e0a129sm327234pjb.38.2022.04.11.13.37.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Apr 2022 13:37:25 -0700 (PDT) Date: Mon, 11 Apr 2022 20:37:22 +0000 From: Sean Christopherson To: Vitaly Kuznetsov Cc: kvm@vger.kernel.org, Paolo Bonzini , Wanpeng Li , Jim Mattson , Michael Kelley , Siddharth Chandrasekaran , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 03/31] KVM: x86: hyper-v: Handle HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST{,EX} calls gently Message-ID: References: <20220407155645.940890-1-vkuznets@redhat.com> <20220407155645.940890-4-vkuznets@redhat.com> <87a6cr7t5k.fsf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a6cr7t5k.fsf@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 11, 2022, Vitaly Kuznetsov wrote: > Sean Christopherson writes: > > > On Thu, Apr 07, 2022, Vitaly Kuznetsov wrote: > >> @@ -1840,15 +1891,47 @@ void kvm_hv_vcpu_flush_tlb(struct kvm_vcpu *vcpu) > >> { > >> struct kvm_vcpu_hv_tlbflush_ring *tlb_flush_ring; > >> struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu); > >> - > >> - kvm_vcpu_flush_tlb_guest(vcpu); > >> - > >> - if (!hv_vcpu) > >> + struct kvm_vcpu_hv_tlbflush_entry *entry; > >> + int read_idx, write_idx; > >> + u64 address; > >> + u32 count; > >> + int i, j; > >> + > >> + if (!tdp_enabled || !hv_vcpu) { > >> + kvm_vcpu_flush_tlb_guest(vcpu); > >> return; > >> + } > >> > >> tlb_flush_ring = &hv_vcpu->tlb_flush_ring; > >> + read_idx = READ_ONCE(tlb_flush_ring->read_idx); > >> + write_idx = READ_ONCE(tlb_flush_ring->write_idx); > >> + > >> + /* Pairs with smp_wmb() in hv_tlb_flush_ring_enqueue() */ > >> + smp_rmb(); > >> > >> - tlb_flush_ring->read_idx = tlb_flush_ring->write_idx; > >> + for (i = read_idx; i != write_idx; i = (i + 1) % KVM_HV_TLB_FLUSH_RING_SIZE) { > >> + entry = &tlb_flush_ring->entries[i]; > >> + > >> + if (entry->flush_all) > >> + goto out_flush_all; > >> + > >> + /* > >> + * Lower 12 bits of 'address' encode the number of additional > >> + * pages to flush. > >> + */ > >> + address = entry->addr & PAGE_MASK; > >> + count = (entry->addr & ~PAGE_MASK) + 1; > >> + for (j = 0; j < count; j++) > >> + static_call(kvm_x86_flush_tlb_gva)(vcpu, address + j * PAGE_SIZE); > >> + } > >> + ++vcpu->stat.tlb_flush; > >> + goto out_empty_ring; > >> + > >> +out_flush_all: > >> + kvm_vcpu_flush_tlb_guest(vcpu); > >> + > >> +out_empty_ring: > >> + tlb_flush_ring->read_idx = write_idx; > > > > Does this need WRITE_ONCE? My usual "I suck at memory ordering" disclaimer applies. > > > > Same here) I *think* we're fine for 'read_idx' as it shouldn't matter at > which point in this function 'tlb_flush_ring->read_idx' gets modified > (relative to other things, e.g. actual TLB flushes) and there's no > concurency as we only have one reader (the vCPU which needs its TLB > flushed). On the other hand, I'm not against adding WRITE_ONCE() here > even if just to aid an unprepared reader (thinking myself couple years > in the future). Ah, read_idx == tail and write_idx == head. I didn't look at the structure very closely, or maybe not at all :-) And IIUC, only the vCPU itself ever writes to tail? In that case, I would omit the READ_ONCE() from both the write to tail here and the read above, and probably add a brief comment stating that the flush must be performed on the target vCPU, i.e. must hold vcpu->mutex, and so it's safe for the compiler to re-read tlb_flush_ring->read_idx in the loop because it cannot change.