[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aPZKVaUT9GZbPHBI@google.com>
Date: Mon, 20 Oct 2025 07:42:29 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Dave Hansen <dave.hansen@...ux.intel.com>, linux-kernel@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>, "Kirill A. Shutemov" <kas@...nel.org>,
Rick Edgecombe <rick.p.edgecombe@...el.com>, Paolo Bonzini <pbonzini@...hat.com>,
Kai Huang <kai.huang@...el.com>, Isaku Yamahata <isaku.yamahata@...el.com>,
Vishal Annapurve <vannapurve@...gle.com>, Thomas Huth <thuth@...hat.com>,
Adrian Hunter <adrian.hunter@...el.com>, linux-coco@...ts.linux.dev, kvm@...r.kernel.org,
Farrah Chen <farrah.chen@...el.com>
Subject: Re: [PATCH] x86/virt/tdx: Use precalculated TDVPR page physical address
On Mon, Oct 20, 2025, Dave Hansen wrote:
> On 10/20/25 06:57, Sean Christopherson wrote:
> > Why do these structures track struct page everywhere?
>
> I asked for it at some point. It allows an unambiguous reference to
> normal, (mostly) allocated physical memory. It means that you (mostly)
> can't accidentally swap in a virtual address, pfn, or something else
> that's not a physical address into the variable.
>
> The TDX ABI is just littered with u64's. There's almost no type safety
> anywhere. This is one place to bring a wee little bit of order to the chaos.
>
> In a perfect world, we'd have sparse annotations for the vaddr, paddr,
> pfn, dma_addr_t and all the other address spaces. Until then, I like
> passing struct page around.
But that clearly doesn't work since now the raw paddr is being passed in many
places, and we end up with goofy code like this where one param takes a raw paddr,
and another uses page_to_phys().
@@ -1583,7 +1578,7 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page)
{
struct tdx_module_args args = {
.rcx = page_to_phys(tdcx_page),
- .rdx = tdx_tdvpr_pa(vp),
+ .rdx = vp->tdvpr_pa,
};
If some form of type safety is the goal, why not do something like this?
typedef void __private *tdx_page_t;
Or maybe even define a new address space.
# define __tdx __attribute__((noderef, address_space(__tdx)))
The effective type safety is limited to sparse, but if you keep the tdx code free
of warnings, then any and all warnings from build bots can be treated as "fatal"
errors from a maintenance perspective.
Powered by blists - more mailing lists