Messages in this thread | | | From | "Kalra, Ashish" <> | Subject | RE: [PATCH Part2 v6 26/49] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_UPDATE command | Date | Wed, 29 Jun 2022 18:14:05 +0000 |
| |
[Public]
>> +static inline void snp_leak_pages(u64 pfn, enum pg_level level) { >> + unsigned int npages = page_level_size(level) >> PAGE_SHIFT; >> + >> + WARN(1, "psc failed pfn 0x%llx pages %d (leaking)\n", pfn, >> + npages); >> + >> + while (npages) { >> + memory_failure(pfn, 0); >> + dump_rmpentry(pfn); >> + npages--; >> + pfn++; >> + } >> +}
>Should this be deduplicated with the snp_leak_pages() in "crypto: ccp: >Handle the legacy TMR allocation when SNP is enabled" ?
Yes, probably should.
>> +static bool is_hva_registered(struct kvm *kvm, hva_t hva, size_t len) >> +{ >> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; >> + struct list_head *head = &sev->regions_list; >> + struct enc_region *i; >> + >> + lockdep_assert_held(&kvm->lock); >> + >> + list_for_each_entry(i, head, list) { >> + u64 start = i->uaddr; >> + u64 end = start + i->size; >> + >> + if (start <= hva && end >= (hva + len)) >> + return true; >> + }
>Given that usersapce could load sev->regions_list with any # of any sized regions. Should we add a cond_resched() like in sev_vm_destroy()?
Actually, is_hva_registered() is also called from PSC handler with kvm->lock mutex held. Even though it is a mutex, I am not really sure if it is a good idea to do cond_resched() with the kvm->lock mutex held ?
>> +e_unpin: >> + /* Content of memory is updated, mark pages dirty */ >> + for (i = 0; i < n; i++) {
>Since |n| is not only a loop variable but actually carries the number of private pages over to e_unpin can we use a more descriptive name? >How about something like 'nprivate_pages'?
Yes.
Thanks, Ashish
| |