search for: dev_private_owner

If system memory is migrated to device private memory and no GPU MMU page table entry exists, the GPU will fault and call hmm_range_fault() to get the PFN for the page. Since the .dev_private_owner pointer in struct hmm_range is not set, hmm_range_fault returns an error which results in the GPU program stopping with a fatal fault. Fix this by setting .dev_private_owner appropriately. Fixes: 08ddddda667b ("mm/hmm: check the device private page owner in hmm_range_fault()") Signed-off...

If system memory is migrated to device private memory and no GPU MMU page table entry exists, the GPU will fault and call hmm_range_fault() to get the PFN for the page. Since the .dev_private_owner pointer in struct hmm_range is not set, hmm_range_fault returns an error which results in the GPU program stopping with a fatal fault. Fix this by setting .dev_private_owner appropriately. Fixes: 08ddddda667b ("mm/hmm: check the device private page owner in hmm_range_fault()") Cc: stable...

...k3s_hv_uvmem.c index 79b1202b1c62..29ed52892d31 100644 --- a/arch/powerpc/kvm/book3s_hv_uvmem.c +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c @@ -386,6 +386,7 @@ kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start, mig.end = end; mig.src = &src_pfn; mig.dst = &dst_pfn; + mig.dev_private_owner = &kvmppc_uvmem_pgmap; /* * We come here with mmap_sem write lock held just for @@ -563,6 +564,7 @@ kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned long start, mig.end = end; mig.src = &src_pfn; mig.dst = &dst_pfn; + mig.dev_private_owner = &kvmppc_uvmem_pgmap;...

...PFN_DEVICE_PRIVATE seems reasonable to me since a driver can > > look at the struct page but what if a driver needs to fault in a page from > > another device's private memory? Should it call handle_mm_fault()? > > Isn't that what this series basically does? > > The dev_private_owner is set to the type of pgmap the device knows how > to handle, and everything else is automatically faulted for the > device. > > If the device does not know how to handle device_private then it sets > dev_private_owner to NULL and it never gets device_private pfns. > > Since...

On Wed, Mar 18, 2020 at 09:28:49PM -0300, Jason Gunthorpe wrote: > > Changes since v1: > > - split out the pgmap->owner addition into a separate patch > > - check pgmap->owner is set for device private mappings > > - rename the dev_private_owner field in struct migrate_vma to src_owner > > - refuse to migrate private pages if src_owner is not set > > - keep the non-fault device private handling in hmm_range_fault > > I'm happy enough to take this, did you have plans for a v3? I think the only open question is if m...

...m.h +++ b/include/linux/hmm.h @@ -132,6 +132,7 @@ enum hmm_pfn_value_e { * @pfn_flags_mask: allows to mask pfn flags so that only default_flags matter * @pfn_shifts: pfn shift value (should be <= PAGE_SHIFT) * @valid: pfns array did not change since it has been fill by an HMM function + * @dev_private_owner: owner of device private pages */ struct hmm_range { struct mmu_interval_notifier *notifier; @@ -144,6 +145,7 @@ struct hmm_range { uint64_t default_flags; uint64_t pfn_flags_mask; uint8_t pfn_shift; + void *dev_private_owner; }; /* diff --git a/mm/hmm.c b/mm/hmm.c index cfad65...

When acting on device private mappings a driver needs to know if the device (or other entity in case of kvmppc) actually owns this private mapping. This series adds an owner field and converts the migrate_vma code over to check it. I looked into doing the same for hmm_range_fault, but as far as I can tell that code has never been wired up to actually work for device private memory, so instead of

On 3/16/20 12:32 PM, Christoph Hellwig wrote: > Remove the code to fault device private pages back into system memory > that has never been used by any driver. Also replace the usage of the > HMM_PFN_DEVICE_PRIVATE flag in the pfns array with a simple > is_device_private_page check in nouveau. > > Signed-off-by: Christoph Hellwig <hch at lst.de> Getting rid of

...the code. We can add it back once we have a working and fully tested code, and then should pass the expected owner in the hmm_range structure. Changes since v1: - split out the pgmap->owner addition into a separate patch - check pgmap->owner is set for device private mappings - rename the dev_private_owner field in struct migrate_vma to src_owner - refuse to migrate private pages if src_owner is not set - keep the non-fault device private handling in hmm_range_fault

...gt; > Getting rid of HMM_PFN_DEVICE_PRIVATE seems reasonable to me since a driver can > look at the struct page but what if a driver needs to fault in a page from > another device's private memory? Should it call handle_mm_fault()? Isn't that what this series basically does? The dev_private_owner is set to the type of pgmap the device knows how to handle, and everything else is automatically faulted for the device. If the device does not know how to handle device_private then it sets dev_private_owner to NULL and it never gets device_private pfns. Since the device_private pfn cannot be dm...

...nable to me since a driver can > > > look at the struct page but what if a driver needs to fault in a page from > > > another device's private memory? Should it call handle_mm_fault()? > > > > Isn't that what this series basically does? > > > > The dev_private_owner is set to the type of pgmap the device knows how > > to handle, and everything else is automatically faulted for the > > device. > > > > If the device does not know how to handle device_private then it sets > > dev_private_owner to NULL and it never gets device_privat...

...m_flags[HMM_PFN_VALID] | >> + dmirror_hmm_flags[HMM_PFN_WRITE]), > > Since pfns is not initialized pfn_flags_mask should be 0. Good point. >> + .default_flags = dmirror_hmm_flags[HMM_PFN_VALID] | >> + (write ? dmirror_hmm_flags[HMM_PFN_WRITE] : 0), >> + .dev_private_owner = dmirror->mdevice, >> + }; >> + int ret = 0; > >> +static int dmirror_snapshot(struct dmirror *dmirror, >> + struct hmm_dmirror_cmd *cmd) >> +{ >> + struct mm_struct *mm = dmirror->mm; >> + unsigned long start, end; >> + unsigned long...

...> > > > look at the struct page but what if a driver needs to fault in a page from > > > > another device's private memory? Should it call handle_mm_fault()? > > > > > > Isn't that what this series basically does? > > > > > > The dev_private_owner is set to the type of pgmap the device knows how > > > to handle, and everything else is automatically faulted for the > > > device. > > > > > > If the device does not know how to handle device_private then it sets > > > dev_private_owner to NULL and it...

On 3/17/20 5:59 AM, Christoph Hellwig wrote: > On Tue, Mar 17, 2020 at 09:47:55AM -0300, Jason Gunthorpe wrote: >> I've been using v7 of Ralph's tester and it is working well - it has >> DEVICE_PRIVATE support so I think it can test this flow too. Ralph are >> you able? >> >> This hunk seems trivial enough to me, can we include it now? > > I can send

...tifier.h> /* - * hmm_pfn_flag_e - HMM flag enums + * On output: + * 0 - The page is faultable and a future call with + * HMM_PFN_REQ_FAULT could succeed. + * HMM_PFN_VALID - the pfn field points to a valid PFN. This PFN is at + * least readable. If dev_private_owner is !NULL then this could + * point at a DEVICE_PRIVATE page. + * HMM_PFN_WRITE - if the page memory can be written to (requires HMM_PFN_VALID) + * HMM_PFN_ERROR - accessing the pfn is impossible and the device should + * fail. ie poisoned memory, special pages, no vm...

...lags, - .values = dmirror_hmm_values, - .pfn_shift = DPT_SHIFT, + .hmm_pfns = pfns, .pfn_flags_mask = 0, - .default_flags = dmirror_hmm_flags[HMM_PFN_VALID] | - (write ? dmirror_hmm_flags[HMM_PFN_WRITE] : 0), + .default_flags = + HMM_PFN_REQ_FAULT | (write ? HMM_PFN_REQ_WRITE : 0), .dev_private_owner = dmirror->mdevice, }; int ret = 0; @@ -754,19 +734,20 @@ static int dmirror_migrate(struct dmirror *dmirror, } static void dmirror_mkentry(struct dmirror *dmirror, struct hmm_range *range, - unsigned char *perm, uint64_t entry) + unsigned char *perm, unsigned long entry) {...

...tifier.h> /* - * hmm_pfn_flag_e - HMM flag enums + * On output: + * 0 - The page is faultable and a future call with + * HMM_PFN_REQ_FAULT could succeed. + * HMM_PFN_VALID - the pfn field points to a valid PFN. This PFN is at + * least readable. If dev_private_owner is !NULL then this could + * point at a DEVICE_PRIVATE page. + * HMM_PFN_WRITE - if the page memory can be written to (requires HMM_PFN_VALID) + * HMM_PFN_ERROR - accessing the pfn is impossible and the device should + * fail. ie poisoned memory, special pages, no vm...

...vate + * page, then leave it as is, otherwise fault it. + */ + hmm_vma_walk->pgmap = get_dev_pagemap( + device_private_entry_to_pfn(entry), + hmm_vma_walk->pgmap); + if (!unlikely(!pgmap)) + return -EBUSY; + if (hmm_vma_walk->pgmap->owner != + hmm_vma_walk->dev_private_owner) + cpu_flags = 0; + hmm_pte_need_fault(hmm_vma_walk, orig_pfn, cpu_flags, &fault, &write_fault); if (fault || write_fault) Jason

...nce we have a working and fully tested code, and > then should pass the expected owner in the hmm_range structure. > > Changes since v1: > - split out the pgmap->owner addition into a separate patch > - check pgmap->owner is set for device private mappings > - rename the dev_private_owner field in struct migrate_vma to src_owner > - refuse to migrate private pages if src_owner is not set > - keep the non-fault device private handling in hmm_range_fault I'm happy enough to take this, did you have plans for a v3? Thanks, Jason

...> + .pfn_flags_mask = ~(dmirror_hmm_flags[HMM_PFN_VALID] | > + dmirror_hmm_flags[HMM_PFN_WRITE]), Since pfns is not initialized pfn_flags_mask should be 0. > + .default_flags = dmirror_hmm_flags[HMM_PFN_VALID] | > + (write ? dmirror_hmm_flags[HMM_PFN_WRITE] : 0), > + .dev_private_owner = dmirror->mdevice, > + }; > + int ret = 0; > +static int dmirror_snapshot(struct dmirror *dmirror, > + struct hmm_dmirror_cmd *cmd) > +{ > + struct mm_struct *mm = dmirror->mm; > + unsigned long start, end; > + unsigned long size = cmd->npages << PAGE_...