On Wed, Jan 18, 2023, Kirill A. Shutemov wrote:
On Wed, Jan 18, 2023 at 01:09:49AM +0000, Sean Christopherson wrote:
On Mon, Dec 05, 2022, Vishal Annapurve wrote:
This series implements selftests targeting the feature floated by Chao via: https://lore.kernel.org/lkml/20221202061347.1070246-10-chao.p.peng@linux.int...
Below changes aim to test the fd based approach for guest private memory in context of normal (non-confidential) VMs executing on non-confidential platforms.
private_mem_test.c file adds selftest to access private memory from the guest via private/shared accesses and checking if the contents can be leaked to/accessed by vmm via shared memory view before/after conversions.
Updates in V2:
- Simplified vcpu run loop implementation API
- Removed VM creation logic from private mem library
I pushed a rework version of this series to:
git@github.com:sean-jc/linux.git x86/upm_base_support
It still has build issue with lockdep enabled that I mentioned before:
Yeah, I haven't updated the branch since last Friday, i.e. I haven't fixed the known bugs yet. I pushed the selftests changes at the same as everything else, just didn't get to typing up the emails until yesterday.
https://lore.kernel.org/all/20230116134845.vboraky2nd56szos@box.shutemov.nam...
And when compiled with lockdep, it triggers a lot of warnings about missed locks, like this:
Ah crud, I knew I was forgetting something. The lockdep assertion can simply be removed, mmu_lock doesn't need to be held to read attributes. I knew the assertion was wrong when I added it, but I wanted to remind myself to take a closer look at the usage of kvm_mem_is_private() and forgot to go back and delete the assertion.
The use of kvm_mem_is_private() in kvm_mmu_do_page_fault() is technically unsafe. Similar to getting the pfn, if mmu_lock isn't held, consuming the attributes (private vs. shared) needs MMU notifier protection, i.e. the attributes are safe to read only after mmu_invalidate_seq is snapshot.
However, is_private gets rechecked by __kvm_faultin_pfn(), which is protected by the sequence counter, and so the technically unsafe read is verified in the end. The obvious alternative is to make is_private an output of kvm_faultin_pfn(), but that's incorrect for SNP and TDX guests, in which case "is_private" is a property of the fault itself.
TL;DR: I'm going to delete the assertion and add a comment in kvm_mmu_do_page_fault() explaining why it's safe to consume kvm_mem_is_private() for "legacy" guests.
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 35a339891aed..da0afe81cf10 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -2310,8 +2310,6 @@ static inline void kvm_account_pgtable_pages(void *virt, int nr) #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn) { - lockdep_assert_held(kvm->mmu_lock); - return xa_to_value(xa_load(&kvm->mem_attr_array, gfn)); }