On Wed, 2025-07-23 at 23:26 +0000, Edgecombe, Rick P wrote:
> On Wed, 2025-07-23 at 23:01 +0000, Huang, Kai wrote:
> > Such renaming goes a little bit far IMHO.
> >
>
> I agree it's not quite necessary churn.
>
> > I respect the value of having
> > "quirk" in the name, but it also seems quite reasonable to me to hide such
> > "quirk" at the last level but just having "reset TDX pages" concept in the
> > higher levels.
>
> Assuming all the comments get corrected, this still leaves "reset" as an
> operation that sometimes eagerly resets the page, or sometimes leaves it to be
> lazily done later by a random access.
>
Thanks for the point.
Yeah I agree it's better to convey such information in the function name.
> Maybe instead of reset which is an action
> that sometimes is skipped, something that says what state we want the page to be
> at the end - ready to use.
>
> tdx_make_page_ready()
> tdx_make_page_usable()
> ...or something in that direction.
>
> But this is still churn. Kai, what do you think about the other option of just
> putting the X86_BUG_TDX_PW_MCE in tdx_reset_page() and letting the
> initialization error path (tdmrs_reset_pamt_all()) keep always zeroing the
> pages. So:
>
> static void tdx_reset_paddr(unsigned long base, unsigned long size)
> {
> /* doing MOVDIR64B ... */
> }
>
> static void tdmr_reset_pamt(struct tdmr_info *tdmr)
> {
> tdmr_do_pamt_func(tdmr, tdx_reset_paddr);
> }
>
> void tdx_quirk_reset_page(struct page *page)
> {
> if (!boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
> return;
>
> tdx_reset_paddr(page_to_phys(page), PAGE_SIZE);
> }
> EXPORT_SYMBOL_GPL(tdx_reset_page);
I don't think it's good idea to treat PAMT and other types of TDX memory
differently. I would rather go with the renaming as shown in Adrian's
patch.
So no objection from me. :-)
