From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f201.google.com (mail-pl1-f201.google.com [209.85.214.201]) (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 3DFA030FF20 for ; Thu, 29 Jan 2026 19:09:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769713788; cv=none; b=BPHY0ZaqONeU62s0UqXjpFmBQddNEQyzTruk8rWw5DoLIAQrAQGFk0ZcvYUBnuwtHsIGXLKMscHH10Rt2gagLo0NxUPURbkeov0M+Np3SE7ppXvshsxopBz2RLsFiMXoyp3BPfB1yUeytcG9uSwpnOP25QQy8lkiGVT+rKFEkdw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769713788; c=relaxed/simple; bh=GWnKwGp1pTa5H5pvTE4nXJQ/OO60moc7qn2x3KKqBE8=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=cfVrLVyL1kXQB5DoM0jmrc1mKk/RS05UwDw8p3SM8ZbgDH9c88q7FukFC1kOhrxmoTqjXzk6bYDiyo8AXFNEu5W7pwDc9ZjYOIiUCw/xfGJph9oCVI7E1eGaoNpnFa3IKNA1ysZWrTHBy1NZmWe2U9JS49G8pua4/FMz5mQuxDQ= 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=AXgbihBf; arc=none smtp.client-ip=209.85.214.201 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="AXgbihBf" Received: by mail-pl1-f201.google.com with SMTP id d9443c01a7336-2a0a0bad5dfso28713555ad.0 for ; Thu, 29 Jan 2026 11:09:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1769713786; x=1770318586; darn=lists.linux.dev; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=wI6/cCp+y2hcCdfZTiCNenInU6VMtVbMj61ls3UA2Zc=; b=AXgbihBfSSOHE0mRwogjMjOECTOgg4rThGPwksOXJ4dwukGXr4FrFLDMVFC9UjMSED 8yGXKK3SxAWmYzJBnGT9WBF10VZhJhGb1tW7JxQ17Y8j4x5zVqouqJAxopVb2KxdeRMy lMkNnRBd8MfeEDkMy5lAmLKOUGIn2ccruDNtbL3DpcxGz1YNNRyh908kHVd1fxIlUeoz 9/p+JZxh2zBFcSDNvG6k3PLizfj64/HkjgC6J1HUjvUMcq7TBC91iLPkWWLyjh8R3+8w JDiQQNQgOMucNAx6DpL+/7bSjSrMuO1+WOzQitzn8ZOQMFU3Ts+1FGlmMvRrsGQ13zYI W1LQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1769713786; x=1770318586; h=content-transfer-encoding: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=wI6/cCp+y2hcCdfZTiCNenInU6VMtVbMj61ls3UA2Zc=; b=l488U4L/VG1OzLsKH+lgny5gPS+tXAZwwblgLGRwTT6aiFijOGi4kb0Jki/xhAl2i3 o0THTujf7WJPGa5LHxwsGUtl2w1IwJXB+cpghpOx9D2e/8opUlEEu/pAyMoFFIf7r6KP ZajNIdr+g1oCrgSpEo/PLThU7xt8BehhDOlY7Bno4ELGGfuktDbvRL+em4yjYJFiJ//8 x1+ML6bGpmOesHfqBuchE1XiAoor0xpn/nAFsLcRZ7JpFIuqK35jmfH/6vtylelECZRk fiTMYoR6r4RV7Bv6mM0J9AlfcBLybWDREkAQGtGgFSdZc31amcDVcNRn4uHX6Y11+2mh jlRA== X-Forwarded-Encrypted: i=1; AJvYcCXRzDu14k7XKaAA6DzKyUPZMeehRIwWLc856Pca2I8SRR9k5pCfbWxS3NggMA9QocSuVkTcg2EE/zM1@lists.linux.dev X-Gm-Message-State: AOJu0YxbmWTIZX1k/nPLaUxzAGtRl5hiOQh7cQahMRl5NkgG8OCOzwK8 /dhdsXgN+75cla+CTs1cC7ur2jAQA6XU5mUqaKrYzeA0Nk0UH4KuPC9x0ND0n5b899v+rDwFdsm xnsSaiQ== X-Received: from pldv14.prod.google.com ([2002:a17:902:ca8e:b0:2a0:a01a:4ee]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:903:11d0:b0:295:745a:8016 with SMTP id d9443c01a7336-2a8d959c542mr1979725ad.11.1769713786398; Thu, 29 Jan 2026 11:09:46 -0800 (PST) Date: Thu, 29 Jan 2026 11:09:44 -0800 In-Reply-To: <9096e7a47742f4a46a7f400aac467ac78e1dfe50.camel@intel.com> Precedence: bulk X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20251121005125.417831-1-rick.p.edgecombe@intel.com> <20251121005125.417831-8-rick.p.edgecombe@intel.com> <12144256-b71a-4331-8309-2e805dc120d1@linux.intel.com> <67d55b24ef1a80af615c3672e8436e0ac32e8efa.camel@intel.com> <9096e7a47742f4a46a7f400aac467ac78e1dfe50.camel@intel.com> Message-ID: Subject: Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers From: Sean Christopherson To: Rick P Edgecombe Cc: "kvm@vger.kernel.org" , "linux-coco@lists.linux.dev" , Kai Huang , Xiaoyao Li , Dave Hansen , Yan Y Zhao , Binbin Wu , "kas@kernel.org" , "binbin.wu@linux.intel.com" , "mingo@redhat.com" , "pbonzini@redhat.com" , "tglx@linutronix.de" , Isaku Yamahata , "linux-kernel@vger.kernel.org" , Vishal Annapurve , Chao Gao , "bp@alien8.de" , "x86@kernel.org" Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Thu, Jan 29, 2026, Rick P Edgecombe wrote: > On Wed, 2026-01-28 at 17:19 -0800, Sean Christopherson wrote: > > Honestly, the entire scheme is a mess.=C2=A0 Four days of staring at th= is > > and I finally undertand what the code is doing.=C2=A0 The whole "struct > > tdx_module_array_args" union is completely unnecessary, the resulting > > args.args crud is ugly, having a pile of duplicate accessors is > > brittle, the code obfuscates a simple concept, and the end result > > doesn't provide any actual protection since the kernel will happily > > overflow the buffer after the WARN. >=20 > The original sin for this, as was spotted by Nikilay in v3, is actually > that it turns out that the whole variable length thing was intended to > give the TDX module flexibility *if* it wanted to increase it in the > future. As in it's not required today. Worse, whether it would actually > grow in the specific way the code assumes is not covered in the spec. > Apparently it was based on some past internal discussions. So the > agreement on v3 was to just support the fixed two page size in the > spec. Heh, I was _this_ close to suggesting we add a compile-time assert on the i= ncoming size of the array (and effective regs array), with a constant max size supp= orted by the kernel. It wouldn't eliminate the array shenanigans, but it would l= et us make them more or less bombproof. > Yea it could probably use another DEFINE or two to make it less error > prone. Vanilla DPAMT has 4 instances of rdx. For me, it's not just a syntax problem. It's the approach of getting a poi= nter to the middle of structure and then doing a memcpy() at a later point in ti= me. More below. > What you have here is close to what I had done when I first took this > series. But it ran afoul of FORTIFY_SOUCE and required some horrible > casting to trick it. I wonder if this code will hit that issue too. AFAICT, FORTIFY_SOURCE doesn't complain. > Dave didn't like the solution and suggested the union actually: > https://lore.kernel.org/kvm/355ad607-52ed-42cc-9a48-63aaa49f4c68@intel.co= m/#t What you proposed is fundamentally quite different than what I'm proposing.= I'm not complaining about the union, I'm complaining about providing a helper t= o grab a pointer to the middle of a struct and then open coding memcpy() calls usi= ng that pointer. I find that _extremely_ difficult to grok, because it does a= poor job of capturing the intent (copy these values to this sequence of register= s). The decouple pointer+memcpy() approach also bleeds gory details about how P= AMT pages are passed in multiple args throughout all SEAMCALL APIs that have su= ch args. I want the APIs to not have to care about the underlying mechanics o= f how PAMT pages are copied from an array to registers, e.g. so that readers of t= he code can focus on the semantics of the SEAMCALL and the pieces of logic tha= t are unique to each SEAMCALL. In other words, I see the necessity for a union as being a symptom of the f= lawed approach. > I'm aware of your tendency to dislike union based solutions. But since > this was purely contained to tip, I went with Dave's preference.