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 B75D1D1D483 for ; Thu, 8 Jan 2026 18:31:44 +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:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=0y1/otFL3ju0i/+D1qE9LxTbEaX4KwTlmLX0xlpRiOg=; b=s4Q8RYEdIE8/pI/Cfy2/BNVj3i xLwLLU5myqTNeRmKKJQ3uxAKQ/CNBjTPVrikdmFEVwHEqM9ifZGPt/vNjHBe/0nZwVgoA0xDQvwot Gcte7GSPn38BlmB3WaURZKFArtaAAOF5zGa2KXW2sLjro3iIFcrnCC5NcP4nyeJaHEXYvjEAsZwxg VsFRR2SBbvur4IljDK5B3hmUA0vsdUk/t0d2wYdlhx3OV/loWY1p1ds86p2rftWy7OHp4rdZ2dwa5 30WuA/BI954nIRuWp9rHhGmJ8Z6EeXPt2yDG8hLHm0DpVYmhTiGTm/vojup0sks9AHKVxA/Cg0aAL N3xK1OjQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vdunA-00000000clB-3MAe; Thu, 08 Jan 2026 18:31:28 +0000 Received: from mail-pj1-x1049.google.com ([2607:f8b0:4864:20::1049]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vdun7-00000000ciu-3c3h for linux-riscv@lists.infradead.org; Thu, 08 Jan 2026 18:31:27 +0000 Received: by mail-pj1-x1049.google.com with SMTP id 98e67ed59e1d1-34ab459c051so7845911a91.0 for ; Thu, 08 Jan 2026 10:31:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1767897084; x=1768501884; darn=lists.infradead.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=nY/l54WU76DsZYqDI43j/uSoqA/+4oD9QHehzkck2fM=; b=S9aF+3xgWRZEfwTrAgpPFdaszrRUGw/zVqcxuyT+3NL0/tyZBVD+fZOmAACeaGplK6 Ta73qJ2R3PFujtyc0Ot0izqgYPv/5e3IlCBen8ZmJlJW3ReJdybHg/TgrKQt5mvSsW0z UBJm2GJbDaLoib0R5yHqc4/882WvlJ1sOJ7ooS0WmHUlbnCI8WG2a/Q6yZ5gBOWnSxU8 68Tb/X9SfDNUtQRJ9NmaCREywc8Pb5ossyXQaBts9Nr8TliCAbDhbPaS4QM8pznLGAX1 tAtxsPRs6jfwtJzENLNHrWG8lnfkbGOKRuSN/7V1XeuniYlYCI34UrSz0iUDotGzfbDZ uHbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767897084; x=1768501884; 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=nY/l54WU76DsZYqDI43j/uSoqA/+4oD9QHehzkck2fM=; b=l+bbrOrjbFt4QlijsRo9w+aXWI8NpzkIfUPhk7J8yBpBOqlENoRM4AAwh9daZw6VF/ Qacs1FGl/K3G8j5DXgGKdoS6zrRnmWs+3vEgpWs7bUuASROFYwZl10VwBaOyN3/bnzVu Am2oOM1q9k5VHKFIXKV+rVjLT/Z+d4Rd9u2DioUJ823gA8LL++I6Ec1YPZFJW+Lp7UyW VlE5FLtC7RxF97wg9PDa+WxgdpUtMRaQiwaA/eiUgl2PRBaWwtXN/jugvsUW3U02u+da 1/B7gcvLTytFhZeK1u4ItPuNpI+Bml6K1h07q9XdrogWGHMTMoy3MCNzHRuVTIiJvZae HGlA== X-Forwarded-Encrypted: i=1; AJvYcCWAYp11zGLOm1CX3TSPTgvW/tVctn+Qju0kIhADxHRF6bZJprp4WajKzWuwE/Nvw7bRYxZK0NXhrfP9LQ==@lists.infradead.org X-Gm-Message-State: AOJu0Yx4zTCHzmP9MaKj6JpQ1hNApgZXimqX3ATFoiq1KUo7pGyJauPE V3hlp7am4N1C5kNOcZd+dV0rmIf4NzAiFmTTADBVNC6SQHoEPnHo4bMw/xv3zDApckKHYZMYGUE jE0SeIw== X-Google-Smtp-Source: AGHT+IHEOdI4r0O9pcHM188bqvdusRA2ROI4oJ6wpZajhgwchTGU2kxOjQW3XQ2z18n6bt3vYCQzcYAxHoc= X-Received: from pjrx16.prod.google.com ([2002:a17:90a:bc90:b0:34c:6f7a:2ab8]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:90b:3c09:b0:349:2154:eef4 with SMTP id 98e67ed59e1d1-34f68b83d71mr6555128a91.5.1767897083980; Thu, 08 Jan 2026 10:31:23 -0800 (PST) Date: Thu, 8 Jan 2026 10:31:22 -0800 In-Reply-To: Mime-Version: 1.0 References: <20251230230150.4150236-1-seanjc@google.com> <20251230230150.4150236-22-seanjc@google.com> Message-ID: Subject: Re: [PATCH v4 21/21] KVM: selftests: Test READ=>WRITE dirty logging behavior for shadow MMU From: Sean Christopherson To: Yosry Ahmed Cc: Paolo Bonzini , Marc Zyngier , Oliver Upton , Tianrui Zhao , Bibo Mao , Huacai Chen , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, loongarch@lists.linux.dev, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260108_103125_897886_1CC49AC7 X-CRM114-Status: GOOD ( 26.30 ) 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, Jan 08, 2026, Yosry Ahmed wrote: > On Thu, Jan 08, 2026 at 08:32:44AM -0800, Sean Christopherson wrote: > > On Fri, Jan 02, 2026, Yosry Ahmed wrote: > > diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c > > index ab869a98bbdc..fab18e9be66c 100644 > > --- a/tools/testing/selftests/kvm/lib/x86/processor.c > > +++ b/tools/testing/selftests/kvm/lib/x86/processor.c > > @@ -390,6 +390,13 @@ static uint64_t *__vm_get_page_table_entry(struct kvm_vm *vm, > > return virt_get_pte(vm, mmu, pte, vaddr, PG_LEVEL_4K); > > } > > > > +uint64_t *tdp_get_pte(struct kvm_vm *vm, uint64_t l2_gpa) > > nested_paddr is the name used by tdp_map(), maybe use that here as well > (and in the header)? Oh hell no :-) nested_paddr is a terrible name (I was *very* tempted to change it on the fly, but restrained myself). "nested" is far too ambigous, e.g. without nested virtualization, "nested_paddr" arguably refers to _L1_ physical addresses (SVM called 'em Nested Page Tables after all). > > + int level = PG_LEVEL_4K; > > + > > + return __vm_get_page_table_entry(vm, &vm->stage2_mmu, l2_gpa, &level); > > +} > > + > > uint64_t *vm_get_pte(struct kvm_vm *vm, uint64_t vaddr) > > { > > int level = PG_LEVEL_4K; > [..] > > @@ -133,35 +220,50 @@ static void test_dirty_log(bool nested_tdp) > > > > /* Add an extra memory slot for testing dirty logging */ > > vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, > > - GUEST_TEST_MEM, > > + TEST_MEM_BASE, > > TEST_MEM_SLOT_INDEX, > > TEST_MEM_PAGES, > > KVM_MEM_LOG_DIRTY_PAGES); > > > > /* > > - * Add an identity map for GVA range [0xc0000000, 0xc0002000). This > > + * Add an identity map for GVA range [0xc0000000, 0xc0004000). This > > * affects both L1 and L2. However... > > */ > > - virt_map(vm, GUEST_TEST_MEM, GUEST_TEST_MEM, TEST_MEM_PAGES); > > + virt_map(vm, TEST_MEM_BASE, TEST_MEM_BASE, TEST_MEM_PAGES); > > > > /* > > - * ... pages in the L2 GPA range [0xc0001000, 0xc0003000) will map to > > - * 0xc0000000. > > + * ... pages in the L2 GPA ranges [0xc0001000, 0xc0002000) and > > + * [0xc0003000, 0xc0004000) will map to 0xc0000000 and 0xc0001000 > > + * respectively. > > Are these ranges correct? I thought L2 GPA range [0xc0002000, > 0xc0004000) will map to [0xc0000000, 0xc0002000). Gah, no. I looked at the comments after changing things around, but my eyes had glazed over by that point. > Also, perhaps it's better to express those in terms of the macros? > > L2 GPA range [TEST_MEM_ALIAS_BASE, TEST_MEM_ALIAS_BASE + 2*PAGE_SIZE) > will map to [TEST_MEM_BASE, TEST_MEM_BASE + 2*PAGE_SIZE)? Hmm, no, at some point we need to concretely state the addresses, so that people debugging this know what to expect, i.e. don't have to manually compute the addresses from the macros in order to debug. > > * > > * When TDP is disabled, the L2 guest code will still access the same L1 > > * GPAs as the TDP enabled case. > > + * > > + * Set the Dirty bit in the PTEs used by L2 so that KVM will create > > + * writable SPTEs when handling read faults (if the Dirty bit isn't > > + * set, KVM must intercept the next write to emulate the Dirty bit > > + * update). > > */ > > if (nested_tdp) { > > + vm_vaddr_t gva0 = TEST_GUEST_ADDR(TEST_MEM_ALIAS_BASE, 0); > > + vm_vaddr_t gva1 = TEST_GUEST_ADDR(TEST_MEM_ALIAS_BASE, 1); > > Why are these gvas? Should these be L2 GPAs? Pure oversight. > Maybe 'uint64_t l2_gpa0' or 'uint64_t nested_paddr0'? For better of worse, vm_paddr_t is the typedef in selftests. Hmm, if/when we go with David M's proposal to switch to u64 (from e.g. uint64_t), it'd probably be a good time to switch to KVM's gva_t and gpa_t as well. > Also maybe add TEST_ALIAS_GPA() macro to keep things consistent? Ya, then the line lengths are short enough to omit the local variables. How's this look? /* * ... pages in the L2 GPA address range [0xc0002000, 0xc0004000) will * map to [0xc0000000, 0xc0002000) when TDP is enabled (for L2). * * When TDP is disabled, the L2 guest code will still access the same L1 * GPAs as the TDP enabled case. * * Set the Dirty bit in the PTEs used by L2 so that KVM will create * writable SPTEs when handling read faults (if the Dirty bit isn't * set, KVM must intercept the next write to emulate the Dirty bit * update). */ if (nested_tdp) { tdp_identity_map_default_memslots(vm); tdp_map(vm, TEST_ALIAS_GPA(0), TEST_GPA(0), PAGE_SIZE); tdp_map(vm, TEST_ALIAS_GPA(1), TEST_GPA(1), PAGE_SIZE); *tdp_get_pte(vm, TEST_ALIAS_GPA(0)) |= PTE_DIRTY_MASK(&vm->stage2_mmu); *tdp_get_pte(vm, TEST_ALIAS_GPA(1)) |= PTE_DIRTY_MASK(&vm->stage2_mmu); } else { *vm_get_pte(vm, TEST_GVA(0)) |= PTE_DIRTY_MASK(&vm->mmu); *vm_get_pte(vm, TEST_GVA(1)) |= PTE_DIRTY_MASK(&vm->mmu); } _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv