Re: [PATCH v2 3/5] drm/i915: terminate reauth at stream management failure

On Wed, Feb 12, 2020 at 03:59:40PM +0530, Ramalingam C wrote:

As per the HDCP2.2 compliance test 1B-10 expectation, when stream management for a repeater fails, we retry thrice and when it fails in all retries, HDCP2.2 reauthentication aborted at kernel.

v2: seq_num_m++ is extended for steam management failures too.[Anshuman] v3: use drm_dbg_kms instead of DRM_DEBUG_KMS [Anshuman] v4: dev_priv is used as i915 [JaniN]

Signed-off-by: Ramalingam C ramalingam.c@intel.com

drivers/gpu/drm/i915/display/intel_hdcp.c | 75 ++++++++++++++--------- 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 30e0a3aa9d57..b24d12efae0a 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1380,7 +1380,7 @@ static int hdcp2_session_key_exchange(struct intel_connector *connector) }

static -int hdcp2_propagate_stream_management_info(struct intel_connector *connector) +int _hdcp2_propagate_stream_management_info(struct intel_connector *connector) { struct intel_digital_port *intel_dig_port = intel_attached_dig_port(connector); struct intel_hdcp *hdcp = &connector->hdcp; @@ -1406,28 +1406,25 @@ int hdcp2_propagate_stream_management_info(struct intel_connector *connector) ret = shim->write_2_2_msg(intel_dig_port, &msgs.stream_manage, sizeof(msgs.stream_manage)); if (ret < 0)

• return ret;
  

• goto err_exit;
  
  

  ret = shim->read_2_2_msg(intel_dig_port, HDCP_2_2_REP_STREAM_READY, &msgs.stream_ready, sizeof(msgs.stream_ready)); if (ret < 0)

• return ret;
  

• goto err_exit;
  
  

  hdcp->port_data.seq_num_m = hdcp->seq_num_m; hdcp->port_data.streams[0].stream_type = hdcp->content_type;

• ret = hdcp2_verify_mprime(connector, &msgs.stream_ready);
• if (ret < 0)

• return ret;
  
  

+err_exit:

This isn't exclusively an error condition, I'd rather it named 'out' instead.

hdcp->seq_num_m++;

• if (hdcp->seq_num_m > HDCP_2_2_SEQ_NUM_MAX) { DRM_DEBUG_KMS("seq_num_m roll over.\n");

• return -1;
  

• ret = -1;
  

What does -1 mean? Please use an errno

}

• return 0;

• return ret;

}

static @@ -1492,17 +1489,6 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector) return 0; }

-static int hdcp2_authenticate_repeater(struct intel_connector *connector) -{

• int ret;
• ret = hdcp2_authenticate_repeater_topology(connector);
• if (ret < 0)

• return ret;
  

• return hdcp2_propagate_stream_management_info(connector);

-}

static int hdcp2_authenticate_sink(struct intel_connector *connector) { struct intel_digital_port *intel_dig_port = intel_attached_dig_port(connector); @@ -1537,18 +1523,13 @@ static int hdcp2_authenticate_sink(struct intel_connector *connector) }

if (hdcp->is_repeater) {

• ret = hdcp2_authenticate_repeater(connector);
  

• ret = hdcp2_authenticate_repeater_topology(connector);
  

  if (ret < 0) { DRM_DEBUG_KMS("Repeater Auth Failed. Err: %d\n", ret); return ret; } }

• hdcp->port_data.streams[0].stream_type = hdcp->content_type;
• ret = hdcp2_authenticate_port(connector);
• if (ret < 0)

• return ret;
  

• return ret;

}

@@ -1626,14 +1607,50 @@ static int hdcp2_disable_encryption(struct intel_connector *connector) return ret; }

+static int +hdcp2_propagate_stream_management_info(struct intel_connector *connector) +{

• struct drm_i915_private *i915 = to_i915(connector->base.dev);
• int i, tries = 3, ret;
• if (!connector->hdcp.is_repeater)

• return 0;
  

• for (i = 0; i < tries; i++) {

• ret = _hdcp2_propagate_stream_management_info(connector);
  

• if (!ret)
  

• 	break;
  

• drm_dbg_kms(&i915->drm,
  

• 	    "HDCP2 stream management %d of %d Failed.(%d)\n",
  

• 	    i + 1, tries, ret);
  

• }
• return ret;

+}

static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector) {

• struct drm_i915_private *i915 = to_i915(connector->base.dev);

• struct intel_hdcp *hdcp = &connector->hdcp; int ret, i, tries = 3;

  for (i = 0; i < tries; i++) { ret = hdcp2_authenticate_sink(connector);

• if (!ret)
  

• 	break;
  

• if (!ret) {
  

• 	ret = hdcp2_propagate_stream_management_info(connector);
  

• 	if (!ret) {
  

• 		hdcp->port_data.streams[0].stream_type =
  

• 					hdcp->content_type;
  

• 		ret = hdcp2_authenticate_port(connector);
  

• 		if (!ret)
  

• 			break;
  

• 	} else {
  

• 		drm_dbg_kms(&i915->drm,
  

• 			    "HDCP2 stream management failed\n");
  

Please print ret values on failures, always.

• 		break;
  

• 	}
  

• }
  

The nesting here is a bit awkward. How about:

if (!ret) { ret = hdcp2_propagate_stream_management_info(connector); if (ret) { drm_dbg_kms(&i915->drm, "HDCP2 stream mgmt fail %d\n", ret); break; }

hdcp->port_data.streams[0].stream_type = hdcp->content_type; ret = hdcp2_authenticate_port(connector); if (ret) drm_dbg_kms(&i915->drm, "HDCP2 auth port failed %d\n", ret); else break; }

/* Clearing the mei hdcp session */
DRM_DEBUG_KMS("HDCP2.2 Auth %d of %d Failed.(%d)\n",

@@ -1642,7 +1659,7 @@ static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector) DRM_DEBUG_KMS("Port deauth failed.\n"); }

• if (i != tries) {

• if (!ret) { /*
  • Ensuring the required 200mSec min time interval between
  • Session Key Exchange and encryption.

-- 2.20.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel