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 X-Spam-Level: X-Spam-Status: No, score=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FSL_HELO_FAKE, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AAA0FC432BE for ; Thu, 5 Aug 2021 16:06:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8FB4D610FC for ; Thu, 5 Aug 2021 16:06:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242894AbhHEQHD (ORCPT ); Thu, 5 Aug 2021 12:07:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36714 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242883AbhHEQHB (ORCPT ); Thu, 5 Aug 2021 12:07:01 -0400 Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D4E5DC0613D5 for ; Thu, 5 Aug 2021 09:06:45 -0700 (PDT) Received: by mail-pj1-x102e.google.com with SMTP id s22-20020a17090a1c16b0290177caeba067so16013625pjs.0 for ; Thu, 05 Aug 2021 09:06:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=gS8FXqlIEEYizkg3i3gXTEp2lHp5Ys6haxL7lS48eI8=; b=miRBqZUA8BbUIlZohBuu/kNx4j1tzah82MIu4dyhQGdEA0VMlmks9FBmM8o1OfLbDE LGg4+1FIyJnQojI0aE/D+9o3NVgjua82cVV6t/gUhboPyFCqN1Ga74AYWV9Jbb2ojx+G uLC4Pgxq4AkM+pZeaTQLCNQU/tyWOi6ibMtFPKZz+vvNd09+DzLB69uQbWRTkmc8Ikfq yXXsy0CRAsx1PBm96IIT769fOoDdkgQRaP/gRlJsFrRJ/tLJTCkzqB8ZYW0zLfb48Ts2 Rdm/QpnNp4zkbO2BE0eQBajw936AYGGnR1Oh5MUJEzCS3tgshADKN0dnaEhaBa0UlrpL oszA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=gS8FXqlIEEYizkg3i3gXTEp2lHp5Ys6haxL7lS48eI8=; b=H3gVQNVAoOdefUvmGOTEMWRdFzNOl9UhNGT4Cd3MRxWNUZU3f8393x5E4jffWaEXbl 3W7xwMKqUt3IImySS0zGLXQsurYJqCpoLLLfUfWvUVGu5pCOqsHV2adVp/z/Uo9XJ1gi b3v0ogYZZMrDf/hTbmKn4i1mR4otTy3fjCnC7AIiWs7/3ueQDMHRr7aF2JucravN7jQi kT5zXwVKQCDnEIVlvMH4sWeB0R+6v7dbzDFXfFy2SFHU6Nhn3CUoeImK2w6+2bgN8vG8 q/lLsHzX9JSuWJmNhPSMYAtSwrAmKGE8uwdQH/bmMwmuQpc4CVWInm3Z4T7eAXBXPAmC NJNA== X-Gm-Message-State: AOAM533tASmz7QVf2DdnwxkhdIdIxcWUUpwHusEeAY9Rw8WoD8IDk0/x NwRZSsBScTVr1RgMLlOKMtJnKA== X-Google-Smtp-Source: ABdhPJxelZ+Nxxc4JwbVy64K4qs/9MVt5N26BOgWzQ/3UeKKo3CG9tIj7GO/UOHPjDVmDR4y9iuAoQ== X-Received: by 2002:a17:90a:150d:: with SMTP id l13mr5379126pja.93.1628179605081; Thu, 05 Aug 2021 09:06:45 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id a21sm10421833pjo.15.2021.08.05.09.06.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Aug 2021 09:06:44 -0700 (PDT) Date: Thu, 5 Aug 2021 16:06:41 +0000 From: Sean Christopherson To: Kai Huang Cc: isaku.yamahata@intel.com, Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , erdemaktas@google.com, Connor Kuehl , x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, isaku.yamahata@gmail.com, Rick Edgecombe Subject: Re: [RFC PATCH v2 41/69] KVM: x86: Add infrastructure for stolen GPA bits Message-ID: References: <20210805234424.d14386b79413845b990a18ac@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210805234424.d14386b79413845b990a18ac@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 05, 2021, Kai Huang wrote: > On Fri, 2 Jul 2021 15:04:47 -0700 isaku.yamahata@intel.com wrote: > > From: Rick Edgecombe > > @@ -2020,6 +2032,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, > > sp = kvm_mmu_alloc_page(vcpu, direct); > > > > sp->gfn = gfn; > > + sp->gfn_stolen_bits = gfn_stolen_bits; > > sp->role = role; > > hlist_add_head(&sp->hash_link, sp_list); > > if (!direct) { > > @@ -2044,6 +2057,13 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, > > return sp; > > } > > > Sorry for replying old thread, Ha, one month isn't old, it's barely even mature. > but to me it looks weird to have gfn_stolen_bits > in 'struct kvm_mmu_page'. If I understand correctly, above code basically > means that GFN with different stolen bit will have different 'struct > kvm_mmu_page', but in the context of this patch, mappings with different > stolen bits still use the same root, You're conflating "mapping" with "PTE". The GFN is a per-PTE value. Yes, there is a final GFN that is representative of the mapping, but more directly the final GFN is associated with the leaf PTE. TDX effectively adds the restriction that all PTEs used for a mapping must have the same shared/private status, so mapping and PTE are somewhat interchangeable when talking about stolen bits (the shared bit), but in the context of this patch, the stolen bits are a property of the PTE. Back to your statement, it's incorrect. PTEs (effectively mappings in TDX) with different stolen bits will _not_ use the same root. kvm_mmu_get_page() includes the stolen bits in both the hash lookup and in the comparison, i.e. restores the stolen bits when looking for an existing shadow page at the target GFN. @@ -1978,9 +1990,9 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, role.quadrant = quadrant; } - sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)]; + sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn(gfn_and_stolen)]; for_each_valid_sp(vcpu->kvm, sp, sp_list) { - if (sp->gfn != gfn) { + if ((sp->gfn | sp->gfn_stolen_bits) != gfn_and_stolen) { collisions++; continue; } > which means gfn_stolen_bits doesn't make a lot of sense at least for root > page table. It does make sense, even without a follow-up patch. In Rick's original series, stealing a bit for execute-only guest memory, there was only a single root. And except for TDX, there can only ever be a single root because the shared EPTP isn't usable, i.e. there's only the regular/private EPTP. > Instead, having gfn_stolen_bits in 'struct kvm_mmu_page' only makes sense in > the context of TDX, since TDX requires two separate roots for private and > shared mappings. > So given we cannot tell whether the same root, or different roots should be > used for different stolen bits, I think we should not add 'gfn_stolen_bits' to > 'struct kvm_mmu_page' and use it to determine whether to allocate a new table > for the same GFN, but should use a new role (i.e role.private) to determine. A new role would work, too, but it has the disadvantage of not automagically working for all uses of stolen bits, e.g. XO support would have to add another role bit. > And removing 'gfn_stolen_bits' in 'struct kvm_mmu_page' could also save some > memory. But I do like saving memory... One potentially bad idea would be to unionize gfn and stolen bits by shifting the stolen bits after they're extracted from the gpa, e.g. union { gfn_t gfn_and_stolen; struct { gfn_t gfn:52; gfn_t stolen:12; } }; the downsides being that accessing just the gfn would require an additional masking operation, and the stolen bits wouldn't align with reality.