Skip to content
  1. Jul 12, 2023
    • Andrzej Hajda's avatar
      drm/i915/perf: add sentinel to xehp_oa_b_counters · 2f42c5af
      Andrzej Hajda authored
      Arrays passed to reg_in_range_table should end with empty record.
      
      The patch solves KASAN detected bug with signature:
      BUG: KASAN: global-out-of-bounds in xehp_is_valid_b_counter_addr+0x2c7/0x350 [i915]
      Read of size 4 at addr ffffffffa1555d90 by task perf/1518
      
      CPU: 4 PID: 1518 Comm: perf Tainted: G U 6.4.0-kasan_438-g3303d06107f3+ #1
      Hardware name: Intel Corporation Meteor Lake Client Platform/MTL-P DDR5 SODIMM SBS RVP, BIOS MTLPFWI1.R00.3223.D80.2305311348 05/31/2023
      Call Trace:
      <TASK>
      ...
      xehp_is_valid_b_counter_addr+0x2c7/0x350 [i915]
      
      Fixes: 0fa9349d
      
       ("drm/i915/perf: complete programming whitelisting for XEHPSDV")
      Signed-off-by: default avatarAndrzej Hajda <andrzej.hajda@intel.com>
      Reviewed-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
      Reviewed-by: default avatarNirmoy Das <nirmoy.das@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230711153410.1224997-1-andrzej.hajda@intel.com
      2f42c5af
  2. Jul 10, 2023
  3. Jul 08, 2023
  4. Jul 07, 2023
  5. Jul 06, 2023
  6. Jul 04, 2023
    • Lucas De Marchi's avatar
      drm/i915/gt: Also check set bits in clr_set() · e3affc7c
      Lucas De Marchi authored
      
      
      When checking if the workarounds were applied successfully, the
      read-back mask should also contain the bits being set: it's possible
      that in a call to wa_write_clr_set(), the cleared bits are not a
      superset of the set bits.
      
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarKenneth Graunke <kenneth@whitecape.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-8-lucas.demarchi@intel.com
      e3affc7c
    • Lucas De Marchi's avatar
      drm/i915/gt: Remove bogus comment on IVB_FBC_RT_BASE_UPPER · 03286f94
      Lucas De Marchi authored
      
      
      The comment on the parameter being 0 to avoid the read back doesn't
      apply as this is not a call to wa_add(), but rather to
      wa_write_clr_set(). So, this register is actually checked and it's
      according to the Bspec that the register is RW, not RO.
      
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarKenneth Graunke <kenneth@whitecape.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-7-lucas.demarchi@intel.com
      03286f94
    • Lucas De Marchi's avatar
      drm/i915/gt: Enable read back on XEHP_FF_MODE2 · 9a54a7c3
      Lucas De Marchi authored
      
      
      Contrary to GEN12_FF_MODE2, platforms using XEHP_FF_MODE2 are not
      affected by Wa_1608008084, hence read back can be enabled.
      
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarKenneth Graunke <kenneth@whitecape.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-6-lucas.demarchi@intel.com
      9a54a7c3
    • Lucas De Marchi's avatar
      drm/i915/gt: Drop read from GEN8_L3CNTLREG in ICL workaround · fc311f11
      Lucas De Marchi authored
      
      
      Now that non-masked registers are already read before programming the
      context reads, the additional read became redudant, so remove it.
      
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarKenneth Graunke <kenneth@whitecape.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-5-lucas.demarchi@intel.com
      fc311f11
    • Lucas De Marchi's avatar
      drm/i915/gt: Fix context workarounds with non-masked regs · 28cf243a
      Lucas De Marchi authored
      
      
      Most of the context workarounds tweak masked registers, but not all. For
      masked registers, when writing the value it's sufficient to just write
      the wa->set_bits since that will take care of both the clr and set bits
      as well as not overwriting other bits.
      
      However there are some workarounds, the registers are non-masked. Up
      until now the driver was simply emitting a MI_LOAD_REGISTER_IMM with the
      set_bits to program the register via the GPU in the WA bb. This has the
      side effect of overwriting the content of the register outside of bits
      that should be set and also doesn't handle the bits that should be
      cleared.
      
      Kenneth reported that on DG2, mesa was seeing a weird behavior due to
      the kernel programming of L3SQCREG5 in dg2_ctx_gt_tuning_init(). With
      the GPU idle, that register could be read via intel_reg as 0x00e001ff,
      but during a 3D workload it would change to 0x0000007f. So the
      programming of that tuning was affecting more than the bits in
      L3_PWM_TIMER_INIT_VAL_MASK. Matt Roper noticed the lack of rmw for the
      context workarounds due to the use of MI_LOAD_REGISTER_IMM.
      
      So, for registers that are not masked, read its value via mmio, modify
      and then set it in the buffer to be written by the GPU. This should take
      care in a simple way of programming just the bits required by the
      tuning/workaround. If in future there are registers that involved that
      can't be read by the CPU, a more complex approach may be required like
      a) issuing additional instructions to read and modify; or b) scan the
      golden context and patch it in place before saving it; or something
      else. But for now this should suffice.
      
      Scanning the context workarounds for all platforms, these are the
      impacted ones with the respective registers
      
      	mtl: DRAW_WATERMARK
      	mtl/dg2: XEHP_L3SQCREG5, XEHP_FF_MODE2
      
      ICL has some non-masked registers in the context workarounds:
      GEN8_L3CNTLREG, IVB_FBC_RT_BASE and VB_FBC_RT_BASE_UPPER, but there
      shouldn't be an impact. The first is already being manually read and the
      other 2 are intentionally overwriting the entire register. Same
      reasoning applies to GEN12_FF_MODE2: the WA is intentionally
      overwriting all the bits to avoid a read-modify-write.
      
      v2:  Reword commit message wrt GEN12_FF_MODE2 and the changed behavior
      on preparatory patches.
      v3: Also skip reading if clear|set bits covers everything
      
      Cc: Kenneth Graunke <kenneth@whitecape.org>
      Cc: Matt Roper <matthew.d.roper@intel.com>
      Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23783#note_1968971
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarKenneth Graunke <kenneth@whitecape.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-4-lucas.demarchi@intel.com
      28cf243a
    • Lucas De Marchi's avatar
      drm/i915/gt: Clear all bits from GEN12_FF_MODE2 · e8f7df16
      Lucas De Marchi authored
      
      
      Right now context workarounds don't do a rmw and instead only write to
      the register. Since 2 separate programmings to the same register are
      coalesced into a single write, this is not problematic for
      GEN12_FF_MODE2 since both TDS and GS timer are going to be written
      together and the other remaining bits be zeroed.
      
      However in order to fix other workarounds that may want to preserve the
      unrelated bits in the same register, context workarounds need to
      be changed to a rmw. To prepare for that, move the programming of
      GEN12_FF_MODE2 to a single place so the value passed for "clear" can
      be all the bits. Otherwise the second workaround would be dropped as
      it'd be detected as overwriting a previously programmed workaround.
      
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-3-lucas.demarchi@intel.com
      e8f7df16
    • Lucas De Marchi's avatar
      drm/i915/gt: Move wal_get_fw_for_rmw() · f567947b
      Lucas De Marchi authored
      
      
      Move helper function to get all the forcewakes required by the wa list
      to the top, so it can be re-used by other functions.
      
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarKenneth Graunke <kenneth@whitecape.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-2-lucas.demarchi@intel.com
      f567947b
  7. Jun 26, 2023
  8. Jun 24, 2023
    • Matt Roper's avatar
      drm/i915: Extend Wa_14015795083 platforms · 6580176f
      Matt Roper authored
      
      
      This workaround was already implemented for DG2, PVC, and some steppings
      of MTL, but the workaround database has now been updated to extend this
      workaround to TGL, RKL, DG1, and ADL.
      
      v2:
       - Skip readback verification for these extra gen12lp platforms.  On
         some of the platforms, the firmware locks this register, preventing
         the driver from making any modifications.  We should still try to
         apply the workaround, but if the register is locked and the value
         doesn't stick, that's semi-expected and not something we want to flag
         as a driver error on debug builds.
      
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarHaridhar Kalvala <haridhar.kalvala@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230616225041.3922719-1-matthew.d.roper@intel.com
      6580176f
  9. Jun 22, 2023
  10. Jun 21, 2023
    • Alan Previn's avatar
      drm/i915/gsc: Fix intel_gsc_uc_fw_proxy_init_done with directed wakerefs · aee90e92
      Alan Previn authored
      intel_gsc_uc_fw_proxy_init_done is used by a few code paths
      and usages. However, certain paths need a wakeref while others
      can't take a wakeref such as from the runtime_pm_resume callstack.
      
      Add a param into this helper to allow callers to direct whether
      to take the wakeref or not. This resolves the following bug:
      
         INFO: task sh:2607 blocked for more than 61 seconds.
         Not tainted 6.3.0-pxp-gsc-final-jun14+ #297
         "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
         task:sh              state:D stack:13016 pid:2607  ppid:2602   flags:0x00004000
         Call Trace:
            <TASK>
            __schedule+0x47b/0xe10
            schedule+0x58/0xd0
            rpm_resume+0x1cc/0x800
            ? __pfx_autoremove_wake_function+0x10/0x10
            __pm_runtime_resume+0x42/0x80
            __intel_runtime_pm_get+0x19/0x80 [i915]
            gsc_uc_get_fw_status+0x10/0x50 [i915]
            intel_gsc_uc_fw_init_done+0x9/0x20 [i915]
            intel_gsc_uc_load_start+0x5b/0x130 [i915]
            __uc_resume+0xa5/0x280 [i915]
            intel_runtime_resume+0xd4/0x250 [i915]
            ? __pfx_pci_pm_runtime_resume+0x10/0x10
         __rpm_callback+0x3c/0x160
      
      Fixes: 8c33c375
      
       ("drm/i915/gsc: take a wakeref for the proxy-init-completion check")
      Signed-off-by: default avatarAlan Previn <alan.previn.teres.alexis@intel.com>
      Reviewed-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Signed-off-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230615211940.4061378-1-alan.previn.teres.alexis@intel.com
      aee90e92
  11. Jun 19, 2023
  12. Jun 16, 2023
  13. Jun 14, 2023
    • Daniele Ceraolo Spurio's avatar
      drm/i915/mtl/gsc: Add a gsc_info debugfs · 561055b8
      Daniele Ceraolo Spurio authored
      
      
      Add a new debugfs to dump information about the GSC. This includes:
      
      - the FW path and SW tracking status;
      - the release, security and compatibility versions;
      - the HECI1 status registers.
      
      Note that those are the same registers that the mei driver dumps in
      their own status sysfs on DG2 (where mei owns the GSC).
      
      To make it simpler to loop through the status register, the code has
      been update to use a PICK macro and the existing code using the regs had
      been adapted to match.
      
      v2: fix includes and copyright dates (Alan)
      v3: actually fix the includes
      
      Signed-off-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
      Cc: John Harrison <John.C.Harrison@Intel.com>
      Reviewed-by: default avatarAlan Previn <alan.previn.teres.alexis@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230612181529.2222451-5-daniele.ceraolospurio@intel.com
      561055b8
    • Daniele Ceraolo Spurio's avatar
      drm/i915/mtl/gsc: query the GSC FW for its compatibility version · a6c13a23
      Daniele Ceraolo Spurio authored
      
      
      The compatibility version is queried via an MKHI command. Right now, the
      only existing interface is 1.0
      This is basically the interface version for the GSC FW, so the plan is
      to use it as the main tracked version, including for the binary naming
      in the fetch code.
      
      v2: use define for the object size (Alan)
      
      Signed-off-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
      Cc: John Harrison <John.C.Harrison@Intel.com>
      Reviewed-by: default avatarAlan Previn <alan.previn.teres.alexis@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230612181529.2222451-4-daniele.ceraolospurio@intel.com
      a6c13a23
    • Daniele Ceraolo Spurio's avatar
      drm/i915/mtl/gsc: extract release and security versions from the gsc binary · 56fafa56
      Daniele Ceraolo Spurio authored
      
      
      The release and security versions of the GSC binary are not used at
      runtime to decide interface compatibility (there is a separate version
      for that), but they're still useful for debug, so it is still worth
      extracting them and printing them out in dmesg.
      
      To get to these version, we need to navigate through various headers in
      the binary. See in-code comment for details.
      
      v2: fix and improve size checks when crawling the binary header, add
      comment about the different version, wrap the partition base/offset
      pairs in the GSC header in a struct (Alan)
      
      Signed-off-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
      Reviewed-by: default avatarAlan Previn <alan.previn.teres.alexis@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230612181529.2222451-3-daniele.ceraolospurio@intel.com
      56fafa56
    • Daniele Ceraolo Spurio's avatar
      drm/i915/gsc: fixes and updates for GSC memory allocation · b267a670
      Daniele Ceraolo Spurio authored
      
      
      A few fixes/updates are required around the GSC memory allocation and it
      is easier to do them all at the same time. The changes are as follows:
      
      1 - Switch the memory allocation to stolen memory. We need to avoid
      accesses from GSC FW to normal memory after the suspend function has
      completed and to do so we can either switch to using stolen or make sure
      the GSC is gone to sleep before the end of the suspend function. Given
      that the GSC waits for a bit before going idle even if there are no
      pending operations, it is easier and quicker to just use stolen memory.
      
      2 - Reduce the GSC allocation size to 4MBs, which is the POR requirement.
      The 8MBs were needed only for early FW and I had misunderstood that as
      being the expected POR size when I sent the original patch.
      
      3 - Perma-map the GSC allocation. This isn't required immediately, but it
      will be needed later to be able to quickly extract the GSC logs, which are
      inside the allocation. Since the mapping code needs to be rewritten due to
      switching to stolen, it makes sense to do the switch immediately to avoid
      having to change it again later.
      
      Note that the explicit setting of CACHE_NONE for Wa_22016122933 has been
      dropped because that's the default setting for stolen memory on !LLC
      platforms.
      
      v2: only memset the memory we're not overwriting (Alan)
      
      Signed-off-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
      Cc: John Harrison <John.C.Harrison@Intel.com>
      Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
      Reviewed-by: default avatarAlan Previn <alan.previn.teres.alexis@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230612181529.2222451-2-daniele.ceraolospurio@intel.com
      b267a670
  14. Jun 12, 2023
  15. Jun 11, 2023
    • Nirmoy Das's avatar
      drm/i915: Fix a VMA UAF for multi-gt platform · f56fe3e9
      Nirmoy Das authored
      
      
      Ensure correct handling of closed VMAs on multi-gt platforms to prevent
      Use-After-Free. Currently, when GT0 goes idle, closed VMAs that are
      exclusively added to GT0's closed_vma link (gt->closed_vma) and
      subsequently freed by i915_vma_parked(), which assumes the entire GPU is
      idle. However, on platforms with multiple GTs, such as MTL, GT1 may
      remain active while GT0 is idle. This causes GT0 to mistakenly consider
      the closed VMAs in its closed_vma list as unnecessary, potentially
      leading to Use-After-Free issues if a job for GT1 attempts to access a
      freed VMA.
      
      Although we do take a wakeref for GT0 but it happens later, after
      evaluating VMAs. To mitigate this, it is necessary to hold a GT0 wakeref
      early.
      
      v2: Use gt id to detect multi-tile(Andi)
          Fix the incorrect error path.
      v3: Add more comment(Andi)
          Use the new gt var when possible(Andrzej)
      
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
      Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
      Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
      Cc: Chris Wilson <chris.p.wilson@intel.com>
      Cc: Andi Shyti <andi.shyti@linux.intel.com>
      Cc: Andrzej Hajda <andrzej.hajda@intel.com>
      Cc: Sushma Venkatesh Reddy <sushma.venkatesh.reddy@intel.com>
      Signed-off-by: default avatarNirmoy Das <nirmoy.das@intel.com>
      Tested-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
      Reviewed-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
      Reviewed-by: default avatarAndrzej Hajda <andrzej.hajda@intel.com>
      Tested-by: default avatarSushma Venkatesh Reddy <sushma.venkatesh.reddy@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230608110103.777594-1-andi.shyti@linux.intel.com
      f56fe3e9
  16. Jun 08, 2023
    • Dan Carpenter's avatar
      drm/i915/gsc: Fix error code in intel_gsc_uc_heci_cmd_submit_nonpriv() · 24335848
      Dan Carpenter authored
      This should return negative -EAGAIN instead of positive EAGAIN.
      
      Fixes: e5e1e6d2
      
       ("drm/i915/pxp: Add MTL helpers to submit Heci-Cmd-Packet to GSC")
      Signed-off-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
      Reviewed-by: default avatarAlan Previn <alan.previn.teres.alexis@intel.com>
      Reviewed-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
      Signed-off-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/ZH7sr+Vs4zOQoouU@moroto
      24335848
    • Umesh Nerlige Ramappa's avatar
      i915/perf: Do not add ggtt offset to hw_tail · 589f4924
      Umesh Nerlige Ramappa authored
      
      
      ggtt offset for hw_tail is not required for the calculations, so drop
      it.
      
      Signed-off-by: default avatarUmesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
      Reviewed-by: default avatarAshutosh Dixit <ashutosh.dixit@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230605193923.1836048-3-umesh.nerlige.ramappa@intel.com
      589f4924
    • Umesh Nerlige Ramappa's avatar
      i915/perf: Drop the aging_tail logic in perf OA · 9cc31938
      Umesh Nerlige Ramappa authored
      
      
      On DG2, capturing OA reports while running heavy render workloads
      sometimes results in invalid OA reports where 64-byte chunks inside
      reports have stale values. Under memory pressure, high OA sampling rates
      (13.3 us) and heavy render workload, occasionally, the OA HW TAIL
      pointer does not progress as fast as the sampling rate. When these
      glitches occur, the TAIL pointer takes approx. 200us to progress.  While
      this is expected behavior from the HW perspective, invalid reports are
      not expected.
      
      In oa_buffer_check_unlocked(), when we execute the if condition, we are
      updating the oa_buffer.tail to the aging tail and then setting pollin
      based on this tail value, however, we do not have a chance to rewind and
      validate the reports prior to setting pollin. The validation happens
      in a subsequent call to oa_buffer_check_unlocked(). If a read occurs
      before this validation, then we end up reading reports up until this
      oa_buffer.tail value which includes invalid reports. Though found on
      DG2, this affects all platforms.
      
      The aging tail logic is no longer necessary since we are explicitly
      checking for landed reports.
      
      Start by dropping the aging tail logic.
      
      v2:
      - Drop extra blank line
      - Add reason to drop aging logic (Ashutosh)
      - Add bug links (Ashutosh)
      - rename aged_tail to read_tail
      - Squash patches 3 and 1
      
      v3: (Ashutosh)
      - Remove extra spaces
      - Remove gtt_offset from the pollin calculation
      - s/Bug:/Link/ in commit message (checkpatch)
      
      Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7484
      Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7757
      Signed-off-by: default avatarUmesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
      Reviewed-by: default avatarAshutosh Dixit <ashutosh.dixit@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230605193923.1836048-2-umesh.nerlige.ramappa@intel.com
      9cc31938
  17. Jun 07, 2023
  18. Jun 06, 2023