Skip to content

Commit fc1e3aa

Browse files
coxuintelzhenyw
authored andcommitted
drm/i915/gvt: Fix incorrect check of enabled bits in mask registers
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 <[email protected]> Signed-off-by: Colin Xu <[email protected]> Signed-off-by: Zhenyu Wang <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent fccd0f7 commit fc1e3aa

File tree

3 files changed

+18
-12
lines changed

3 files changed

+18
-12
lines changed

drivers/gpu/drm/i915/gvt/handlers.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,13 +1726,13 @@ static int ring_mode_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
17261726
(*(u32 *)p_data) &= ~_MASKED_BIT_ENABLE(2);
17271727
write_vreg(vgpu, offset, p_data, bytes);
17281728

1729-
if (data & _MASKED_BIT_ENABLE(1)) {
1729+
if (IS_MASKED_BITS_ENABLED(data, 1)) {
17301730
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);
17311731
return 0;
17321732
}
17331733

17341734
if (IS_COFFEELAKE(vgpu->gvt->gt->i915) &&
1735-
data & _MASKED_BIT_ENABLE(2)) {
1735+
IS_MASKED_BITS_ENABLED(data, 2)) {
17361736
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);
17371737
return 0;
17381738
}
@@ -1741,14 +1741,14 @@ static int ring_mode_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
17411741
* pvinfo, if not, we will treat this guest as non-gvtg-aware
17421742
* guest, and stop emulating its cfg space, mmio, gtt, etc.
17431743
*/
1744-
if (((data & _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE)) ||
1745-
(data & _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE)))
1746-
&& !vgpu->pv_notified) {
1744+
if ((IS_MASKED_BITS_ENABLED(data, GFX_PPGTT_ENABLE) ||
1745+
IS_MASKED_BITS_ENABLED(data, GFX_RUN_LIST_ENABLE)) &&
1746+
!vgpu->pv_notified) {
17471747
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);
17481748
return 0;
17491749
}
1750-
if ((data & _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE))
1751-
|| (data & _MASKED_BIT_DISABLE(GFX_RUN_LIST_ENABLE))) {
1750+
if (IS_MASKED_BITS_ENABLED(data, GFX_RUN_LIST_ENABLE) ||
1751+
IS_MASKED_BITS_DISABLED(data, GFX_RUN_LIST_ENABLE)) {
17521752
enable_execlist = !!(data & GFX_RUN_LIST_ENABLE);
17531753

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

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

1830-
if (data & _MASKED_BIT_ENABLE(0x10) || data & _MASKED_BIT_ENABLE(0x8))
1830+
if (IS_MASKED_BITS_ENABLED(data, 0x10) ||
1831+
IS_MASKED_BITS_ENABLED(data, 0x8))
18311832
enter_failsafe_mode(vgpu, GVT_FAILSAFE_UNSUPPORTED_GUEST);
18321833

18331834
return 0;

drivers/gpu/drm/i915/gvt/mmio_context.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ bool is_inhibit_context(struct intel_context *ce);
5454

5555
int intel_vgpu_restore_inhibit_context(struct intel_vgpu *vgpu,
5656
struct i915_request *req);
57-
#define IS_RESTORE_INHIBIT(a) \
58-
(_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) == \
59-
((a) & _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT)))
57+
58+
#define IS_RESTORE_INHIBIT(a) \
59+
IS_MASKED_BITS_ENABLED(a, CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT)
6060

6161
#endif

drivers/gpu/drm/i915/gvt/reg.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@
9494
#define GFX_MODE_BIT_SET_IN_MASK(val, bit) \
9595
((((bit) & 0xffff0000) == 0) && !!((val) & (((bit) << 16))))
9696

97+
#define IS_MASKED_BITS_ENABLED(_val, _b) \
98+
(((_val) & _MASKED_BIT_ENABLE(_b)) == _MASKED_BIT_ENABLE(_b))
99+
#define IS_MASKED_BITS_DISABLED(_val, _b) \
100+
((_val) & _MASKED_BIT_DISABLE(_b))
101+
97102
#define FORCEWAKE_RENDER_GEN9_REG 0xa278
98103
#define FORCEWAKE_ACK_RENDER_GEN9_REG 0x0D84
99104
#define FORCEWAKE_BLITTER_GEN9_REG 0xa188

0 commit comments

Comments
 (0)