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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z4dMgs7BKmmuaXeM@yzhao56-desk.sh.intel.com>
Date: Wed, 15 Jan 2025 13:49:54 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: Paolo Bonzini <pbonzini@...hat.com>
CC: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Huang, Kai" <kai.huang@...el.com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "Yamahata,
 Isaku" <isaku.yamahata@...el.com>
Subject: Re: [PATCH 08/13] x86/virt/tdx: Add SEAMCALL wrappers to add TD
 private pages

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
>