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@xxxxxxxxx/ 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 >
