From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f202.google.com (mail-pf1-f202.google.com [209.85.210.202]) (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 4679B33F597 for ; Fri, 5 Jun 2026 20:03:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780689826; cv=none; b=PvfjFhigxJY3bCjM9NbJMTGnfefndmuWXSY63JzwWczlNJQ1VJEEk+WZD0IiCbAR9ZcSX0ID5LRnW2UhurthzJD0hP8OHfACX/W1C7Fj9rHWNrHPPQMllpZ1bVkeix4gIeBRDvu6a66Dv91X9oPV1WxtDbyCEfXrvTvM89JrA+8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780689826; c=relaxed/simple; bh=+4CIY7Ynis+rMHOl6QrsbGIprSmxQOXY5/oYOOses1g=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=MD9e1JPLZcDDu+DmTLpxtAkWmvw789vddXbaKpfQU9OJqJC4PLozZsVcvd4ymPYpxJ5Rqrp0ceEjwr8/18LOI1zP+h8Me6My//twWVlUH9m1jsZNvAVJLt8uTEYWKQLBgv73SDYHWNaWgGXy9hYReopUm+JkU2MegDmIrgoYTiM= 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=d3MzVZN2; arc=none smtp.client-ip=209.85.210.202 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="d3MzVZN2" Received: by mail-pf1-f202.google.com with SMTP id d2e1a72fcca58-8423970cb30so1388482b3a.2 for ; Fri, 05 Jun 2026 13:03:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1780689823; x=1781294623; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=wBLcJpUQejqUn4D2+44Z/s9VjbxJzGZ5eXm9TJWqVPA=; b=d3MzVZN2qNxiwhBjwWHLxBJiIAFYZtxnA1X19aByQE69UnEFilMlHJXAO8ZV4fDALn tKL1zJFjQWuF8crRFfECoFo81PXyOmD8+jn+PTQZRY81JlstEDI9iqjKDh9idl15KaIi BcAacd7eWidND9nRSfkDbhxg0odnM+Tu0oc5PtsQ4cTHT0iY/CUUZIlWZ/2DphcAGXk/ zpKXKjOzGl3IfEl8lNuMIyznL9UMXBgb1RQE2vagN9SW7d/ulODHBg+WNa6kCkp/yp5C ZixniH7xI7d2KsvugHBOUXDS0ZGSv24cihWkfxfLamNIu8pZEXanxr/pzwQD0Ae6L0JG R9xQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780689823; x=1781294623; h=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=wBLcJpUQejqUn4D2+44Z/s9VjbxJzGZ5eXm9TJWqVPA=; b=r04RYJ9Vwa5lR+iVapZtqelUApJFp7n+N2y59oM+gJsRxOw+h5XwjFAgUhNDwkQoEM J7HIN4jDaMSfhaQ/lZSBi3PFzp3xQxaOcIrlUITgXqIOs2FBCaGVaJ/TKZIbH2ZPQ92T N6qGXp6m2JXHF3HODSvNFHWZMx8YfFGpvXg/EHsRzLX0G24AqKaQ8MvmTay08ldoWDJu IV1avwC7kwVf+ZLfCD9s5nC1IAumLF5s011EqtzjndnnFYtsRzzSvMmZ+r9p+UHxz8bO HQywNDUzAcwkw2KGNjF47+XBTg3tMaTDfSdZMeaPbr0sYzokbyQ0uYMZEmP2h3StZDZE 2TIA== X-Forwarded-Encrypted: i=1; AFNElJ8J4jmb5PmH34tl7aX5Azc9BYwxRzh6aZJ3uv+DgCmYgEuMBt69gh1qhA/1DUyVcT4nC1Yrl7iV00cGmA0=@vger.kernel.org X-Gm-Message-State: AOJu0Yz2507s7DYAzk8dKfo8aokiIHdRDtKMDRQQ8FA4cdF3ccMcsovc 06qlwMyuRbkRZfpIYo+4Jk3DqDKvwR6JnERjhHhS6WfOH9BhgD/SMcP9MInmLylgWL4+UCk21nR PFgTIaQ== X-Received: from pfbhx1.prod.google.com ([2002:a05:6a00:8981:b0:842:5a44:a83c]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a00:3ccf:b0:82f:72e6:ed4 with SMTP id d2e1a72fcca58-842b0a9d376mr5173975b3a.0.1780689823138; Fri, 05 Jun 2026 13:03:43 -0700 (PDT) Date: Fri, 5 Jun 2026 13:03:42 -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: <20260529222223.870923-1-seanjc@google.com> <20260529222223.870923-32-seanjc@google.com> Message-ID: Subject: Re: [PATCH v3 31/40] KVM: x86: Move MMU helper declarations from kvm_host.h => mmu.h From: Sean Christopherson To: Kai Huang Cc: "pbonzini@redhat.com" , "vkuznets@redhat.com" , "dwmw2@infradead.org" , "paul@xen.org" , "kvm@vger.kernel.org" , "dwmw@amazon.co.uk" , "linux-kernel@vger.kernel.org" , "yosry@kernel.org" , "binbin.wu@linux.intel.com" Content-Type: text/plain; charset="us-ascii" On Fri, Jun 05, 2026, Kai Huang wrote: > > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index 1143140592df..f217403e18fc 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -161,12 +161,6 @@ > > #define KVM_HPAGE_MASK(x) (~(KVM_HPAGE_SIZE(x) - 1)) > > #define KVM_PAGES_PER_HPAGE(x) (KVM_HPAGE_SIZE(x) / PAGE_SIZE) > > > > -#define KVM_MEMSLOT_PAGES_TO_MMU_PAGES_RATIO 50 > > -#define KVM_MIN_ALLOC_MMU_PAGES 64UL > > -#define KVM_MMU_HASH_SHIFT 12 > > -#define KVM_NUM_MMU_PAGES (1 << KVM_MMU_HASH_SHIFT) > > -#define KVM_MIN_FREE_MMU_PAGES 5 > > -#define KVM_REFILL_PAGES 25 > > > > [...] > > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > > index d30676935fff..a6b871253bd7 100644 > > --- a/arch/x86/kvm/mmu.h > > +++ b/arch/x86/kvm/mmu.h > > @@ -15,6 +15,13 @@ extern bool tdp_mmu_enabled; > > extern bool __read_mostly enable_mmio_caching; > > extern bool eager_page_split; > > > > +#define KVM_MEMSLOT_PAGES_TO_MMU_PAGES_RATIO 50 > > +#define KVM_MIN_ALLOC_MMU_PAGES 64UL > > +#define KVM_MMU_HASH_SHIFT 12 > > +#define KVM_NUM_MMU_PAGES (1 << KVM_MMU_HASH_SHIFT) > > +#define KVM_MIN_FREE_MMU_PAGES 5 > > +#define KVM_REFILL_PAGES 25 > > + > > Btw, I found below defines can be moved to "mmu.h" too: > > #define INVALID_PAGE (~(hpa_t)0) > #define VALID_PAGE(x) ((x) != INVALID_PAGE) > > #define KVM_MMU_ROOT_INFO_INVALID \ > ((struct kvm_mmu_root_info) { .pgd = INVALID_PAGE, .hpa = INVALID_PAGE > }) I saw this one too, but I don't want to split KVM_MMU_ROOT_INFO_INVALID from the related defs: #define KVM_MMU_NUM_PREV_ROOTS 3 #define KVM_MMU_ROOT_CURRENT BIT(0) #define KVM_MMU_ROOT_PREVIOUS(i) BIT(1+i) #define KVM_MMU_ROOTS_ALL (BIT(1 + KVM_MMU_NUM_PREV_ROOTS) - 1) Hrm, but KVM_MMU_ROOT_INFO_INVALID has exactly one user: for (i = 0; i < KVM_MMU_NUM_PREV_ROOTS; i++) mmu->prev_roots[i] = KVM_MMU_ROOT_INFO_INVALID; so leaving it in kvm_host.h is silly. Oof, but that's odd, the code above it does: mmu->root.hpa = INVALID_PAGE; mmu->root.pgd = 0; as does kvm_mmu_free_roots(): mmu->root.hpa = INVALID_PAGE; mmu->root.pgd = 0; mmu_alloc_direct_roots() deliberately zeros "pgd" to guarantee matches on the pgd, as guest CR3 is ignored for direct roots, but the "invalidation" code looks wrong. Let's handle KVM_MMU_ROOT_INFO_INVALID separately, as simply moving it probably isn't what we want to do. > And even below can be moved to "mmu.h", but perhaps you won't like that: > > #define KVM_HPAGE_GFN_SHIFT(x) (((x) - 1) * 9) > #define KVM_HPAGE_SHIFT(x) (PAGE_SHIFT + KVM_HPAGE_GFN_SHIFT(x)) > #define KVM_HPAGE_SIZE(x) (1UL << KVM_HPAGE_SHIFT(x)) > #define KVM_HPAGE_MASK(x) (~(KVM_HPAGE_SIZE(x) - 1)) > #define KVM_PAGES_PER_HPAGE(x) (KVM_HPAGE_SIZE(x) / PAGE_SIZE) Huh. Oh, /facepalm. I looked at these, but saw there were references in kvm_host.h and moved on. But they're all "self" references. So yeah, I agree moving these and INVALID_PAGE+VALID_PAGE makes sense. > #define KVM_MMU_ROOT_CURRENT BIT(0) > #define KVM_MMU_ROOT_PREVIOUS(i) BIT(1+i) > #define KVM_MMU_ROOTS_ALL (BIT(1 + KVM_MMU_NUM_PREV_ROOTS) - 1) These I want to keep in kvm_host.h because they're tied to KVM_MMU_NUM_PREV_ROOTS, which needs to stay in kvm_host.h. Thanks so much for the thorough review(s)!