[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4f8c63f8-6b5f-40a2-bc7e-34c4ccd30593@intel.com>
Date: Wed, 29 Oct 2025 16:51:17 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Sean Christopherson <seanjc@...gle.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 10/20/25 07:42, Sean Christopherson wrote:
...
> 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)))
Sean,
I hacked up a TDX physical address namespace for sparse. It's not awful.
It doesn't make the .c files any uglier (or prettier really). It
definitely adds code because it needs a handful of conversion functions.
But those are all one-liner functions.
Net, this approach seems to add a few conversion functions versus the
'struct page' approach. That's because there are at least a couple of
places that *need* a 'struct page' like tdx_unpin().
There's some wonkiness in this like using virtual addresses to back the
"paddr" type. I did that so we could still do NULL checks instead of
keeping some explicit "invalid paddr" value. It's hidden in the helpers
and not exposed to the users, but it is weird for sure. The important
part isn't what the type is in the end, it's that something is making it
opaque.
This can definitely be taken further like getting rid of
tdx->vp.tdvpr_pa precalcuation. But it's mostly a straight s/struct page
*/tdx_paddr_t/ replacement.
I'm not looking at this and jumping up and down for how much better it
makes the code. It certainly *can* find a few things by leveraging
sparse. But, honestly, after seeing that nobody runs or cares about
sparse on this code, it's hard to take it seriously.
Was this generally what you had in mind? Should I turn this into a real
series?
View attachment "tdx-sparse.patch" of type "text/x-patch" (20766 bytes)
Powered by blists - more mailing lists
 
