- 06 Nov, 2017 4 commits
-
-
Chris Wilson authored
Silence smatch by demonstrating that guc->stage_desc_pool is allocated following a successful guc_stage_desc_pool_create(), drivers/gpu/drm/i915/i915_guc_submission.c:1293 i915_guc_submission_init() error: we previously assumed 'guc->stage_desc_pool' could be null (see line 1261) Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Oscar Mateo <oscar.mateo@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171106114833.31199-1-chris@chris-wilson.co.ukReviewed-by:
Michal Wajdeczko <michal.wajdeczko@intel.com>
-
Chris Wilson authored
An oversight in commit 87701b4b ("drm/i915: Only free the oldest stale object before a fresh allocation") was that not only do we have to serialise concurrent users of llist_del_first(), but we also have to lock llist_del_first() vs llist_del_all(). From llist.h, * This can be summarized as follows: * * | add | del_first | del_all * add | - | - | - * del_first | | L | L * del_all | | | - * * Where, a particular row's operation can happen concurrently with a column's * operation, with "-" being no lock needed, while "L" being lock is needed. This should hopefully explain: <4>[ 89.287106] general protection fault: 0000 [#1] PREEMPT SMP <4>[ 89.287126] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic x86_pkg_temp_thermal intel_powerclamp coretemp i915 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core r8169 mii mei_me mei snd_pcm prime_numbers i2c_hid pinctrl_geminilake pinctrl_intel <4>[ 89.287226] CPU: 2 PID: 23 Comm: ksoftirqd/2 Tainted: G U 4.14.0-rc8-CI-CI_DRM_3315+ #1 <4>[ 89.287247] Hardware name: Intel Corp. Geminilake/GLK RVP2 LP4SD (07), BIOS GELKRVPA.X64.0062.B30.1708222146 08/22/2017 <4>[ 89.287270] task: ffff88017ab34ec0 task.stack: ffffc90000128000 <4>[ 89.287290] RIP: 0010:llist_add_batch+0x4/0x20 <4>[ 89.287301] RSP: 0018:ffffc9000012bdb8 EFLAGS: 00010296 <4>[ 89.287314] RAX: ffffffff811017ad RBX: 6e468801a1560000 RCX: ef3e53fceecdeb81 <4>[ 89.287330] RDX: 6e468801a1566130 RSI: ffff880103d73d98 RDI: ffff880103d73d98 <4>[ 89.287346] RBP: ffffc9000012bdb8 R08: ffff88017ab35780 R09: 0000000000000000 <4>[ 89.287361] R10: ffffc9000012bd68 R11: 00000000abb18c3d R12: ffffffffa01369e0 <4>[ 89.287377] R13: ffff88017fd1b8f8 R14: ffff88017ab34ec0 R15: 000000000000000a <4>[ 89.287393] FS: 0000000000000000(0000) GS:ffff88017fd00000(0000) knlGS:0000000000000000 <4>[ 89.287411] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4>[ 89.287424] CR2: 00007ff0c0755018 CR3: 000000016df9b000 CR4: 00000000003406e0 <4>[ 89.287440] Call Trace: <4>[ 89.287511] __i915_gem_free_object_rcu+0x20/0x40 [i915] <4>[ 89.287527] rcu_process_callbacks+0x27a/0x730 <4>[ 89.287544] __do_softirq+0xc0/0x4ae <4>[ 89.287559] ? smpboot_thread_fn+0x2d/0x280 <4>[ 89.287571] run_ksoftirqd+0x1f/0x70 <4>[ 89.287582] smpboot_thread_fn+0x18a/0x280 <4>[ 89.287595] kthread+0x114/0x150 <4>[ 89.287605] ? sort_range+0x30/0x30 <4>[ 89.287615] ? kthread_create_on_node+0x40/0x40 <4>[ 89.287628] ret_from_fork+0x27/0x40 <4>[ 89.287641] Code: 0d 48 83 ea 01 4c 89 c1 48 83 fa ff 74 12 48 23 0c d7 74 ed 48 c1 e2 06 48 0f bd c9 48 8d 04 0a 5d c3 90 90 90 90 90 55 48 89 e5 <48> 8b 0a 48 89 0e 48 89 c8 f0 48 0f b1 3a 48 39 c1 75 ed 48 85 <1>[ 89.287774] RIP: llist_add_batch+0x4/0x20 RSP: ffffc9000012bdb8 <4>[ 89.287826] ---[ end trace e775d15174d8ae02 ]--- (Lockless lists are only easy (and lockless) when only using llist_add/llist_del_all!) Fixes: 87701b4b ("drm/i915: Only free the oldest stale object before a fresh allocation") Signed-off-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com>
-
Chris Wilson authored
Some tests are designed to exercise the limits of the HW and may trigger unintended side-effects making the machine unusable. This should not be executed by default, but are still useful for early platform validation. References: https://bugs.freedesktop.org/show_bug.cgi?id=103453Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
-
Chris Wilson authored
As we bind, and unbind on error, we want to be sure that the vma->flags are updated to reflect the binding state so that on the next invocation all is well. v2: Take two. v3: Take three; vma-misplaced is checking map-and-fenceable so keep it last! Signed-off-by:
Matthew Auld <matthew.william.auld@gmail.com>
-
- 03 Nov, 2017 3 commits
-
-
Chris Wilson authored
After a reset, we may immediately begin executing requests on restarting the engines. Ergo this has to be last step with all re-initialisation completed beforehand. The mocs setup was after we started executing the requests; do it earlier! Signed-off-by:
-
Tvrtko Ursulin authored
GEM_BUG_ON if the packed bits do not fit into the specified width. v2: Avoid using the macro argument twice. v3: Drop unnecessary braces. (Joonas) Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1) Reviewed-by:
-
Tvrtko Ursulin authored
We have to reject unknown flags for uAPI considerations, and also because the curent implementation limits their i915 storage space to two bits. v2: (Chris Wilson) * Fix fail in ABI check. * Added unknown flags and BUILD_BUG_ON. v3: * Use ARCH_KMALLOC_MINALIGN instead of alignof. (Chris Wilson) Signed-off-by:
-
- 02 Nov, 2017 6 commits
-
-
Lionel Landwerlin authored
Because dev_priv is 0-ed it's not currently an issue, but since we have dev_priv->perf.oa.test_config.uuid size at uuid + 1, we could just copy the null character. v2: Use strlcpy instead of strncpy (Chris) Signed-off-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
-
Chris Wilson authored
Before we assert that the engine is idle, make sure we flush any residual tasklet. After that point, if the engine is not idle, more work may be queued despite us trying to park the engine and go to sleep. References: https://bugs.freedesktop.org/show_bug.cgi?id=103479Signed-off-by:
-
Mika Kuoppala authored
There is a possibility on gen9 hardware to miss the forcewake ack message. The recommended workaround is to use another free bit and toggle it until original bit is successfully acknowledged. Some future gen9 revs might or might not fix the underlying issue but using fallback forcewake bit dance can be considered as harmless: without the ack timeout we never reach the fallback bit forcewake. Thus as of now we adopt a blanket approach for all gen9 and leave the bypassing the fallback bit approach for future patches if corresponding hw revisions do appear. Commit 83e33372 ("drm/i915: Increase maximum polling time to 50ms for forcewake request/clear ack") did increase the forcewake timeout. If the issue was a delayed ack, future work could include finding a suitable timeout value both for primary ack and reserve toggle to reduce the worst case latency. v2: use bit 15, naming, comment (Chris), only wait fallback ack v3: fix return on fallback, backoff after fallback write (Chris) v4: udelay on first pass, grammar (Chris) v4: s/reserve/fallback References: HSDES #1604254524 References: https://bugs.freedesktop.org/show_bug.cgi?id=102051 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Signed-off-by:
-
Michel Thierry authored
This patch adds per engine reset and recovery (TDR) support when GuC is used to submit workloads to GPU. In the case of i915 directly submission to ELSP, driver manages hang detection, recovery and resubmission. With GuC submission these tasks are shared between driver and GuC. i915 is still responsible for detecting a hang, and when it does it only requests GuC to reset that Engine. GuC internally manages acquiring forcewake and idling the engine before resetting it. Once the reset is successful, i915 takes over again and handles the resubmission. The scheduler in i915 knows which requests are pending so after resetting a engine, pending workloads/requests are resubmitted again. v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the non-guc function names. v3: Removed debug message about engine restarting from which request, since the new baseline do it regardless of submission mode. (Chris) v4: Rebase. v5: Do not pass unnecessary reporting flags to the fw (Jeff); tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase. v6: Rename the existing reset engine function and share a similar interface between guc and non-guc paths (Chris). Signed-off-by:
Michel Thierry <michel.thierry@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20171031225309.10888-1-michel.thierry@intel.comReviewed-by:
Jeff McGee <jeff.mcgee@intel.com> Signed-off-by:
-
Michel Thierry authored
intel_guc_reset sounds more like the microcontroller is the one performing a reset, while in this case is the opposite. intel_reset_guc not only makes it clearer, it follows the other intel_reset functions available. v2: Print error message in English (Tvrtko). Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by:
-
Jeff McGee authored
If GuC firmware performs an engine reset while that engine had a preemption pending, it will set the terminated attribute bit on our preemption stage descriptor. GuC firmware retains all pending work items for a high-priority GuC client, unlike the normal-priority GuC client where work items are dropped. It wants to make sure the preempt- to-idle work doesn't run when scheduling resumes, and uses this bit to inform its scheduler and presumably us as well. Our job is to clear it for the next preemption after reset, otherwise that and future preemptions will never complete. We'll just clear it every time. Signed-off-by:
-
- 01 Nov, 2017 5 commits
-
-
Mika Kahola authored
For Cannonlake the number of scalers for each pipe is 2. Let's increase the number of scalers for pipe C. v2: Use INTEL_GEN() instead of IS_CANNONLAKE() Signed-off-by:
Mika Kahola <mika.kahola@intel.com> Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
-
Chris Wilson authored
If the we think the engine is still active when we attempt to park it, we want more details -- so dump the engine state. References: https://bugs.freedesktop.org/show_bug.cgi?id=103479Signed-off-by:
-
Chris Wilson authored
We will want to break this down to give detailed per-engine warnings as to why we still think we are active as we attempt to park the engines. For the first step, just move the warning verbatim from the idle-worker to intel_engines_park(). Signed-off-by:
-
Chris Wilson authored
We will disarm the breadcrumb interrupt if we see a user interrupt whilst no one is waiting. This may race with the call to intel_engine_disarm_breadcrumbs() triggering an assert that we aren't trying to do the same job twice. Prevent this by checking that the irq is still armed after flushing the interrupt (for the irq spinlock). Fixes: bcbd5c33 ("drm/i915/guc: Always enable the breadcrumbs irq") Signed-off-by:
-
Chris Wilson authored
In case the object has changed tiling between calls to execbuf, we need to check if the existing offset inside the GTT matches the new tiling constraint. We even need to do this for "unfenced" tiled objects, where the 3D commands use an implied fence and so the object still needs to match the physical fence restrictions on alignment (only required for gen2 and early gen3). In commit 2889caa9 ("drm/i915: Eliminate lots of iterations over the execobjects array"), the idea was to remove the second guessing and only set the NEEDS_MAP flag when required. However, the entire check for an unusable offset for fencing was removed and not just the secondary check. I.e. /* avoid costly ping-pong once a batch bo ended up non-mappable */ if (entry->flags & __EXEC_OBJECT_NEEDS_MAP && !i915_vma_is_map_and_fenceable(vma)) return !only_mappable_for_reloc(entry->flags); was entirely removed as the ping-pong between execbuf passes was fixed, but its primary purpose in forcing unaligned unfenced access to be rebound was forgotten. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103502 Fixes: 2889caa9 ("drm/i915: Eliminate lots of iterations over the execobjects array") Signed-off-by:
-
- 31 Oct, 2017 3 commits
-
-
Michel Thierry authored
There is no need check if PPGTT is disabled because that not possible in CNL. Execlists and GuC submission modes rely on at least aliasing PPGTT and even intel_sanitize_enable_ppgtt says: "We don't allow disabling PPGTT for gen9+ as it's a requirement for execlists, the sole mechanism available to submit work." Suggested-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
-
Ville Syrjälä authored
encoder->type isn't genreally safe around DDI ports, so let's replace some uses in the audio code with the crtc state's output_types instead. Actually in these cases encoder->type would work since the DP SST case is only relevant for VLV/CHV and encoder->type==DP is a thing on those platforms. The DP MST cases would work as well since MST encoder->type==DP_MST always. But I think it's best to try and minimize the encoder->type use in general to avoid showing a bad example to people. Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171030184654.17429-2-ville.syrjala@linux.intel.comReviewed-by:
Jani Nikula <jani.nikula@intel.com>
-
Ville Syrjälä authored
Explicitly pass the crtc and connector states into the audio code enable/disable hooks, and plumb them all the way down. This gets rid of almost all crtc->config and encoder->crtc uses. The one place where we still use them is i915_audio_component_sync_audio_rate() since that gets called from the audio driver and we don't have explicit states around then. Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
- 30 Oct, 2017 9 commits
-
-
Chris Wilson authored
To quote kbuild/makefiles.txt: cc-disable-warning checks if gcc supports a given warning and returns the commandline switch to disable it. This special function is needed, because gcc 4.4 and later accept any unknown -Wno-* option and only warn about it if there is another warning in the source file. This is exactly what we were trying to achieve with cc-option -Wno-foo and failed miserably. Reported-by: kbuild-all@01.org Fixes: 39bf4de8 ("drm/i915: Add -Wall -Wextra to our build, set warnings to full") Signed-off-by: -
Ville Syrjälä authored
Eliminate the partially duplicated DDI readout code from MST, and instead just call intel_ddi_get_config(). As a nice bonus we get more cross checking as intel_ddi_get_config() will populate output_types based on the actual mode of the DDI port. Additonally intel_ddi_get_config() must be changed to get the crtc from the passed in crtc state rather than from the encoder->crtc link. encoder->crtc really shouldn't be used anyway. v2: Rebased on BXT MST latency_optim fix Make intel_ddi_clock_get() static Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
Ville Syrjälä authored
Pass an old crtc state to intel_ddi_post_disable() from the MST code. Note that this crtc state won't necessaitly match the one that was passed to intel_ddi_pre_enable() if the first stream to be enabled isn't the last stream to be disabled. But this is fine since the states should be identical in every important way. This does mean people frobbing the DDI pre_enable/post_disable hooks have to pay attention in what parts of the state they consult. The alternative would be to inline the relevant code into the MST code. That is actually what we used to do for pre_enable before commit e081c846 ("drm/i915: Remove duplicate DDI enabling logic from MST path"). For post_disable we've always called the DDI hook. v2: Pimp up the comments explaining the MST issues Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
We should be using the DPLL hw state we got from the current crtc state to determine the corresponding port clock frequency rather than getting it via the current state programmed into the DPLL. v2: Rebase due to intel_dpll_id changes Signed-off-by:
-
Ville Syrjälä authored
encoder->port works for FDI, and it also works for MST (regardless of whether we're dealing with the "fake" MST encoder, or mst->primary). So let's eliminate intel_ddi_get_encoder_port(). Signed-off-by:
-
Ville Syrjälä authored
Currently the DDI encoder->type will change at runtime depending on what kind of hotplugs we've processed. That's quite bad since we can't really trust that that current value of encoder->type actually matches the type of signal we're trying to drive through it. Let's eliminate that problem by declaring that non-eDP DDI port will always have the encoder type as INTEL_OUTPUT_DDI. This means the code can no longer try to distinguish DP vs. HDMI based on encoder->type. We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and there's a bunch of code that relies on that value to identify eDP encoders. We'll introduce a new encoder .compute_output_type() hook. This allows us to compute the full output_types before any encoder .compute_config() hooks get called, thus those hooks can rely on output_types being correct, which is useful for cloning on oldr platforms. For now we'll just look at the connector type and pick the correct mode based on that. In the future the new hook could be used to implement dynamic switching between LS and PCON modes for LSPCON. v2: Fix BXT/GLK PPS explosion with DSI/MST encoders v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg v4: Rebase v5: Populate output_types in .get_config() rather than in the caller v5: Split out populating output_types in .get_config() (Maarten) Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
Rather than having the caller of .get_config() set output_types based on encoder->type, let's just have .get_config() itself populate output_types. This way we are isolated from encoder->type, which won't be useable for this purpose anyway soon (at least for DDI encoders). Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
Starting from version 204 VBT can specify the max TMDS clock we are allowed to use with HDMI ports. Parse that information and take it into account when filtering modes and computing a crtc state. Also take the opportunity to sort the platform check if ladder from new to old. v2: Add defines for the values into intel_vbt_defs.h (Jani) Don't fall back to 0 silently for unknown values (Jani) Skip the debug print for the 0 case (Jani) Cc: Jani Nikula <jani.nikula@intel.com> Reviewed-by: -
Ville Syrjälä authored
The HDMI level shifter value should be 5 bits and the max data rate 3 bits. Cc: Jani Nikula <jani.nikula@intel.com> Reported-by:
-
- 27 Oct, 2017 10 commits
-
-
Ville Syrjälä authored
Move the crtc state related 12bpc checks into hdmi_12bpc_possible() since that one already examines other parts of the crtc state. Note that we can drop the !force_dvi check since crtc_state->has_hdmi_sink already accounts for that. Signed-off-by:
-
Ville Syrjälä authored
DP dongles may signal downstream HPD via short HPD pulses. Setting the sink to DPMS off apparently kills the downstream HPD (at least on my DP->VGA dongle), so skip the DPMS off for such dongles when we turn off the port. v2: Deal with DDI as well by moving the check into intel_dp_sink_dpms() (Dhinakaran) Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: David Woodhouse <dwmw2@infradead.org> Cc: Imre Deak <imre.deak@intel.com> Cc: Pablo <pablodebiase@nanalysis.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114Signed-off-by: -
Ville Syrjälä authored
Call the DDI .pre_pll_enable() hook from the MST code so that BXT gets the correct lane latency optimal setting applied. And we obviously need to compute the correct value, and read it out to keep the state checker happy. While at it drop the useless 'encoder' parameter to bxt_ddi_phy_calc_lane_lat_optim_mask() Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Ville Syrjälä authored
intel_ddi_enable_transcoder_func() already has the crtc state so we can use that instead of the untrustworthy encoder->type. Signed-off-by:
-
Ville Syrjälä authored
encoder->type is not realiable for DP/HDMI so let's switch the DPLL selection over to using output_types. Signed-off-by:
-
Ville Syrjälä authored
Eliminate intel_prepare_dp_ddi_buffers()'s reliance on the encoder->type by passing in the crtc state. Signed-off-by:
-
Ville Syrjälä authored
encoder->type isn't reliable for DP/HDMI so instead extract the correct type from the crtc state in intel_ddi_set_pipe_settings(). Signed-off-by:
-
Chris Wilson authored
Kasan spotted [IGT] gem_tiled_pread_pwrite: exiting, ret=0 ================================================================== BUG: KASAN: use-after-free in __i915_gem_object_reset_page_iter+0x15c/0x170 [i915] Read of size 8 at addr ffff8801359da310 by task kworker/3:2/182 CPU: 3 PID: 182 Comm: kworker/3:2 Tainted: G U 4.14.0-rc6-CI-Custom_3340+ #1 Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0062.B30.1708222146 08/22/2017 Workqueue: events __i915_gem_free_work [i915] Call Trace: dump_stack+0x68/0xa0 print_address_description+0x78/0x290 ? __i915_gem_object_reset_page_iter+0x15c/0x170 [i915] kasan_report+0x23d/0x350 __asan_report_load8_noabort+0x19/0x20 __i915_gem_object_reset_page_iter+0x15c/0x170 [i915] ? i915_gem_object_truncate+0x100/0x100 [i915] ? lock_acquire+0x380/0x380 __i915_gem_object_put_pages+0x30d/0x530 [i915] __i915_gem_free_objects+0x551/0xbd0 [i915] ? lock_acquire+0x13e/0x380 __i915_gem_free_work+0x4e/0x70 [i915] process_one_work+0x6f6/0x1590 ? pwq_dec_nr_in_flight+0x2b0/0x2b0 worker_thread+0xe6/0xe90 ? pci_mmcfg_check_reserved+0x110/0x110 kthread+0x309/0x410 ? process_one_work+0x1590/0x1590 ? kthread_create_on_node+0xb0/0xb0 ret_from_fork+0x27/0x40 Allocated by task 1801: save_stack_trace+0x1b/0x20 kasan_kmalloc+0xee/0x190 kasan_slab_alloc+0x12/0x20 kmem_cache_alloc+0xdc/0x2e0 radix_tree_node_alloc.constprop.12+0x48/0x330 __radix_tree_create+0x274/0x480 __radix_tree_insert+0xa2/0x610 i915_gem_object_get_sg+0x224/0x670 [i915] i915_gem_object_get_page+0xb5/0x1c0 [i915] i915_gem_pread_ioctl+0x822/0xf60 [i915] drm_ioctl_kernel+0x13f/0x1c0 drm_ioctl+0x6cf/0x980 do_vfs_ioctl+0x184/0xf30 SyS_ioctl+0x41/0x70 entry_SYSCALL_64_fastpath+0x1c/0xb1 Freed by task 37: save_stack_trace+0x1b/0x20 kasan_slab_free+0xaf/0x190 kmem_cache_free+0xbf/0x340 radix_tree_node_rcu_free+0x79/0x90 rcu_process_callbacks+0x46d/0xf40 __do_softirq+0x21c/0x8d3 The buggy address belongs to the object at ffff8801359da0f0 which belongs to the cache radix_tree_node of size 576 The buggy address is located 544 bytes inside of 576-byte region [ffff8801359da0f0, ffff8801359da330) The buggy address belongs to the page: page:ffffea0004d67600 count:1 mapcount:0 mapping: (null) index:0x0 compound_mapcount: 0 flags: 0x8000000000008100(slab|head) raw: 8000000000008100 0000000000000000 0000000000000000 0000000100110011 raw: ffffea0004b52920 ffffea0004b38020 ffff88015b416a80 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff8801359da200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff8801359da280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb >ffff8801359da300: fb fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc ^ ffff8801359da380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff8801359da400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ================================================================== Disabling lock debugging due to kernel taint which looks like the slab containing the radixtree iter was freed as we traversed the tree, taking the rcu read lock across the loop should prevent that (deferring all the frees until the end). Reported-by:Tomi Sarvela <tomi.p.sarvela@intel.com> Fixes: d1b48c1e ("drm/i915: Replace execbuf vma ht with an idr") Signed-off-by:
-
Chris Wilson authored
Kasan spotted [IGT] gem_tiled_pread_pwrite: exiting, ret=0 ================================================================== BUG: KASAN: use-after-free in __i915_gem_object_reset_page_iter+0x15c/0x170 [i915] Read of size 8 at addr ffff8801359da310 by task kworker/3:2/182 CPU: 3 PID: 182 Comm: kworker/3:2 Tainted: G U 4.14.0-rc6-CI-Custom_3340+ #1 Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0062.B30.1708222146 08/22/2017 Workqueue: events __i915_gem_free_work [i915] Call Trace: dump_stack+0x68/0xa0 print_address_description+0x78/0x290 ? __i915_gem_object_reset_page_iter+0x15c/0x170 [i915] kasan_report+0x23d/0x350 __asan_report_load8_noabort+0x19/0x20 __i915_gem_object_reset_page_iter+0x15c/0x170 [i915] ? i915_gem_object_truncate+0x100/0x100 [i915] ? lock_acquire+0x380/0x380 __i915_gem_object_put_pages+0x30d/0x530 [i915] __i915_gem_free_objects+0x551/0xbd0 [i915] ? lock_acquire+0x13e/0x380 __i915_gem_free_work+0x4e/0x70 [i915] process_one_work+0x6f6/0x1590 ? pwq_dec_nr_in_flight+0x2b0/0x2b0 worker_thread+0xe6/0xe90 ? pci_mmcfg_check_reserved+0x110/0x110 kthread+0x309/0x410 ? process_one_work+0x1590/0x1590 ? kthread_create_on_node+0xb0/0xb0 ret_from_fork+0x27/0x40 Allocated by task 1801: save_stack_trace+0x1b/0x20 kasan_kmalloc+0xee/0x190 kasan_slab_alloc+0x12/0x20 kmem_cache_alloc+0xdc/0x2e0 radix_tree_node_alloc.constprop.12+0x48/0x330 __radix_tree_create+0x274/0x480 __radix_tree_insert+0xa2/0x610 i915_gem_object_get_sg+0x224/0x670 [i915] i915_gem_object_get_page+0xb5/0x1c0 [i915] i915_gem_pread_ioctl+0x822/0xf60 [i915] drm_ioctl_kernel+0x13f/0x1c0 drm_ioctl+0x6cf/0x980 do_vfs_ioctl+0x184/0xf30 SyS_ioctl+0x41/0x70 entry_SYSCALL_64_fastpath+0x1c/0xb1 Freed by task 37: save_stack_trace+0x1b/0x20 kasan_slab_free+0xaf/0x190 kmem_cache_free+0xbf/0x340 radix_tree_node_rcu_free+0x79/0x90 rcu_process_callbacks+0x46d/0xf40 __do_softirq+0x21c/0x8d3 The buggy address belongs to the object at ffff8801359da0f0 which belongs to the cache radix_tree_node of size 576 The buggy address is located 544 bytes inside of 576-byte region [ffff8801359da0f0, ffff8801359da330) The buggy address belongs to the page: page:ffffea0004d67600 count:1 mapcount:0 mapping: (null) index:0x0 compound_mapcount: 0 flags: 0x8000000000008100(slab|head) raw: 8000000000008100 0000000000000000 0000000000000000 0000000100110011 raw: ffffea0004b52920 ffffea0004b38020 ffff88015b416a80 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff8801359da200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff8801359da280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb >ffff8801359da300: fb fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc ^ ffff8801359da380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff8801359da400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ================================================================== Disabling lock debugging due to kernel taint which looks like the slab containing the radixtree iter was freed as we traversed the tree, taking the rcu read lock across the loop should prevent that (deferring all the frees until the end). Reported-by: -
Chris Wilson authored
An interesting snippet from Sandybridge's prm: "Although a Ring Buffer can be enabled in the non-empty state, it must not be disabled unless it is empty. Attempting to disable a Ring Buffer in the non-empty state is UNDEFINED." Let's avoid the undefined behaviour as we disable the rings prior to reset and resume. v2: Tell HEAD to catch up to TAIL (empty ring) first, then reset both to 0 (supposedly while stopped). Signed-off-by:
-
