Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdcp: Fix 1B-06 HDCP2.2 Comp test

[Ramalingam C <ramalingam.c@intel.com>]

On 2020-02-06 at 20:34:41 +0530, Anshuman Gupta wrote:
> HDCP Repeater initializes seq_num_V to 0 at the beginning of
> hdcp Session i.e. after AKE_init received.
> 
> HDCP 2.2 Comp specs 1B-06 test verifies that whether DUT
> considers failures of authentication if the repeater provides a
> non-zero value in seq_num_V in the first,
> RepeaterAuth_Send_ReceiverID_List message after first AKE_Init.
> Fixing this broken test.
Instead of "Fixing the broken test" could we say, we mandate the first
seq_num_v to be zero? in fact i would keep this as commit subject also. 
> 
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  3 +++
>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 +++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 7ae0bc8b80d1..2ae540e986ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -360,6 +360,9 @@ struct intel_hdcp {
>  	/* HDCP2.2 Encryption status */
>  	bool hdcp2_encrypted;
>  
> +	/* Flag indicate if it is a first ReceiverID_List msg after AKE_Init */
> +	bool first_recvid_msg;
This extra flag is not needed, see below comment
> +
>  	/*
>  	 * Content Stream Type defined by content owner. TYPE0(0x0) content can
>  	 * flow in the link protected by HDCP2.2 or HDCP1.4, where as TYPE1(0x1)
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 4d1a33d13105..3e24a6df503a 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -1251,6 +1251,8 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
>  	size_t size;
>  	int ret;
>  
> +	hdcp->first_recvid_msg = true;
> +
>  	/* Init for seq_num */
>  	hdcp->seq_num_v = 0;
>  	hdcp->seq_num_m = 0;
> @@ -1462,6 +1464,16 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
>  	seq_num_v =
>  		drm_hdcp_be24_to_cpu((const u8 *)msgs.recvid_list.seq_num_v);
>  
> +	/*
> +	 * HDCP 2.2 Spec HDMI PAGE 19, DP PAGE 20
> +	 * HDCP 2.2 Comp 1B-06 test requires to disable encryption if there is
> +	 * non zero seq_num_V from recevier.
IMHO In commit message this kind of reasoning make sense, but here this is
not needed. As every line in the file will be as per the spec so we dont
need to call them out.
> +	 */
> +	if (hdcp->first_recvid_msg && seq_num_v) {
if (!hdcp->seq_num_v && seq_num_v) {

IMO This is all we need it.

-Ram
> +		drm_dbg_kms(&dev_priv->drm, "Non zero Seq_num_v at beginning of HDCP Session\n");
> +		return -EINVAL;
> +	}
> +
>  	if (seq_num_v < hdcp->seq_num_v) {
>  		/* Roll over of the seq_num_v from repeater. Reauthenticate. */
>  		DRM_DEBUG_KMS("Seq_num_v roll over.\n");
> @@ -1484,6 +1496,7 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
>  		return ret;
>  
>  	hdcp->seq_num_v = seq_num_v;
> +	hdcp->first_recvid_msg = false;
>  	ret = shim->write_2_2_msg(intel_dig_port, &msgs.rep_ack,
>  				  sizeof(msgs.rep_ack));
>  	if (ret < 0)
> -- 
> 2.24.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx