[PATCH 3/6] drm/amdgpu: IOCTL interface for PRT support v3

Bas Nieuwenhuizen bas at basnieuwenhuizen.nl
Sat Feb 4 20:11:01 UTC 2017



On Sat, Feb 4, 2017, at 20:14, Bas Nieuwenhuizen wrote:
> I get an error when trying to map a PRT region:
> 
> [   41.588224] BUG: unable to handle kernel NULL pointer dereference at
> 00000000000001e8
> [   41.589899] IP: [<ffffffffa0578fa6>]
> ttm_eu_reserve_buffers+0x136/0x370 [ttm]
> [   41.590424] PGD 0 
> 
> [   41.590943] Oops: 0000 [#1] PREEMPT SMP
> [   41.591468] Modules linked in: uvcvideo videobuf2_vmalloc
> videobuf2_memops videobuf2_v4l2 videobuf2_core snd_usb_audio videodev
> snd_usbmidi_lib snd_rawmidi media snd_seq_device nct6775 hwmon_vid
> cdc_acm ext4 crc16 jbd2 fscrypto mbcache nls_iso8859_1 intel_rapl
> x86_pkg_temp_thermal nls_cp437 intel_powerclamp vfat coretemp fat
> kvm_intel kvm irqbypass iTCO_wdt crct10dif_pclmul amdkfd crc32_pclmul
> ghash_clmulni_intel amd_iommu_v2 iTCO_vendor_support amdgpu radeon
> mei_wdt aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper
> cryptd ttm drm_kms_helper intel_cstate intel_rapl_perf psmouse pcspkr
> drm syscopyarea sysfillrect input_leds sysimgblt i2c_i801 evdev alx
> joydev led_class mousedev fb_sys_fops lpc_ich i2c_smbus mac_hid mdio
> i2c_algo_bit snd_hda_codec_realtek snd_hda_codec_generic
> snd_hda_codec_hdmi
> [   41.593415]  battery snd_hda_intel fjes nuvoton_cir snd_hda_codec
> rc_core video soc_button_array snd_hda_core snd_hwdep mei_me button
> snd_pcm mei snd_timer snd tpm_tis shpchp tpm_tis_core soundcore tpm
> sch_fq_codel fuse ip_tables x_tables btrfs xor raid6_pq hid_generic
> usbhid hid sd_mod serio_raw atkbd libps2 xhci_pci ahci libahci ehci_pci
> xhci_hcd ehci_hcd libata crc32c_intel scsi_mod usbcore usb_common i8042
> serio
> [   41.595473] CPU: 1 PID: 469 Comm: deqp-vk Not tainted 4.9.0-amdgpu+
> #2
> [   41.596148] Hardware name: To Be Filled By O.E.M. To Be Filled By
> O.E.M./B85 Killer, BIOS P1.50 07/11/2014
> [   41.596829] task: ffff880429280000 task.stack: ffffc9000a5cc000
> [   41.597517] RIP: 0010:[<ffffffffa0578fa6>]  [<ffffffffa0578fa6>]
> ttm_eu_reserve_buffers+0x136/0x370 [ttm]
> [   41.598220] RSP: 0018:ffffc9000a5cfb00  EFLAGS: 00010286
> [   41.598978] RAX: 0000000000000000 RBX: ffffc9000a5cfb68 RCX:
> ffffc9000a5cfb78
> [   41.599760] RDX: ffff88040c415d00 RSI: ffffc9000a5cfb88 RDI:
> 0000000000000000
> [   41.600558] RBP: ffffc9000a5cfb50 R08: 0000000000000004 R09:
> 00000000001000a0
> [   41.601348] R10: 00000000fffffff2 R11: ffff880429280000 R12:
> 0000000000000000
> [   41.602154] R13: 0000000000000058 R14: ffff880410ee76c0 R15:
> ffffc9000a5cfba0
> [   41.602926] FS:  00007f75deecb780(0000) GS:ffff88043dc80000(0000)
> knlGS:0000000000000000
> [   41.603690] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   41.604447] CR2: 00000000000001e8 CR3: 000000040e64a000 CR4:
> 00000000001406e0
> [   41.605205] Stack:
> [   41.605972]  ffff88042982a600 ffffc9000a5cfb78 01000000fffffe00
> ffffc9000a5cfba0
> [   41.606766]  ffffc9000a5cfb88 ffff88041d1f0000 0000000000000001
> ffffc9000a5cfb68
> [   41.607582]  ffff880410ee76c0 ffffc9000a5cfb78 ffffc9000a5cfc40
> ffffffffa0808e38
> [   41.608408] Call Trace:
> [   41.609248]  [<ffffffffa0808e38>] amdgpu_gem_va_update_vm+0xb8/0x1c0
> [amdgpu]
> [   41.610091]  [<ffffffff8135c701>] ? interval_tree_iter_next+0x21/0x70
> [   41.610946]  [<ffffffffa080a1b7>] amdgpu_gem_va_ioctl+0x2b7/0x350
> [amdgpu]
> [   41.611773]  [<ffffffff8136b67b>] ?
> __percpu_counter_compare+0x3b/0x90
> [   41.612578]  [<ffffffffa021ce01>] ?
> __btrfs_btree_balance_dirty+0x41/0x80 [btrfs]
> [   41.613409]  [<ffffffffa04b4277>] drm_ioctl+0x227/0x4c0 [drm]
> [   41.614342]  [<ffffffffa0809f00>] ?
> amdgpu_gem_metadata_ioctl+0x1e0/0x1e0 [amdgpu]
> [   41.615166]  [<ffffffffa02403ff>] ? btrfs_file_write_iter+0x1df/0x560
> [btrfs]
> [   41.615989]  [<ffffffff814446df>] ? tty_write+0x1df/0x300
> [   41.616862]  [<ffffffffa07ed062>] amdgpu_drm_ioctl+0x62/0xa0 [amdgpu]
> [   41.617738]  [<ffffffff8124b612>] do_vfs_ioctl+0xb2/0x600
> [   41.618601]  [<ffffffff812370cb>] ? vfs_write+0x14b/0x1b0
> [   41.619448]  [<ffffffff8124bbe8>] SyS_ioctl+0x88/0xa0
> [   41.620311]  [<ffffffff81679cb7>] entry_SYSCALL_64_fastpath+0x1a/0xa9
> [   41.621212] Code: 0a 48 89 71 08 49 89 0f 49 89 57 08 48 89 32 49 89
> c7 4d 8b 3f 4c 39 fb 4c 89 7d c8 0f 84 67 01 00 00 48 83 7d d0 00 4d 8b
> 6f 10 <49> 8b bd 90 01 00 00 0f 84 a7 00 00 00 80 7d c7 00 48 8b 75 d0 
> [   41.622165] RIP  [<ffffffffa0578fa6>]
> ttm_eu_reserve_buffers+0x136/0x370 [ttm]
> [   41.623138]  RSP <ffffc9000a5cfb00>
> [   41.624017] CR2: 00000000000001e8
> [   41.630632] ---[ end trace f1fc48bc614df131 ]---
> 
> However, when I specify AMDGPU_VM_DELAY_UPDATE too, it is gone, and
> seems to work.

Scratch that, using AMDGPU_VM_DELAY_UPDATE I get a VM fault, and the
warning from gmc_v8_0_set_prt is never printed. 

> 
> (side note: when is that flag useful and when not? I was under the
> impression that map/unmap operations always waited until the last CS
> submitted before the op was complete. In what case would it be useful to
> map/unmap earlier than next CS?)
> 
> - Bas
> 
> On Thu, Feb 2, 2017, at 11:25, Christian König wrote:
> > From: Junwei Zhang <Jerry.Zhang at amd.com>
> > 
> > Till GFX8 we can only enable PRT support globally, but with the next
> > hardware
> > generation we can do this on a per page basis.
> > 
> > Keep the interface consistent by adding PRT mappings and enable
> > support globally on current hardware when the first mapping is made.
> > 
> > v2: disable PRT support delayed and on all error paths
> > v3: PRT and other permissions are mutal exclusive,
> >     PRT mappings don't need a BO.
> > 
> > Signed-off-by: Junwei Zhang <Jerry.Zhang at amd.com>
> > Signed-off-by: Christian König <christian.koenig at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 62
> >  ++++++++++++++++++++-------------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 ++++++
> >  include/uapi/drm/amdgpu_drm.h           |  2 ++
> >  4 files changed, 51 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 34a971a..99ca5e8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -703,6 +703,7 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr);
> >  
> >  struct amdgpu_fpriv {
> >  	struct amdgpu_vm	vm;
> > +       struct amdgpu_bo_va     *prt_va;
> >  	struct mutex		bo_list_lock;
> >  	struct idr		bo_list_handles;
> >  	struct amdgpu_ctx_mgr	ctx_mgr;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > index 1dc59aa..f3e9051 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > @@ -540,6 +540,12 @@ static void amdgpu_gem_va_update_vm(struct
> > amdgpu_device *adev,
> >  int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
> >  			  struct drm_file *filp)
> >  {
> > +       const uint32_t valid_flags = AMDGPU_VM_DELAY_UPDATE |
> > +               AMDGPU_VM_PAGE_READABLE | AMDGPU_VM_PAGE_WRITEABLE |
> > +               AMDGPU_VM_PAGE_EXECUTABLE;
> > +       const uint32_t prt_flags = AMDGPU_VM_DELAY_UPDATE |
> > +               AMDGPU_VM_PAGE_PRT;
> > +
> >  	struct drm_amdgpu_gem_va *args = data;
> >  	struct drm_gem_object *gobj;
> >  	struct amdgpu_device *adev = dev->dev_private;
> > @@ -550,7 +556,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void
> > *data,
> >  	struct ttm_validate_buffer tv;
> >  	struct ww_acquire_ctx ticket;
> >  	struct list_head list;
> > -       uint32_t invalid_flags, va_flags = 0;
> > +       uint32_t va_flags = 0;
> >  	int r = 0;
> >  
> >  	if (!adev->vm_manager.enabled)
> > @@ -564,11 +570,9 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void
> > *data,
> >  		return -EINVAL;
> >  	}
> >  
> > -       invalid_flags = ~(AMDGPU_VM_DELAY_UPDATE |
> > AMDGPU_VM_PAGE_READABLE |
> > -                       AMDGPU_VM_PAGE_WRITEABLE |
> > AMDGPU_VM_PAGE_EXECUTABLE);
> > -       if ((args->flags & invalid_flags)) {
> > -               dev_err(&dev->pdev->dev, "invalid flags 0x%08X vs
> > 0x%08X\n",
> > -                       args->flags, invalid_flags);
> > +       if ((args->flags & ~valid_flags) && (args->flags & ~prt_flags)) {
> > +               dev_err(&dev->pdev->dev, "invalid flags combination
> > 0x%08X\n",
> > +                       args->flags);
> >  		return -EINVAL;
> >  	}
> >  
> > @@ -582,28 +586,34 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev,
> > void *data,
> >  		return -EINVAL;
> >  	}
> >  
> > -       gobj = drm_gem_object_lookup(filp, args->handle);
> > -       if (gobj == NULL)
> > -               return -ENOENT;
> > -       abo = gem_to_amdgpu_bo(gobj);
> >  	INIT_LIST_HEAD(&list);
> > -       tv.bo = &abo->tbo;
> > -       tv.shared = false;
> > -       list_add(&tv.head, &list);
> > +       if (!(args->flags & AMDGPU_VM_PAGE_PRT)) {
> > +               gobj = drm_gem_object_lookup(filp, args->handle);
> > +               if (gobj == NULL)
> > +                       return -ENOENT;
> > +               abo = gem_to_amdgpu_bo(gobj);
> > +               tv.bo = &abo->tbo;
> > +               tv.shared = false;
> > +               list_add(&tv.head, &list);
> > +       } else {
> > +               gobj = NULL;
> > +               abo = NULL;
> > +       }
> >  
> >  	amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd);
> >  
> >  	r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
> > -       if (r) {
> > -               drm_gem_object_unreference_unlocked(gobj);
> > -               return r;
> > -       }
> > +       if (r)
> > +               goto error_unref;
> >  
> > -       bo_va = amdgpu_vm_bo_find(&fpriv->vm, abo);
> > -       if (!bo_va) {
> > -               ttm_eu_backoff_reservation(&ticket, &list);
> > -               drm_gem_object_unreference_unlocked(gobj);
> > -               return -ENOENT;
> > +       if (abo) {
> > +               bo_va = amdgpu_vm_bo_find(&fpriv->vm, abo);
> > +               if (!bo_va) {
> > +                       r = -ENOENT;
> > +                       goto error_backoff;
> > +               }
> > +       } else {
> > +               bo_va = fpriv->prt_va;
> >  	}
> >  
> >  	switch (args->operation) {
> > @@ -614,6 +624,8 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void
> > *data,
> >  			va_flags |= AMDGPU_PTE_WRITEABLE;
> >  		if (args->flags & AMDGPU_VM_PAGE_EXECUTABLE)
> >  			va_flags |= AMDGPU_PTE_EXECUTABLE;
> > +               if (args->flags & AMDGPU_VM_PAGE_PRT)
> > +                       va_flags |= AMDGPU_PTE_PRT;
> >  		r = amdgpu_vm_bo_map(adev, bo_va, args->va_address,
> >  				     args->offset_in_bo, args->map_size,
> >  				     va_flags);
> > @@ -624,11 +636,13 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev,
> > void *data,
> >  	default:
> >  		break;
> >  	}
> > -       if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) &&
> > -           !amdgpu_vm_debug)
> > +       if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) &&
> > !amdgpu_vm_debug)
> >  		amdgpu_gem_va_update_vm(adev, bo_va, &list, args->operation);
> > +
> > +error_backoff:
> >  	ttm_eu_backoff_reservation(&ticket, &list);
> >  
> > +error_unref:
> >  	drm_gem_object_unreference_unlocked(gobj);
> >  	return r;
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index 215f73b..d5f9d6a4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -656,6 +656,14 @@ int amdgpu_driver_open_kms(struct drm_device *dev,
> > struct drm_file *file_priv)
> >  		goto out_suspend;
> >  	}
> >  
> > +       fpriv->prt_va = amdgpu_vm_bo_add(adev, &fpriv->vm, NULL);
> > +       if (!fpriv->prt_va) {
> > +               r = -ENOMEM;
> > +               amdgpu_vm_fini(adev, &fpriv->vm);
> > +               kfree(fpriv);
> > +               goto out_suspend;
> > +       }
> > +
> >  	if (amdgpu_sriov_vf(adev)) {
> >  		r = amdgpu_map_static_csa(adev, &fpriv->vm);
> >  		if (r)
> > @@ -700,6 +708,8 @@ void amdgpu_driver_postclose_kms(struct drm_device
> > *dev,
> >  	amdgpu_uvd_free_handles(adev, file_priv);
> >  	amdgpu_vce_free_handles(adev, file_priv);
> >  
> > +       amdgpu_vm_bo_rmv(adev, fpriv->prt_va);
> > +
> >  	if (amdgpu_sriov_vf(adev)) {
> >  		/* TODO: how to handle reserve failure */
> >  		BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false));
> > diff --git a/include/uapi/drm/amdgpu_drm.h
> > b/include/uapi/drm/amdgpu_drm.h
> > index 2cf8df8..07e3710 100644
> > --- a/include/uapi/drm/amdgpu_drm.h
> > +++ b/include/uapi/drm/amdgpu_drm.h
> > @@ -363,6 +363,8 @@ struct drm_amdgpu_gem_op {
> >  #define AMDGPU_VM_PAGE_WRITEABLE        (1 << 2)
> >  /* executable mapping, new for VI */
> >  #define AMDGPU_VM_PAGE_EXECUTABLE       (1 << 3)
> > +/* partially resident texture */
> > +#define AMDGPU_VM_PAGE_PRT             (1 << 4)
> >  
> >  struct drm_amdgpu_gem_va {
> >  	/** GEM object handle */
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list