From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f201.google.com (mail-pg1-f201.google.com [209.85.215.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8855D4A11 for ; Mon, 28 Jul 2025 21:38:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753738738; cv=none; b=NywMxguYy/df3ShAltWghaHjrKwk4yFbw6MlxHPkjsunLvDzSQK2aIJ3QUoIOpQT34eMQ2IzmHKQ/b6A76z9gWQBr+BZSDSJMq8cSycPlDjVVLHn0pCNaybLaRswsD+5hTqSe5vwj9VbRjS6sM0mLNj70+8Ocr9rRIOMWyjFajU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753738738; c=relaxed/simple; bh=LOfd0vHjMNejVUvbvNWDBiGEYNfSLM8vMq7V6Z9EF7Q=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=R3iclh3hJAaKt6gypx1kdKiLlbhLx7ZeRMJ6/RjmuxWUCqrD61YUdGydr2YFKR9Sp4/ZPW+lYuiBMp7tUdcKrp0aZpZIiBdUTUL4zBKMjMcni20udP+0bH3J4y5gw9N6GgbQU4fB1oJk9xAjuyy2BHSVxuNsWHcPVlhqgsxKYwQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=3yQSOS4j; arc=none smtp.client-ip=209.85.215.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="3yQSOS4j" Received: by mail-pg1-f201.google.com with SMTP id 41be03b00d2f7-b38fc4d8dbaso6081824a12.2 for ; Mon, 28 Jul 2025 14:38:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1753738736; x=1754343536; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=+Qzsw1VtUTY4pn1HE8xcAZoBHNUjQcfll7dBGQyXeZ8=; b=3yQSOS4jS8DExoaCU89AKXdHCoxApqdRPqCdjg+Lu0WX3V119XPa4lYC1Hri/eI4/G 29nR28nWXpsxfAyPDtVTFeqoPAmUnYMPvV0kxpWKvobq8qlJg7D//eEDcOueqtCXssro jTYjwwxA8hdoLgaUxwD+P5Lkqbsqd3cMmOplhe+W7DN2AFMvp5GwMIfTSJ8cgVMLpAXH IwuRex9Q+mbwZ6w7+e3aRHJ7nKsbCftiYkiwMjBN4dNIhoVnzpGDkYRzMpX5OQ2IAzsc jeCcbC9tYjIeWckzJOWLLOkuDHh1X+5RzajOzsVlLyEPe8t0R2VKL6JiSX0UuiJQeUx+ agLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753738736; x=1754343536; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=+Qzsw1VtUTY4pn1HE8xcAZoBHNUjQcfll7dBGQyXeZ8=; b=JOlRfRhEayZ9zAuaGHg4g1rThfJOW7Rs4Xc/7GMuuCIFytguoeEjDYfczEbD0+GrBn 4eoFx4Ai/IZHMtR+WjBuPqpOvtdUuwBoqtW4Rw7EuWy118B6aNEj2yu411B9TtJ+q6GX 4yvGMhI+nC23WnyNmVVmSgzUNPyq+ycZgrmMaeojd81eOY7MNpC1zc7ir6g0eqMv+FbP 2vIWsXvmp9Wbdfylsh42R0/ewBC1xSbaC3Jnxwe5t7y17Ep0vI/oBow7h3CtC0IvhkVI w5KF4MNHVucoPDewT6jxGE15pph/RTPYv3eBKDLU1RXvd9+2O+br5bA2xSJDqx+hs1Yz YrMg== X-Forwarded-Encrypted: i=1; AJvYcCVg8sTk0MAwnMmS1vZSwHYeWRhEvaSuLKSsiT2UtgefmAEKE6m5rcb6Dt4XlJ7Bq2beu4Ta1S3l8zEvtj0=@vger.kernel.org X-Gm-Message-State: AOJu0YxVxjgRVjQ7icp1xwgOVEkaNKzIo8VGFuFkIKpQOVGIBJpIUkxi 4ZDJkAylkH7Y32A351bOqnAj4h8x2S+6IIJrC4X8iG3VKd4hU6TtjiYc+9C27cXx9r0hfEAvvYe ByVyxbw== X-Google-Smtp-Source: AGHT+IEZrkPEJG3cebOIkuivVavvJbQnzIudGhOMIpdjHX58AE2AU/k8vYvp80MKmMTuR8uwmPUPq0yanhw= X-Received: from pjqq12.prod.google.com ([2002:a17:90b:584c:b0:2ff:6132:8710]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:903:320a:b0:23d:d2d2:b511 with SMTP id d9443c01a7336-23fb30cb6c3mr186721765ad.19.1753738735880; Mon, 28 Jul 2025 14:38:55 -0700 (PDT) Date: Mon, 28 Jul 2025 14:38:54 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250707224720.4016504-1-jthoughton@google.com> <20250707224720.4016504-4-jthoughton@google.com> Message-ID: Subject: Re: [PATCH v5 3/7] KVM: x86/mmu: Recover TDP MMU NX huge pages using MMU read lock From: Sean Christopherson To: David Matlack Cc: James Houghton , Paolo Bonzini , Vipin Sharma , kvm@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Mon, Jul 28, 2025, David Matlack wrote: > On Mon, Jul 28, 2025 at 11:08=E2=80=AFAM James Houghton wrote: > > On Wed, Jul 23, 2025 at 1:35=E2=80=AFPM Sean Christopherson wrote: > > > > @@ -7559,8 +7590,17 @@ static void kvm_recover_nx_huge_pages(struct= kvm *kvm, > > > > rcu_read_lock(); > > > > > > > > for ( ; to_zap; --to_zap) { > > > > - if (list_empty(nx_huge_pages)) > > > > +#ifdef CONFIG_X86_64 > > > > > > These #ifdefs still make me sad, but I also still think they're the l= east awful > > > solution. And hopefully we will jettison 32-bit sooner than later :-= ) > > > > Yeah I couldn't come up with anything better. :( >=20 > Could we just move the definition of tdp_mmu_pages_lock outside of > CONFIG_X86_64? The only downside I can think of is slightly larger kvm > structs for 32-bit builds. Hmm, I was going to say "no, because we'd also need to do spin_lock_init()"= , but obviously spin_(un)lock() will only ever be invoked for 64-bit kernels. I = still don't love the idea of making tdp_mmu_pages_lock visible outside of CONFIG_= X86_64, it feels like we're just asking to introduce (likely benign) bugs. Ugh, and I just noticed this as well: #ifndef CONFIG_X86_64 #define KVM_TDP_MMU -1 #endif Rather than expose kvm->arch.tdp_mmu_pages_lock, what about using a single = #ifdef section to bury both is_tdp_mmu and a local kvm->arch.tdp_mmu_pages_lock po= inter? Alternatively, we could do: const bool is_tdp_mmu =3D IS_ENABLED(CONFIG_X86_64) && mmu_type !=3D KVM_S= HADOW_MMU; to avoid referencing KVM_TDP_MMU, but that's quite ugly. Overall, I think = the below strikes the best balance between polluting the code with #ifdefs, and generating robust code. diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_hos= t.h index 52bf6a886bfd..c038d7cd187d 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1372,10 +1372,6 @@ enum kvm_mmu_type { KVM_NR_MMU_TYPES, }; =20 -#ifndef CONFIG_X86_64 -#define KVM_TDP_MMU -1 -#endif - struct kvm_arch { unsigned long n_used_mmu_pages; unsigned long n_requested_mmu_pages; diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index a6a1fb42b2d1..e2bde6a5e346 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -7624,8 +7624,14 @@ static bool kvm_mmu_sp_dirty_logging_enabled(struct = kvm *kvm, static void kvm_recover_nx_huge_pages(struct kvm *kvm, const enum kvm_mmu_type mmu_type) { +#ifdef CONFIG_X86_64 + const bool is_tdp_mmu =3D mmu_type =3D=3D KVM_TDP_MMU; + spinlock_t *tdp_mmu_pages_lock =3D &kvm->arch.tdp_mmu_pages_lock; +#else + const bool is_tdp_mmu =3D false; + spinlock_t *tdp_mmu_pages_lock =3D NULL; +#endif unsigned long to_zap =3D nx_huge_pages_to_zap(kvm, mmu_type); - bool is_tdp_mmu =3D mmu_type =3D=3D KVM_TDP_MMU; struct list_head *nx_huge_pages; struct kvm_mmu_page *sp; LIST_HEAD(invalid_list); @@ -7648,15 +7654,12 @@ static void kvm_recover_nx_huge_pages(struct kvm *k= vm, rcu_read_lock(); =20 for ( ; to_zap; --to_zap) { -#ifdef CONFIG_X86_64 if (is_tdp_mmu) - spin_lock(&kvm->arch.tdp_mmu_pages_lock); -#endif + spin_lock(tdp_mmu_pages_lock); + if (list_empty(nx_huge_pages)) { -#ifdef CONFIG_X86_64 if (is_tdp_mmu) - spin_unlock(&kvm->arch.tdp_mmu_pages_lock); -#endif + spin_unlock(tdp_mmu_pages_lock); break; } =20 @@ -7675,10 +7678,8 @@ static void kvm_recover_nx_huge_pages(struct kvm *kv= m, =20 unaccount_nx_huge_page(kvm, sp); =20 -#ifdef CONFIG_X86_64 if (is_tdp_mmu) - spin_unlock(&kvm->arch.tdp_mmu_pages_lock); -#endif + spin_unlock(tdp_mmu_pages_lock); =20 /* * Do not attempt to recover any NX Huge Pages that are bei= ng --