On Mon, Nov 04, 2019 at 09:47:55AM +0530, Bharata B Rao wrote: > A secure guest will share some of its pages with hypervisor (Eg. virtio > bounce buffers etc). Support sharing of pages between hypervisor and > ultravisor. > > Shared page is reachable via both HV and UV side page tables. Once a > secure page is converted to shared page, the device page that represents > the secure page is unmapped from the HV side page tables. I'd like to understand a little better what's going on - see below... > +/* > + * Shares the page with HV, thus making it a normal page. > + * > + * - If the page is already secure, then provision a new page and share > + * - If the page is a normal page, share the existing page > + * > + * In the former case, uses dev_pagemap_ops.migrate_to_ram handler > + * to unmap the device page from QEMU's page tables. > + */ > +static unsigned long > +kvmppc_share_page(struct kvm *kvm, unsigned long gpa, unsigned long page_shift) > +{ > + > + int ret = H_PARAMETER; > + struct page *uvmem_page; > + struct kvmppc_uvmem_page_pvt *pvt; > + unsigned long pfn; > + unsigned long gfn = gpa >> page_shift; > + int srcu_idx; > + unsigned long uvmem_pfn; > + > + srcu_idx = srcu_read_lock(&kvm->srcu); > + mutex_lock(&kvm->arch.uvmem_lock); > + if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, &uvmem_pfn)) { > + uvmem_page = pfn_to_page(uvmem_pfn); > + pvt = uvmem_page->zone_device_data; > + pvt->skip_page_out = true; > + } > + > +retry: > + mutex_unlock(&kvm->arch.uvmem_lock); > + pfn = gfn_to_pfn(kvm, gfn); At this point, pfn is the value obtained from the page table for userspace (e.g. QEMU), right? I would think it should be equal to uvmem_pfn in most cases, shouldn't it? If not, what is it going to be? > + if (is_error_noslot_pfn(pfn)) > + goto out; > + > + mutex_lock(&kvm->arch.uvmem_lock); > + if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, &uvmem_pfn)) { > + uvmem_page = pfn_to_page(uvmem_pfn); > + pvt = uvmem_page->zone_device_data; > + pvt->skip_page_out = true; > + kvm_release_pfn_clean(pfn); This is going to do a put_page(), unless pfn is a reserved pfn. If it does a put_page(), where did we do the corresponding get_page()? However, since kvmppc_gfn_is_uvmem_pfn() returned true, doesn't that mean that pfn here should be a device pfn, and in fact should be the same as uvmem_pfn (possibly with some extra bit(s) set)? What does kvm_is_reserved_pfn() return for a device pfn? Paul.