From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f202.google.com (mail-pl1-f202.google.com [209.85.214.202]) (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 0F0112F6179 for ; Thu, 29 Jan 2026 19:09:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769713788; cv=none; b=Ktn/htG2OC2q+GpQHaqcq0BDSbeCEWmugGdlrIC/n1vQ9KBgG9XRaqjF9ywP4fK/gwrKU5J9Qh9JBqW+F49oDTcsKCaM1jL0RE+NxajQXo3OO+Ytc7h2E+TVZx5idaDhLhweUw6Yx46mhs/8pn+35lGy5Yc/ZLyLYpQqlVSw51g= 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=Rjq0ixXG; arc=none smtp.client-ip=209.85.214.202 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="Rjq0ixXG" Received: by mail-pl1-f202.google.com with SMTP id d9443c01a7336-2a8c54bbe46so16923665ad.2 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=vger.kernel.org; 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=Rjq0ixXGG7ewXIHUjYw3wviCW+JgSaXWoOyXHFVN+7qiH5CxQoYR6VUVKwmZefv9EV 5K0OZfEThR8SQSSIj9z/bndQ9GrJK1ujyizlJHCxT87ZMxkCg5OgKJ1DorkhlHi47j0d D+c4sq+MqKHe2GGrjME+2fpDp2cvGPFHhdfBhwlb9Kmgf45Lh+5lHkqTCe9QbSvmk63Y cE4ecpeBZ6UiCmyu2YbH3P/DNEwrgrQ2MonbjRQOXd/4J2NXb+w+MhBMwb7+WeRVq4US +STaENdunMRLbPO/TrYxyUSEi9iidykmsTbBJUq8Z3DxK8DJH/BOu4kbneJC77B4gLHh v/ig== 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=k8ch7DOFWYFfVQRhKnuHwFgwPRyJ+JEZ/o4wNEtNVw9aLKuil5EuuRITZ3REsnGUw4 NFEi2+1VSzlws8BYLxewQZDfJ40P+FB1zJA7Cg5ipReTqy9s390KG3bRqNrwE/U4Y4G9 nGPru/aaCjopfbfG5ZEE0ddFY86AVqgtNfgF5trH5mUJUrwuaeO7J1MT2PCHisGHwJLq zm2GRfzVuks1aWnm0GiTEOAwdY5L+W3sJwOPRfN1fQgDXBGusRM+D6LnulnRB1wP4gQv AoeTWFuuyRYmF6nvWKN9/DWIbp3pMFZxvr58sNiSfRAWxY6y0m0HalIA3tOPmdYEhaFd xwQw== X-Forwarded-Encrypted: i=1; AJvYcCXLgmBNG83OLjmiz9tMqpD4jTyVT+Q5yWzX2vMuddrIffm0nFn+q8ZYfTTj3zj0vH6FfMmgSKiwSAT9GSg=@vger.kernel.org X-Gm-Message-State: AOJu0Yw1VtPtI3syCaa+ByF08P6+/f8kyeazWisgjGSeOvy+6l4rk2py tnn5DKJhEjviM7ioykThlIIi0+GEWqTDHWgMp+HCz9tes4b9a+OhKugEH+AdKPtcNKDsyGJqmbM 31qp6Fw== 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-kernel@vger.kernel.org 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.