[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
