Re: [PATCH 08/13] x86/virt/tdx: Add SEAMCALL wrappers to add TD private pages

Yan Zhao
2025-01-15T05:51:03Z

On Wed, Jan 15, 2025 at 12:32:24AM +0100, Paolo Bonzini wrote:
> On 1/6/25 06:50, Yan Zhao wrote:
> > Yeah.
> > So, do you think we need to have tdh_mem_page_aug() to support 4K level page
> > only and ask for Dave's review again for huge page?
>
> You're right that TDH.MEM.PAGE.AUG is basically the only case in which a
> struct folio is involved; on the other hand that also means that the
> arch/x86/virt part of large page support will be tiny and I don't think it
> will be a problem to review it again (for either Dave or myself).
>
> > Do we need to add param "level" ?
> > - if yes, "struct page" looks not fit.
>
> Maybe, but I think adding folio knowledge now would be a bit too
> hypothetical.
>
> > - if not, hardcode it as 0 in the wrapper and convert "pfn" to "struct page"?
>
> I think it makes sense to add "int level" now everywhere, even if it is just
> to match the SEPT API and to have the same style for computing the SEAMCALL
> arguments. I'd rather keep the arguments simple with just "gpa | level"
> (i.e. gpa/level instead of gfn/level) as the computation: that's because gpa
> is more obviously a u64.
>
> I've pushed to kvm-coco-queue; if you have some time to double check what I
> did that's great, otherwise if I don't hear from you I'll post around noon
> European time the v3 of this series.
For tdh_mem_sept_add(), tdh_mem_page_aug(), tdh_mem_page_add()
- Use tdx_clflush_page() instead of invoking clflush_cache_range() directly to
share the common comment of tdx_clflush_page().
- prefer page_to_phy() over page_to_pfn()?
https://lore.kernel.org/kvm/0070a616-5233-4a8d-8797-eb9f182f074d@intel.com/

4d0824a1daba ("x86/virt/tdx: Add SEAMCALL wrappers to add TD private pages")
contains unexpected changes to tdh_mem_sept_add().
u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2)
{
+ u64 hpa = page_to_pfn(page) << PAGE_SHIFT;
struct tdx_module_args args = {
.rcx = gpa | level,
.rdx = tdx_tdr_pa(td),
- .r8 = page_to_pfn(page) << PAGE_SHIFT,
+ .r8 = hpa,
};
u64 ret;

- clflush_cache_range(page_to_virt(page), PAGE_SIZE);
+ clflush_cache_range(__va(hpa), PAGE_SIZE);
ret = seamcall_ret(TDH_MEM_SEPT_ADD, &args);

*ext_err1 = args.rcx;
@@ -1522,6 +1544,26 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page)
}
EXPORT_SYMBOL_GPL(tdh_vp_addcx);

>
> I have also asked Amazon, since they use KVM without struct page, whether it
> is a problem to have struct page pervasively in the API and they don't care.
>
> Paolo
>