Received: by 2002:a05:6a10:c604:0:0:0:0 with SMTP id y4csp934555pxt; Thu, 5 Aug 2021 15:44:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyWURphKc/ySvCkRnQapc1rv1FWuFrYVqbUFx2WjCcL+rA7ndLyTUd249Ty2PUbQX9GdulL X-Received: by 2002:a17:906:4e89:: with SMTP id v9mr7101502eju.300.1628203499154; Thu, 05 Aug 2021 15:44:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1628203499; cv=none; d=google.com; s=arc-20160816; b=DBYv/6u6Wz0Y4LgHaO5NHpmhBd5b/x+wA/txaN/dgUvnX7y2XVNyvibSqu+zS8o20M ZD6ghbbYpDDukTC9k7OKEMhMRBxKqgfjzK7pc9FYUd6zftqSKdgBOLUAFllMP/+wnCJQ Y/ocwd33hzLkChUZEsP0LX/wCktZTV6p19UghRcl5byoPp5CeeGH2CS9hBspuDfKxShR thMEz8foRqdpds3JSuW59fl3jOYi52dzeAcBQ6E6PJ39262VxN5PK3W623YranACFl7C LoJa7k3dyKMo2TS20QyxS0YuQX05MDr2XQSaZy/ShFQWcSyDje7Ah2eC/qGLJMJe3bqV 9gmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=gS8FXqlIEEYizkg3i3gXTEp2lHp5Ys6haxL7lS48eI8=; b=PwZ5MbmiAgrAbrE5dUElWI3q2Gu6chQ6Rsm8YnJiK4C6xmYQkMtDPrTmrYR9UrokJs 2OolVKNftcPWQP1nf5TYQ5lN+WEqWIuirNAiq/G80mALQTyZAkDPxE+2ZeGHqbA5V+t3 9QH/nwAJU4AV28BPHMPsDCSnhwVv7QgOyCHbC5ezUUoX5OIeULNMqJZBCjfZe7cnNuy1 hCn7B+gXKdQLx6/GVesAsFYWl+BU3fdByjx4sPxtMaJc4C9QiybqtcNPmJj9WCnhSnHE TTYlsRtLB5zvCda41rRzPCfr4pYRDcnmGl0uZTCRKW1CiHPtTIWZUnXnSK7UTX5fcqOy q9tg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=miRBqZUA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id t2si7336479ejf.408.2021.08.05.15.44.33; Thu, 05 Aug 2021 15:44:59 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=miRBqZUA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242655AbhHEQHD (ORCPT + 99 others); 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-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.