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 <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

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index f39a6b2..fadd2ad 100644 (file)
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -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;
        }
@@ -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",
@@ -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;
@@ -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;

diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.h b/drivers/gpu/drm/i915/gvt/mmio_context.h
index 970704b..3b25e7f 100644 (file)
--- a/drivers/gpu/drm/i915/gvt/mmio_context.h
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.h
@@ -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

diff --git a/drivers/gpu/drm/i915/gvt/reg.h b/drivers/gpu/drm/i915/gvt/reg.h
index 5b66e14..b88e033 100644 (file)
--- a/drivers/gpu/drm/i915/gvt/reg.h
+++ b/drivers/gpu/drm/i915/gvt/reg.h
@@ -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