From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:48:39
This is the next iteration of implementing fd-based (instead of vma-based)
memory for KVM guests. If you want the full background of why we are doing
this, please go read the v10 cover letter[1].
The biggest change from v10 is to implement the backing storage in KVM
itself, and expose it via a KVM ioctl() instead of a "generic" sycall.
See link[2] for details on why we pivoted to a KVM-specific approach.
Key word is "biggest". Relative to v10, there are many big changes.
Highlights below (I can't remember everything that got changed at
this point).
Tagged RFC as there are a lot of empty changelogs, and a lot of missing
documentation. And ideally, we'll have even more tests before merging.
There are also several gaps/opens (to be discussed in tomorrow's PUCK).
v11:
- Test private<=>shared conversions *without* doing fallocate()
- PUNCH_HOLE all memory between iterations of the conversion test so that
KVM doesn't retain pages in the guest_memfd
- Rename hugepage control to be a very generic ALLOW_HUGEPAGE, instead of
giving it a THP or PMD specific name.
- Fold in fixes from a lot of people (thank you!)
- Zap SPTEs *before* updating attributes to ensure no weirdness, e.g. if
KVM handles a page fault and looks at inconsistent attributes
- Refactor MMU interaction with attributes updates to reuse much of KVM's
framework for mmu_notifiers.
[1] https://lore.kernel.org/all/20221202061347.1070246-1-chao.p.peng@linux.intel.com
[2] https://lore.kernel.org/all/ZEM5Zq8oo+xnApW9@google.com
Ackerley Tng (1):
KVM: selftests: Test KVM exit behavior for private memory/access
Chao Peng (7):
KVM: Use gfn instead of hva for mmu_notifier_retry
KVM: Add KVM_EXIT_MEMORY_FAULT exit
KVM: Introduce per-page memory attributes
KVM: x86: Disallow hugepages when memory attributes are mixed
KVM: x86/mmu: Handle page fault for private memory
KVM: selftests: Add KVM_SET_USER_MEMORY_REGION2 helper
KVM: selftests: Expand set_memory_region_test to validate
guest_memfd()
Sean Christopherson (18):
KVM: Wrap kvm_gfn_range.pte in a per-action union
KVM: Tweak kvm_hva_range and hva_handler_t to allow reusing for gfn
ranges
KVM: PPC: Drop dead code related to KVM_ARCH_WANT_MMU_NOTIFIER
KVM: Convert KVM_ARCH_WANT_MMU_NOTIFIER to
CONFIG_KVM_GENERIC_MMU_NOTIFIER
KVM: Introduce KVM_SET_USER_MEMORY_REGION2
mm: Add AS_UNMOVABLE to mark mapping as completely unmovable
security: Export security_inode_init_security_anon() for use by KVM
KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing
memory
KVM: Add transparent hugepage support for dedicated guest memory
KVM: Drop superfluous __KVM_VCPU_MULTIPLE_ADDRESS_SPACE macro
KVM: Allow arch code to track number of memslot address spaces per VM
KVM: x86: Add support for "protected VMs" that can utilize private
memory
KVM: selftests: Drop unused kvm_userspace_memory_region_find() helper
KVM: selftests: Convert lib's mem regions to
KVM_SET_USER_MEMORY_REGION2
KVM: selftests: Add support for creating private memslots
KVM: selftests: Introduce VM "shape" to allow tests to specify the VM
type
KVM: selftests: Add GUEST_SYNC[1-6] macros for synchronizing more data
KVM: selftests: Add basic selftest for guest_memfd()
Vishal Annapurve (3):
KVM: selftests: Add helpers to convert guest memory b/w private and
shared
KVM: selftests: Add helpers to do KVM_HC_MAP_GPA_RANGE hypercalls
(x86)
KVM: selftests: Add x86-only selftest for private memory conversions
Documentation/virt/kvm/api.rst | 114 ++++
arch/arm64/include/asm/kvm_host.h | 2 -
arch/arm64/kvm/Kconfig | 2 +-
arch/arm64/kvm/mmu.c | 2 +-
arch/mips/include/asm/kvm_host.h | 2 -
arch/mips/kvm/Kconfig | 2 +-
arch/mips/kvm/mmu.c | 2 +-
arch/powerpc/include/asm/kvm_host.h | 2 -
arch/powerpc/kvm/Kconfig | 8 +-
arch/powerpc/kvm/book3s_hv.c | 2 +-
arch/powerpc/kvm/powerpc.c | 5 +-
arch/riscv/include/asm/kvm_host.h | 2 -
arch/riscv/kvm/Kconfig | 2 +-
arch/riscv/kvm/mmu.c | 2 +-
arch/x86/include/asm/kvm_host.h | 17 +-
arch/x86/include/uapi/asm/kvm.h | 3 +
arch/x86/kvm/Kconfig | 14 +-
arch/x86/kvm/debugfs.c | 2 +-
arch/x86/kvm/mmu/mmu.c | 287 +++++++-
arch/x86/kvm/mmu/mmu_internal.h | 4 +
arch/x86/kvm/mmu/mmutrace.h | 1 +
arch/x86/kvm/mmu/tdp_mmu.c | 8 +-
arch/x86/kvm/vmx/vmx.c | 11 +-
arch/x86/kvm/x86.c | 24 +-
include/linux/kvm_host.h | 129 +++-
include/linux/pagemap.h | 11 +
include/uapi/linux/kvm.h | 50 ++
include/uapi/linux/magic.h | 1 +
mm/compaction.c | 4 +
mm/migrate.c | 2 +
security/security.c | 1 +
tools/testing/selftests/kvm/Makefile | 3 +
tools/testing/selftests/kvm/dirty_log_test.c | 2 +-
.../testing/selftests/kvm/guest_memfd_test.c | 114 ++++
.../selftests/kvm/include/kvm_util_base.h | 141 +++-
.../testing/selftests/kvm/include/test_util.h | 5 +
.../selftests/kvm/include/ucall_common.h | 12 +
.../selftests/kvm/include/x86_64/processor.h | 15 +
.../selftests/kvm/kvm_page_table_test.c | 2 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 230 ++++---
tools/testing/selftests/kvm/lib/memstress.c | 3 +-
.../selftests/kvm/set_memory_region_test.c | 99 +++
.../kvm/x86_64/private_mem_conversions_test.c | 408 +++++++++++
.../kvm/x86_64/private_mem_kvm_exits_test.c | 115 ++++
.../kvm/x86_64/ucna_injection_test.c | 2 +-
virt/kvm/Kconfig | 17 +
virt/kvm/Makefile.kvm | 1 +
virt/kvm/dirty_ring.c | 2 +-
virt/kvm/guest_mem.c | 635 ++++++++++++++++++
virt/kvm/kvm_main.c | 384 +++++++++--
virt/kvm/kvm_mm.h | 38 ++
51 files changed, 2700 insertions(+), 246 deletions(-)
create mode 100644 tools/testing/selftests/kvm/guest_memfd_test.c
create mode 100644 tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
create mode 100644 tools/testing/selftests/kvm/x86_64/private_mem_kvm_exits_test.c
create mode 100644 virt/kvm/guest_mem.c
base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
--
2.41.0.255.g8b1d071c50-goog
@@ -1241,7 +1241,7 @@ static bool set_spte_gfn(struct kvm *kvm, struct tdp_iter *iter,u64new_spte;/* Huge pages aren't expected to be modified without first being zapped. */-WARN_ON(pte_huge(range->pte)||range->start+1!=range->end);+WARN_ON(pte_huge(range->arg.pte)||range->start+1!=range->end);if(iter->level!=PG_LEVEL_4K||!is_shadow_present_pte(iter->old_spte))
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:49:17
From: Chao Peng <redacted>
Currently in mmu_notifier invalidate path, hva range is recorded and
then checked against by mmu_notifier_retry_hva() in the page fault
handling path. However, for the to be introduced private memory, a page
fault may not have a hva associated, checking gfn(gpa) makes more sense.
For existing hva based shared memory, gfn is expected to also work. The
only downside is when aliasing multiple gfns to a single hva, the
current algorithm of checking multiple ranges could result in a much
larger range being rejected. Such aliasing should be uncommon, so the
impact is expected small.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Chao Peng <redacted>
Reviewed-by: Fuad Tabba <redacted>
Tested-by: Fuad Tabba <redacted>
[sean: convert vmx_set_apic_access_page_addr() to gfn-based API]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/mmu/mmu.c | 10 ++++++----
arch/x86/kvm/vmx/vmx.c | 11 +++++------
include/linux/kvm_host.h | 33 +++++++++++++++++++++------------
virt/kvm/kvm_main.c | 40 +++++++++++++++++++++++++++++++---------
4 files changed, 63 insertions(+), 31 deletions(-)
@@ -85,7 +85,7 @@ config KVM_BOOK3S_64_HVtristate"KVM for POWER7 and later using hypervisor mode in host"depends onKVM_BOOK3S_64&&PPC_POWERNVselectKVM_BOOK3S_HV_POSSIBLE-selectMMU_NOTIFIER+selectKVM_GENERIC_MMU_NOTIFIERselectCMAhelpSupportrunningunmodifiedbook3s_64guestkernelsin
@@ -4779,15 +4788,28 @@ static long kvm_vm_ioctl(struct file *filp,r=kvm_vm_ioctl_enable_cap_generic(kvm,&cap);break;}+caseKVM_SET_USER_MEMORY_REGION2:caseKVM_SET_USER_MEMORY_REGION:{-structkvm_userspace_memory_regionkvm_userspace_mem;+structkvm_userspace_memory_region2mem;+unsignedlongsize;++if(ioctl==KVM_SET_USER_MEMORY_REGION)+size=sizeof(structkvm_userspace_memory_region);+else+size=sizeof(structkvm_userspace_memory_region2);++/* Ensure the common parts of the two structs are identical. */+SANITY_CHECK_MEM_REGION_FIELD(slot);+SANITY_CHECK_MEM_REGION_FIELD(flags);+SANITY_CHECK_MEM_REGION_FIELD(guest_phys_addr);+SANITY_CHECK_MEM_REGION_FIELD(memory_size);+SANITY_CHECK_MEM_REGION_FIELD(userspace_addr);r=-EFAULT;-if(copy_from_user(&kvm_userspace_mem,argp,-sizeof(kvm_userspace_mem)))+if(copy_from_user(&mem,argp,size))gotoout;-r=kvm_vm_ioctl_set_memory_region(kvm,&kvm_userspace_mem);+r=kvm_vm_ioctl_set_memory_region(kvm,&mem);break;}caseKVM_GET_DIRTY_LOG:{
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:49:56
From: Chao Peng <redacted>
This new KVM exit allows userspace to handle memory-related errors. It
indicates an error happens in KVM at guest memory range [gpa, gpa+size).
The flags includes additional information for userspace to handle the
error. Currently bit 0 is defined as 'private memory' where '1'
indicates error happens due to private memory access and '0' indicates
error happens due to shared memory access.
When private memory is enabled, this new exit will be used for KVM to
exit to userspace for shared <-> private memory conversion in memory
encryption usage. In such usage, typically there are two kind of memory
conversions:
- explicit conversion: happens when guest explicitly calls into KVM
to map a range (as private or shared), KVM then exits to userspace
to perform the map/unmap operations.
- implicit conversion: happens in KVM page fault handler where KVM
exits to userspace for an implicit conversion when the page is in a
different state than requested (private or shared).
Suggested-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Yu Zhang <redacted>
Signed-off-by: Yu Zhang <redacted>
Signed-off-by: Chao Peng <redacted>
Reviewed-by: Fuad Tabba <redacted>
Tested-by: Fuad Tabba <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++++++
include/uapi/linux/kvm.h | 8 ++++++++
2 files changed, 30 insertions(+)
@@ -6700,6 +6700,28 @@ array field represents return values. The userspace should update the return values of SBI call before resuming the VCPU. For more details on RISC-V SBI spec refer, https://github.com/riscv/riscv-sbi-doc.+::++ /* KVM_EXIT_MEMORY_FAULT */+ struct {+ #define KVM_MEMORY_EXIT_FLAG_PRIVATE (1ULL << 3)+ __u64 flags;+ __u64 gpa;+ __u64 size;+ } memory;++If exit reason is KVM_EXIT_MEMORY_FAULT then it indicates that the VCPU has+encountered a memory error which is not handled by KVM kernel module and+userspace may choose to handle it. The 'flags' field indicates the memory+properties of the exit.++- KVM_MEMORY_EXIT_FLAG_PRIVATE - indicates the memory error is caused by+ private memory access when the bit is set. Otherwise the memory error is+ caused by shared memory access when the bit is clear.++'gpa' and 'size' indicate the memory range the error occurs at. The userspace+may handle the error and return to KVM to retry the previous memory access.+ :: /* KVM_EXIT_NOTIFY */
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:50:10
From: Chao Peng <redacted>
In confidential computing usages, whether a page is private or shared is
necessary information for KVM to perform operations like page fault
handling, page zapping etc. There are other potential use cases for
per-page memory attributes, e.g. to make memory read-only (or no-exec,
or exec-only, etc.) without having to modify memslots.
Introduce two ioctls (advertised by KVM_CAP_MEMORY_ATTRIBUTES) to allow
userspace to operate on the per-page memory attributes.
- KVM_SET_MEMORY_ATTRIBUTES to set the per-page memory attributes to
a guest memory range.
- KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES to return the KVM supported
memory attributes.
Use an xarray to store the per-page attributes internally, with a naive,
not fully optimized implementation, i.e. prioritize correctness over
performance for the initial implementation.
Because setting memory attributes is roughly analogous to mprotect() on
memory that is mapped into the guest, zap existing mappings prior to
updating the memory attributes. Opportunistically provide an arch hook
for the post-set path (needed to complete invalidation anyways) in
anticipation of x86 needing the hook to update metadata related to
determining whether or not a given gfn can be backed with various sizes
of hugepages.
It's possible that future usages may not require an invalidation, e.g.
if KVM ends up supporting RWX protections and userspace grants _more_
protections, but again opt for simplicity and punt optimizations to
if/when they are needed.
Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/all/Y2WB48kD0J4VGynX@google.com
Cc: Fuad Tabba <redacted>
Signed-off-by: Chao Peng <redacted>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Documentation/virt/kvm/api.rst | 60 ++++++++++++
include/linux/kvm_host.h | 14 +++
include/uapi/linux/kvm.h | 14 +++
virt/kvm/Kconfig | 4 +
virt/kvm/kvm_main.c | 170 +++++++++++++++++++++++++++++++++
5 files changed, 262 insertions(+)
@@ -6068,6 +6068,56 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG interface. No error will be returned, but the resulting offset will not be applied.+4.139 KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES+-----------------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm ioctl+:Parameters: u64 memory attributes bitmask(out)+:Returns: 0 on success, <0 on error++Returns supported memory attributes bitmask. Supported memory attributes will+have the corresponding bits set in u64 memory attributes bitmask.++The following memory attributes are defined::++ #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)++4.140 KVM_SET_MEMORY_ATTRIBUTES+-----------------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm ioctl+:Parameters: struct kvm_memory_attributes(in/out)+:Returns: 0 on success, <0 on error++Sets memory attributes for pages in a guest memory range. Parameters are+specified via the following structure::++ struct kvm_memory_attributes {+ __u64 address;+ __u64 size;+ __u64 attributes;+ __u64 flags;+ };++The user sets the per-page memory attributes to a guest memory range indicated+by address/size, and in return KVM adjusts address and size to reflect the+actual pages of the memory range have been successfully set to the attributes.+If the call returns 0, "address" is updated to the last successful address + 1+and "size" is updated to the remaining address size that has not been set+successfully. The user should check the return value as well as the size to+decide if the operation succeeded for the whole range or not. The user may want+to retry the operation with the returned address/size if the previous range was+partially successful.++Both address and size should be page aligned and the supported attributes can be+retrieved with KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES.++The "flags" field may be used for future extensions and should be set to 0s.+5. The kvm_run structure ========================
@@ -8494,6 +8544,16 @@ block sizes is exposed in KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES as a 64-bit bitmap (each bit describing a block size). The default value is 0, to disable the eager page splitting.+8.41 KVM_CAP_MEMORY_ATTRIBUTES+------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm++This capability indicates KVM supports per-page memory attributes and ioctls+KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES are available.+9. Known KVM API problems =========================
@@ -2301,4 +2305,14 @@ static inline void kvm_account_pgtable_pages(void *virt, int nr)/* Max number of entries allowed for each kvm dirty ring */#define KVM_DIRTY_RING_MAX_ENTRIES 65536+#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES+staticinlineunsignedlongkvm_get_memory_attributes(structkvm*kvm,gfn_tgfn)+{+returnxa_to_value(xa_load(&kvm->mem_attr_array,gfn));+}++boolkvm_arch_post_set_memory_attributes(structkvm*kvm,+structkvm_gfn_range*range);+#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */+#endif
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:50:12
From: Chao Peng <redacted>
Disallow creating hugepages with mixed memory attributes, e.g. shared
versus private, as mapping a hugepage in this case would allow the guest
to access memory with the wrong attributes, e.g. overlaying private memory
with a shared hugepage.
Tracking whether or not attributes are mixed via the existing
disallow_lpage field, but use the most significant bit in 'disallow_lpage'
to indicate a hugepage has mixed attributes instead using the normal
refcounting. Whether or not attributes are mixed is binary; either they
are or they aren't. Attempting to squeeze that info into the refcount is
unnecessarily complex as it would require knowing the previous state of
the mixed count when updating attributes. Using a flag means KVM just
needs to ensure the current status is reflected in the memslots.
Signed-off-by: Chao Peng <redacted>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 3 +
arch/x86/kvm/mmu/mmu.c | 185 +++++++++++++++++++++++++++++++-
arch/x86/kvm/x86.c | 4 +
3 files changed, 190 insertions(+), 2 deletions(-)
@@ -7223,3 +7234,173 @@ void kvm_mmu_pre_destroy_vm(struct kvm *kvm)if(kvm->arch.nx_huge_page_recovery_thread)kthread_stop(kvm->arch.nx_huge_page_recovery_thread);}++#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES+staticboolhugepage_test_mixed(structkvm_memory_slot*slot,gfn_tgfn,+intlevel)+{+returnlpage_info_slot(gfn,slot,level)->disallow_lpage&KVM_LPAGE_MIXED_FLAG;+}++staticvoidhugepage_clear_mixed(structkvm_memory_slot*slot,gfn_tgfn,+intlevel)+{+lpage_info_slot(gfn,slot,level)->disallow_lpage&=~KVM_LPAGE_MIXED_FLAG;+}++staticvoidhugepage_set_mixed(structkvm_memory_slot*slot,gfn_tgfn,+intlevel)+{+lpage_info_slot(gfn,slot,level)->disallow_lpage|=KVM_LPAGE_MIXED_FLAG;+}++staticboolrange_has_attrs(structkvm*kvm,gfn_tstart,gfn_tend,+unsignedlongattrs)+{+XA_STATE(xas,&kvm->mem_attr_array,start);+unsignedlongindex;+boolhas_attrs;+void*entry;++rcu_read_lock();++if(!attrs){+has_attrs=!xas_find(&xas,end);+gotoout;+}++has_attrs=true;+for(index=start;index<end;index++){+do{+entry=xas_next(&xas);+}while(xas_retry(&xas,entry));++if(xas.xa_index!=index||xa_to_value(entry)!=attrs){+has_attrs=false;+break;+}+}++out:+rcu_read_unlock();+returnhas_attrs;+}++staticboolhugepage_has_attrs(structkvm*kvm,structkvm_memory_slot*slot,+gfn_tgfn,intlevel,unsignedlongattrs)+{+constunsignedlongstart=gfn;+constunsignedlongend=start+KVM_PAGES_PER_HPAGE(level);++if(level==PG_LEVEL_2M)+returnrange_has_attrs(kvm,start,end,attrs);++for(gfn=start;gfn<end;gfn+=KVM_PAGES_PER_HPAGE(level-1)){+if(hugepage_test_mixed(slot,gfn,level-1)||+attrs!=kvm_get_memory_attributes(kvm,gfn))+returnfalse;+}+returntrue;+}++boolkvm_arch_post_set_memory_attributes(structkvm*kvm,+structkvm_gfn_range*range)+{+unsignedlongattrs=range->arg.attributes;+structkvm_memory_slot*slot=range->slot;+intlevel;++lockdep_assert_held_write(&kvm->mmu_lock);+lockdep_assert_held(&kvm->slots_lock);++/*+*KVMx86currentlyonlysupportsKVM_MEMORY_ATTRIBUTE_PRIVATE,skip+*theslotiftheslotwillneverconsumethePRIVATEattribute.+*/+if(!kvm_slot_can_be_private(slot))+returnfalse;++/*+*Thesequencemattershere:upperlevelsconsumetheresultoflower+*level'sscanning.+*/+for(level=PG_LEVEL_2M;level<=KVM_MAX_HUGEPAGE_LEVEL;level++){+gfn_tnr_pages=KVM_PAGES_PER_HPAGE(level);+gfn_tgfn=gfn_round_for_level(range->start,level);++/* Process the head page if it straddles the range. */+if(gfn!=range->start||gfn+nr_pages>range->end){+/*+*Skipmixedtrackingifthealignedgfnisn'tcovered+*bythememslot,KVMcan'tuseahugepageduetothe+*misalignedaddressregardlessofmemoryattributes.+*/+if(gfn>=slot->base_gfn){+if(hugepage_has_attrs(kvm,slot,gfn,level,attrs))+hugepage_clear_mixed(slot,gfn,level);+else+hugepage_set_mixed(slot,gfn,level);+}+gfn+=nr_pages;+}++/*+*Pagesentirelycoveredbytherangeareguaranteedtohave+*onlytheattributeswhichwerejustset.+*/+for(;gfn+nr_pages<=range->end;gfn+=nr_pages)+hugepage_clear_mixed(slot,gfn,level);++/*+*Processthelasttailpageifitstraddlestherangeandis+*containedbythememslot.Liketheheadpage,KVMcan't+*createahugepageiftheslotsizeismisaligned.+*/+if(gfn<range->end&&+(gfn+nr_pages)<=(slot->base_gfn+slot->npages)){+if(hugepage_has_attrs(kvm,slot,gfn,level,attrs))+hugepage_clear_mixed(slot,gfn,level);+else+hugepage_set_mixed(slot,gfn,level);+}+}+returnfalse;+}++voidkvm_mmu_init_memslot_memory_attributes(structkvm*kvm,+structkvm_memory_slot*slot)+{+intlevel;++if(!kvm_slot_can_be_private(slot))+return;++for(level=PG_LEVEL_2M;level<=KVM_MAX_HUGEPAGE_LEVEL;level++){+/*+*Don'tbothertrackingmixedattributesforpagesthatcan't+*behugeduetoalignment,i.e.processonlypagesthatare+*entirelycontainedbythememslot.+*/+gfn_tend=gfn_round_for_level(slot->base_gfn+slot->npages,level);+gfn_tstart=gfn_round_for_level(slot->base_gfn,level);+gfn_tnr_pages=KVM_PAGES_PER_HPAGE(level);+gfn_tgfn;++if(start<slot->base_gfn)+start+=nr_pages;++/*+*Unlikesettingattributes,everypotentialhugepageneedsto+*bemanuallycheckedastheattributesmayalreadybemixed.+*/+for(gfn=start;gfn<end;gfn+=nr_pages){+unsignedlongattrs=kvm_get_memory_attributes(kvm,gfn);++if(hugepage_has_attrs(kvm,slot,gfn,level,attrs))+hugepage_clear_mixed(slot,gfn,level);+else+hugepage_set_mixed(slot,gfn,level);+}+}+}+#endif
@@ -203,6 +203,7 @@ enum mapping_flags {/* writeback related tags are not used */AS_NO_WRITEBACK_TAGS=5,AS_LARGE_FOLIO_SUPPORT=6,+AS_UNMOVABLE=7,/* The mapping cannot be moved, ever */};/**
@@ -273,6 +274,16 @@ static inline int mapping_use_writeback_tags(struct address_space *mapping)return!test_bit(AS_NO_WRITEBACK_TAGS,&mapping->flags);}+staticinlinevoidmapping_set_unmovable(structaddress_space*mapping)+{+set_bit(AS_UNMOVABLE,&mapping->flags);+}++staticinlineboolmapping_unmovable(structaddress_space*mapping)+{+returntest_bit(AS_UNMOVABLE,&mapping->flags);+}+staticinlinegfp_tmapping_gfp_mask(structaddress_space*mapping){returnmapping->gfp_mask;
@@ -0,0 +1,591 @@+// SPDX-License-Identifier: GPL-2.0+#include<linux/backing-dev.h>+#include<linux/falloc.h>+#include<linux/kvm_host.h>+#include<linux/pagemap.h>+#include<linux/pseudo_fs.h>++#include<uapi/linux/magic.h>++#include"kvm_mm.h"++staticstructvfsmount*kvm_gmem_mnt;++structkvm_gmem{+structkvm*kvm;+structxarraybindings;+structlist_headentry;+};++staticstructfolio*kvm_gmem_get_folio(structfile*file,pgoff_tindex)+{+structfolio*folio;++/* TODO: Support huge pages. */+folio=filemap_grab_folio(file->f_mapping,index);+if(!folio)+returnNULL;++/*+*Usetheup-to-dateflagtotrackwhetherornotthememoryhasbeen+*zeroedbeforebeinghandedofftotheguest.Thereisnobacking+*storageforthememory,sothefoliowillremainup-to-dateuntil+*it'sremoved.+*+*TODO:Skipclearingpageswhentrustedfirmwarewilldoitwhen+*assigningmemorytotheguest.+*/+if(!folio_test_uptodate(folio)){+unsignedlongnr_pages=folio_nr_pages(folio);+unsignedlongi;++for(i=0;i<nr_pages;i++)+clear_highpage(folio_page(folio,i));++folio_mark_uptodate(folio);+}++/*+*Ignoreaccessed,referenced,anddirtyflags.Thememoryis+*unevictableandthereisnostoragetowritebackto.+*/+returnfolio;+}++staticvoidkvm_gmem_invalidate_begin(structkvm_gmem*gmem,pgoff_tstart,+pgoff_tend)+{+structkvm_memory_slot*slot;+structkvm*kvm=gmem->kvm;+unsignedlongindex;+boolflush=false;++KVM_MMU_LOCK(kvm);++kvm_mmu_invalidate_begin(kvm);++xa_for_each_range(&gmem->bindings,index,slot,start,end-1){+pgoff_tpgoff=slot->gmem.pgoff;++structkvm_gfn_rangegfn_range={+.start=slot->base_gfn+max(pgoff,start)-pgoff,+.end=slot->base_gfn+min(pgoff+slot->npages,end)-pgoff,+.slot=slot,+.may_block=true,+};++flush|=kvm_mmu_unmap_gfn_range(kvm,&gfn_range);+}++if(flush)+kvm_flush_remote_tlbs(kvm);++KVM_MMU_UNLOCK(kvm);+}++staticvoidkvm_gmem_invalidate_end(structkvm_gmem*gmem,pgoff_tstart,+pgoff_tend)+{+structkvm*kvm=gmem->kvm;++KVM_MMU_LOCK(kvm);+if(xa_find(&gmem->bindings,&start,end-1,XA_PRESENT))+kvm_mmu_invalidate_end(kvm);+KVM_MMU_UNLOCK(kvm);+}++staticlongkvm_gmem_punch_hole(structinode*inode,loff_toffset,loff_tlen)+{+structlist_head*gmem_list=&inode->i_mapping->private_list;+pgoff_tstart=offset>>PAGE_SHIFT;+pgoff_tend=(offset+len)>>PAGE_SHIFT;+structkvm_gmem*gmem;++/*+*Bindingsmuststableacrossinvalidationtoensurethestart+end+*arebalanced.+*/+filemap_invalidate_lock(inode->i_mapping);++list_for_each_entry(gmem,gmem_list,entry)+kvm_gmem_invalidate_begin(gmem,start,end);++truncate_inode_pages_range(inode->i_mapping,offset,offset+len-1);++list_for_each_entry(gmem,gmem_list,entry)+kvm_gmem_invalidate_end(gmem,start,end);++filemap_invalidate_unlock(inode->i_mapping);++return0;+}++staticlongkvm_gmem_allocate(structinode*inode,loff_toffset,loff_tlen)+{+structaddress_space*mapping=inode->i_mapping;+pgoff_tstart,index,end;+intr;++/* Dedicated guest is immutable by default. */+if(offset+len>i_size_read(inode))+return-EINVAL;++filemap_invalidate_lock_shared(mapping);++start=offset>>PAGE_SHIFT;+end=(offset+len)>>PAGE_SHIFT;++r=0;+for(index=start;index<end;){+structfolio*folio;++if(signal_pending(current)){+r=-EINTR;+break;+}++folio=kvm_gmem_get_folio(inode,index);+if(!folio){+r=-ENOMEM;+break;+}++index=folio_next_index(folio);++folio_unlock(folio);+folio_put(folio);++/* 64-bit only, wrapping the index should be impossible. */+if(WARN_ON_ONCE(!index))+break;++cond_resched();+}++filemap_invalidate_unlock_shared(mapping);++returnr;+}++staticlongkvm_gmem_fallocate(structfile*file,intmode,loff_toffset,+loff_tlen)+{+intret;++if(!(mode&FALLOC_FL_KEEP_SIZE))+return-EOPNOTSUPP;++if(mode&~(FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE))+return-EOPNOTSUPP;++if(!PAGE_ALIGNED(offset)||!PAGE_ALIGNED(len))+return-EINVAL;++if(mode&FALLOC_FL_PUNCH_HOLE)+ret=kvm_gmem_punch_hole(file_inode(file),offset,len);+else+ret=kvm_gmem_allocate(file_inode(file),offset,len);++if(!ret)+file_modified(file);+returnret;+}++staticintkvm_gmem_release(structinode*inode,structfile*file)+{+structkvm_gmem*gmem=file->private_data;+structkvm_memory_slot*slot;+structkvm*kvm=gmem->kvm;+unsignedlongindex;++filemap_invalidate_lock(inode->i_mapping);++/*+*Preventconcurrentattemptsto*unbind*amemslot.Thisisthelast+*referencetothefileandthusnonewbindingscanbecreated,but+*dereferencingtheslotforexistingbindingsneedstobeprotected+*againstmemslotupdates,specificallysothatunbinddoesn'trace+*andfreethememslot(kvm_gmem_get_file()willreturnNULL).+*/+mutex_lock(&kvm->slots_lock);++xa_for_each(&gmem->bindings,index,slot)+rcu_assign_pointer(slot->gmem.file,NULL);++synchronize_rcu();++/*+*Allin-flightoperationsaregoneandnewbindingscanbecreated.+*ZapallSPTEspointedatbythisfile.Donotfreethebacking+*memory,asitslifetimeisassociatedwiththeinode,notthefile.+*/+kvm_gmem_invalidate_begin(gmem,0,-1ul);+kvm_gmem_invalidate_end(gmem,0,-1ul);++mutex_unlock(&kvm->slots_lock);++list_del(&gmem->entry);++filemap_invalidate_unlock(inode->i_mapping);++xa_destroy(&gmem->bindings);+kfree(gmem);++kvm_put_kvm(kvm);++return0;+}++staticstructfile*kvm_gmem_get_file(structkvm_memory_slot*slot)+{+structfile*file;++rcu_read_lock();++file=rcu_dereference(slot->gmem.file);+if(file&&!get_file_rcu(file))+file=NULL;++rcu_read_unlock();++returnfile;+}++staticconststructfile_operationskvm_gmem_fops={+.open=generic_file_open,+.release=kvm_gmem_release,+.fallocate=kvm_gmem_fallocate,+};++staticintkvm_gmem_migrate_folio(structaddress_space*mapping,+structfolio*dst,structfolio*src,+enummigrate_modemode)+{+WARN_ON_ONCE(1);+return-EINVAL;+}++staticintkvm_gmem_error_page(structaddress_space*mapping,structpage*page)+{+structlist_head*gmem_list=&mapping->private_list;+structkvm_memory_slot*slot;+structkvm_gmem*gmem;+unsignedlongindex;+pgoff_tstart,end;+gfn_tgfn;++filemap_invalidate_lock_shared(mapping);++start=page->index;+end=start+thp_nr_pages(page);++list_for_each_entry(gmem,gmem_list,entry){+xa_for_each_range(&gmem->bindings,index,slot,start,end-1){+for(gfn=start;gfn<end;gfn++){+if(WARN_ON_ONCE(gfn<slot->base_gfn||+gfn>=slot->base_gfn+slot->npages))+continue;++/*+*FIXME:Telluserspacethatthe*private*+*memoryencounteredanerror.+*/+send_sig_mceerr(BUS_MCEERR_AR,+(void__user*)gfn_to_hva_memslot(slot,gfn),+PAGE_SHIFT,current);+}+}+}++filemap_invalidate_unlock_shared(mapping);++return0;+}++staticconststructaddress_space_operationskvm_gmem_aops={+.dirty_folio=noop_dirty_folio,+#ifdef CONFIG_MIGRATION+.migrate_folio=kvm_gmem_migrate_folio,+#endif+.error_remove_page=kvm_gmem_error_page,+};++staticintkvm_gmem_getattr(structmnt_idmap*idmap,+conststructpath*path,structkstat*stat,+u32request_mask,unsignedintquery_flags)+{+structinode*inode=path->dentry->d_inode;++/* TODO */+generic_fillattr(idmap,inode,stat);+return0;+}++staticintkvm_gmem_setattr(structmnt_idmap*idmap,structdentry*dentry,+structiattr*attr)+{+/* TODO */+return-EINVAL;+}+staticconststructinode_operationskvm_gmem_iops={+.getattr=kvm_gmem_getattr,+.setattr=kvm_gmem_setattr,+};++staticint__kvm_gmem_create(structkvm*kvm,loff_tsize,structvfsmount*mnt)+{+constchar*anon_name="[kvm-gmem]";+conststructqstrqname=QSTR_INIT(anon_name,strlen(anon_name));+structkvm_gmem*gmem;+structinode*inode;+structfile*file;+intfd,err;++inode=alloc_anon_inode(mnt->mnt_sb);+if(IS_ERR(inode))+returnPTR_ERR(inode);++err=security_inode_init_security_anon(inode,&qname,NULL);+if(err)+gotoerr_inode;++inode->i_private=(void*)(unsignedlong)flags;+inode->i_op=&kvm_gmem_iops;+inode->i_mapping->a_ops=&kvm_gmem_aops;+inode->i_mode|=S_IFREG;+inode->i_size=size;+mapping_set_gfp_mask(inode->i_mapping,GFP_HIGHUSER);+mapping_set_unevictable(inode->i_mapping);+mapping_set_unmovable(inode->i_mapping);++fd=get_unused_fd_flags(0);+if(fd<0){+err=fd;+gotoerr_inode;+}++file=alloc_file_pseudo(inode,mnt,"kvm-gmem",O_RDWR,&kvm_gmem_fops);+if(IS_ERR(file)){+err=PTR_ERR(file);+gotoerr_fd;+}++file->f_flags|=O_LARGEFILE;+file->f_mapping=inode->i_mapping;++gmem=kzalloc(sizeof(*gmem),GFP_KERNEL);+if(!gmem){+err=-ENOMEM;+gotoerr_file;+}++kvm_get_kvm(kvm);+gmem->kvm=kvm;+xa_init(&gmem->bindings);++file->private_data=gmem;++list_add(&gmem->entry,&inode->i_mapping->private_list);++fd_install(fd,file);+returnfd;++err_file:+fput(file);+err_fd:+put_unused_fd(fd);+err_inode:+iput(inode);+returnerr;+}++staticboolkvm_gmem_is_valid_size(loff_tsize,u64flags)+{+if(size<0||!PAGE_ALIGNED(size))+returnfalse;++returntrue;+}++intkvm_gmem_create(structkvm*kvm,structkvm_create_guest_memfd*args)+{+loff_tsize=args->size;+u64flags=args->flags;+u64valid_flags=0;++if(flags&~valid_flags)+return-EINVAL;++if(!kvm_gmem_is_valid_size(size,flags))+return-EINVAL;++return__kvm_gmem_create(kvm,size,flags,kvm_gmem_mnt);+}++intkvm_gmem_bind(structkvm*kvm,structkvm_memory_slot*slot,+unsignedintfd,loff_toffset)+{+loff_tsize=slot->npages<<PAGE_SHIFT;+unsignedlongstart,end,flags;+structkvm_gmem*gmem;+structinode*inode;+structfile*file;++BUILD_BUG_ON(sizeof(gfn_t)!=sizeof(slot->gmem.pgoff));++file=fget(fd);+if(!file)+return-EINVAL;++if(file->f_op!=&kvm_gmem_fops)+gotoerr;++gmem=file->private_data;+if(gmem->kvm!=kvm)+gotoerr;++inode=file_inode(file);+flags=(unsignedlong)inode->i_private;++/*+*Forsimplicity,requiretheoffsetintothefileandthesizeofthe+*memslottobealignedtothelargestpossiblepagesizeusedtoback+*thefile(sameasthesizeofthefileitself).+*/+if(!kvm_gmem_is_valid_size(offset,flags)||+!kvm_gmem_is_valid_size(size,flags))+gotoerr;++if(offset+size>i_size_read(inode))+gotoerr;++filemap_invalidate_lock(inode->i_mapping);++start=offset>>PAGE_SHIFT;+end=start+slot->npages;++if(!xa_empty(&gmem->bindings)&&+xa_find(&gmem->bindings,&start,end-1,XA_PRESENT)){+filemap_invalidate_unlock(inode->i_mapping);+gotoerr;+}++/*+*Nosynchronize_rcu()needed,anyin-flightreadersareguaranteedto+*beseeeitheraNULLfileorthisnewfile,noneedforthemtogo+*away.+*/+rcu_assign_pointer(slot->gmem.file,file);+slot->gmem.pgoff=start;++xa_store_range(&gmem->bindings,start,end-1,slot,GFP_KERNEL);+filemap_invalidate_unlock(inode->i_mapping);++/*+*Dropthereferencetothefile,evenonsuccess.ThefilepinsKVM,+*nottheotherway'round.Activebindingsareinvalidatedifthe+*fileisclosedbeforememslotsaredestroyed.+*/+fput(file);+return0;++err:+fput(file);+return-EINVAL;+}++voidkvm_gmem_unbind(structkvm_memory_slot*slot)+{+unsignedlongstart=slot->gmem.pgoff;+unsignedlongend=start+slot->npages;+structkvm_gmem*gmem;+structfile*file;++/*+*Nothingtodoiftheunderlyingfilewasalreadyclosed(orisbeing+*closedrightnow),kvm_gmem_release()invalidatesallbindings.+*/+file=kvm_gmem_get_file(slot);+if(!file)+return;++gmem=file->private_data;++filemap_invalidate_lock(file->f_mapping);+xa_store_range(&gmem->bindings,start,end-1,NULL,GFP_KERNEL);+rcu_assign_pointer(slot->gmem.file,NULL);+synchronize_rcu();+filemap_invalidate_unlock(file->f_mapping);++fput(file);+}++intkvm_gmem_get_pfn(structkvm*kvm,structkvm_memory_slot*slot,+gfn_tgfn,kvm_pfn_t*pfn,int*max_order)+{+pgoff_tindex=gfn-slot->base_gfn+slot->gmem.pgoff;+structkvm_gmem*gmem;+structfolio*folio;+structpage*page;+structfile*file;++file=kvm_gmem_get_file(slot);+if(!file)+return-EFAULT;++gmem=file->private_data;++if(WARN_ON_ONCE(xa_load(&gmem->bindings,index)!=slot)){+fput(file);+return-EIO;+}++folio=kvm_gmem_get_folio(file_inode(file),index);+if(!folio){+fput(file);+return-ENOMEM;+}++page=folio_file_page(folio,index);++*pfn=page_to_pfn(page);+*max_order=compound_order(compound_head(page));++folio_unlock(folio);+fput(file);++return0;+}+EXPORT_SYMBOL_GPL(kvm_gmem_get_pfn);++staticintkvm_gmem_init_fs_context(structfs_context*fc)+{+if(!init_pseudo(fc,GUEST_MEMORY_MAGIC))+return-ENOMEM;++return0;+}++staticstructfile_system_typekvm_gmem_fs={+.name="kvm_guest_memory",+.init_fs_context=kvm_gmem_init_fs_context,+.kill_sb=kill_anon_super,+};++intkvm_gmem_init(void)+{+kvm_gmem_mnt=kern_mount(&kvm_gmem_fs);+if(IS_ERR(kvm_gmem_mnt))+returnPTR_ERR(kvm_gmem_mnt);++/* For giggles. Userspace can never map this anyways. */+kvm_gmem_mnt->mnt_flags|=MNT_NOEXEC;++return0;+}++voidkvm_gmem_exit(void)+{+kern_unmount(kvm_gmem_mnt);+kvm_gmem_mnt=NULL;+}
@@ -992,6 +992,9 @@ static void kvm_destroy_dirty_bitmap(struct kvm_memory_slot *memslot)/* This does not remove the slot from struct kvm_memslots data structures */staticvoidkvm_free_memslot(structkvm*kvm,structkvm_memory_slot*slot){+if(slot->flags&KVM_MEM_PRIVATE)+kvm_gmem_unbind(slot);+kvm_destroy_dirty_bitmap(slot);kvm_arch_free_memslot(kvm,slot);
@@ -1556,10 +1559,18 @@ static void kvm_replace_memslot(struct kvm *kvm,}}-staticintcheck_memory_region_flags(conststructkvm_userspace_memory_region2*mem)+staticintcheck_memory_region_flags(structkvm*kvm,+conststructkvm_userspace_memory_region2*mem){u32valid_flags=KVM_MEM_LOG_DIRTY_PAGES;+if(kvm_arch_has_private_mem(kvm))+valid_flags|=KVM_MEM_PRIVATE;++/* Dirty logging private memory is not currently supported. */+if(mem->flags&KVM_MEM_PRIVATE)+valid_flags&=~KVM_MEM_LOG_DIRTY_PAGES;+#ifdef __KVM_HAVE_READONLY_MEMvalid_flags|=KVM_MEM_READONLY;#endif
@@ -1968,7 +1979,7 @@ int __kvm_set_memory_region(struct kvm *kvm,intas_id,id;intr;-r=check_memory_region_flags(mem);+r=check_memory_region_flags(kvm,mem);if(r)returnr;
@@ -1987,6 +1998,10 @@ int __kvm_set_memory_region(struct kvm *kvm,!access_ok((void__user*)(unsignedlong)mem->userspace_addr,mem->memory_size))return-EINVAL;+if(mem->flags&KVM_MEM_PRIVATE&&+(mem->gmem_offset&(PAGE_SIZE-1)||+mem->gmem_offset+mem->memory_size<mem->gmem_offset))+return-EINVAL;if(as_id>=KVM_ADDRESS_SPACE_NUM||id>=KVM_MEM_SLOTS_NUM)return-EINVAL;if(mem->guest_phys_addr+mem->memory_size<mem->guest_phys_addr)
@@ -2025,6 +2040,9 @@ int __kvm_set_memory_region(struct kvm *kvm,if((kvm->nr_memslot_pages+npages)<kvm->nr_memslot_pages)return-EINVAL;}else{/* Modify an existing slot. */+/* Private memslots are immutable, they can only be deleted. */+if(mem->flags&KVM_MEM_PRIVATE)+return-EINVAL;if((mem->userspace_addr!=old->userspace_addr)||(npages!=old->npages)||((mem->flags^old->flags)&KVM_MEM_READONLY))
@@ -2053,10 +2071,23 @@ int __kvm_set_memory_region(struct kvm *kvm,new->npages=npages;new->flags=mem->flags;new->userspace_addr=mem->userspace_addr;+if(mem->flags&KVM_MEM_PRIVATE){+r=kvm_gmem_bind(kvm,new,mem->gmem_fd,mem->gmem_offset);+if(r)+gotoout;+}r=kvm_set_memslot(kvm,old,new,change);if(r)-kfree(new);+gotoout_restricted;++return0;++out_restricted:+if(mem->flags&KVM_MEM_PRIVATE)+kvm_gmem_unbind(new);+out:+kfree(new);returnr;}EXPORT_SYMBOL_GPL(__kvm_set_memory_region);
@@ -2356,6 +2387,8 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTESstaticu64kvm_supported_mem_attributes(structkvm*kvm){+if(kvm_arch_has_private_mem(kvm))+returnKVM_MEMORY_ATTRIBUTE_PRIVATE;return0;}
@@ -5134,6 +5167,16 @@ static long kvm_vm_ioctl(struct file *filp,caseKVM_GET_STATS_FD:r=kvm_vm_ioctl_get_stats_fd(kvm);break;+caseKVM_CREATE_GUEST_MEMFD:{+structkvm_create_guest_memfdguest_memfd;++r=-EFAULT;+if(copy_from_user(&guest_memfd,argp,sizeof(guest_memfd)))+gotoout;++r=kvm_gmem_create(kvm,&guest_memfd);+break;+}default:r=kvm_arch_vm_ioctl(filp,ioctl,arg);}
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:51:34
From: Chao Peng <redacted>
A KVM_MEM_PRIVATE memslot can include both fd-based private memory and
hva-based shared memory. Architecture code (like TDX code) can tell
whether the on-going fault is private or not. This patch adds a
'is_private' field to kvm_page_fault to indicate this and architecture
code is expected to set it.
To handle page fault for such memslot, the handling logic is different
depending on whether the fault is private or shared. KVM checks if
'is_private' matches the host's view of the page (maintained in
mem_attr_array).
- For a successful match, private pfn is obtained with
restrictedmem_get_page() and shared pfn is obtained with existing
get_user_pages().
- For a failed match, KVM causes a KVM_EXIT_MEMORY_FAULT exit to
userspace. Userspace then can convert memory between private/shared
in host's view and retry the fault.
Co-developed-by: Yu Zhang <redacted>
Signed-off-by: Yu Zhang <redacted>
Signed-off-by: Chao Peng <redacted>
Reviewed-by: Fuad Tabba <redacted>
Tested-by: Fuad Tabba <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/mmu/mmu.c | 82 +++++++++++++++++++++++++++++++--
arch/x86/kvm/mmu/mmu_internal.h | 3 ++
arch/x86/kvm/mmu/mmutrace.h | 1 +
3 files changed, 81 insertions(+), 5 deletions(-)
@@ -203,6 +203,7 @@ struct kvm_page_fault {/* Derived from mmu and global state. */constboolis_tdp;+constboolis_private;constboolnx_huge_page_workaround_enabled;/*
@@ -748,9 +753,9 @@ struct kvm {structmm_struct*mm;/* userspace tied to this vm */unsignedlongnr_memslot_pages;/* The two memslot sets - active and inactive (per address space) */-structkvm_memslots__memslots[KVM_ADDRESS_SPACE_NUM][2];+structkvm_memslots__memslots[KVM_MAX_NR_ADDRESS_SPACES][2];/* The current active memslot set for each address space */-structkvm_memslots__rcu*memslots[KVM_ADDRESS_SPACE_NUM];+structkvm_memslots__rcu*memslots[KVM_MAX_NR_ADDRESS_SPACES];structxarrayvcpu_array;/**Protectedbyslots_lock,butcanbereadoutsideifan
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:52:15
Drop kvm_userspace_memory_region_find(), it's unused and a terrible API
(probably why it's unused). If anything outside of kvm_util.c needs to
get at the memslot, userspace_mem_region_find() can be exposed to give
others full access to all memory region/slot information.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/include/kvm_util_base.h | 4 ---
tools/testing/selftests/kvm/lib/kvm_util.c | 29 -------------------
2 files changed, 33 deletions(-)
@@ -147,10 +147,29 @@ described as 'basic' will be available. The new VM has no virtual cpus and no memory. You probably want to use 0 as machine type.+X86:+^^^^++Supported X86 VM types can be queried via KVM_CAP_VM_TYPES.++S390:+^^^^^+ In order to create user controlled virtual machines on S390, check KVM_CAP_S390_UCONTROL and use the flag KVM_VM_S390_UCONTROL as privileged user (CAP_SYS_ADMIN).+MIPS:+^^^^^++To use hardware assisted virtualization on MIPS (VZ ASE) rather than+the default trap & emulate implementation (which changes the virtual+memory layout to fit in user mode), check KVM_CAP_MIPS_VZ and use the+flag KVM_VM_MIPS_VZ.++ARM64:+^^^^^^+ On arm64, the physical address size for a VM (IPA Size limit) is limited to 40bits by default. The limit can be configured if the host supports the extension KVM_CAP_ARM_VM_IPA_SIZE. When supported, use
@@ -8554,6 +8573,19 @@ block sizes is exposed in KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES as a This capability indicates KVM supports per-page memory attributes and ioctls KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES are available.+8.41 KVM_CAP_VM_TYPES+---------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: system ioctl++This capability returns a bitmap of support VM types. The 1-setting of bit @n+means the VM type with value @n is supported. Possible values of @n are::++ #define KVM_X86_DEFAULT_VM 0+ #define KVM_X86_SW_PROTECTED_VM 1+9. Known KVM API problems =========================
@@ -4427,6 +4427,13 @@ static int kvm_ioctl_get_supported_hv_cpuid(struct kvm_vcpu *vcpu,return0;}+staticboolkvm_is_vm_type_supported(unsignedlongtype)+{+returntype==KVM_X86_DEFAULT_VM||+(type==KVM_X86_SW_PROTECTED_VM&&+IS_ENABLED(CONFIG_KVM_SW_PROTECTED_VM)&&tdp_enabled);+}+intkvm_vm_ioctl_check_extension(structkvm*kvm,longext){intr=0;
@@ -4617,6 +4624,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)caseKVM_CAP_X86_NOTIFY_VMEXIT:r=kvm_caps.has_notify_vmexit;break;+caseKVM_CAP_VM_TYPES:+r=BIT(KVM_X86_DEFAULT_VM);+if(kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM))+r|=BIT(KVM_X86_SW_PROTECTED_VM);+break;default:break;}
@@ -12274,9 +12286,11 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)intret;unsignedlongflags;-if(type)+if(!kvm_is_vm_type_supported(type))return-EINVAL;+kvm->arch.vm_type=type;+ret=kvm_page_track_init(kvm);if(ret)gotoout;
@@ -143,6 +143,11 @@ static inline bool backing_src_is_shared(enum vm_mem_backing_src_type t)returnvm_mem_backing_src_alias(t)->flag&MAP_SHARED;}+staticinlineboolbacking_src_can_be_huge(enumvm_mem_backing_src_typet)+{+returnt!=VM_MEM_SRC_ANONYMOUS&&t!=VM_MEM_SRC_SHMEM;+}+/* Aligns x up to the next multiple of size. Size must be a power of 2. */staticinlineuint64_talign_up(uint64_tx,uint64_tsize){
@@ -865,36 +867,15 @@ void vm_set_user_memory_region(struct kvm_vm *vm, uint32_t slot, uint32_t flags,errno,strerror(errno));}-/*-*VMUserspaceMemoryRegionAdd-*-*InputArgs:-*vm-VirtualMachine-*src_type-Storagesourceforthisregion.-*NULLtouseanonymousmemory.-*guest_paddr-Startingguestphysicaladdress-*slot-KVMregionslot-*npages-Numberofphysicalpages-*flags-KVMmemoryregionflags(e.g.KVM_MEM_LOG_DIRTY_PAGES)-*-*OutputArgs:None-*-*Return:None-*-*Allocatesamemoryareaofthenumberofpagesspecifiedbynpages-*andmapsittotheVMspecifiedbyvm,atastartingphysicaladdress-*givenbyguest_paddr.TheregioniscreatedwithaKVMregionslot-*givenbyslot,whichmustbeuniqueand<KVM_MEM_SLOTS_NUM.The-*regioniscreatedwiththeflagsgivenbyflags.-*/-voidvm_userspace_mem_region_add(structkvm_vm*vm,-enumvm_mem_backing_src_typesrc_type,-uint64_tguest_paddr,uint32_tslot,uint64_tnpages,-uint32_tflags)+/* FIXME: This thing needs to be ripped apart and rewritten. */+voidvm_mem_add(structkvm_vm*vm,enumvm_mem_backing_src_typesrc_type,+uint64_tguest_paddr,uint32_tslot,uint64_tnpages,+uint32_tflags,intgmem_fd,uint64_tgmem_offset){intret;structuserspace_mem_region*region;size_tbacking_src_pagesz=get_backing_src_pagesz(src_type);+size_tmem_size=npages*vm->page_size;size_talignment;TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode,npages)==npages,
@@ -947,7 +928,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,/* Allocate and initialize new mem region structure. */region=calloc(1,sizeof(*region));TEST_ASSERT(region!=NULL,"Insufficient Memory");-region->mmap_size=npages*vm->page_size;+region->mmap_size=mem_size;#ifdef __s390x__/* On s390x, the host address must be aligned to 1M (due to PGSTEs) */
@@ -994,14 +975,47 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,/* As needed perform madvise */if((src_type==VM_MEM_SRC_ANONYMOUS||src_type==VM_MEM_SRC_ANONYMOUS_THP)&&thp_configured()){-ret=madvise(region->host_mem,npages*vm->page_size,+ret=madvise(region->host_mem,mem_size,src_type==VM_MEM_SRC_ANONYMOUS?MADV_NOHUGEPAGE:MADV_HUGEPAGE);TEST_ASSERT(ret==0,"madvise failed, addr: %p length: 0x%lx src_type: %s",-region->host_mem,npages*vm->page_size,+region->host_mem,mem_size,vm_mem_backing_src_alias(src_type)->name);}region->backing_src_type=src_type;++if(flags&KVM_MEM_PRIVATE){+if(gmem_fd<0){+uint32_tgmem_flags=0;++/*+*Allowhugepagesfortheguestmemfdbackingifthe+*"normal"backingisallowed/requiredtobehuge.+*/+if(src_type!=VM_MEM_SRC_ANONYMOUS&&+src_type!=VM_MEM_SRC_SHMEM)+gmem_flags|=KVM_GUEST_MEMFD_ALLOW_HUGEPAGE;++TEST_ASSERT(!gmem_offset,+"Offset must be zero when creating new guest_memfd");+gmem_fd=vm_create_guest_memfd(vm,mem_size,gmem_flags);+}else{+/*+*Installauniquefdforeachmemslotsothatthefd+*canbeclosedwhentheregionisdeletedwithout+*needingtotrackifthefdisownedbytheframework+*orbythecaller.+*/+gmem_fd=dup(gmem_fd);+TEST_ASSERT(gmem_fd>=0,__KVM_SYSCALL_ERROR("dup()",gmem_fd));+}++region->region.gmem_fd=gmem_fd;+region->region.gmem_offset=gmem_offset;+}else{+region->region.gmem_fd=-1;+}+region->unused_phy_pages=sparsebit_alloc();sparsebit_set_num(region->unused_phy_pages,guest_paddr>>vm->page_shift,npages);
@@ -1171,6 +1171,32 @@ void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot)__vm_mem_region_delete(vm,memslot2region(vm,slot),true);}+voidvm_guest_mem_fallocate(structkvm_vm*vm,uint64_tgpa,uint64_tsize,+boolpunch_hole)+{+structuserspace_mem_region*region;+uint64_tend=gpa+size-1;+off_tfd_offset;+intmode,ret;++region=userspace_mem_region_find(vm,gpa,gpa);+TEST_ASSERT(region&®ion->region.flags&KVM_MEM_PRIVATE,+"Private memory region not found for GPA 0x%lx",gpa);++TEST_ASSERT(region==userspace_mem_region_find(vm,end,end),+"fallocate() for guest_memfd must act on a single memslot");++fd_offset=region->region.gmem_offset++(gpa-region->region.guest_phys_addr);++mode=FALLOC_FL_KEEP_SIZE|(punch_hole?FALLOC_FL_PUNCH_HOLE:0);++ret=fallocate(region->region.gmem_fd,mode,fd_offset,size);+TEST_ASSERT(!ret,"fallocate() failed to %s at %lx[%lu], fd = %d, mode = %x, offset = %lx\n",+punch_hole?"punch hole":"allocate",gpa,size,+region->region.gmem_fd,mode,fd_offset);+}+/* Returns the size of a vCPU's kvm_run structure. */staticintvcpu_mmap_sz(void){
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:53:00
From: Vishal Annapurve <redacted>
Add a selftest to exercise implicit/explicit conversion functionality
within KVM and verify:
- Shared memory is visible to host userspace
- Private memory is not visible to host userspace
- Host userspace and guest can communicate over shared memory
- Data in shared backing is preserved across conversions (test's
host userspace doesn't free the data)
- Private memory is bound to the lifetime of the VM
TODO: rewrite this to allow backing a single region of guest memory with
multiple memslots for _all_ backing types and shapes, i.e. make the code
for using a single backing fd across multiple memslots apply to regular
memory as well.
Signed-off-by: Vishal Annapurve <redacted>
Co-developed-by: Ackerley Tng <redacted>
Signed-off-by: Ackerley Tng <redacted>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../kvm/x86_64/private_mem_conversions_test.c | 408 ++++++++++++++++++
2 files changed, 409 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
@@ -0,0 +1,408 @@+// SPDX-License-Identifier: GPL-2.0+/*+*Copyright(C)2022,GoogleLLC.+*/+#define _GNU_SOURCE /* for program_invocation_short_name */+#include<fcntl.h>+#include<limits.h>+#include<pthread.h>+#include<sched.h>+#include<signal.h>+#include<stdio.h>+#include<stdlib.h>+#include<string.h>+#include<sys/ioctl.h>++#include<linux/compiler.h>+#include<linux/kernel.h>+#include<linux/kvm_para.h>+#include<linux/memfd.h>+#include<linux/sizes.h>++#include<test_util.h>+#include<kvm_util.h>+#include<processor.h>++#define BASE_DATA_SLOT 10+#define BASE_DATA_GPA ((uint64_t)(1ull << 32))+#define PER_CPU_DATA_SIZE ((uint64_t)(SZ_2M + PAGE_SIZE))++/* Horrific macro so that the line info is captured accurately :-( */+#define memcmp_g(gpa, pattern, size) \+do{\+uint8_t*mem=(uint8_t*)gpa;\+size_ti;\+\+for(i=0;i<size;i++)\+GUEST_ASSERT_4(mem[i]==pattern,\+gpa,i,mem[i],pattern);\+}while(0)++staticvoidmemcmp_h(uint8_t*mem,uint8_tpattern,size_tsize)+{+size_ti;++for(i=0;i<size;i++)+TEST_ASSERT(mem[i]==pattern,+"Expected 0x%x at offset %lu, got 0x%x",+pattern,i,mem[i]);+}++/*+*Runmemoryconversiontestswithexplicitconversion:+*ExecuteKVMhypercalltomap/unmapgparangewhichwillcauseuserspaceexit+*toback/unbackprivatememory.Subsequentaccessesbyguesttothegparange+*willnotcauseexittouserspace.+*+*Testmemoryconversionscenarioswithfollowingsteps:+*1)Accessprivatememoryusingprivateaccessandverifythatmemorycontents+*arenotvisibletouserspace.+*2)Convertmemorytosharedusingexplicitconversionsandensurethat+*userspaceisabletoaccessthesharedregions.+*3)Convertmemorybacktoprivateusingexplicitconversionsandensurethat+*userspaceisagainnotabletoaccessconvertedprivateregions.+*/++#define GUEST_STAGE(o, s) { .offset = o, .size = s }++enumucall_syncs{+SYNC_SHARED,+SYNC_PRIVATE,+};++staticvoidguest_sync_shared(uint64_tgpa,uint64_tsize,+uint8_tcurrent_pattern,uint8_tnew_pattern)+{+GUEST_SYNC5(SYNC_SHARED,gpa,size,current_pattern,new_pattern);+}++staticvoidguest_sync_private(uint64_tgpa,uint64_tsize,uint8_tpattern)+{+GUEST_SYNC4(SYNC_PRIVATE,gpa,size,pattern);+}++/* Arbitrary values, KVM doesn't care about the attribute flags. */+#define MAP_GPA_SHARED BIT(0)+#define MAP_GPA_DO_FALLOCATE BIT(1)++staticvoidguest_map_mem(uint64_tgpa,uint64_tsize,boolmap_shared,+booldo_fallocate)+{+uint64_tflags=0;++if(map_shared)+flags|=MAP_GPA_SHARED;+if(do_fallocate)+flags|=MAP_GPA_DO_FALLOCATE;+kvm_hypercall_map_gpa_range(gpa,size,flags);+}++staticvoidguest_map_shared(uint64_tgpa,uint64_tsize,booldo_fallocate)+{+guest_map_mem(gpa,size,true,do_fallocate);+}++staticvoidguest_map_private(uint64_tgpa,uint64_tsize,booldo_fallocate)+{+guest_map_mem(gpa,size,false,do_fallocate);+}++staticvoidguest_run_test(uint64_tbase_gpa,booldo_fallocate)+{+struct{+uint64_toffset;+uint64_tsize;+uint8_tpattern;+}stages[]={+GUEST_STAGE(0,PAGE_SIZE),+GUEST_STAGE(0,SZ_2M),+GUEST_STAGE(PAGE_SIZE,PAGE_SIZE),+GUEST_STAGE(PAGE_SIZE,SZ_2M),+GUEST_STAGE(SZ_2M,PAGE_SIZE),+};+constuint8_tinit_p=0xcc;+uint64_tj;+inti;++/* Memory should be shared by default. */+memset((void*)base_gpa,~init_p,PER_CPU_DATA_SIZE);+guest_sync_shared(base_gpa,PER_CPU_DATA_SIZE,(uint8_t)~init_p,init_p);+memcmp_g(base_gpa,init_p,PER_CPU_DATA_SIZE);++for(i=0;i<ARRAY_SIZE(stages);i++){+uint64_tgpa=base_gpa+stages[i].offset;+uint64_tsize=stages[i].size;+uint8_tp1=0x11;+uint8_tp2=0x22;+uint8_tp3=0x33;+uint8_tp4=0x44;++/*+*Setthetestregiontopatternonetodifferentiateitfrom+*thedatarangeasawhole(containstheinitialpattern).+*/+memset((void*)gpa,p1,size);++/*+*Converttoprivate,setandverifytheprivatedata,and+*thenverifythattherestofthedata(mapshared)still+*holdstheinitialpattern,andthatthehostalwaysseesthe+*sharedmemory(initialpattern).Unlikesharedmemory,+*punchingaholeinprivatememoryisdestructive,i.e.+*previousvaluesaren'tguaranteedtobepreserved.+*/+guest_map_private(gpa,size,do_fallocate);++if(size>PAGE_SIZE){+memset((void*)gpa,p2,PAGE_SIZE);+gotoskip;+}++memset((void*)gpa,p2,size);+guest_sync_private(gpa,size,p1);++/*+*Verifythattheprivatememorywassettopatterntwo,and+*thatsharedmemorystillholdstheinitialpattern.+*/+memcmp_g(gpa,p2,size);+if(gpa>base_gpa)+memcmp_g(base_gpa,init_p,gpa-base_gpa);+if(gpa+size<base_gpa+PER_CPU_DATA_SIZE)+memcmp_g(gpa+size,init_p,+(base_gpa+PER_CPU_DATA_SIZE)-(gpa+size));++/*+*Convertodd-numberpageframesbacktosharedtoverifyKVM+*alsocorrectlyhandlesholesinprivateranges.+*/+for(j=0;j<size;j+=PAGE_SIZE){+if((j>>PAGE_SHIFT)&1){+guest_map_shared(gpa+j,PAGE_SIZE,do_fallocate);+guest_sync_shared(gpa+j,PAGE_SIZE,p1,p3);++memcmp_g(gpa+j,p3,PAGE_SIZE);+}else{+guest_sync_private(gpa+j,PAGE_SIZE,p1);+}+}++skip:+/*+*Converttheentireregionbacktoshared,explicitlywrite+*patternthreetofillintheeven-numberframesbefore+*askingthehosttoverify(andwritepatternfour).+*/+guest_map_shared(gpa,size,do_fallocate);+memset((void*)gpa,p3,size);+guest_sync_shared(gpa,size,p3,p4);+memcmp_g(gpa,p4,size);++/* Reset the shared memory back to the initial pattern. */+memset((void*)gpa,init_p,size);++/*+*Free(viaPUNCH_HOLE)*all*privatememorysothatthenext+*iterationstartsfromacleanslate,e.g.withrespectto+*whetherornottherearepages/foliosinguest_mem.+*/+guest_map_shared(base_gpa,PER_CPU_DATA_SIZE,true);+}+}++staticvoidguest_code(uint64_tbase_gpa)+{+/*+*Runeverythingtwice,withandwithoutdoingfallocate()onthe+*guest_memfdbackingwhenconvertingbetweensharedandprivate.+*/+guest_run_test(base_gpa,false);+guest_run_test(base_gpa,true);+GUEST_DONE();+}++staticvoidhandle_exit_hypercall(structkvm_vcpu*vcpu)+{+structkvm_run*run=vcpu->run;+uint64_tgpa=run->hypercall.args[0];+uint64_tsize=run->hypercall.args[1]*PAGE_SIZE;+boolmap_shared=run->hypercall.args[2]&MAP_GPA_SHARED;+booldo_fallocate=run->hypercall.args[2]&MAP_GPA_DO_FALLOCATE;+structkvm_vm*vm=vcpu->vm;++TEST_ASSERT(run->hypercall.nr==KVM_HC_MAP_GPA_RANGE,+"Wanted MAP_GPA_RANGE (%u), got '%llu'",+KVM_HC_MAP_GPA_RANGE,run->hypercall.nr);++if(do_fallocate)+vm_guest_mem_fallocate(vm,gpa,size,map_shared);++vm_set_memory_attributes(vm,gpa,size,+map_shared?0:KVM_MEMORY_ATTRIBUTE_PRIVATE);+run->hypercall.ret=0;+}++staticboolrun_vcpus;++staticvoid*__test_mem_conversions(void*__vcpu)+{+structkvm_vcpu*vcpu=__vcpu;+structkvm_run*run=vcpu->run;+structkvm_vm*vm=vcpu->vm;+structucalluc;++while(!READ_ONCE(run_vcpus))+;++for(;;){+vcpu_run(vcpu);++if(run->exit_reason==KVM_EXIT_HYPERCALL){+handle_exit_hypercall(vcpu);+continue;+}++TEST_ASSERT(run->exit_reason==KVM_EXIT_IO,+"Wanted KVM_EXIT_IO, got exit reason: %u (%s)",+run->exit_reason,exit_reason_str(run->exit_reason));++switch(get_ucall(vcpu,&uc)){+caseUCALL_ABORT:+REPORT_GUEST_ASSERT_4(uc,"%lx %lx %lx %lx");+caseUCALL_SYNC:{+uint8_t*hva=addr_gpa2hva(vm,uc.args[1]);+uint64_tsize=uc.args[2];++TEST_ASSERT(uc.args[0]==SYNC_SHARED||+uc.args[0]==SYNC_PRIVATE,+"Unknown sync command '%ld'",uc.args[0]);++/* In all cases, the host should observe the shared data. */+memcmp_h(hva,uc.args[3],size);++/* For shared, write the new pattern to guest memory. */+if(uc.args[0]==SYNC_SHARED)+memset(hva,uc.args[4],size);+break;+}+caseUCALL_DONE:+returnNULL;+default:+TEST_FAIL("Unknown ucall 0x%lx.",uc.cmd);+}+}+}++staticvoidtest_mem_conversions(enumvm_mem_backing_src_typesrc_type,uint32_tnr_vcpus,+uint32_tnr_memslots)+{+/*+*AllocateenoughmemorysothateachvCPU'schunkofmemorycanbe+*naturallyalignedwithrespecttothesizeofthebackingstore.+*/+constsize_tsize=align_up(PER_CPU_DATA_SIZE,get_backing_src_pagesz(src_type));+constsize_tmemfd_size=size*nr_vcpus;+structkvm_vcpu*vcpus[KVM_MAX_VCPUS];+pthread_tthreads[KVM_MAX_VCPUS];+uint64_tgmem_flags;+structkvm_vm*vm;+intmemfd,i,r;++conststructvm_shapeshape={+.mode=VM_MODE_DEFAULT,+.type=KVM_X86_SW_PROTECTED_VM,+};++vm=__vm_create_with_vcpus(shape,nr_vcpus,0,guest_code,vcpus);++vm_enable_cap(vm,KVM_CAP_EXIT_HYPERCALL,(1<<KVM_HC_MAP_GPA_RANGE));++if(backing_src_can_be_huge(src_type))+gmem_flags=KVM_GUEST_MEMFD_ALLOW_HUGEPAGE;+else+gmem_flags=0;+memfd=vm_create_guest_memfd(vm,memfd_size,gmem_flags);++for(i=0;i<nr_memslots;i++)+vm_mem_add(vm,src_type,BASE_DATA_GPA+size*i,+BASE_DATA_SLOT+i,size/vm->page_size,+KVM_MEM_PRIVATE,memfd,size*i);++for(i=0;i<nr_vcpus;i++){+uint64_tgpa=BASE_DATA_GPA+i*size;++vcpu_args_set(vcpus[i],1,gpa);++virt_map(vm,gpa,gpa,size/vm->page_size);++pthread_create(&threads[i],NULL,__test_mem_conversions,vcpus[i]);+}++WRITE_ONCE(run_vcpus,true);++for(i=0;i<nr_vcpus;i++)+pthread_join(threads[i],NULL);++kvm_vm_free(vm);++/*+*Allocateandfreememoryfromtheguest_memfdafterclosingtheVM+*fd.Theguest_memfdisgiftedareferencetoitsowningVM,i.e.+*shouldpreventtheVMfrombeingfullydestroyeduntilthelast+*referencetotheguest_memfdisalsoput.+*/+r=fallocate(memfd,FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE,0,memfd_size);+TEST_ASSERT(!r,__KVM_SYSCALL_ERROR("fallocate()",r));++r=fallocate(memfd,FALLOC_FL_KEEP_SIZE,0,memfd_size);+TEST_ASSERT(!r,__KVM_SYSCALL_ERROR("fallocate()",r));+}++staticvoidusage(constchar*cmd)+{+puts("");+printf("usage: %s [-h] [-m] [-s mem_type] [-n nr_vcpus]\n",cmd);+puts("");+backing_src_help("-s");+puts("");+puts(" -n: specify the number of vcpus (default: 1)");+puts("");+puts(" -m: use multiple memslots (default: 1)");+puts("");+}++intmain(intargc,char*argv[])+{+enumvm_mem_backing_src_typesrc_type=DEFAULT_VM_MEM_SRC;+booluse_multiple_memslots=false;+uint32_tnr_vcpus=1;+uint32_tnr_memslots;+intopt;++TEST_REQUIRE(kvm_has_cap(KVM_CAP_EXIT_HYPERCALL));+TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES)&BIT(KVM_X86_SW_PROTECTED_VM));++while((opt=getopt(argc,argv,"hms:n:"))!=-1){+switch(opt){+case's':+src_type=parse_backing_src_type(optarg);+break;+case'n':+nr_vcpus=atoi_positive("nr_vcpus",optarg);+break;+case'm':+use_multiple_memslots=true;+break;+case'h':+default:+usage(argv[0]);+exit(0);+}+}++nr_memslots=use_multiple_memslots?nr_vcpus:1;++test_mem_conversions(src_type,nr_vcpus,nr_memslots);++return0;+}
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:53:07
From: Chao Peng <redacted>
Provide a raw version as well as an assert-success version to reduce
the amount of boilerplate code need for basic usage.
Signed-off-by: Chao Peng <redacted>
Signed-off-by: Ackerley Tng <redacted>
---
.../selftests/kvm/include/kvm_util_base.h | 7 +++++
tools/testing/selftests/kvm/lib/kvm_util.c | 29 +++++++++++++++++++
2 files changed, 36 insertions(+)
@@ -382,6 +382,98 @@ static void test_add_max_memory_regions(void)kvm_vm_free(vm);}++staticvoidtest_invalid_guest_memfd(structkvm_vm*vm,intmemfd,+size_toffset,constchar*msg)+{+intr=__vm_set_user_memory_region2(vm,MEM_REGION_SLOT,KVM_MEM_PRIVATE,+MEM_REGION_GPA,MEM_REGION_SIZE,+0,memfd,offset);+TEST_ASSERT(r==-1&&errno==EINVAL,"%s",msg);+}++staticvoidtest_add_private_memory_region(void)+{+structkvm_vm*vm,*vm2;+intmemfd,i;++pr_info("Testing ADD of KVM_MEM_PRIVATE memory regions\n");++vm=vm_create_barebones_protected_vm();++test_invalid_guest_memfd(vm,vm->kvm_fd,0,"KVM fd should fail");+test_invalid_guest_memfd(vm,vm->fd,0,"VM's fd should fail");++memfd=kvm_memfd_alloc(MEM_REGION_SIZE,false);+test_invalid_guest_memfd(vm,vm->fd,0,"Regular memfd() should fail");+close(memfd);++vm2=vm_create_barebones_protected_vm();+memfd=vm_create_guest_memfd(vm2,MEM_REGION_SIZE,0);+test_invalid_guest_memfd(vm,memfd,0,"Other VM's guest_memfd() should fail");++vm_set_user_memory_region2(vm2,MEM_REGION_SLOT,KVM_MEM_PRIVATE,+MEM_REGION_GPA,MEM_REGION_SIZE,0,memfd,0);+close(memfd);+kvm_vm_free(vm2);++memfd=vm_create_guest_memfd(vm,MEM_REGION_SIZE,0);+for(i=1;i<PAGE_SIZE;i++)+test_invalid_guest_memfd(vm,memfd,i,"Unaligned offset should fail");++vm_set_user_memory_region2(vm,MEM_REGION_SLOT,KVM_MEM_PRIVATE,+MEM_REGION_GPA,MEM_REGION_SIZE,0,memfd,0);+close(memfd);++kvm_vm_free(vm);+}++staticvoidtest_add_overlapping_private_memory_regions(void)+{+structkvm_vm*vm;+intmemfd;+intr;++pr_info("Testing ADD of overlapping KVM_MEM_PRIVATE memory regions\n");++vm=vm_create_barebones_protected_vm();++memfd=vm_create_guest_memfd(vm,MEM_REGION_SIZE*4,0);++vm_set_user_memory_region2(vm,MEM_REGION_SLOT,KVM_MEM_PRIVATE,+MEM_REGION_GPA,MEM_REGION_SIZE*2,0,memfd,0);++vm_set_user_memory_region2(vm,MEM_REGION_SLOT+1,KVM_MEM_PRIVATE,+MEM_REGION_GPA*2,MEM_REGION_SIZE*2,+0,memfd,MEM_REGION_SIZE*2);++/*+*Deletethefirstmemslot,andthenattempttorecreateitexcept+*witha"bad"offsetthatresultsinoverlapintheguest_memfd().+*/+vm_set_user_memory_region2(vm,MEM_REGION_SLOT,KVM_MEM_PRIVATE,+MEM_REGION_GPA,0,NULL,-1,0);++/* Overlap the front half of the other slot. */+r=__vm_set_user_memory_region2(vm,MEM_REGION_SLOT,KVM_MEM_PRIVATE,+MEM_REGION_GPA*2-MEM_REGION_SIZE,+MEM_REGION_SIZE*2,+0,memfd,0);+TEST_ASSERT(r==-1&&errno==EEXIST,"%s",+"Overlapping guest_memfd() bindings should fail with EEXIST");++/* And now the back half of the other slot. */+r=__vm_set_user_memory_region2(vm,MEM_REGION_SLOT,KVM_MEM_PRIVATE,+MEM_REGION_GPA*2+MEM_REGION_SIZE,+MEM_REGION_SIZE*2,+0,memfd,0);+TEST_ASSERT(r==-1&&errno==EEXIST,"%s",+"Overlapping guest_memfd() bindings should fail with EEXIST");++close(memfd);+kvm_vm_free(vm);+}+intmain(intargc,char*argv[]){#ifdef __x86_64__
@@ -398,6 +490,13 @@ int main(int argc, char *argv[])test_add_max_memory_regions();+if(kvm_check_cap(KVM_CAP_VM_TYPES)&BIT(KVM_X86_SW_PROTECTED_VM)){+test_add_private_memory_region();+test_add_overlapping_private_memory_regions();+}else{+pr_info("Skipping tests for KVM_MEM_PRIVATE memory regions\n");+}+#ifdef __x86_64__if(argc>1)loops=atoi_positive("Number of iterations",argv[1]);
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:54:01
Add a selftest to verify the basic functionality of guest_memfd():
+ file descriptor created with the guest_memfd() ioctl does not allow
read/write/mmap operations
+ file size and block size as returned from fstat are as expected
+ fallocate on the fd checks that offset/length on
fallocate(FALLOC_FL_PUNCH_HOLE) should be page aligned
Signed-off-by: Chao Peng <redacted>
Co-developed-by: Ackerley Tng <redacted>
Signed-off-by: Ackerley Tng <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../testing/selftests/kvm/guest_memfd_test.c | 114 ++++++++++++++++++
2 files changed, 115 insertions(+)
create mode 100644 tools/testing/selftests/kvm/guest_memfd_test.c
@@ -0,0 +1,114 @@+// SPDX-License-Identifier: GPL-2.0+/*+*CopyrightIntelCorporation,2023+*+*Author:ChaoPeng<chao.p.peng@linux.intel.com>+*/++#define _GNU_SOURCE+#include"test_util.h"+#include"kvm_util_base.h"+#include<linux/falloc.h>+#include<sys/mman.h>+#include<sys/types.h>+#include<sys/stat.h>++#include<stdlib.h>+#include<string.h>+#include<unistd.h>+#include<errno.h>+#include<stdio.h>+#include<fcntl.h>++staticvoidtest_file_read_write(intfd)+{+charbuf[64];++TEST_ASSERT(read(fd,buf,sizeof(buf))<0,+"read on a guest_mem fd should fail");+TEST_ASSERT(write(fd,buf,sizeof(buf))<0,+"write on a guest_mem fd should fail");+TEST_ASSERT(pread(fd,buf,sizeof(buf),0)<0,+"pread on a guest_mem fd should fail");+TEST_ASSERT(pwrite(fd,buf,sizeof(buf),0)<0,+"pwrite on a guest_mem fd should fail");+}++staticvoidtest_mmap(intfd,size_tpage_size)+{+char*mem;++mem=mmap(NULL,page_size,PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);+ASSERT_EQ(mem,MAP_FAILED);+}++staticvoidtest_file_size(intfd,size_tpage_size,size_ttotal_size)+{+structstatsb;+intret;++ret=fstat(fd,&sb);+TEST_ASSERT(!ret,"fstat should succeed");+ASSERT_EQ(sb.st_size,total_size);+ASSERT_EQ(sb.st_blksize,page_size);+}++staticvoidtest_fallocate(intfd,size_tpage_size,size_ttotal_size)+{+intret;++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE,0,total_size);+TEST_ASSERT(!ret,"fallocate with aligned offset and size should succeed");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE,+page_size-1,page_size);+TEST_ASSERT(ret,"fallocate with unaligned offset should fail");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE,total_size,page_size);+TEST_ASSERT(ret,"fallocate beginning at total_size should fail");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE,total_size+page_size,page_size);+TEST_ASSERT(ret,"fallocate beginning at total_size should fail");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE,+total_size,page_size);+TEST_ASSERT(!ret,"fallocate(PUNCH_HOLE) at total_size should succeed");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE,+total_size+page_size,page_size);+TEST_ASSERT(!ret,"fallocate(PUNCH_HOLE) after total_size should succeed");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE,+page_size,page_size-1);+TEST_ASSERT(ret,"fallocate with unaligned size should fail");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE,+page_size,page_size);+TEST_ASSERT(!ret,"fallocate(PUNCH_HOLE) with aligned offset and size should succeed");++ret=fallocate(fd,FALLOC_FL_KEEP_SIZE,page_size,page_size);+TEST_ASSERT(!ret,"fallocate to restore punched hole should succeed");+}+++intmain(intargc,char*argv[])+{+size_tpage_size;+size_ttotal_size;+intfd;+structkvm_vm*vm;++page_size=getpagesize();+total_size=page_size*4;++vm=vm_create_barebones();++fd=vm_create_guest_memfd(vm,total_size,0);++test_file_read_write(fd);+test_mmap(fd,page_size);+test_file_size(fd,page_size,total_size);+test_fallocate(fd,page_size,total_size);++close(fd);+}
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-18 23:54:23
From: Ackerley Tng <redacted>
"Testing private access when memslot gets deleted" tests the behavior
of KVM when a private memslot gets deleted while the VM is using the
private memslot. When KVM looks up the deleted (slot = NULL) memslot,
KVM should exit to userspace with KVM_EXIT_MEMORY_FAULT.
In the second test, upon a private access to non-private memslot, KVM
should also exit to userspace with KVM_EXIT_MEMORY_FAULT.
sean: These testcases belong in set_memory_region_test.c, they're private
variants on existing testscases and aren't as robust, e.g. don't ensure
the vCPU is actually running and accessing memory when converting and
deleting.
Signed-off-by: Ackerley Tng <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../kvm/x86_64/private_mem_kvm_exits_test.c | 115 ++++++++++++++++++
2 files changed, 116 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/private_mem_kvm_exits_test.c
@@ -254,7 +254,7 @@ static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)/* Create a VM with enough guest pages */guest_num_pages=test_mem_size/guest_page_size;-vm=__vm_create_with_vcpus(mode,nr_vcpus,guest_num_pages,+vm=__vm_create_with_vcpus(VM_SHAPE(mode),nr_vcpus,guest_num_pages,guest_code,test_args.vcpus);/* Align down GPA of the testing memslot */
@@ -85,7 +85,7 @@ config KVM_BOOK3S_64_HVtristate"KVM for POWER7 and later using hypervisor mode in host"depends onKVM_BOOK3S_64&&PPC_POWERNVselectKVM_BOOK3S_HV_POSSIBLE-selectMMU_NOTIFIER+selectKVM_GENERIC_MMU_NOTIFIERselectCMAhelpSupportrunningunmodifiedbook3s_64guestkernelsin
On Tue, Jul 18, 2023 at 04:44:50PM -0700, Sean Christopherson wrote:
From: Chao Peng <redacted>
This new KVM exit allows userspace to handle memory-related errors. It
indicates an error happens in KVM at guest memory range [gpa, gpa+size).
The flags includes additional information for userspace to handle the
error. Currently bit 0 is defined as 'private memory' where '1'
indicates error happens due to private memory access and '0' indicates
error happens due to shared memory access.
Now it's bit 3:
#define KVM_MEMORY_EXIT_FLAG_PRIVATE (1ULL << 3)
I remember some other attributes were introduced in v10 yet:
#define KVM_MEMORY_ATTRIBUTE_READ (1ULL << 0)
#define KVM_MEMORY_ATTRIBUTE_WRITE (1ULL << 1)
#define KVM_MEMORY_ATTRIBUTE_EXECUTE (1ULL << 2)
#define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)
So KVM_MEMORY_EXIT_FLAG_PRIVATE changed to bit 3 due to above things,
or other reason ? (Sorry I didn't follow v10 too much before).
quoted hunk
When private memory is enabled, this new exit will be used for KVM to
exit to userspace for shared <-> private memory conversion in memory
encryption usage. In such usage, typically there are two kind of memory
conversions:
- explicit conversion: happens when guest explicitly calls into KVM
to map a range (as private or shared), KVM then exits to userspace
to perform the map/unmap operations.
- implicit conversion: happens in KVM page fault handler where KVM
exits to userspace for an implicit conversion when the page is in a
different state than requested (private or shared).
Suggested-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Yu Zhang <redacted>
Signed-off-by: Yu Zhang <redacted>
Signed-off-by: Chao Peng <redacted>
Reviewed-by: Fuad Tabba <redacted>
Tested-by: Fuad Tabba <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++++++
include/uapi/linux/kvm.h | 8 ++++++++
2 files changed, 30 insertions(+)
@@ -6700,6 +6700,28 @@ array field represents return values. The userspace should update the return values of SBI call before resuming the VCPU. For more details on RISC-V SBI spec refer, https://github.com/riscv/riscv-sbi-doc.+::++ /* KVM_EXIT_MEMORY_FAULT */+ struct {+ #define KVM_MEMORY_EXIT_FLAG_PRIVATE (1ULL << 3)+ __u64 flags;+ __u64 gpa;+ __u64 size;+ } memory;++If exit reason is KVM_EXIT_MEMORY_FAULT then it indicates that the VCPU has+encountered a memory error which is not handled by KVM kernel module and+userspace may choose to handle it. The 'flags' field indicates the memory+properties of the exit.++- KVM_MEMORY_EXIT_FLAG_PRIVATE - indicates the memory error is caused by+ private memory access when the bit is set. Otherwise the memory error is+ caused by shared memory access when the bit is clear.++'gpa' and 'size' indicate the memory range the error occurs at. The userspace+may handle the error and return to KVM to retry the previous memory access.+ :: /* KVM_EXIT_NOTIFY */
On Wed Jul 19, 2023 at 2:44 AM EEST, Sean Christopherson wrote:
/* Huge pages aren't expected to be modified without first being zapped. */
- WARN_ON(pte_huge(range->pte) || range->start + 1 != range->end);
+ WARN_ON(pte_huge(range->arg.pte) || range->start + 1 != range->end);
Not familiar with this code. Just checking whether whether instead
pr_{warn,err}() combined with return false would be a more graceful
option?
BR, Jarkko
Not sure why a declaration here, it's defined for ARCHs which defined
KVM_ARCH_WANT_MMU_NOTIFIER before.
The forward declaration exists to handle cases where CONFIG_KVM=n, specifically
arch/powerpc/include/asm/kvm_ppc.h's declaration of hooks to forward calls to
uarch modules:
bool (*unmap_gfn_range)(struct kvm *kvm, struct kvm_gfn_range *range);
bool (*age_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
bool (*test_age_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
bool (*set_spte_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
Prior to using a Kconfig, a forward declaration wasn't necessary because
arch/powerpc/include/asm/kvm_host.h would #define KVM_ARCH_WANT_MMU_NOTIFIER even
if CONFIG_KVM=n.
Alternatively, kvm_ppc.h could declare the struct. I went this route mainly to
avoid the possibility of someone encountering the same problem on a different
architecture.
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-19 14:16:59
On Wed, Jul 19, 2023, Yuan Yao wrote:
On Tue, Jul 18, 2023 at 04:44:50PM -0700, Sean Christopherson wrote:
quoted
From: Chao Peng <redacted>
This new KVM exit allows userspace to handle memory-related errors. It
indicates an error happens in KVM at guest memory range [gpa, gpa+size).
The flags includes additional information for userspace to handle the
error. Currently bit 0 is defined as 'private memory' where '1'
indicates error happens due to private memory access and '0' indicates
error happens due to shared memory access.
Now it's bit 3:
Yeah, I need to update (or write) a lot of changelogs.
#define KVM_MEMORY_EXIT_FLAG_PRIVATE (1ULL << 3)
I remember some other attributes were introduced in v10 yet:
#define KVM_MEMORY_ATTRIBUTE_READ (1ULL << 0)
#define KVM_MEMORY_ATTRIBUTE_WRITE (1ULL << 1)
#define KVM_MEMORY_ATTRIBUTE_EXECUTE (1ULL << 2)
#define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)
So KVM_MEMORY_EXIT_FLAG_PRIVATE changed to bit 3 due to above things,
or other reason ? (Sorry I didn't follow v10 too much before).
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-19 15:39:25
On Wed, Jul 19, 2023, Jarkko Sakkinen wrote:
On Wed Jul 19, 2023 at 2:44 AM EEST, Sean Christopherson wrote:
quoted
/* Huge pages aren't expected to be modified without first being zapped. */
- WARN_ON(pte_huge(range->pte) || range->start + 1 != range->end);
+ WARN_ON(pte_huge(range->arg.pte) || range->start + 1 != range->end);
Not familiar with this code. Just checking whether whether instead
pr_{warn,err}()
The "full" WARN is desirable, this is effecitvely an assert on the contract between
the primary MMU, generic KVM code, and x86's TDP MMU. The .change_pte() mmu_notifier
callback doesn't allow for hugepages, i.e. it's a (likely fatal) kernel bug if a
hugepage is encountered at this point. Ditto for the "start + 1 == end" check,
if that fails then generic KVM likely has a fatal bug.
combined with return false would be a more graceful option?
The return value communicates whether or not a TLB flush is needed, not whether
or not the operation was successful, i.e. there is no way to cancel the unexpected
PTE change.
I think these should be static assertions near the definition of the
structs. However another possibility is to remove 'raw' and just assign
the whole union.
Apart from this,
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2023-07-19 17:13:13
On 7/19/23 01:44, Sean Christopherson wrote:
From: Chao Peng<redacted>
Currently in mmu_notifier invalidate path, hva range is recorded and
then checked against by mmu_notifier_retry_hva() in the page fault
handling path. However, for the to be introduced private memory, a page
fault may not have a hva associated, checking gfn(gpa) makes more sense.
For existing hva based shared memory, gfn is expected to also work. The
only downside is when aliasing multiple gfns to a single hva, the
current algorithm of checking multiple ranges could result in a much
larger range being rejected. Such aliasing should be uncommon, so the
impact is expected small.
Does it make sense to replicate what happens with MCE handling on
tmpfs backed guest memory:
1) Unmap gpa from guest
2) On the next guest EPT fault, exit to userspace to handle/log the
mce error for the gpa.
IIUC, such MCEs could be asynchronous and "current" might not always
be the intended recipient of this signal.
@@ -634,10 +634,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)caseKVM_CAP_SYNC_MMU:#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLEr=hv_enabled;
This could actually be unnecessarily conservative. Even book3s_pr.c
knows how to do unmap and set_spte, so it should be able to support
KVM_CAP_SYNC_MMU. Alex, Nick, do you remember any of this? This would
mean moving KVM_CAP_SYNC_MMU to virt/kvm/kvm_main.c, which is nice.
Paolo
-#elif defined(KVM_ARCH_WANT_MMU_NOTIFIER)
- r = 1;
#else
- r = 0;
+#ifndef KVM_ARCH_WANT_MMU_NOTIFIER
+ BUILD_BUG();
+#endif
+ r = 1;
#endif
break;
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
Does it make sense to replicate what happens with MCE handling on
tmpfs backed guest memory:
1) Unmap gpa from guest
2) On the next guest EPT fault, exit to userspace to handle/log the
mce error for the gpa.
Hmm, yes, that would be much better. Ah, and kvm_gmem_get_pfn() needs to check
folio_test_hwpoison() and potentially PageHWPoison(). E.g. if the folio is huge,
KVM needs to restrict the mapping to order-0 (target page isn't poisoned), or
return KVM_PFN_ERR_HWPOISON (taget page IS poisoned).
Alternatively, KVM could punch a hole in kvm_gmem_error_page(), but I don't think
we want to do that because that would prevent forwarding the #MC to the guest.
Not sure why a declaration here, it's defined for ARCHs which defined
KVM_ARCH_WANT_MMU_NOTIFIER before.
The forward declaration exists to handle cases where CONFIG_KVM=n, specifically
arch/powerpc/include/asm/kvm_ppc.h's declaration of hooks to forward calls to
uarch modules:
bool (*unmap_gfn_range)(struct kvm *kvm, struct kvm_gfn_range *range);
bool (*age_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
bool (*test_age_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
bool (*set_spte_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
Prior to using a Kconfig, a forward declaration wasn't necessary because
arch/powerpc/include/asm/kvm_host.h would #define KVM_ARCH_WANT_MMU_NOTIFIER even
if CONFIG_KVM=n.
Alternatively, kvm_ppc.h could declare the struct. I went this route mainly to
avoid the possibility of someone encountering the same problem on a different
architecture.
On Tue, Jul 18, 2023 at 04:44:51PM -0700, Sean Christopherson wrote:
quoted hunk
From: Chao Peng <redacted>
In confidential computing usages, whether a page is private or shared is
necessary information for KVM to perform operations like page fault
handling, page zapping etc. There are other potential use cases for
per-page memory attributes, e.g. to make memory read-only (or no-exec,
or exec-only, etc.) without having to modify memslots.
Introduce two ioctls (advertised by KVM_CAP_MEMORY_ATTRIBUTES) to allow
userspace to operate on the per-page memory attributes.
- KVM_SET_MEMORY_ATTRIBUTES to set the per-page memory attributes to
a guest memory range.
- KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES to return the KVM supported
memory attributes.
Use an xarray to store the per-page attributes internally, with a naive,
not fully optimized implementation, i.e. prioritize correctness over
performance for the initial implementation.
Because setting memory attributes is roughly analogous to mprotect() on
memory that is mapped into the guest, zap existing mappings prior to
updating the memory attributes. Opportunistically provide an arch hook
for the post-set path (needed to complete invalidation anyways) in
anticipation of x86 needing the hook to update metadata related to
determining whether or not a given gfn can be backed with various sizes
of hugepages.
It's possible that future usages may not require an invalidation, e.g.
if KVM ends up supporting RWX protections and userspace grants _more_
protections, but again opt for simplicity and punt optimizations to
if/when they are needed.
Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/all/Y2WB48kD0J4VGynX@google.com
Cc: Fuad Tabba <redacted>
Signed-off-by: Chao Peng <redacted>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Documentation/virt/kvm/api.rst | 60 ++++++++++++
include/linux/kvm_host.h | 14 +++
include/uapi/linux/kvm.h | 14 +++
virt/kvm/Kconfig | 4 +
virt/kvm/kvm_main.c | 170 +++++++++++++++++++++++++++++++++
5 files changed, 262 insertions(+)
@@ -6068,6 +6068,56 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG interface. No error will be returned, but the resulting offset will not be applied.+4.139 KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES+-----------------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm ioctl+:Parameters: u64 memory attributes bitmask(out)+:Returns: 0 on success, <0 on error++Returns supported memory attributes bitmask. Supported memory attributes will+have the corresponding bits set in u64 memory attributes bitmask.++The following memory attributes are defined::++ #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)++4.140 KVM_SET_MEMORY_ATTRIBUTES+-----------------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm ioctl+:Parameters: struct kvm_memory_attributes(in/out)+:Returns: 0 on success, <0 on error++Sets memory attributes for pages in a guest memory range. Parameters are+specified via the following structure::++ struct kvm_memory_attributes {+ __u64 address;+ __u64 size;+ __u64 attributes;+ __u64 flags;+ };++The user sets the per-page memory attributes to a guest memory range indicated+by address/size, and in return KVM adjusts address and size to reflect the+actual pages of the memory range have been successfully set to the attributes.+If the call returns 0, "address" is updated to the last successful address + 1+and "size" is updated to the remaining address size that has not been set+successfully. The user should check the return value as well as the size to+decide if the operation succeeded for the whole range or not. The user may want+to retry the operation with the returned address/size if the previous range was+partially successful.++Both address and size should be page aligned and the supported attributes can be+retrieved with KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES.++The "flags" field may be used for future extensions and should be set to 0s.+5. The kvm_run structure ========================
@@ -8494,6 +8544,16 @@ block sizes is exposed in KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES as a 64-bit bitmap (each bit describing a block size). The default value is 0, to disable the eager page splitting.+8.41 KVM_CAP_MEMORY_ATTRIBUTES+------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm++This capability indicates KVM supports per-page memory attributes and ioctls+KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES are available.+9. Known KVM API problems =========================
@@ -2301,4 +2305,14 @@ static inline void kvm_account_pgtable_pages(void *virt, int nr)/* Max number of entries allowed for each kvm dirty ring */#define KVM_DIRTY_RING_MAX_ENTRIES 65536+#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES+staticinlineunsignedlongkvm_get_memory_attributes(structkvm*kvm,gfn_tgfn)+{+returnxa_to_value(xa_load(&kvm->mem_attr_array,gfn));+}++boolkvm_arch_post_set_memory_attributes(structkvm*kvm,+structkvm_gfn_range*range);
Used but no definition in this patch, it's defined in next patch 09.
How about add weak version in this patch and let ARCHs to overide it ?
From: Xiaoyao Li <hidden> Date: 2023-07-20 14:46:31
On 7/19/2023 7:44 AM, Sean Christopherson wrote:
quoted hunk
@@ -5134,6 +5167,16 @@ static long kvm_vm_ioctl(struct file *filp, case KVM_GET_STATS_FD: r = kvm_vm_ioctl_get_stats_fd(kvm); break;+ case KVM_CREATE_GUEST_MEMFD: {+ struct kvm_create_guest_memfd guest_memfd;++ r = -EFAULT;+ if (copy_from_user(&guest_memfd, argp, sizeof(guest_memfd)))+ goto out;++ r = kvm_gmem_create(kvm, &guest_memfd);+ break;+ }
Does it need a new CAP to indicate the support of guest_memfd?
This is patch series introduces 3 new CAPs and it seems any one of them
can serve as the indicator of guest_memfd.
+#define KVM_CAP_USER_MEMORY2 230
+#define KVM_CAP_MEMORY_ATTRIBUTES 231
+#define KVM_CAP_VM_TYPES 232
or we just go and try the ioctl, the return value will tell the result?
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-20 15:15:39
On Thu, Jul 20, 2023, Xiaoyao Li wrote:
On 7/19/2023 7:44 AM, Sean Christopherson wrote:
quoted
@@ -5134,6 +5167,16 @@ static long kvm_vm_ioctl(struct file *filp, case KVM_GET_STATS_FD: r = kvm_vm_ioctl_get_stats_fd(kvm); break;+ case KVM_CREATE_GUEST_MEMFD: {+ struct kvm_create_guest_memfd guest_memfd;++ r = -EFAULT;+ if (copy_from_user(&guest_memfd, argp, sizeof(guest_memfd)))+ goto out;++ r = kvm_gmem_create(kvm, &guest_memfd);+ break;+ }
Does it need a new CAP to indicate the support of guest_memfd?
Yeah, I meant to add that to the TODO list and forgot (obviously).
This is patch series introduces 3 new CAPs and it seems any one of them can
serve as the indicator of guest_memfd.
+#define KVM_CAP_USER_MEMORY2 230
+#define KVM_CAP_MEMORY_ATTRIBUTES 231
+#define KVM_CAP_VM_TYPES 232
The number of new caps being added is the main why I didn't just add another one.
On the other hand, we have room for a few billion caps, so one more isn't a big
deal. So yeah, KVM_CAP_GUEST_MEMFD is probably the way to go.
On Thu, Jul 20, 2023 at 04:09:12PM +0800,
Yuan Yao [off-list ref] wrote:
On Tue, Jul 18, 2023 at 04:44:51PM -0700, Sean Christopherson wrote:
quoted
From: Chao Peng <redacted>
In confidential computing usages, whether a page is private or shared is
necessary information for KVM to perform operations like page fault
handling, page zapping etc. There are other potential use cases for
per-page memory attributes, e.g. to make memory read-only (or no-exec,
or exec-only, etc.) without having to modify memslots.
Introduce two ioctls (advertised by KVM_CAP_MEMORY_ATTRIBUTES) to allow
userspace to operate on the per-page memory attributes.
- KVM_SET_MEMORY_ATTRIBUTES to set the per-page memory attributes to
a guest memory range.
- KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES to return the KVM supported
memory attributes.
Use an xarray to store the per-page attributes internally, with a naive,
not fully optimized implementation, i.e. prioritize correctness over
performance for the initial implementation.
Because setting memory attributes is roughly analogous to mprotect() on
memory that is mapped into the guest, zap existing mappings prior to
updating the memory attributes. Opportunistically provide an arch hook
for the post-set path (needed to complete invalidation anyways) in
anticipation of x86 needing the hook to update metadata related to
determining whether or not a given gfn can be backed with various sizes
of hugepages.
It's possible that future usages may not require an invalidation, e.g.
if KVM ends up supporting RWX protections and userspace grants _more_
protections, but again opt for simplicity and punt optimizations to
if/when they are needed.
Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/all/Y2WB48kD0J4VGynX@google.com
Cc: Fuad Tabba <redacted>
Signed-off-by: Chao Peng <redacted>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Documentation/virt/kvm/api.rst | 60 ++++++++++++
include/linux/kvm_host.h | 14 +++
include/uapi/linux/kvm.h | 14 +++
virt/kvm/Kconfig | 4 +
virt/kvm/kvm_main.c | 170 +++++++++++++++++++++++++++++++++
5 files changed, 262 insertions(+)
@@ -6068,6 +6068,56 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG interface. No error will be returned, but the resulting offset will not be applied.+4.139 KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES+-----------------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm ioctl+:Parameters: u64 memory attributes bitmask(out)+:Returns: 0 on success, <0 on error++Returns supported memory attributes bitmask. Supported memory attributes will+have the corresponding bits set in u64 memory attributes bitmask.++The following memory attributes are defined::++ #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)++4.140 KVM_SET_MEMORY_ATTRIBUTES+-----------------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm ioctl+:Parameters: struct kvm_memory_attributes(in/out)+:Returns: 0 on success, <0 on error++Sets memory attributes for pages in a guest memory range. Parameters are+specified via the following structure::++ struct kvm_memory_attributes {+ __u64 address;+ __u64 size;+ __u64 attributes;+ __u64 flags;+ };++The user sets the per-page memory attributes to a guest memory range indicated+by address/size, and in return KVM adjusts address and size to reflect the+actual pages of the memory range have been successfully set to the attributes.+If the call returns 0, "address" is updated to the last successful address + 1+and "size" is updated to the remaining address size that has not been set+successfully. The user should check the return value as well as the size to+decide if the operation succeeded for the whole range or not. The user may want+to retry the operation with the returned address/size if the previous range was+partially successful.++Both address and size should be page aligned and the supported attributes can be+retrieved with KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES.++The "flags" field may be used for future extensions and should be set to 0s.+5. The kvm_run structure ========================
@@ -8494,6 +8544,16 @@ block sizes is exposed in KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES as a 64-bit bitmap (each bit describing a block size). The default value is 0, to disable the eager page splitting.+8.41 KVM_CAP_MEMORY_ATTRIBUTES+------------------------------++:Capability: KVM_CAP_MEMORY_ATTRIBUTES+:Architectures: x86+:Type: vm++This capability indicates KVM supports per-page memory attributes and ioctls+KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES are available.+9. Known KVM API problems =========================
@@ -2301,4 +2305,14 @@ static inline void kvm_account_pgtable_pages(void *virt, int nr)/* Max number of entries allowed for each kvm dirty ring */#define KVM_DIRTY_RING_MAX_ENTRIES 65536+#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES+staticinlineunsignedlongkvm_get_memory_attributes(structkvm*kvm,gfn_tgfn)+{+returnxa_to_value(xa_load(&kvm->mem_attr_array,gfn));+}++boolkvm_arch_post_set_memory_attributes(structkvm*kvm,+structkvm_gfn_range*range);
Used but no definition in this patch, it's defined in next patch 09.
How about add weak version in this patch and let ARCHs to overide it ?
It is guarded by CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES.
--
Isaku Yamahata [off-list ref]
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-20 20:20:18
On Thu, Jul 20, 2023, Isaku Yamahata wrote:
On Thu, Jul 20, 2023 at 04:09:12PM +0800,
Yuan Yao [off-list ref] wrote:
quoted
On Tue, Jul 18, 2023 at 04:44:51PM -0700, Sean Christopherson wrote:
quoted
@@ -2301,4 +2305,14 @@ static inline void kvm_account_pgtable_pages(void *virt, int nr) /* Max number of entries allowed for each kvm dirty ring */ #define KVM_DIRTY_RING_MAX_ENTRIES 65536+#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES+static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)+{+ return xa_to_value(xa_load(&kvm->mem_attr_array, gfn));+}++bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,+ struct kvm_gfn_range *range);
Used but no definition in this patch, it's defined in next patch 09.
How about add weak version in this patch and let ARCHs to overide it ?
It is guarded by CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES.
Yep. I don't love the ordering, e.g. this patch can't even be compile tested
until later in the series, but I wanted to separate x86 usage from the generic
support code.
On Tue, Jul 18, 2023 at 04:44:55PM -0700,
Sean Christopherson [off-list ref] wrote:
+static int kvm_gmem_release(struct inode *inode, struct file *file)
+{
+ struct kvm_gmem *gmem = file->private_data;
+ struct kvm_memory_slot *slot;
+ struct kvm *kvm = gmem->kvm;
+ unsigned long index;
+
+ filemap_invalidate_lock(inode->i_mapping);
+
+ /*
+ * Prevent concurrent attempts to *unbind* a memslot. This is the last
+ * reference to the file and thus no new bindings can be created, but
+ * dereferencing the slot for existing bindings needs to be protected
+ * against memslot updates, specifically so that unbind doesn't race
+ * and free the memslot (kvm_gmem_get_file() will return NULL).
+ */
+ mutex_lock(&kvm->slots_lock);
+
+ xa_for_each(&gmem->bindings, index, slot)
+ rcu_assign_pointer(slot->gmem.file, NULL);
+
+ synchronize_rcu();
+
+ /*
+ * All in-flight operations are gone and new bindings can be created.
+ * Zap all SPTEs pointed at by this file. Do not free the backing
+ * memory, as its lifetime is associated with the inode, not the file.
+ */
+ kvm_gmem_invalidate_begin(gmem, 0, -1ul);
+ kvm_gmem_invalidate_end(gmem, 0, -1ul);
+
+ mutex_unlock(&kvm->slots_lock);
+
+ list_del(&gmem->entry);
+
+ filemap_invalidate_unlock(inode->i_mapping);
+
+ xa_destroy(&gmem->bindings);
+ kfree(gmem);
+
+ kvm_put_kvm(kvm);
+
+ return 0;
+}
The lockdep complains with the filemapping lock and the kvm slot lock.
From bc45eb084a761f93a87ba1f6d3a9949c17adeb31 Mon Sep 17 00:00:00 2001
Message-Id: [off-list ref]
From: Isaku Yamahata <redacted>
Date: Thu, 20 Jul 2023 14:16:21 -0700
Subject: [PATCH] KVM/gmem: Fix locking ordering in kvm_gmem_release()
The lockdep complains the locking order. Fix kvm_gmem_release()
VM destruction:
- fput()
...
\-kvm_gmem_release()
\-filemap_invalidate_lock(inode->i_mapping);
lock(&kvm->slots_lock);
slot creation:
kvm_set_memory_region()
mutex_lock(&kvm->slots_lock);
__kvm_set_memory_region(kvm, mem);
\-kvm_gmem_bind()
\-filemap_invalidate_lock(inode->i_mapping);
======================================================
WARNING: possible circular locking dependency detected
------------------------------------------------------
...
the existing dependency chain (in reverse order) is:
-> #1 (mapping.invalidate_lock#4){+.+.}-{4:4}:
...
down_write+0x40/0xe0
kvm_gmem_bind+0xd9/0x1b0 [kvm]
__kvm_set_memory_region.part.0+0x4fc/0x620 [kvm]
__kvm_set_memory_region+0x6b/0x90 [kvm]
kvm_vm_ioctl+0x350/0xa00 [kvm]
__x64_sys_ioctl+0x95/0xd0
do_syscall_64+0x39/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
-> #0 (&kvm->slots_lock){+.+.}-{4:4}:
...
mutex_lock_nested+0x1b/0x30
kvm_gmem_release+0x56/0x1b0 [kvm]
__fput+0x115/0x2e0
____fput+0xe/0x20
task_work_run+0x5e/0xb0
do_exit+0x2dd/0x5b0
do_group_exit+0x3b/0xb0
__x64_sys_exit_group+0x18/0x20
do_syscall_64+0x39/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(mapping.invalidate_lock#4);
lock(&kvm->slots_lock);
lock(mapping.invalidate_lock#4);
lock(&kvm->slots_lock);
Signed-off-by: Isaku Yamahata <redacted>
---
virt/kvm/guest_mem.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
@@ -0,0 +1,591 @@+// SPDX-License-Identifier: GPL-2.0+#include<linux/backing-dev.h>+#include<linux/falloc.h>+#include<linux/kvm_host.h>+#include<linux/pagemap.h>+#include<linux/pseudo_fs.h>++#include<uapi/linux/magic.h>++#include"kvm_mm.h"++staticstructvfsmount*kvm_gmem_mnt;++structkvm_gmem{+structkvm*kvm;+structxarraybindings;+structlist_headentry;+};++staticstructfolio*kvm_gmem_get_folio(structfile*file,pgoff_tindex)+{+structfolio*folio;++/* TODO: Support huge pages. */+folio=filemap_grab_folio(file->f_mapping,index);+if(!folio)+returnNULL;++/*+*Usetheup-to-dateflagtotrackwhetherornotthememoryhasbeen+*zeroedbeforebeinghandedofftotheguest.Thereisnobacking+*storageforthememory,sothefoliowillremainup-to-dateuntil+*it'sremoved.+*+*TODO:Skipclearingpageswhentrustedfirmwarewilldoitwhen+*assigningmemorytotheguest.+*/+if(!folio_test_uptodate(folio)){+unsignedlongnr_pages=folio_nr_pages(folio);+unsignedlongi;++for(i=0;i<nr_pages;i++)+clear_highpage(folio_page(folio,i));++folio_mark_uptodate(folio);+}++/*+*Ignoreaccessed,referenced,anddirtyflags.Thememoryis+*unevictableandthereisnostoragetowritebackto.+*/+returnfolio;+}++staticvoidkvm_gmem_invalidate_begin(structkvm_gmem*gmem,pgoff_tstart,+pgoff_tend)+{+structkvm_memory_slot*slot;+structkvm*kvm=gmem->kvm;+unsignedlongindex;+boolflush=false;++KVM_MMU_LOCK(kvm);++kvm_mmu_invalidate_begin(kvm);++xa_for_each_range(&gmem->bindings,index,slot,start,end-1){+pgoff_tpgoff=slot->gmem.pgoff;++structkvm_gfn_rangegfn_range={+.start=slot->base_gfn+max(pgoff,start)-pgoff,+.end=slot->base_gfn+min(pgoff+slot->npages,end)-pgoff,+.slot=slot,+.may_block=true,+};++flush|=kvm_mmu_unmap_gfn_range(kvm,&gfn_range);+}++if(flush)+kvm_flush_remote_tlbs(kvm);++KVM_MMU_UNLOCK(kvm);+}++staticvoidkvm_gmem_invalidate_end(structkvm_gmem*gmem,pgoff_tstart,+pgoff_tend)+{+structkvm*kvm=gmem->kvm;++KVM_MMU_LOCK(kvm);+if(xa_find(&gmem->bindings,&start,end-1,XA_PRESENT))+kvm_mmu_invalidate_end(kvm);+KVM_MMU_UNLOCK(kvm);+}++staticlongkvm_gmem_punch_hole(structinode*inode,loff_toffset,loff_tlen)+{+structlist_head*gmem_list=&inode->i_mapping->private_list;+pgoff_tstart=offset>>PAGE_SHIFT;+pgoff_tend=(offset+len)>>PAGE_SHIFT;+structkvm_gmem*gmem;++/*+*Bindingsmuststableacrossinvalidationtoensurethestart+end+*arebalanced.+*/+filemap_invalidate_lock(inode->i_mapping);++list_for_each_entry(gmem,gmem_list,entry)+kvm_gmem_invalidate_begin(gmem,start,end);++truncate_inode_pages_range(inode->i_mapping,offset,offset+len-1);++list_for_each_entry(gmem,gmem_list,entry)+kvm_gmem_invalidate_end(gmem,start,end);++filemap_invalidate_unlock(inode->i_mapping);++return0;+}++staticlongkvm_gmem_allocate(structinode*inode,loff_toffset,loff_tlen)+{+structaddress_space*mapping=inode->i_mapping;+pgoff_tstart,index,end;+intr;++/* Dedicated guest is immutable by default. */+if(offset+len>i_size_read(inode))+return-EINVAL;++filemap_invalidate_lock_shared(mapping);++start=offset>>PAGE_SHIFT;+end=(offset+len)>>PAGE_SHIFT;++r=0;+for(index=start;index<end;){+structfolio*folio;++if(signal_pending(current)){+r=-EINTR;+break;+}++folio=kvm_gmem_get_folio(inode,index);+if(!folio){+r=-ENOMEM;+break;+}++index=folio_next_index(folio);++folio_unlock(folio);+folio_put(folio);++/* 64-bit only, wrapping the index should be impossible. */+if(WARN_ON_ONCE(!index))+break;++cond_resched();+}++filemap_invalidate_unlock_shared(mapping);++returnr;+}++staticlongkvm_gmem_fallocate(structfile*file,intmode,loff_toffset,+loff_tlen)+{+intret;++if(!(mode&FALLOC_FL_KEEP_SIZE))+return-EOPNOTSUPP;++if(mode&~(FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE))+return-EOPNOTSUPP;++if(!PAGE_ALIGNED(offset)||!PAGE_ALIGNED(len))+return-EINVAL;++if(mode&FALLOC_FL_PUNCH_HOLE)+ret=kvm_gmem_punch_hole(file_inode(file),offset,len);+else+ret=kvm_gmem_allocate(file_inode(file),offset,len);++if(!ret)+file_modified(file);+returnret;+}++staticintkvm_gmem_release(structinode*inode,structfile*file)+{+structkvm_gmem*gmem=file->private_data;+structkvm_memory_slot*slot;+structkvm*kvm=gmem->kvm;+unsignedlongindex;++filemap_invalidate_lock(inode->i_mapping);++/*+*Preventconcurrentattemptsto*unbind*amemslot.Thisisthelast+*referencetothefileandthusnonewbindingscanbecreated,but+*dereferencingtheslotforexistingbindingsneedstobeprotected+*againstmemslotupdates,specificallysothatunbinddoesn'trace+*andfreethememslot(kvm_gmem_get_file()willreturnNULL).+*/+mutex_lock(&kvm->slots_lock);++xa_for_each(&gmem->bindings,index,slot)+rcu_assign_pointer(slot->gmem.file,NULL);++synchronize_rcu();++/*+*Allin-flightoperationsaregoneandnewbindingscanbecreated.+*ZapallSPTEspointedatbythisfile.Donotfreethebacking+*memory,asitslifetimeisassociatedwiththeinode,notthefile.+*/+kvm_gmem_invalidate_begin(gmem,0,-1ul);+kvm_gmem_invalidate_end(gmem,0,-1ul);++mutex_unlock(&kvm->slots_lock);++list_del(&gmem->entry);++filemap_invalidate_unlock(inode->i_mapping);++xa_destroy(&gmem->bindings);+kfree(gmem);++kvm_put_kvm(kvm);++return0;+}++staticstructfile*kvm_gmem_get_file(structkvm_memory_slot*slot)+{+structfile*file;++rcu_read_lock();++file=rcu_dereference(slot->gmem.file);+if(file&&!get_file_rcu(file))+file=NULL;++rcu_read_unlock();++returnfile;+}++staticconststructfile_operationskvm_gmem_fops={+.open=generic_file_open,+.release=kvm_gmem_release,+.fallocate=kvm_gmem_fallocate,+};++staticintkvm_gmem_migrate_folio(structaddress_space*mapping,+structfolio*dst,structfolio*src,+enummigrate_modemode)+{+WARN_ON_ONCE(1);+return-EINVAL;+}++staticintkvm_gmem_error_page(structaddress_space*mapping,structpage*page)+{+structlist_head*gmem_list=&mapping->private_list;+structkvm_memory_slot*slot;+structkvm_gmem*gmem;+unsignedlongindex;+pgoff_tstart,end;+gfn_tgfn;++filemap_invalidate_lock_shared(mapping);++start=page->index;+end=start+thp_nr_pages(page);++list_for_each_entry(gmem,gmem_list,entry){+xa_for_each_range(&gmem->bindings,index,slot,start,end-1){+for(gfn=start;gfn<end;gfn++){
Why the start end range used as gfn here ?
the page->index is offset of inode's page cache mapping and
gmem address space, IIUC, gfn calculation should follow same
way as kvm_gmem_invalidate_begin().
quoted hunk
+ if (WARN_ON_ONCE(gfn < slot->base_gfn ||
+ gfn >= slot->base_gfn + slot->npages))
+ continue;
+
+ /*
+ * FIXME: Tell userspace that the *private*
+ * memory encountered an error.
+ */
+ send_sig_mceerr(BUS_MCEERR_AR,
+ (void __user *)gfn_to_hva_memslot(slot, gfn),
+ PAGE_SHIFT, current);
+ }
+ }
+ }
+
+ filemap_invalidate_unlock_shared(mapping);
+
+ return 0;
+}
+
+static const struct address_space_operations kvm_gmem_aops = {
+ .dirty_folio = noop_dirty_folio,
+#ifdef CONFIG_MIGRATION
+ .migrate_folio = kvm_gmem_migrate_folio,
+#endif
+ .error_remove_page = kvm_gmem_error_page,
+};
+
+static int kvm_gmem_getattr(struct mnt_idmap *idmap,
+ const struct path *path, struct kstat *stat,
+ u32 request_mask, unsigned int query_flags)
+{
+ struct inode *inode = path->dentry->d_inode;
+
+ /* TODO */
+ generic_fillattr(idmap, inode, stat);
+ return 0;
+}
+
+static int kvm_gmem_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+ struct iattr *attr)
+{
+ /* TODO */
+ return -EINVAL;
+}
+static const struct inode_operations kvm_gmem_iops = {
+ .getattr = kvm_gmem_getattr,
+ .setattr = kvm_gmem_setattr,
+};
+
+static int __kvm_gmem_create(struct kvm *kvm, loff_t size, struct vfsmount *mnt)
+{
+ const char *anon_name = "[kvm-gmem]";
+ const struct qstr qname = QSTR_INIT(anon_name, strlen(anon_name));
+ struct kvm_gmem *gmem;
+ struct inode *inode;
+ struct file *file;
+ int fd, err;
+
+ inode = alloc_anon_inode(mnt->mnt_sb);
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
+
+ err = security_inode_init_security_anon(inode, &qname, NULL);
+ if (err)
+ goto err_inode;
+
+ inode->i_private = (void *)(unsigned long)flags;
+ inode->i_op = &kvm_gmem_iops;
+ inode->i_mapping->a_ops = &kvm_gmem_aops;
+ inode->i_mode |= S_IFREG;
+ inode->i_size = size;
+ mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
+ mapping_set_unevictable(inode->i_mapping);
+ mapping_set_unmovable(inode->i_mapping);
+
+ fd = get_unused_fd_flags(0);
+ if (fd < 0) {
+ err = fd;
+ goto err_inode;
+ }
+
+ file = alloc_file_pseudo(inode, mnt, "kvm-gmem", O_RDWR, &kvm_gmem_fops);
+ if (IS_ERR(file)) {
+ err = PTR_ERR(file);
+ goto err_fd;
+ }
+
+ file->f_flags |= O_LARGEFILE;
+ file->f_mapping = inode->i_mapping;
+
+ gmem = kzalloc(sizeof(*gmem), GFP_KERNEL);
+ if (!gmem) {
+ err = -ENOMEM;
+ goto err_file;
+ }
+
+ kvm_get_kvm(kvm);
+ gmem->kvm = kvm;
+ xa_init(&gmem->bindings);
+
+ file->private_data = gmem;
+
+ list_add(&gmem->entry, &inode->i_mapping->private_list);
+
+ fd_install(fd, file);
+ return fd;
+
+err_file:
+ fput(file);
+err_fd:
+ put_unused_fd(fd);
+err_inode:
+ iput(inode);
+ return err;
+}
+
+static bool kvm_gmem_is_valid_size(loff_t size, u64 flags)
+{
+ if (size < 0 || !PAGE_ALIGNED(size))
+ return false;
+
+ return true;
+}
+
+int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
+{
+ loff_t size = args->size;
+ u64 flags = args->flags;
+ u64 valid_flags = 0;
+
+ if (flags & ~valid_flags)
+ return -EINVAL;
+
+ if (!kvm_gmem_is_valid_size(size, flags))
+ return -EINVAL;
+
+ return __kvm_gmem_create(kvm, size, flags, kvm_gmem_mnt);
+}
+
+int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
+ unsigned int fd, loff_t offset)
+{
+ loff_t size = slot->npages << PAGE_SHIFT;
+ unsigned long start, end, flags;
+ struct kvm_gmem *gmem;
+ struct inode *inode;
+ struct file *file;
+
+ BUILD_BUG_ON(sizeof(gfn_t) != sizeof(slot->gmem.pgoff));
+
+ file = fget(fd);
+ if (!file)
+ return -EINVAL;
+
+ if (file->f_op != &kvm_gmem_fops)
+ goto err;
+
+ gmem = file->private_data;
+ if (gmem->kvm != kvm)
+ goto err;
+
+ inode = file_inode(file);
+ flags = (unsigned long)inode->i_private;
+
+ /*
+ * For simplicity, require the offset into the file and the size of the
+ * memslot to be aligned to the largest possible page size used to back
+ * the file (same as the size of the file itself).
+ */
+ if (!kvm_gmem_is_valid_size(offset, flags) ||
+ !kvm_gmem_is_valid_size(size, flags))
+ goto err;
+
+ if (offset + size > i_size_read(inode))
+ goto err;
+
+ filemap_invalidate_lock(inode->i_mapping);
+
+ start = offset >> PAGE_SHIFT;
+ end = start + slot->npages;
+
+ if (!xa_empty(&gmem->bindings) &&
+ xa_find(&gmem->bindings, &start, end - 1, XA_PRESENT)) {
+ filemap_invalidate_unlock(inode->i_mapping);
+ goto err;
+ }
+
+ /*
+ * No synchronize_rcu() needed, any in-flight readers are guaranteed to
+ * be see either a NULL file or this new file, no need for them to go
+ * away.
+ */
+ rcu_assign_pointer(slot->gmem.file, file);
+ slot->gmem.pgoff = start;
+
+ xa_store_range(&gmem->bindings, start, end - 1, slot, GFP_KERNEL);
+ filemap_invalidate_unlock(inode->i_mapping);
+
+ /*
+ * Drop the reference to the file, even on success. The file pins KVM,
+ * not the other way 'round. Active bindings are invalidated if the
+ * file is closed before memslots are destroyed.
+ */
+ fput(file);
+ return 0;
+
+err:
+ fput(file);
+ return -EINVAL;
+}
+
+void kvm_gmem_unbind(struct kvm_memory_slot *slot)
+{
+ unsigned long start = slot->gmem.pgoff;
+ unsigned long end = start + slot->npages;
+ struct kvm_gmem *gmem;
+ struct file *file;
+
+ /*
+ * Nothing to do if the underlying file was already closed (or is being
+ * closed right now), kvm_gmem_release() invalidates all bindings.
+ */
+ file = kvm_gmem_get_file(slot);
+ if (!file)
+ return;
+
+ gmem = file->private_data;
+
+ filemap_invalidate_lock(file->f_mapping);
+ xa_store_range(&gmem->bindings, start, end - 1, NULL, GFP_KERNEL);
+ rcu_assign_pointer(slot->gmem.file, NULL);
+ synchronize_rcu();
+ filemap_invalidate_unlock(file->f_mapping);
+
+ fput(file);
+}
+
+int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
+ gfn_t gfn, kvm_pfn_t *pfn, int *max_order)
+{
+ pgoff_t index = gfn - slot->base_gfn + slot->gmem.pgoff;
+ struct kvm_gmem *gmem;
+ struct folio *folio;
+ struct page *page;
+ struct file *file;
+
+ file = kvm_gmem_get_file(slot);
+ if (!file)
+ return -EFAULT;
+
+ gmem = file->private_data;
+
+ if (WARN_ON_ONCE(xa_load(&gmem->bindings, index) != slot)) {
+ fput(file);
+ return -EIO;
+ }
+
+ folio = kvm_gmem_get_folio(file_inode(file), index);
+ if (!folio) {
+ fput(file);
+ return -ENOMEM;
+ }
+
+ page = folio_file_page(folio, index);
+
+ *pfn = page_to_pfn(page);
+ *max_order = compound_order(compound_head(page));
+
+ folio_unlock(folio);
+ fput(file);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_gmem_get_pfn);
+
+static int kvm_gmem_init_fs_context(struct fs_context *fc)
+{
+ if (!init_pseudo(fc, GUEST_MEMORY_MAGIC))
+ return -ENOMEM;
+
+ return 0;
+}
+
+static struct file_system_type kvm_gmem_fs = {
+ .name = "kvm_guest_memory",
+ .init_fs_context = kvm_gmem_init_fs_context,
+ .kill_sb = kill_anon_super,
+};
+
+int kvm_gmem_init(void)
+{
+ kvm_gmem_mnt = kern_mount(&kvm_gmem_fs);
+ if (IS_ERR(kvm_gmem_mnt))
+ return PTR_ERR(kvm_gmem_mnt);
+
+ /* For giggles. Userspace can never map this anyways. */
+ kvm_gmem_mnt->mnt_flags |= MNT_NOEXEC;
+
+ return 0;
+}
+
+void kvm_gmem_exit(void)
+{
+ kern_unmount(kvm_gmem_mnt);
+ kvm_gmem_mnt = NULL;
+}
@@ -992,6 +992,9 @@ static void kvm_destroy_dirty_bitmap(struct kvm_memory_slot *memslot)/* This does not remove the slot from struct kvm_memslots data structures */staticvoidkvm_free_memslot(structkvm*kvm,structkvm_memory_slot*slot){+if(slot->flags&KVM_MEM_PRIVATE)+kvm_gmem_unbind(slot);+kvm_destroy_dirty_bitmap(slot);kvm_arch_free_memslot(kvm,slot);
@@ -1556,10 +1559,18 @@ static void kvm_replace_memslot(struct kvm *kvm,}}-staticintcheck_memory_region_flags(conststructkvm_userspace_memory_region2*mem)+staticintcheck_memory_region_flags(structkvm*kvm,+conststructkvm_userspace_memory_region2*mem){u32valid_flags=KVM_MEM_LOG_DIRTY_PAGES;+if(kvm_arch_has_private_mem(kvm))+valid_flags|=KVM_MEM_PRIVATE;++/* Dirty logging private memory is not currently supported. */+if(mem->flags&KVM_MEM_PRIVATE)+valid_flags&=~KVM_MEM_LOG_DIRTY_PAGES;+#ifdef __KVM_HAVE_READONLY_MEMvalid_flags|=KVM_MEM_READONLY;#endif
@@ -1968,7 +1979,7 @@ int __kvm_set_memory_region(struct kvm *kvm,intas_id,id;intr;-r=check_memory_region_flags(mem);+r=check_memory_region_flags(kvm,mem);if(r)returnr;
@@ -1987,6 +1998,10 @@ int __kvm_set_memory_region(struct kvm *kvm,!access_ok((void__user*)(unsignedlong)mem->userspace_addr,mem->memory_size))return-EINVAL;+if(mem->flags&KVM_MEM_PRIVATE&&+(mem->gmem_offset&(PAGE_SIZE-1)||+mem->gmem_offset+mem->memory_size<mem->gmem_offset))+return-EINVAL;if(as_id>=KVM_ADDRESS_SPACE_NUM||id>=KVM_MEM_SLOTS_NUM)return-EINVAL;if(mem->guest_phys_addr+mem->memory_size<mem->guest_phys_addr)
@@ -2025,6 +2040,9 @@ int __kvm_set_memory_region(struct kvm *kvm,if((kvm->nr_memslot_pages+npages)<kvm->nr_memslot_pages)return-EINVAL;}else{/* Modify an existing slot. */+/* Private memslots are immutable, they can only be deleted. */+if(mem->flags&KVM_MEM_PRIVATE)+return-EINVAL;if((mem->userspace_addr!=old->userspace_addr)||(npages!=old->npages)||((mem->flags^old->flags)&KVM_MEM_READONLY))
@@ -2053,10 +2071,23 @@ int __kvm_set_memory_region(struct kvm *kvm,new->npages=npages;new->flags=mem->flags;new->userspace_addr=mem->userspace_addr;+if(mem->flags&KVM_MEM_PRIVATE){+r=kvm_gmem_bind(kvm,new,mem->gmem_fd,mem->gmem_offset);+if(r)+gotoout;+}r=kvm_set_memslot(kvm,old,new,change);if(r)-kfree(new);+gotoout_restricted;++return0;++out_restricted:+if(mem->flags&KVM_MEM_PRIVATE)+kvm_gmem_unbind(new);+out:+kfree(new);returnr;}EXPORT_SYMBOL_GPL(__kvm_set_memory_region);
@@ -2356,6 +2387,8 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTESstaticu64kvm_supported_mem_attributes(structkvm*kvm){+if(kvm_arch_has_private_mem(kvm))+returnKVM_MEMORY_ATTRIBUTE_PRIVATE;return0;}
@@ -5134,6 +5167,16 @@ static long kvm_vm_ioctl(struct file *filp,caseKVM_GET_STATS_FD:r=kvm_vm_ioctl_get_stats_fd(kvm);break;+caseKVM_CREATE_GUEST_MEMFD:{+structkvm_create_guest_memfdguest_memfd;++r=-EFAULT;+if(copy_from_user(&guest_memfd,argp,sizeof(guest_memfd)))+gotoout;++r=kvm_gmem_create(kvm,&guest_memfd);+break;+}default:r=kvm_arch_vm_ioctl(filp,ioctl,arg);}
On Tue, Jul 18, 2023 at 04:44:44PM -0700, Sean Christopherson wrote:
May I know why KVM now needs to register to callback .change_pte()?
As also commented in kvm_mmu_notifier_change_pte(), .change_pte() must be
surrounded by .invalidate_range_{start,end}().
While kvm_mmu_notifier_invalidate_range_start() has called kvm_unmap_gfn_range()
to zap all leaf SPTEs, and page fault path will not install new SPTEs
successfully before kvm_mmu_notifier_invalidate_range_end(),
kvm_set_spte_gfn() should not be able to find any shadow present leaf entries to
update PFN.
Or could we just delete completely
"kvm_handle_hva_range(mn, address, address + 1, pte, kvm_set_spte_gfn);"
from kvm_mmu_notifier_change_pte() ?
@@ -1241,7 +1241,7 @@ static bool set_spte_gfn(struct kvm *kvm, struct tdp_iter *iter,u64new_spte;/* Huge pages aren't expected to be modified without first being zapped. */-WARN_ON(pte_huge(range->pte)||range->start+1!=range->end);+WARN_ON(pte_huge(range->arg.pte)||range->start+1!=range->end);if(iter->level!=PG_LEVEL_4K||!is_shadow_present_pte(iter->old_spte))
@@ -4779,15 +4788,28 @@ static long kvm_vm_ioctl(struct file *filp,r=kvm_vm_ioctl_enable_cap_generic(kvm,&cap);break;}+caseKVM_SET_USER_MEMORY_REGION2:caseKVM_SET_USER_MEMORY_REGION:{-structkvm_userspace_memory_regionkvm_userspace_mem;+structkvm_userspace_memory_region2mem;+unsignedlongsize;++if(ioctl==KVM_SET_USER_MEMORY_REGION)+size=sizeof(structkvm_userspace_memory_region);+else+size=sizeof(structkvm_userspace_memory_region2);++/* Ensure the common parts of the two structs are identical. */+SANITY_CHECK_MEM_REGION_FIELD(slot);+SANITY_CHECK_MEM_REGION_FIELD(flags);+SANITY_CHECK_MEM_REGION_FIELD(guest_phys_addr);+SANITY_CHECK_MEM_REGION_FIELD(memory_size);+SANITY_CHECK_MEM_REGION_FIELD(userspace_addr);r=-EFAULT;-if(copy_from_user(&kvm_userspace_mem,argp,-sizeof(kvm_userspace_mem)))+if(copy_from_user(&mem,argp,size))gotoout;-r=kvm_vm_ioctl_set_memory_region(kvm,&kvm_userspace_mem);+r=kvm_vm_ioctl_set_memory_region(kvm,&mem);break;}caseKVM_GET_DIRTY_LOG:{
On Tue, Jul 18, 2023 at 04:44:44PM -0700, Sean Christopherson wrote:
May I know why KVM now needs to register to callback .change_pte()?
I can see the original purpose is to "setting a pte in the shadow page
table directly, instead of flushing the shadow page table entry and then
getting vmexit to set it"[1].
IIUC, KVM is expected to directly make the new pte present for new
pages in this callback, like for COW.
As also commented in kvm_mmu_notifier_change_pte(), .change_pte() must be
surrounded by .invalidate_range_{start,end}().
While kvm_mmu_notifier_invalidate_range_start() has called kvm_unmap_gfn_range()
to zap all leaf SPTEs, and page fault path will not install new SPTEs
successfully before kvm_mmu_notifier_invalidate_range_end(),
kvm_set_spte_gfn() should not be able to find any shadow present leaf entries to
update PFN.
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2023-07-21 10:58:02
On 7/19/23 01:44, Sean Christopherson wrote:
From: Chao Peng <redacted>
In confidential computing usages, whether a page is private or shared is
necessary information for KVM to perform operations like page fault
handling, page zapping etc. There are other potential use cases for
per-page memory attributes, e.g. to make memory read-only (or no-exec,
or exec-only, etc.) without having to modify memslots.
Introduce two ioctls (advertised by KVM_CAP_MEMORY_ATTRIBUTES) to allow
userspace to operate on the per-page memory attributes.
- KVM_SET_MEMORY_ATTRIBUTES to set the per-page memory attributes to
a guest memory range.
- KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES to return the KVM supported
memory attributes.
Use an xarray to store the per-page attributes internally, with a naive,
not fully optimized implementation, i.e. prioritize correctness over
performance for the initial implementation.
Because setting memory attributes is roughly analogous to mprotect() on
memory that is mapped into the guest, zap existing mappings prior to
updating the memory attributes. Opportunistically provide an arch hook
for the post-set path (needed to complete invalidation anyways) in
anticipation of x86 needing the hook to update metadata related to
determining whether or not a given gfn can be backed with various sizes
of hugepages.
It's possible that future usages may not require an invalidation, e.g.
if KVM ends up supporting RWX protections and userspace grants _more_
protections, but again opt for simplicity and punt optimizations to
if/when they are needed.
Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/all/Y2WB48kD0J4VGynX@google.com
Cc: Fuad Tabba <redacted>
Signed-off-by: Chao Peng <redacted>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2023-07-21 15:11:01
On 7/19/23 01:44, Sean Christopherson wrote:
quoted hunk
From: Chao Peng <redacted>
A KVM_MEM_PRIVATE memslot can include both fd-based private memory and
hva-based shared memory. Architecture code (like TDX code) can tell
whether the on-going fault is private or not. This patch adds a
'is_private' field to kvm_page_fault to indicate this and architecture
code is expected to set it.
To handle page fault for such memslot, the handling logic is different
depending on whether the fault is private or shared. KVM checks if
'is_private' matches the host's view of the page (maintained in
mem_attr_array).
- For a successful match, private pfn is obtained with
restrictedmem_get_page() and shared pfn is obtained with existing
get_user_pages().
- For a failed match, KVM causes a KVM_EXIT_MEMORY_FAULT exit to
userspace. Userspace then can convert memory between private/shared
in host's view and retry the fault.
Co-developed-by: Yu Zhang <redacted>
Signed-off-by: Yu Zhang <redacted>
Signed-off-by: Chao Peng <redacted>
Reviewed-by: Fuad Tabba <redacted>
Tested-by: Fuad Tabba <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/mmu/mmu.c | 82 +++++++++++++++++++++++++++++++--
arch/x86/kvm/mmu/mmu_internal.h | 3 ++
arch/x86/kvm/mmu/mmutrace.h | 1 +
3 files changed, 81 insertions(+), 5 deletions(-)
@@ -203,6 +203,7 @@ struct kvm_page_fault {/* Derived from mmu and global state. */constboolis_tdp;+constboolis_private;constboolnx_huge_page_workaround_enabled;/*
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2023-07-21 15:16:13
On 7/19/23 01:45, Sean Christopherson wrote:
quoted hunk
Drop kvm_userspace_memory_region_find(), it's unused and a terrible API
(probably why it's unused). If anything outside of kvm_util.c needs to
get at the memslot, userspace_mem_region_find() can be exposed to give
others full access to all memory region/slot information.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/include/kvm_util_base.h | 4 ---
tools/testing/selftests/kvm/lib/kvm_util.c | 29 -------------------
2 files changed, 33 deletions(-)
why kvm_gmem_init() needs to be called again? by mistake?
I'm sure it's a mistake.
I'm testing the gmem QEMU with this series. SW_PROTECTED_VM gets stuck
in a loop in early OVMF code due to two shared page of OVMF get zapped
and re-mapped infinitely. Removing the second call of kvm_gmem_init()
can solve the issue, though I'm not sure about the reason.
From: Xiaoyao Li <hidden> Date: 2023-07-21 15:57:00
On 7/19/2023 7:44 AM, Sean Christopherson wrote:
+4.140 KVM_SET_MEMORY_ATTRIBUTES
+-----------------------------------------
+
+:Capability: KVM_CAP_MEMORY_ATTRIBUTES
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_memory_attributes(in/out)
+:Returns: 0 on success, <0 on error
+
+Sets memory attributes for pages in a guest memory range. Parameters are
+specified via the following structure::
+
+ struct kvm_memory_attributes {
+ __u64 address;
+ __u64 size;
+ __u64 attributes;
+ __u64 flags;
+ };
+
+The user sets the per-page memory attributes to a guest memory range indicated
+by address/size, and in return KVM adjusts address and size to reflect the
+actual pages of the memory range have been successfully set to the attributes.
+If the call returns 0, "address" is updated to the last successful address + 1
+and "size" is updated to the remaining address size that has not been set
+successfully. The user should check the return value as well as the size to
+decide if the operation succeeded for the whole range or not. The user may want
+to retry the operation with the returned address/size if the previous range was
+partially successful.
This does not match with the implementation. Please fix either one to
make them consistent.
I think it should be always allowed. The outcome would just be "never have
a hugepage" if thp is not enabled in the kernel.
I don't have a strong preference. My thinking was that userspace would probably
rather have an explicit error, as opposed to silently running with a misconfigured
setup.
I don't understand the need to have a separate filesystem. If it is to
fully setup the inode before it's given a struct file, why not just
export anon_inode_make_secure_inode instead of
security_inode_init_security_anon?
Paolo
why kvm_gmem_init() needs to be called again? by mistake?
I'm sure it's a mistake.
Yeah, definitely a bug.
I'm testing the gmem QEMU with this series. SW_PROTECTED_VM gets stuck in a
loop in early OVMF code due to two shared page of OVMF get zapped and
re-mapped infinitely. Removing the second call of kvm_gmem_init() can solve
the issue, though I'm not sure about the reason.
Not worth investigating unless you want to satiate your curiosity :-)
I don't understand the need to have a separate filesystem. If it is to
fully setup the inode before it's given a struct file, why not just export
anon_inode_make_secure_inode instead of security_inode_init_security_anon?
Ugh, this is why comments are important, I can't remember either.
I suspect I implemented a dedicated filesystem to kinda sorta show that we could
allow userspace to provide the mount point with e.g. NUMA hints[*]. But my
preference would be to not support a userspace provided mount and instead implement
fbind() to let userspace control NUMA and whatnot.
[*] https://lore.kernel.org/all/ef48935e5e6f947f6f0c6d748232b14ef5d5ad70.1681176340.git.ackerleytng@google.com
@@ -0,0 +1,591 @@+// SPDX-License-Identifier: GPL-2.0+#include<linux/backing-dev.h>+#include<linux/falloc.h>+#include<linux/kvm_host.h>+#include<linux/pagemap.h>+#include<linux/pseudo_fs.h>++#include<uapi/linux/magic.h>++#include"kvm_mm.h"++staticstructvfsmount*kvm_gmem_mnt;++structkvm_gmem{+structkvm*kvm;+structxarraybindings;+structlist_headentry;+};++staticstructfolio*kvm_gmem_get_folio(structfile*file,pgoff_tindex)+{+structfolio*folio;++/* TODO: Support huge pages. */+folio=filemap_grab_folio(file->f_mapping,index);+if(!folio)+returnNULL;++/*+*Usetheup-to-dateflagtotrackwhetherornotthememoryhasbeen+*zeroedbeforebeinghandedofftotheguest.Thereisnobacking+*storageforthememory,sothefoliowillremainup-to-dateuntil+*it'sremoved.+*+*TODO:Skipclearingpageswhentrustedfirmwarewilldoitwhen+*assigningmemorytotheguest.+*/+if(!folio_test_uptodate(folio)){+unsignedlongnr_pages=folio_nr_pages(folio);+unsignedlongi;++for(i=0;i<nr_pages;i++)+clear_highpage(folio_page(folio,i));++folio_mark_uptodate(folio);+}++/*+*Ignoreaccessed,referenced,anddirtyflags.Thememoryis+*unevictableandthereisnostoragetowritebackto.+*/+returnfolio;+}++staticvoidkvm_gmem_invalidate_begin(structkvm_gmem*gmem,pgoff_tstart,+pgoff_tend)+{+structkvm_memory_slot*slot;+structkvm*kvm=gmem->kvm;+unsignedlongindex;+boolflush=false;++KVM_MMU_LOCK(kvm);++kvm_mmu_invalidate_begin(kvm);++xa_for_each_range(&gmem->bindings,index,slot,start,end-1){+pgoff_tpgoff=slot->gmem.pgoff;++structkvm_gfn_rangegfn_range={+.start=slot->base_gfn+max(pgoff,start)-pgoff,+.end=slot->base_gfn+min(pgoff+slot->npages,end)-pgoff,+.slot=slot,+.may_block=true,+};++flush|=kvm_mmu_unmap_gfn_range(kvm,&gfn_range);+}++if(flush)+kvm_flush_remote_tlbs(kvm);++KVM_MMU_UNLOCK(kvm);+}++staticvoidkvm_gmem_invalidate_end(structkvm_gmem*gmem,pgoff_tstart,+pgoff_tend)+{+structkvm*kvm=gmem->kvm;++KVM_MMU_LOCK(kvm);+if(xa_find(&gmem->bindings,&start,end-1,XA_PRESENT))+kvm_mmu_invalidate_end(kvm);+KVM_MMU_UNLOCK(kvm);+}++staticlongkvm_gmem_punch_hole(structinode*inode,loff_toffset,loff_tlen)+{+structlist_head*gmem_list=&inode->i_mapping->private_list;+pgoff_tstart=offset>>PAGE_SHIFT;+pgoff_tend=(offset+len)>>PAGE_SHIFT;+structkvm_gmem*gmem;++/*+*Bindingsmuststableacrossinvalidationtoensurethestart+end+*arebalanced.+*/+filemap_invalidate_lock(inode->i_mapping);++list_for_each_entry(gmem,gmem_list,entry)+kvm_gmem_invalidate_begin(gmem,start,end);++truncate_inode_pages_range(inode->i_mapping,offset,offset+len-1);++list_for_each_entry(gmem,gmem_list,entry)+kvm_gmem_invalidate_end(gmem,start,end);++filemap_invalidate_unlock(inode->i_mapping);++return0;+}++staticlongkvm_gmem_allocate(structinode*inode,loff_toffset,loff_tlen)+{+structaddress_space*mapping=inode->i_mapping;+pgoff_tstart,index,end;+intr;++/* Dedicated guest is immutable by default. */+if(offset+len>i_size_read(inode))+return-EINVAL;++filemap_invalidate_lock_shared(mapping);++start=offset>>PAGE_SHIFT;+end=(offset+len)>>PAGE_SHIFT;++r=0;+for(index=start;index<end;){+structfolio*folio;++if(signal_pending(current)){+r=-EINTR;+break;+}++folio=kvm_gmem_get_folio(inode,index);+if(!folio){+r=-ENOMEM;+break;+}++index=folio_next_index(folio);++folio_unlock(folio);+folio_put(folio);++/* 64-bit only, wrapping the index should be impossible. */+if(WARN_ON_ONCE(!index))+break;++cond_resched();+}++filemap_invalidate_unlock_shared(mapping);++returnr;+}++staticlongkvm_gmem_fallocate(structfile*file,intmode,loff_toffset,+loff_tlen)+{+intret;++if(!(mode&FALLOC_FL_KEEP_SIZE))+return-EOPNOTSUPP;++if(mode&~(FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE))+return-EOPNOTSUPP;++if(!PAGE_ALIGNED(offset)||!PAGE_ALIGNED(len))+return-EINVAL;++if(mode&FALLOC_FL_PUNCH_HOLE)+ret=kvm_gmem_punch_hole(file_inode(file),offset,len);+else+ret=kvm_gmem_allocate(file_inode(file),offset,len);++if(!ret)+file_modified(file);+returnret;+}++staticintkvm_gmem_release(structinode*inode,structfile*file)+{+structkvm_gmem*gmem=file->private_data;+structkvm_memory_slot*slot;+structkvm*kvm=gmem->kvm;+unsignedlongindex;++filemap_invalidate_lock(inode->i_mapping);++/*+*Preventconcurrentattemptsto*unbind*amemslot.Thisisthelast+*referencetothefileandthusnonewbindingscanbecreated,but+*dereferencingtheslotforexistingbindingsneedstobeprotected+*againstmemslotupdates,specificallysothatunbinddoesn'trace+*andfreethememslot(kvm_gmem_get_file()willreturnNULL).+*/+mutex_lock(&kvm->slots_lock);++xa_for_each(&gmem->bindings,index,slot)+rcu_assign_pointer(slot->gmem.file,NULL);++synchronize_rcu();++/*+*Allin-flightoperationsaregoneandnewbindingscanbecreated.+*ZapallSPTEspointedatbythisfile.Donotfreethebacking+*memory,asitslifetimeisassociatedwiththeinode,notthefile.+*/+kvm_gmem_invalidate_begin(gmem,0,-1ul);+kvm_gmem_invalidate_end(gmem,0,-1ul);++mutex_unlock(&kvm->slots_lock);++list_del(&gmem->entry);++filemap_invalidate_unlock(inode->i_mapping);++xa_destroy(&gmem->bindings);+kfree(gmem);++kvm_put_kvm(kvm);++return0;+}++staticstructfile*kvm_gmem_get_file(structkvm_memory_slot*slot)+{+structfile*file;++rcu_read_lock();++file=rcu_dereference(slot->gmem.file);+if(file&&!get_file_rcu(file))+file=NULL;++rcu_read_unlock();++returnfile;+}++staticconststructfile_operationskvm_gmem_fops={+.open=generic_file_open,+.release=kvm_gmem_release,+.fallocate=kvm_gmem_fallocate,+};++staticintkvm_gmem_migrate_folio(structaddress_space*mapping,+structfolio*dst,structfolio*src,+enummigrate_modemode)+{+WARN_ON_ONCE(1);+return-EINVAL;+}++staticintkvm_gmem_error_page(structaddress_space*mapping,structpage*page)+{+structlist_head*gmem_list=&mapping->private_list;+structkvm_memory_slot*slot;+structkvm_gmem*gmem;+unsignedlongindex;+pgoff_tstart,end;+gfn_tgfn;++filemap_invalidate_lock_shared(mapping);++start=page->index;+end=start+thp_nr_pages(page);++list_for_each_entry(gmem,gmem_list,entry){+xa_for_each_range(&gmem->bindings,index,slot,start,end-1){+for(gfn=start;gfn<end;gfn++){
Why the start end range used as gfn here ?
the page->index is offset of inode's page cache mapping and
gmem address space, IIUC, gfn calculation should follow same
way as kvm_gmem_invalidate_begin().
Also instead of sending signal multiple times, we can utilize lsb argument.
Something like this?
Math is hard? I almost always mess up these types of things, and then catch my
bugs via tests. But I don't have tests for this particular flow... Which
reminds me, we need tests for this :-) Hopefully error injection provides most
of what we need?
quoted
the page->index is offset of inode's page cache mapping and
gmem address space, IIUC, gfn calculation should follow same
way as kvm_gmem_invalidate_begin().
Also instead of sending signal multiple times, we can utilize lsb argument.
As Vishal pointed out, this code shouldn't be sending signals in the first place.
On 2023-07-18 at 16:44:51 -0700, Sean Christopherson wrote:
From: Chao Peng <redacted>
In confidential computing usages, whether a page is private or shared is
necessary information for KVM to perform operations like page fault
handling, page zapping etc. There are other potential use cases for
per-page memory attributes, e.g. to make memory read-only (or no-exec,
or exec-only, etc.) without having to modify memslots.
Introduce two ioctls (advertised by KVM_CAP_MEMORY_ATTRIBUTES) to allow
userspace to operate on the per-page memory attributes.
- KVM_SET_MEMORY_ATTRIBUTES to set the per-page memory attributes to
a guest memory range.
- KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES to return the KVM supported
memory attributes.
Use an xarray to store the per-page attributes internally, with a naive,
not fully optimized implementation, i.e. prioritize correctness over
performance for the initial implementation.
Because setting memory attributes is roughly analogous to mprotect() on
memory that is mapped into the guest, zap existing mappings prior to
updating the memory attributes. Opportunistically provide an arch hook
for the post-set path (needed to complete invalidation anyways) in
anticipation of x86 needing the hook to update metadata related to
determining whether or not a given gfn can be backed with various sizes
of hugepages.
It's possible that future usages may not require an invalidation, e.g.
if KVM ends up supporting RWX protections and userspace grants _more_
protections, but again opt for simplicity and punt optimizations to
if/when they are needed.
Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/all/Y2WB48kD0J4VGynX@google.com
Cc: Fuad Tabba <redacted>
Signed-off-by: Chao Peng <redacted>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Documentation/virt/kvm/api.rst | 60 ++++++++++++
include/linux/kvm_host.h | 14 +++
include/uapi/linux/kvm.h | 14 +++
virt/kvm/Kconfig | 4 +
virt/kvm/kvm_main.c | 170 +++++++++++++++++++++++++++++++++
5 files changed, 262 insertions(+)
Is it better to make the destruction in reverse order from the creation?
To put xa_destroy(&kvm->mem_attr_array) after cleanup_srcu_struct(&kvm->srcu),
or put xa_init(&kvm->mem_attr_array) after init_srcu_struct(&kvm->irq_srcu).
+static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
+ struct kvm_memory_attributes *attrs)
+{
+ gfn_t start, end;
+
+ /* flags is currently not used. */
+ if (attrs->flags)
+ return -EINVAL;
+ if (attrs->attributes & ~kvm_supported_mem_attributes(kvm))
+ return -EINVAL;
+ if (attrs->size == 0 || attrs->address + attrs->size < attrs->address)
+ return -EINVAL;
+ if (!PAGE_ALIGNED(attrs->address) || !PAGE_ALIGNED(attrs->size))
+ return -EINVAL;
+
+ start = attrs->address >> PAGE_SHIFT;
+ end = (attrs->address + attrs->size - 1 + PAGE_SIZE) >> PAGE_SHIFT;
As the attrs->address/size are both garanteed to be non-zero, non-wrap
and page aligned in prevous check. Is it OK to simplify the calculation,
like:
end = (attrs->address + attrs->size) >> PAGE_SHIFT;
+
+ if (WARN_ON_ONCE(start == end))
+ return -EINVAL;
Also, is this check possible to be hit? Maybe remove it?
Thanks,
Yilun
+
+ /*
+ * xarray tracks data using "unsigned long", and as a result so does
+ * KVM. For simplicity, supports generic attributes only on 64-bit
+ * architectures.
+ */
+ BUILD_BUG_ON(sizeof(attrs->attributes) != sizeof(unsigned long));
+
+ return kvm_vm_set_mem_attributes(kvm, attrs->attributes, start, end);
+}
+#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
From: Nikunj A. Dadhania <hidden> Date: 2023-07-24 06:40:00
On 7/19/2023 5:14 AM, Sean Christopherson wrote:
This is the next iteration of implementing fd-based (instead of vma-based)
memory for KVM guests. If you want the full background of why we are doing
this, please go read the v10 cover letter[1].
The biggest change from v10 is to implement the backing storage in KVM
itself, and expose it via a KVM ioctl() instead of a "generic" sycall.
See link[2] for details on why we pivoted to a KVM-specific approach.
Key word is "biggest". Relative to v10, there are many big changes.
Highlights below (I can't remember everything that got changed at
this point).
Tagged RFC as there are a lot of empty changelogs, and a lot of missing
documentation. And ideally, we'll have even more tests before merging.
There are also several gaps/opens (to be discussed in tomorrow's PUCK).
As per our discussion on the PUCK call, here are the memory/NUMA accounting
related observations that I had while working on SNP guest secure page migration:
* gmem allocations are currently treated as file page allocations
accounted to the kernel and not to the QEMU process.
Starting an SNP guest with 40G memory with memory interleave between
Node2 and Node3
$ numactl -i 2,3 ./bootg_snp.sh
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
242179 root 20 0 40.4g 99580 51676 S 78.0 0.0 0:56.58 qemu-system-x86
-> Incorrect process resident memory and shared memory is reported
Accounting of the memory happens in the host page fault handler path,
but for private guest pages we will never hit that.
* NUMA allocation does use the process mempolicy for appropriate node
allocation (Node2 and Node3), but they again do not get attributed to
the QEMU process
Every 1.0s: sudo numastat -m -p qemu-system-x86 | egrep -i "qemu|PID|Node|Filepage" gomati: Mon Jul 24 11:51:34 2023
Per-node process memory usage (in MBs)
PID Node 0 Node 1 Node 2 Node 3 Total
242179 (qemu-system-x86) 21.14 1.61 39.44 39.38 101.57
Per-node system memory usage (in MBs):
Node 0 Node 1 Node 2 Node 3 Total
FilePages 2475.63 2395.83 23999.46 23373.22 52244.14
* Most of the memory accounting relies on the VMAs and as private-fd of
gmem doesn't have a VMA(and that was the design goal), user-space fails
to attribute the memory appropriately to the process.
/proc/<qemu pid>/numa_maps
7f528be00000 interleave:2-3 file=/memfd:memory-backend-memfd-shared\040(deleted) anon=1070 dirty=1070 mapped=1987 mapmax=256 active=1956 N2=582 N3=1405 kernelpagesize_kB=4
7f5c90200000 interleave:2-3 file=/memfd:rom-backend-memfd-shared\040(deleted)
7f5c90400000 interleave:2-3 file=/memfd:rom-backend-memfd-shared\040(deleted) dirty=32 active=0 N2=32 kernelpagesize_kB=4
7f5c90800000 interleave:2-3 file=/memfd:rom-backend-memfd-shared\040(deleted) dirty=892 active=0 N2=512 N3=380 kernelpagesize_kB=4
/proc/<qemu pid>/smaps
7f528be00000-7f5c8be00000 rw-p 00000000 00:01 26629 /memfd:memory-backend-memfd-shared (deleted)
7f5c90200000-7f5c90220000 rw-s 00000000 00:01 44033 /memfd:rom-backend-memfd-shared (deleted)
7f5c90400000-7f5c90420000 rw-s 00000000 00:01 44032 /memfd:rom-backend-memfd-shared (deleted)
7f5c90800000-7f5c90b7c000 rw-s 00000000 00:01 1025 /memfd:rom-backend-memfd-shared (deleted)
* QEMU based NUMA bindings will not work. Memory backend uses mbind()
to set the policy for a particular virtual memory range but gmem
private-FD does not have a virtual memory range visible in the host.
Regards,
Nikunj
From: Sean Christopherson <seanjc@google.com> Date: 2023-07-24 17:00:43
On Mon, Jul 24, 2023, Nikunj A. Dadhania wrote:
On 7/19/2023 5:14 AM, Sean Christopherson wrote:
quoted
This is the next iteration of implementing fd-based (instead of vma-based)
memory for KVM guests. If you want the full background of why we are doing
this, please go read the v10 cover letter[1].
The biggest change from v10 is to implement the backing storage in KVM
itself, and expose it via a KVM ioctl() instead of a "generic" sycall.
See link[2] for details on why we pivoted to a KVM-specific approach.
Key word is "biggest". Relative to v10, there are many big changes.
Highlights below (I can't remember everything that got changed at
this point).
Tagged RFC as there are a lot of empty changelogs, and a lot of missing
documentation. And ideally, we'll have even more tests before merging.
There are also several gaps/opens (to be discussed in tomorrow's PUCK).
As per our discussion on the PUCK call, here are the memory/NUMA accounting
related observations that I had while working on SNP guest secure page migration:
* gmem allocations are currently treated as file page allocations
accounted to the kernel and not to the QEMU process.
We need to level set on terminology: these are all *stats*, not accounting. That
distinction matters because we have wiggle room on stats, e.g. we can probably get
away with just about any definition of how guest_memfd memory impacts stats, so
long as the information that is surfaced to userspace is useful and expected.
But we absolutely need to get accounting correct, specifically the allocations
need to be correctly accounted in memcg. And unless I'm missing something,
nothing in here shows anything related to memcg.
Starting an SNP guest with 40G memory with memory interleave between
Node2 and Node3
$ numactl -i 2,3 ./bootg_snp.sh
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
242179 root 20 0 40.4g 99580 51676 S 78.0 0.0 0:56.58 qemu-system-x86
-> Incorrect process resident memory and shared memory is reported
I don't know that I would call these "incorrect". Shared memory definitely is
correct, because by definition guest_memfd isn't shared. RSS is less clear cut;
gmem memory is resident in RAM, but if we show gmem in RSS then we'll end up with
scenarios where RSS > VIRT, which will be quite confusing for unaware users (I'm
assuming the 40g of VIRT here comes from QEMU mapping the shared half of gmem
memslots).
Accounting of the memory happens in the host page fault handler path,
but for private guest pages we will never hit that.
* NUMA allocation does use the process mempolicy for appropriate node
allocation (Node2 and Node3), but they again do not get attributed to
the QEMU process
Every 1.0s: sudo numastat -m -p qemu-system-x86 | egrep -i "qemu|PID|Node|Filepage" gomati: Mon Jul 24 11:51:34 2023
Per-node process memory usage (in MBs)
PID Node 0 Node 1 Node 2 Node 3 Total
242179 (qemu-system-x86) 21.14 1.61 39.44 39.38 101.57
Per-node system memory usage (in MBs):
Node 0 Node 1 Node 2 Node 3 Total
FilePages 2475.63 2395.83 23999.46 23373.22 52244.14
* Most of the memory accounting relies on the VMAs and as private-fd of
gmem doesn't have a VMA(and that was the design goal), user-space fails
to attribute the memory appropriately to the process.
/proc/<qemu pid>/numa_maps
7f528be00000 interleave:2-3 file=/memfd:memory-backend-memfd-shared\040(deleted) anon=1070 dirty=1070 mapped=1987 mapmax=256 active=1956 N2=582 N3=1405 kernelpagesize_kB=4
7f5c90200000 interleave:2-3 file=/memfd:rom-backend-memfd-shared\040(deleted)
7f5c90400000 interleave:2-3 file=/memfd:rom-backend-memfd-shared\040(deleted) dirty=32 active=0 N2=32 kernelpagesize_kB=4
7f5c90800000 interleave:2-3 file=/memfd:rom-backend-memfd-shared\040(deleted) dirty=892 active=0 N2=512 N3=380 kernelpagesize_kB=4
/proc/<qemu pid>/smaps
7f528be00000-7f5c8be00000 rw-p 00000000 00:01 26629 /memfd:memory-backend-memfd-shared (deleted)
7f5c90200000-7f5c90220000 rw-s 00000000 00:01 44033 /memfd:rom-backend-memfd-shared (deleted)
7f5c90400000-7f5c90420000 rw-s 00000000 00:01 44032 /memfd:rom-backend-memfd-shared (deleted)
7f5c90800000-7f5c90b7c000 rw-s 00000000 00:01 1025 /memfd:rom-backend-memfd-shared (deleted)
This is all expected, and IMO correct. There are no userspace mappings, and so
not accounting anything is working as intended.
* QEMU based NUMA bindings will not work. Memory backend uses mbind()
to set the policy for a particular virtual memory range but gmem
private-FD does not have a virtual memory range visible in the host.
Yes, adding a generic fbind() is the way to solve silve.
@@ -1047,6 +1047,10 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,if(!mapping&&(folio_ref_count(folio)-1)>folio_mapcount(folio))gotoisolate_fail_put;+/* The mapping truly isn't movable. */+if(mapping&&mapping_unmovable(mapping))+gotoisolate_fail_put;+
I doubt that it is safe to dereference mapping here. I believe the folio
can be truncated from under us and the mapping freed with the inode.
The folio has to be locked to dereference mapping safely (given that the
mapping is still tied to the folio).
Vlastimil, any comments?
--
Kiryl Shutsemau / Kirill A. Shutemov