Re: [RFC PATCH v2 03/23] x86/tdx: Enhance tdh_phymem_page_wbinvd_hkid() to invalidate huge pages

[Huang, Kai]

On Wed, 2025-11-12 at 16:43 +0800, Yan Zhao wrote:
> On Tue, Nov 11, 2025 at 05:23:30PM +0800, Huang, Kai wrote:
> > On Thu, 2025-08-07 at 17:42 +0800, Yan Zhao wrote:
> > > -u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page)
> > > +u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct folio *folio,
> > > +				unsigned long start_idx, unsigned long npages)
> > >  {
> > > +	struct page *start = folio_page(folio, start_idx);
> > >  	struct tdx_module_args args = {};
> > > +	u64 err;
> > > +
> > > +	if (start_idx + npages > folio_nr_pages(folio))
> > > +		return TDX_OPERAND_INVALID;
> > >  
> > > -	args.rcx = mk_keyed_paddr(hkid, page);
> > > +	for (unsigned long i = 0; i < npages; i++) {
> > > +		args.rcx = mk_keyed_paddr(hkid, nth_page(start, i));
> > >  
> > 
> > Just FYI: seems there's a series to remove nth_page() completely:
> > 
> > https://lore.kernel.org/kvm/20250901150359.867252-1-david@xxxxxxxxxx/
> Ah, thanks!
> Then we can get rid of the "unsigned long i".
> 
> -       for (unsigned long i = 0; i < npages; i++) {
> -               args.rcx = mk_keyed_paddr(hkid, nth_page(start, i));
> +       while (npages--) {
> +               args.rcx = mk_keyed_paddr(hkid, start++);
> 

You may want to be careful about doing '++' on a 'struct page *'.  I am not
expert, but I saw below discussion on the thread [*] which led to the series
to get rid of nth_page():

  > 
  > I wish we didn't have nth_page() at all. I really don't think it's a
  > valid operation. It's been around forever, but I think it was broken
  > as introduced, exactly because I don't think you can validly even have
  > allocations that cross section boundaries.

  Ordinary buddy allocations cannot exceed a memory section, but hugetlb and
  dax can with gigantic folios ... :(

  We had some weird bugs with that, because people keep forgetting that you
  cannot just use page++ unconditionally with such folios.

So, why not just get the actual page for each index within the loop?

[*]:
https://lore.kernel.org/all/CAHk-=wiCYfNp4AJLBORU-c7ZyRBUp66W2-Et6cdQ4REx-GyQ_A@xxxxxxxxxxxxxx/T/#m49ba78f5f630b27fa6d3d0737271f047af599c60