Should we add HMM as a dependency in the Kconfig? At least for KFD functionality we depend on userptrs. Regards,  Felix On 2018-09-12 03:15 AM, Christian König wrote: > Am 11.09.2018 um 21:31 schrieb Philip Yang: >> Replace our MMU notifier with hmm_mirror_ops.sync_cpu_device_pagetables >> callback if kernel configured > >> HMM. Kenel configured without HMM still uses >> our own MMU notifier. > > Please drop that and always use the HMM path. > > When a kernel doesn't support HMM we should not support userptr either. > > Christian. > >> >> It supports both KFD userptr and gfx userptr paths. >> >> This depends on several HMM patchset from Jérôme Glisse queued for >> upstream. See >> http://172.27.226.38/root/kernel_amd/commits/hmm-dev-v01 (for AMD >> intranet) >> >> Change-Id: Ie62c3c5e3c5b8521ab3b438d1eff2aa2a003835e >> Signed-off-by: Philip Yang <Philip.Yang at amd.com> >> --- >>  drivers/gpu/drm/amd/amdgpu/Makefile    | 1 + >>  drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c | 77 >> +++++++++++++++++++++++++++++++++ >>  drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h | 41 ++++++++++++++++++ >>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 49 ++++++++++++++++++++- >>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h | 6 +++ >>  5 files changed, 173 insertions(+), 1 deletion(-) >>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile >> b/drivers/gpu/drm/amd/amdgpu/Makefile >> index 138cb78..ee691e8 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/Makefile >> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile >> @@ -172,6 +172,7 @@ amdgpu-$(CONFIG_COMPAT) += amdgpu_ioc32.o >>  amdgpu-$(CONFIG_VGA_SWITCHEROO) += amdgpu_atpx_handler.o >>  amdgpu-$(CONFIG_ACPI) += amdgpu_acpi.o >>  amdgpu-$(CONFIG_MMU_NOTIFIER) += amdgpu_mn.o >> +amdgpu-$(CONFIG_HMM) += amdgpu_hmm.o >>   include $(FULL_AMD_PATH)/powerplay/Makefile >>  diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >> new file mode 100644 >> index 0000000..a502c11 >> --- /dev/null >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >> @@ -0,0 +1,77 @@ >> +/* >> + * Copyright 2018 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without >> limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom >> the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be >> included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO >> EVENT SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, >> DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >> OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE >> USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. >> + */ >> + >> +#include <linux/mutex.h> >> +#include <linux/slab.h> >> +#include <linux/notifier.h> >> +#include <linux/compat.h> >> +#include <linux/mman.h> >> +#include <linux/hmm.h> >> +#include <asm/page.h> >> +#include "amdgpu.h" >> +#include "amdgpu_mn.h" >> + >> +static void amdgpu_hmm_release(struct hmm_mirror *mirror) >> +{ >> +   pr_debug("mirror=%p\n", mirror); >> +} >> + >> +static int amdgpu_hmm_sync_cpu_device_pagetables(struct hmm_mirror >> *mirror, >> +           const struct hmm_update *update) >> +{ >> +   struct hmm *hmm; >> +   struct mm_struct *mm; >> +   unsigned long start; >> +   unsigned long end; >> + >> +   start = update->start; >> +   end = update->end; >> + >> +   pr_debug("mirror %p start %lx end %lx\n", mirror, start, end); >> + >> +   hmm = mirror->hmm; >> +   mm = *(struct mm_struct **)hmm; >> + >> +   return amdgpu_mn_invalidate_range(mirror, mm, start, end, >> +                   update->blockable); >> +} >> + >> +static struct hmm_mirror_ops amdgpu_hmm_mirror_ops = { >> +   .sync_cpu_device_pagetables = >> amdgpu_hmm_sync_cpu_device_pagetables, >> +   .release = amdgpu_hmm_release >> +}; >> + >> +int amdgpu_hmm_register(struct hmm_mirror *mirror, struct mm_struct >> *mm) >> +{ >> +   pr_debug("mirror=%p\n", mirror); >> + >> +   mirror->ops = &amdgpu_hmm_mirror_ops; >> + >> +   return hmm_mirror_register(mirror, mm); >> +} >> + >> +void amdgpu_hmm_unregister(struct hmm_mirror *mirror) >> +{ >> +   pr_debug("mirror=%p\n", mirror); >> + >> +   hmm_mirror_unregister(mirror); >> +} >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >> new file mode 100644 >> index 0000000..a21a5f6 >> --- /dev/null >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >> @@ -0,0 +1,41 @@ >> +/* >> + * Copyright 2018 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without >> limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom >> the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be >> included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO >> EVENT SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, >> DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >> OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE >> USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. >> + */ >> + >> +#ifndef __AMDGPU_HMM_H__ >> +#define __AMDGPU_HMM_H__ >> + >> +#include <linux/mm.h> >> +#include <linux/hmm.h> >> + >> +#if IS_ENABLED(CONFIG_HMM) >> + >> +int amdgpu_hmm_register(struct hmm_mirror *mirror, struct mm_struct >> *mm); >> +void amdgpu_hmm_unregister(struct hmm_mirror *mirror); >> + >> +#else >> + >> +#define amdgpu_hmm_register(x, y) (0) >> +#define amdgpu_hmm_unregister(x) >> + >> +#endif >> + >> +#endif >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c >> index e55508b..2b60631 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c >> @@ -47,11 +47,13 @@ >>  #include <linux/module.h> >>  #include <linux/mmu_notifier.h> >>  #include <linux/interval_tree.h> >> +#include <linux/hmm.h> >>  #include <drm/drmP.h> >>  #include <drm/drm.h> >>   #include "amdgpu.h" >>  #include "amdgpu_amdkfd.h" >> +#include "amdgpu_hmm.h" >>   /** >>   * struct amdgpu_mn >> @@ -66,6 +68,7 @@ >>   * @objects: interval tree containing amdgpu_mn_nodes >>   * @read_lock: mutex for recursive locking of @lock >>   * @recursion: depth of recursion >> + * @hmm_mirror: HMM mirror function support >>   * >>   * Data for each amdgpu device and process address space. >>   */ >> @@ -87,6 +90,11 @@ struct amdgpu_mn { >>      struct rb_root_cached   objects; >>      struct mutex       read_lock; >>      atomic_t       recursion; >> + >> +#if IS_ENABLED(CONFIG_HMM) >> +   /* HMM mirror */ >> +   struct hmm_mirror   hmm_mirror; >> +#endif >>  }; >>   /** >> @@ -130,6 +138,9 @@ static void amdgpu_mn_destroy(struct work_struct >> *work) >>      up_write(&amn->lock); >>      mutex_unlock(&adev->mn_lock); >>      mmu_notifier_unregister_no_release(&amn->mn, amn->mm); >> + >> +   amdgpu_hmm_unregister(&amn->hmm_mirror); >> + >>      kfree(amn); >>  } >>  @@ -372,6 +383,39 @@ static const struct mmu_notifier_ops >> amdgpu_mn_ops[] = { >>   */ >>  #define AMDGPU_MN_KEY(mm, type) ((unsigned long)(mm) + (type)) >>  +#if IS_ENABLED(CONFIG_HMM) >> + >> +int amdgpu_mn_invalidate_range(struct hmm_mirror *mirror, >> +               struct mm_struct *mm, >> +               unsigned long start, >> +               unsigned long end, >> +               bool blockable) >> + >> +{ >> +   unsigned long key; >> +   struct amdgpu_mn *amn; >> +   int r = 0; >> + >> +   amn = container_of(mirror, struct amdgpu_mn, hmm_mirror); >> +   key = AMDGPU_MN_KEY(mm, amn->type); >> + >> +   hash_for_each_possible(amn->adev->mn_hash, amn, node, key) >> +       if (AMDGPU_MN_KEY(amn->mm, amn->type) == key) { >> +           r = amn->mn.ops->invalidate_range_start(&amn->mn, mm, >> +                           start, end, blockable); >> +           amn->mn.ops->invalidate_range_end(&amn->mn, mm, >> +                           start, end); >> +           if (r) { >> +               DRM_ERROR("failed to invalidate %lx\n", start); >> +               break; >> +           } >> +       } >> + >> +   return r; >> +} >> + >> +#endif >> + >>  /** >>   * amdgpu_mn_get - create notifier context >>   * >> @@ -413,7 +457,10 @@ struct amdgpu_mn *amdgpu_mn_get(struct >> amdgpu_device *adev, >>      mutex_init(&amn->read_lock); >>      atomic_set(&amn->recursion, 0); >>  -   r = __mmu_notifier_register(&amn->mn, mm); >> +   if (IS_ENABLED(CONFIG_HMM)) >> +       r = amdgpu_hmm_register(&amn->hmm_mirror, mm); >> +   else >> +       r = __mmu_notifier_register(&amn->mn, mm); >>      if (r) >>          goto free_amn; >>  diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h >> index eb0f432..c628add 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h >> @@ -28,6 +28,7 @@ >>   * MMU Notifier >>   */ >>  struct amdgpu_mn; >> +struct hmm_mirror; >>   enum amdgpu_mn_type { >>      AMDGPU_MN_TYPE_GFX, >> @@ -41,6 +42,11 @@ struct amdgpu_mn *amdgpu_mn_get(struct >> amdgpu_device *adev, >>                  enum amdgpu_mn_type type); >>  int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr); >>  void amdgpu_mn_unregister(struct amdgpu_bo *bo); >> +int amdgpu_mn_invalidate_range(struct hmm_mirror *mirror, >> +               struct mm_struct *mm, >> +               unsigned long start, >> +               unsigned long end, >> +               bool blockable); >>  #else >>  static inline void amdgpu_mn_lock(struct amdgpu_mn *mn) {} >>  static inline void amdgpu_mn_unlock(struct amdgpu_mn *mn) {} > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx