Skip to content

Commit

Permalink
drm/i915/gvt: Fix incorrect check of enabled bits in mask registers
Browse files Browse the repository at this point in the history
Using _MASKED_BIT_ENABLE macro to set mask register bits is straight
forward and not likely to go wrong. However when checking which bit(s)
is(are) enabled, simply bitwise AND value and _MASKED_BIT_ENABLE() won't
output expected result. Suppose the register write is disabling bit 1
by setting 0xFFFF0000, however "& _MASKED_BIT_ENABLE(1)" outputs
0x00010000, and the non-zero check will pass which cause the old code
consider the new value set as an enabling operation.

We found guest set 0x80008000 on boot, and set 0xffff8000 during resume.
Both are legal settings but old code will block latter and force vgpu
enter fail-safe mode.

Introduce two new macro and make proper masked bit check in mmio handler:
IS_MASKED_BITS_ENABLED()
IS_MASKED_BITS_DISABLED()

V2: Rebase.

Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Colin Xu <colin.xu@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20200601030721.17129-1-colin.xu@intel.com
  • Loading branch information
Colin Xu authored and Zhenyu Wang committed Jun 17, 2020
1 parent fccd0f7 commit fc1e3aa
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 12 deletions.
19 changes: 10 additions & 9 deletions drivers/gpu/drm/i915/gvt/handlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -1726,13 +1726,13 @@ static int ring_mode_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
(*(u32 *)p_data) &= ~_MASKED_BIT_ENABLE(2);
write_vreg(vgpu, offset, p_data, bytes);

if (data & _MASKED_BIT_ENABLE(1)) {
if (IS_MASKED_BITS_ENABLED(data, 1)) {
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);
return 0;
}

if (IS_COFFEELAKE(vgpu->gvt->gt->i915) &&
data & _MASKED_BIT_ENABLE(2)) {
IS_MASKED_BITS_ENABLED(data, 2)) {
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);
return 0;
}
Expand All @@ -1741,14 +1741,14 @@ static int ring_mode_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
* pvinfo, if not, we will treat this guest as non-gvtg-aware
* guest, and stop emulating its cfg space, mmio, gtt, etc.
*/
if (((data & _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE)) ||
(data & _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE)))
&& !vgpu->pv_notified) {
if ((IS_MASKED_BITS_ENABLED(data, GFX_PPGTT_ENABLE) ||
IS_MASKED_BITS_ENABLED(data, GFX_RUN_LIST_ENABLE)) &&
!vgpu->pv_notified) {
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);
return 0;
}
if ((data & _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE))
|| (data & _MASKED_BIT_DISABLE(GFX_RUN_LIST_ENABLE))) {
if (IS_MASKED_BITS_ENABLED(data, GFX_RUN_LIST_ENABLE) ||
IS_MASKED_BITS_DISABLED(data, GFX_RUN_LIST_ENABLE)) {
enable_execlist = !!(data & GFX_RUN_LIST_ENABLE);

gvt_dbg_core("EXECLIST %s on ring %s\n",
Expand Down Expand Up @@ -1809,7 +1809,7 @@ static int ring_reset_ctl_write(struct intel_vgpu *vgpu,
write_vreg(vgpu, offset, p_data, bytes);
data = vgpu_vreg(vgpu, offset);

if (data & _MASKED_BIT_ENABLE(RESET_CTL_REQUEST_RESET))
if (IS_MASKED_BITS_ENABLED(data, RESET_CTL_REQUEST_RESET))
data |= RESET_CTL_READY_TO_RESET;
else if (data & _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET))
data &= ~RESET_CTL_READY_TO_RESET;
Expand All @@ -1827,7 +1827,8 @@ static int csfe_chicken1_mmio_write(struct intel_vgpu *vgpu,
(*(u32 *)p_data) &= ~_MASKED_BIT_ENABLE(0x18);
write_vreg(vgpu, offset, p_data, bytes);

if (data & _MASKED_BIT_ENABLE(0x10) || data & _MASKED_BIT_ENABLE(0x8))
if (IS_MASKED_BITS_ENABLED(data, 0x10) ||
IS_MASKED_BITS_ENABLED(data, 0x8))
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);

return 0;
Expand Down
6 changes: 3 additions & 3 deletions drivers/gpu/drm/i915/gvt/mmio_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ bool is_inhibit_context(struct intel_context *ce);

int intel_vgpu_restore_inhibit_context(struct intel_vgpu *vgpu,
struct i915_request *req);
#define IS_RESTORE_INHIBIT(a) \
(_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) == \
((a) & _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT)))

#define IS_RESTORE_INHIBIT(a) \
IS_MASKED_BITS_ENABLED(a, CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT)

#endif
5 changes: 5 additions & 0 deletions drivers/gpu/drm/i915/gvt/reg.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@
#define GFX_MODE_BIT_SET_IN_MASK(val, bit) \
((((bit) & 0xffff0000) == 0) && !!((val) & (((bit) << 16))))

#define IS_MASKED_BITS_ENABLED(_val, _b) \
(((_val) & _MASKED_BIT_ENABLE(_b)) == _MASKED_BIT_ENABLE(_b))
#define IS_MASKED_BITS_DISABLED(_val, _b) \
((_val) & _MASKED_BIT_DISABLE(_b))

#define FORCEWAKE_RENDER_GEN9_REG 0xa278
#define FORCEWAKE_ACK_RENDER_GEN9_REG 0x0D84
#define FORCEWAKE_BLITTER_GEN9_REG 0xa188
Expand Down

0 comments on commit fc1e3aa

Please sign in to comment.