On Thu, 2025-07-31 at 01:06 +0000, Edgecombe, Rick P wrote: > On Wed, 2025-06-25 at 11:06 -0700, Dave Hansen wrote: > > This is the wrong place to do this. > > > > Hide it in tdmr_get_pamt_sz(). Don't inject it in the main code flow > > here and complicate the for loop. > > I'm finding this tdmr_get_pamt_sz() maybe too strange to build on top of. > It iterates through these special TDX page sizes once, and calls into > tdmr_get_pamt_sz() for each, which in turn has a case statement for each > index. So the loop doesn't add much - each index still has its own line > of code inside tdmr_get_pamt_sz(). And then despite prepping the base/size > in an array via the loop, it has to be packed manually at the end for each > index. So I'm not sure if the general wisdom of doing things in a single way > is really adding much here. > > I'm wondering if something like the below might be a better base to build > on. For dpamt the "tdmr->pamt_4k_size =" line could just branch on > tdx_supports_dynamic_pamt(). Any thoughts on it as an alternative to the > suggestion to add the dpamt logic to tdmr_get_pamt_sz()? The code change LGTM, albeit I am not sure whether it is definitely better. For where to add dynamic PAMT logic, I think it's reasonable to put such logic into tdmr_get_pamt_sz() because it changes the amount of memory that we need to allocate for 4K page size. If we do dynamic PAMT logic at higher level, the code logic in tdmr_get_pamt_sz() to calculate PAMT size for 4K page will not be accurate, i.e., it is only correct w/o dynamic PAMT.