On Thu, Jul 23, 2020 at 10:18:30AM +0530, Bharata B Rao wrote: > On Fri, Jul 17, 2020 at 01:00:24AM -0700, Ram Pai wrote: > > pvt->gpa = gpa; ..snip.. > > pvt->kvm = kvm; > > @@ -524,6 +663,7 @@ static unsigned long kvmppc_share_page(struct kvm *kvm, unsigned long gpa, > > uvmem_page = pfn_to_page(uvmem_pfn); > > pvt = uvmem_page->zone_device_data; > > pvt->skip_page_out = true; > > + pvt->remove_gfn = false; > > } > > > > retry: > > @@ -537,12 +677,16 @@ static unsigned long kvmppc_share_page(struct kvm *kvm, unsigned long gpa, > > uvmem_page = pfn_to_page(uvmem_pfn); > > pvt = uvmem_page->zone_device_data; > > pvt->skip_page_out = true; > > + pvt->remove_gfn = false; > > This is the case of making an already secure page as shared page. > A comment here as to why remove_gfn is set to false here will help. > > Also isn't it by default false? Is there a situation where it starts > out by default false, becomes true later and you are required to > explicitly mark it false here? It is by default false. And will be true when the GFN is released/invalidated through kvmppc_uvmem_drop_pages(). It is marked false explicitly here, just to be safe, and protect against any implicit changes. > > Otherwise, Reviewed-by: Bharata B Rao <bharata@xxxxxxxxxxxxx> > Thanks for the review. RP