linux-kernel - Re: [PATCH v7 000/102] KVM TDX basic feature support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Ys9rcnyIZlUc76iG@google.com>
Date:   Thu, 14 Jul 2022 01:03:46 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     isaku.yamahata@...el.com
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        isaku.yamahata@...il.com, Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v7 000/102] KVM TDX basic feature support

On Mon, Jun 27, 2022, isaku.yamahata@...el.com wrote:
> From: Isaku Yamahata <isaku.yamahata@...el.com>
> 
> KVM TDX basic feature support
> 
> Hello.  This is v7 the patch series vof KVM TDX support.
> This is based on v5.19-rc1 + kvm/queue branch + TDX HOST patch series.
> The tree can be found at https://github.com/intel/tdx/tree/kvm-upstream
> How to run/test: It's describe at https://github.com/intel/tdx/wiki/TDX-KVM
> 
> Major changes from v6:
> - rebased to v5.19 base
> 
> TODO:
> - integrate fd-based guest memory. As the discussion is still on-going, I
>   intentionally dropped fd-based guest memory support yet.  The integration can
>   be found at https://github.com/intel/tdx/tree/kvm-upstream-workaround.
> - 2M large page support. It's work-in-progress.
> For large page support, there are several design choices. Here is the design options.
> Any thoughts/feedback?

Apologies, I didn't read beyond the intro paragraph.  In case something like this
comes up again, it's probably best to send a standalone email tagged RFC, I doubt
I'm the only one that missed this embedded RFC.

> KVM MMU Large page support for TDX
 
...

> * options to track private or shared
> At each page size (4KB, 2MB, and 1GB), track private, shared, or mixed (2MB and
> 1GB case). For 4KB each page, 1 bit per page is needed. private or shared.  For
> large pages (2MB and 1GB), 2 bits per large page is needed. (private, shared, or
> mixed).  When resolving KVM page fault, we don't want to check the lower-size
> pages to check if the given GPA can be a large for performance.  On MapGPA check
> it instead.
> 
> Option A). enhance kvm_arch_memory_slot
>   enum kvm_page_type {
>        KVM_PAGE_TYPE_INVALID,
>        KVM_PAGE_TYPE_SHARED,
>        KVM_PAGE_TYPE_PRIVATE,
>        KVM_PAGE_TYPE_MIXED,
>   };
> 
>   struct kvm_page_attr {
>        enum kvm_page_type type;
>   };
> 
>  struct kvm_arch_memory_slot {
>  +      struct kvm_page_attr *page_attr[KVM_NR_PAGE_SIZES];
> 
> Option B). steal one more bit SPTE_MIXED_MASK in addition to SPTE_SHARED_MASK
> If !SPTE_MIXED_MASK, it can be large page.
> 
> Option C). use SPTE_SHARED_MASK and kvm_mmu_page::mixed bitmap
> kvm_mmu_page::mixed bitmap of 1GB, root indicates mixed for 2MB, 1GB.
> 
> 
> * comparison
> A).
> + straightforward to implement
> + SPTE_SHARED_MASK isn't needed
> - memory overhead compared to B). or C).
> - more memory reference on KVM page fault
> 
> B).
> + simpler than C) (complex than A)?)
> + efficient on KVM page fault. (only SPTE reference)
> + low memory overhead
> - Waste precious SPTE bits.
> 
> C).
> + efficient on KVM page fault. (only SPTE reference)
> + low memory overhead
> - complicates MapGPA
> - scattered data structure

Option D). track shared regions in an Xarray, update kvm_arch_memory_slot.lpage_info
on insertion/removal to (dis)allow hugepages as needed.

  + efficient on KVM page fault (no new lookups)
  + zero memory overhead (assuming KVM has to eat the cost of the Xarray anyways)
  + straightforward to implement
  + can (and should) be merged as part of the UPM series

I believe xa_for_each_range() can be used to see if a given 2mb/1gb range is
completely covered (fully shared) or not covered at all (fully private), but I'm
not 100% certain that xa_for_each_range() works the way I think it does.