<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> True. Thank you for spotting that. <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">MMAP-related code was added after I inititally drafted this change earlier this year.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">Regards,</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">Yong</div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Kuehling, Felix <Felix.Kuehling@amd.com> Sent: Thursday, November 7, 2019 12:05 AM To: Zhao, Yong <Yong.Zhao@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Subject: Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">On 2019-11-05 18:18, Zhao, Yong wrote: > The new code uses straightforward bit shifts and thus has better > readability. You're missing the MMAP-related code for mmio remapping. In kfd_ioctl_alloc_memory_of_gpu: /* MMIO is mapped through kfd device * Generate a kfd mmap offset */ if (flags & KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP) { args->mmap_offset = KFD_MMAP_TYPE_MMIO | KFD_MMAP_GPU_ID(args->gpu_id); args->mmap_offset <<= PAGE_SHIFT; } Regards, Felix > > Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373 > Signed-off-by: Yong Zhao <Yong.Zhao@amd.com> > --- > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 10 ++++------ > drivers/gpu/drm/amd/amdkfd/kfd_events.c | 1 - > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++------ > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 3 +-- > 4 files changed, 8 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > index b91993753b82..34078df36621 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > @@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file > *filep, struct kfd_process *p, > /* Return gpu_id as doorbell offset for mmap usage */ > args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL; > args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id); > - args->doorbell_offset <<= PAGE_SHIFT; > if (KFD_IS_SOC15(dev->device_info->asic_family)) > /* On SOC15 ASICs, include the doorbell offset within the > * process doorbell frame, which could be 1 page or 2 pages. > @@ -1938,20 +1937,19 @@ static int kfd_mmap(struct file *filp, struct > vm_area_struct *vma) > { > struct kfd_process *process; > struct kfd_dev *dev = NULL; > - unsigned long vm_pgoff; > + unsigned long mmap_offset; > unsigned int gpu_id; > process = kfd_get_process(current); > if (IS_ERR(process)) > return PTR_ERR(process); > - vm_pgoff = vma->vm_pgoff; > - vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff); > - gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff); > + mmap_offset = vma->vm_pgoff << PAGE_SHIFT; > + gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset); > if (gpu_id) > dev = kfd_device_by_id(gpu_id); > - switch (vm_pgoff & KFD_MMAP_TYPE_MASK) { > + switch (mmap_offset & KFD_MMAP_TYPE_MASK) { > case KFD_MMAP_TYPE_DOORBELL: > if (!dev) > return -ENODEV; > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c > b/drivers/gpu/drm/amd/amdkfd/kfd_events.c > index 908081c85de1..1f8365575b12 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c > @@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct > kfd_process *p, > ret = create_signal_event(devkfd, p, ev); > if (!ret) { > *event_page_offset = KFD_MMAP_TYPE_EVENTS; > - *event_page_offset <<= PAGE_SHIFT; > *event_slot_index = ev->event_id; > } > break; > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index 66bae8f2dad1..8eecd2cd1fd2 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -59,24 +59,21 @@ > * NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these > * defines are w.r.t to PAGE_SIZE > */ > -#define KFD_MMAP_TYPE_SHIFT (62 - PAGE_SHIFT) > +#define KFD_MMAP_TYPE_SHIFT (62) > #define KFD_MMAP_TYPE_MASK (0x3ULL << KFD_MMAP_TYPE_SHIFT) > #define KFD_MMAP_TYPE_DOORBELL (0x3ULL << KFD_MMAP_TYPE_SHIFT) > #define KFD_MMAP_TYPE_EVENTS (0x2ULL << KFD_MMAP_TYPE_SHIFT) > #define KFD_MMAP_TYPE_RESERVED_MEM (0x1ULL << KFD_MMAP_TYPE_SHIFT) > #define KFD_MMAP_TYPE_MMIO (0x0ULL << KFD_MMAP_TYPE_SHIFT) > -#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT) > +#define KFD_MMAP_GPU_ID_SHIFT (46) > #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \ > << KFD_MMAP_GPU_ID_SHIFT) > #define KFD_MMAP_GPU_ID(gpu_id) ((((uint64_t)gpu_id) << > KFD_MMAP_GPU_ID_SHIFT)\ > & KFD_MMAP_GPU_ID_MASK) > -#define KFD_MMAP_GPU_ID_GET(offset) ((offset & KFD_MMAP_GPU_ID_MASK) \ > +#define KFD_MMAP_GET_GPU_ID(offset) ((offset & KFD_MMAP_GPU_ID_MASK) \ > >> KFD_MMAP_GPU_ID_SHIFT) > -#define KFD_MMAP_OFFSET_VALUE_MASK (0x3FFFFFFFFFFFULL >> PAGE_SHIFT) > -#define KFD_MMAP_OFFSET_VALUE_GET(offset) (offset & > KFD_MMAP_OFFSET_VALUE_MASK) > - > /* > * When working with cp scheduler we should assign the HIQ manually or via > * the amdgpu driver to a fixed hqd slot, here are the fixed HIQ hqd slot > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c > b/drivers/gpu/drm/amd/amdkfd/kfd_process.c > index 6abfb77ae540..39dc49b8fd85 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c > @@ -554,8 +554,7 @@ static int kfd_process_init_cwsr_apu(struct > kfd_process *p, struct file *filep) > if (!dev->cwsr_enabled || qpd->cwsr_kaddr || qpd->cwsr_base) > continue; > - offset = (KFD_MMAP_TYPE_RESERVED_MEM | KFD_MMAP_GPU_ID(dev->id)) > - << PAGE_SHIFT; > + offset = KFD_MMAP_TYPE_RESERVED_MEM | KFD_MMAP_GPU_ID(dev->id); > qpd->tba_addr = (int64_t)vm_mmap(filep, 0, > KFD_CWSR_TBA_TMA_SIZE, PROT_READ | PROT_EXEC, > MAP_SHARED, offset); </div> </div> </body> </html>