Linux Confidential Computing Development
 help / color / mirror / Atom feed
From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
To: "Hansen, Dave" <dave.hansen@intel.com>,
	"kas@kernel.org" <kas@kernel.org>,
	"Gao, Chao" <chao.gao@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>,
	"Huang, Kai" <kai.huang@intel.com>,
	"Zhao, Yan Y" <yan.y.zhao@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"seanjc@google.com" <seanjc@google.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"nik.borisov@suse.com" <nik.borisov@suse.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"tglx@kernel.org" <tglx@kernel.org>,
	"Annapurve, Vishal" <vannapurve@google.com>,
	"bp@alien8.de" <bp@alien8.de>,
	"kirill.shutemov@linux.intel.com"
	<kirill.shutemov@linux.intel.com>,
	"x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH v6 06/11] x86/virt/tdx: Optimize tdx_pamt_get/put()
Date: Wed, 1 Jul 2026 01:05:18 +0000	[thread overview]
Message-ID: <bbc43329805cdb164f240841b45ce0d5151eeb5c.camel@intel.com> (raw)
In-Reply-To: <572868d7-4794-4fec-b80f-97d8434d5fb6@intel.com>

On Fri, 2026-06-05 at 09:23 -0700, Dave Hansen wrote:
> On 6/5/26 04:42, Kiryl Shutsemau wrote:
> > > > I don't see a reason why we can't keep the scoped_guard() on get side.
> > > One additional reason to drop scoped_guard() is that it mixes cleanup helpers
> > > with goto, which is discouraged. See [*]
> > > 
> > >   :Lastly, given that the benefit of cleanup helpers is removal of “goto”, and
> > >   :that the “goto” statement can jump between scopes, the expectation is that
> > >   :usage of “goto” and cleanup helpers is never mixed in the same function.
> > Fair enough.
> > 
> > But it can also be address if we free the PAMT page array with the guard
> > too :P
> 
> How important is this patch? I see "Optimize" but I read "Optional".
> 
> If we're arguing about it, maybe we should just kick it out and focus on
> the more important bits.

I had done some testing previously to see if the refcount solution avoided
contention enough:
   V2 of the series includes a global lock to be used around actual 
   installation/removal of the DPAMT backing, combined with opportunistic 
   checking outside the lock to avoid taking it most of the time. In testing, 
   booting 10 16GB TDs, the lock only hit contention 1136 times, with 4ms 
   waiting. This is very small for an operation that took 60s of wall time. 
   So despite being an (ugly) global lock, the actual impact was small. It 
   will probably further be reduced in the case of huge pages, where most of 
   the time 4KB DPAMT installation will not be necessary.
   https://lore.kernel.org/all/20250918232224.2202592-1-rick.p.edgecombe@intel.com/
   
But I did not test without this global lock avoiding optimization. I can look
into it.

The other way to drop this patch I was considering was to limit the situations
when dynamic PAMT gets enabled somehow, like early LASS support. A boot time
param would be the simplest. Or define a policy based on the number of keyids
with the reasoning that if you are only able to run one TD, you will at least
not contend with other TDs. Then we could revisit the optimization after huge
pages is settled.

For the subject of the arguing though, my vote would be to drop the scope guard
stuff and stick with the old pattern from the beginning.

  parent reply	other threads:[~2026-07-01  1:05 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-26  2:35 [PATCH v6 00/11] Dynamic PAMT Rick Edgecombe
2026-05-26  2:35 ` [PATCH v6 01/11] x86/virt/tdx: Simplify tdmr_get_pamt_sz() Rick Edgecombe
2026-06-04 16:05   ` Kiryl Shutsemau
2026-07-01  0:08     ` Edgecombe, Rick P
2026-06-11 18:25   ` Vishal Annapurve
2026-05-26  2:35 ` [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT Rick Edgecombe
2026-06-04 16:14   ` Kiryl Shutsemau
2026-07-01  0:14     ` Edgecombe, Rick P
2026-06-11 18:47   ` Vishal Annapurve
2026-05-26  2:35 ` [PATCH v6 03/11] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers Rick Edgecombe
2026-06-08  2:11   ` Binbin Wu
2026-06-08  2:18     ` Yan Zhao
2026-07-01  0:15       ` Edgecombe, Rick P
2026-05-26  2:35 ` [PATCH v6 04/11] x86/virt/tdx: Allocate ref counts for Dynamic PAMT memory Rick Edgecombe
2026-07-02  7:20   ` Binbin Wu
2026-05-26  2:35 ` [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put() Rick Edgecombe
2026-07-02  7:39   ` Binbin Wu
2026-05-26  2:35 ` [PATCH v6 06/11] x86/virt/tdx: Optimize tdx_pamt_get/put() Rick Edgecombe
2026-05-26  8:57   ` Chao Gao
2026-05-26 16:42     ` Edgecombe, Rick P
2026-06-04 16:59       ` Kiryl Shutsemau
2026-06-05  5:40         ` Chao Gao
2026-06-05 11:42           ` Kiryl Shutsemau
2026-06-05 16:23             ` Dave Hansen
2026-06-08  9:14               ` Kiryl Shutsemau
2026-06-08  9:50               ` Yan Zhao
2026-07-01  1:45                 ` Edgecombe, Rick P
2026-07-01  5:37                   ` Yan Zhao
2026-07-01  1:05               ` Edgecombe, Rick P [this message]
2026-05-26  2:35 ` [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures Rick Edgecombe
2026-05-26  2:35 ` [PATCH v6 08/11] x86/tdx: Add APIs to support Dynamic PAMT ops from KVM's fault path Rick Edgecombe
2026-06-04 17:11   ` Kiryl Shutsemau
2026-05-26  2:35 ` [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory Rick Edgecombe
2026-05-26  2:35 ` [PATCH v6 10/11] x86/virt/tdx: Enable Dynamic PAMT Rick Edgecombe
2026-06-04 17:14   ` Kiryl Shutsemau
2026-06-05  5:25     ` Chao Gao
2026-07-01  1:20       ` Edgecombe, Rick P
2026-05-26  2:35 ` [PATCH v6 11/11] Documentation/x86: Add documentation for TDX's " Rick Edgecombe
2026-06-08  5:45 ` [PATCH v6 00/11] " Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bbc43329805cdb164f240841b45ce0d5151eeb5c.camel@intel.com \
    --to=rick.p.edgecombe@intel.com \
    --cc=bp@alien8.de \
    --cc=chao.gao@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=kai.huang@intel.com \
    --cc=kas@kernel.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nik.borisov@suse.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@kernel.org \
    --cc=vannapurve@google.com \
    --cc=x86@kernel.org \
    --cc=yan.y.zhao@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox