drm/i915/gvt: refactor the force-nonpriv handler

Submitted by Zhao, Yan Y on April 28, 2018, 7:37 a.m.

Details

Message ID 1524901069-21356-1-git-send-email-yan.y.zhao@intel.com
State New
Headers show
Series "drm/i915/gvt: refactor the force-nonpriv handler" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Zhao, Yan Y April 28, 2018, 7:37 a.m.
Each ring has 12 force_to_nonpriv registers, the registers written to those
registers will be converted to nonprivleged from privileged.

This patch
1. adds force_to_nonpriv registers to handlers list for all rings
2. adds NOPID register of each ring to valid value of force_to_nonpriv
registers, and regards it as default value
3. does not return error on detecting MMIO write of values outside
whitelist, but just prevents it from writing into virtual reg and prints
error message.
4. in cmdparser part, just converts invalid value to default value before
sending down to hardware

Signed-off-by: Zhao Yan <yan.y.zhao@intel.com>
Reviewed-by: Zhang Yulei <yulei.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/cmd_parser.c | 34 ++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/gvt/handlers.c   | 20 ++++++++++++++------
 2 files changed, 40 insertions(+), 14 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index d895069..4424098 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -807,20 +807,38 @@  static bool is_shadowed_mmio(unsigned int offset)
 	return ret;
 }
 
-static inline bool is_force_nonpriv_mmio(unsigned int offset)
+static bool is_force_nonpriv_mmio(struct parser_exec_state *s,
+		unsigned int offset)
 {
-	return (offset >= 0x24d0 && offset < 0x2500);
+	u32 base = s->vgpu->gvt->dev_priv->engine[s->ring_id]->mmio_base;
+
+	return (offset >= (base + 0x4d0) && offset < (base + 0x500));
 }
 
 static int force_nonpriv_reg_handler(struct parser_exec_state *s,
-				     unsigned int offset, unsigned int index)
+		unsigned int offset, unsigned int index, char *cmd)
 {
 	struct intel_gvt *gvt = s->vgpu->gvt;
-	unsigned int data = cmd_val(s, index + 1);
+	unsigned int data;
+	u32 ring_base;
+	u32 nopid;
+	struct drm_i915_private *dev_priv = s->vgpu->gvt->dev_priv;
 
-	if (!intel_gvt_in_force_nonpriv_whitelist(gvt, data)) {
+	if (!strcmp(cmd, "lri"))
+		data = cmd_val(s, index + 1);
+	else {
+		gvt_err("Unexpected forcenonpriv 0x%x write from cmd %s\n",
+			offset, cmd);
+		return -EINVAL;
+	}
+
+	ring_base = dev_priv->engine[s->ring_id]->mmio_base;
+	nopid = i915_mmio_reg_offset(RING_NOPID(ring_base));
+	if (!intel_gvt_in_force_nonpriv_whitelist(gvt, data) &&
+			data != nopid) {
 		gvt_err("Unexpected forcenonpriv 0x%x LRI write, value=0x%x\n",
 			offset, data);
+		patch_value(s, cmd_ptr(s, index), nopid);
 		return -EPERM;
 	}
 	return 0;
@@ -868,9 +886,9 @@  static int cmd_reg_handler(struct parser_exec_state *s,
 	    mocs_cmd_reg_handler(s, offset, index))
 		return -EINVAL;
 
-	if (is_force_nonpriv_mmio(offset) &&
-		force_nonpriv_reg_handler(s, offset, index))
-		return -EPERM;
+	if (is_force_nonpriv_mmio(s, offset) &&
+		force_nonpriv_reg_handler(s, offset, index, cmd))
+		return 0;
 
 	if (offset == i915_mmio_reg_offset(DERRMR) ||
 		offset == i915_mmio_reg_offset(FORCEWAKE_MT)) {
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index b77adcc..1c06578d 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -477,22 +477,28 @@  static int force_nonpriv_write(struct intel_vgpu *vgpu,
 	unsigned int offset, void *p_data, unsigned int bytes)
 {
 	u32 reg_nonpriv = *(u32 *)p_data;
+	int ring_id = intel_gvt_render_mmio_to_ring_id(vgpu->gvt, offset);
+	u32 ring_base;
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
 	int ret = -EINVAL;
 
-	if ((bytes != 4) || ((offset & (bytes - 1)) != 0)) {
+	if ((bytes != 4) || ((offset & (bytes - 1)) != 0) || ring_id < 0) {
 		gvt_err("vgpu(%d) Invalid FORCE_NONPRIV offset %x(%dB)\n",
 			vgpu->id, offset, bytes);
 		return ret;
 	}
 
-	if (in_whitelist(reg_nonpriv)) {
+	ring_base = dev_priv->engine[ring_id]->mmio_base;
+
+	if (in_whitelist(reg_nonpriv) ||
+		reg_nonpriv == i915_mmio_reg_offset(RING_NOPID(ring_base))) {
 		ret = intel_vgpu_default_mmio_write(vgpu, offset, p_data,
 			bytes);
 	} else {
-		gvt_err("vgpu(%d) Invalid FORCE_NONPRIV write %x\n",
-			vgpu->id, reg_nonpriv);
+		gvt_err("vgpu(%d) Invalid FORCE_NONPRIV write %x at offset %x\n",
+			vgpu->id, reg_nonpriv, offset);
 	}
-	return ret;
+	return 0;
 }
 
 static int ddi_buf_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
@@ -2594,8 +2600,10 @@  static int init_broadwell_mmio_info(struct intel_gvt *gvt)
 	MMIO_DFH(_MMIO(0xb10c), D_BDW, F_CMD_ACCESS, NULL, NULL);
 	MMIO_D(_MMIO(0xb110), D_BDW);
 
-	MMIO_F(_MMIO(0x24d0), 48, F_CMD_ACCESS, 0, 0, D_BDW_PLUS,
+#define RING_REG(base) _MMIO((base) + 0x4d0)
+	MMIO_RING_F(RING_REG, 48, F_CMD_ACCESS, 0, 0, D_BDW_PLUS,
 		NULL, force_nonpriv_write);
+#undef RING_REG
 
 	MMIO_D(_MMIO(0x44484), D_BDW_PLUS);
 	MMIO_D(_MMIO(0x4448c), D_BDW_PLUS);

Comments


On 2018.04.28 15:37:49 +0800, Zhao Yan wrote:
> Each ring has 12 force_to_nonpriv registers, the registers written to those
> registers will be converted to nonprivleged from privileged.
> 
> This patch
> 1. adds force_to_nonpriv registers to handlers list for all rings
> 2. adds NOPID register of each ring to valid value of force_to_nonpriv
> registers, and regards it as default value
> 3. does not return error on detecting MMIO write of values outside
> whitelist, but just prevents it from writing into virtual reg and prints
> error message.
> 4. in cmdparser part, just converts invalid value to default value before
> sending down to hardware
>

please split these into seperated ones, don't mix different issues in one patch.

thanks