Re: [PATCH v8 08/35] drm/i915: Implement HDCP2.2 repeater authentication

From: Daniel Vetter <daniel@ffwll.ch>
To: Ramalingam C <ramalingam.c@intel.com>
Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org,
	tomas.winkler@intel.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v8 08/35] drm/i915: Implement HDCP2.2 repeater authentication
Date: Thu, 6 Dec 2018 11:45:49 +0100	[thread overview]
Message-ID: <20181206104549.GQ21184@phenom.ffwll.local> (raw)
In-Reply-To: <1543315413-24302-9-git-send-email-ramalingam.c@intel.com>

On Tue, Nov 27, 2018 at 04:13:06PM +0530, Ramalingam C wrote:
> Implements the HDCP2.2 repeaters authentication steps such as verifying
> the downstream topology and sending stream management information.
> 
> v2:
>   Rebased.
> v3:
>   No Changes.
> v4:
>   -EINVAL is returned for topology error and rollover scenario.
>   Endianness conversion func from drm_hdcp.h is used [Uma]
> v5:
>   Rebased as part of patches reordering.
>   Defined the mei service functions [Daniel]
> v6:
>   Redefined the mei service functions as per comp redesign.
> v7:
>   %s/uintxx_t/uxx
>   Check for comp_master is removed.
> v8:
>   Adjust to the new mei interface.
>   style issue fixed.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 125 +++++++++++++++++++++++++++++++++++++-
>  include/drm/drm_hdcp.h            |  15 +++++
>  2 files changed, 138 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 0d7fea9c9bb1..679f3c164582 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -1069,7 +1069,7 @@ static int hdcp2_prepare_skey(struct intel_connector *connector,
>  	return ret;
>  }
>  
> -static __attribute__((unused)) int
> +static int
>  hdcp2_verify_rep_topology_prepare_ack(struct intel_connector *connector,
>  				      struct hdcp2_rep_send_receiverid_list
>  								*rep_topology,
> @@ -1099,7 +1099,7 @@ hdcp2_verify_rep_topology_prepare_ack(struct intel_connector *connector,
>  	return ret;
>  }
>  
> -static __attribute__((unused)) int
> +static int
>  hdcp2_verify_mprime(struct intel_connector *connector,
>  		    struct hdcp2_rep_stream_ready *stream_ready)
>  {
> @@ -1315,6 +1315,121 @@ static int hdcp2_session_key_exchange(struct intel_connector *connector)
>  	return 0;
>  }
>  
> +static
> +int hdcp2_propagate_stream_management_info(struct intel_connector *connector)
> +{
> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> +	struct intel_hdcp *hdcp = &connector->hdcp;
> +	union {
> +		struct hdcp2_rep_stream_manage stream_manage;
> +		struct hdcp2_rep_stream_ready stream_ready;
> +	} msgs;
> +	const struct intel_hdcp_shim *shim = hdcp->shim;
> +	int ret;
> +
> +	/* Prepare RepeaterAuth_Stream_Manage msg */
> +	msgs.stream_manage.msg_id = HDCP_2_2_REP_STREAM_MANAGE;
> +	reverse_endianness(msgs.stream_manage.seq_num_m, HDCP_2_2_SEQ_NUM_LEN,
> +			   (u8 *)&hdcp->seq_num_m);
> +
> +	/* K no of streams is fixed as 1. Stored as big-endian. */
> +	msgs.stream_manage.k = __swab16(1);
> +
> +	/* For HDMI this is forced to be 0x0. For DP SST also this is 0x0. */
> +	msgs.stream_manage.streams[0].stream_id = 0;
> +	msgs.stream_manage.streams[0].stream_type = hdcp->content_type;
> +
> +	/* Send it to Repeater */
> +	ret = shim->write_2_2_msg(intel_dig_port, &msgs.stream_manage,
> +				  sizeof(msgs.stream_manage));
> +	if (ret < 0)
> +		return ret;
> +
> +	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;
> +
> +	hdcp->mei_data.seq_num_m = hdcp->seq_num_m;
> +	hdcp->mei_data.streams[0].stream_type = hdcp->content_type;
> +
> +	ret = hdcp2_verify_mprime(connector, &msgs.stream_ready);
> +	if (ret < 0)
> +		return ret;
> +
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
> +static
> +int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
> +{
> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> +	struct intel_hdcp *hdcp = &connector->hdcp;
> +	union {
> +		struct hdcp2_rep_send_receiverid_list recvid_list;
> +		struct hdcp2_rep_send_ack rep_ack;
> +	} msgs;
> +	const struct intel_hdcp_shim *shim = hdcp->shim;
> +	u8 *rx_info;
> +	u32 seq_num_v;
> +	int ret;
> +
> +	ret = shim->read_2_2_msg(intel_dig_port, HDCP_2_2_REP_SEND_RECVID_LIST,
> +				 &msgs.recvid_list, sizeof(msgs.recvid_list));
> +	if (ret < 0)
> +		return ret;
> +
> +	rx_info = msgs.recvid_list.rx_info;
> +
> +	if (HDCP_2_2_MAX_CASCADE_EXCEEDED(rx_info[1]) ||
> +	    HDCP_2_2_MAX_DEVS_EXCEEDED(rx_info[1])) {
> +		DRM_DEBUG_KMS("Topology Max Size Exceeded\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Converting and Storing the seq_num_v to local variable as DWORD */
> +	reverse_endianness((u8 *)&seq_num_v, HDCP_2_2_SEQ_NUM_LEN,
> +			   msgs.recvid_list.seq_num_v);
> +
> +	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");
> +		return -EINVAL;
> +	}
> +
> +	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
> +						    &msgs.recvid_list,
> +						    &msgs.rep_ack);
> +	if (ret < 0)
> +		return ret;
> +
> +	hdcp->seq_num_v = seq_num_v;
> +	ret = shim->write_2_2_msg(intel_dig_port, &msgs.rep_ack,
> +				  sizeof(msgs.rep_ack));
> +	if (ret < 0)
> +		return ret;
> +
> +	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 = conn_to_dig_port(connector);
> @@ -1356,6 +1471,12 @@ static int hdcp2_authenticate_sink(struct intel_connector *connector)
>  					       sizeof(stream_type_msg));
>  		if (ret < 0)
>  			return ret;
> +	} else if (hdcp->is_repeater) {
> +		ret = hdcp2_authenticate_repeater(connector);
> +		if (ret < 0) {
> +			DRM_DEBUG_KMS("Repeater Auth Failed. Err: %d\n", ret);
> +			return ret;
> +		}
>  	}
>  
>  	hdcp->mei_data.streams[0].stream_type = hdcp->content_type;
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index a6de09c5e47f..3e45a7618ab3 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -250,4 +250,19 @@ struct hdcp2_dp_errata_stream_type {
>  #define HDCP_2_2_HDMI_RXSTATUS_READY(x)		((x) & BIT(2))
>  #define HDCP_2_2_HDMI_RXSTATUS_REAUTH_REQ(x)	((x) & BIT(3))
>  
> +/*
> + * Library functions for endianness are aligned for 16/32/64 bits.
> + * But hdcp sequence numbers are 24bits. So for their Byte swapping,
> + * a conversion function is developed.
> + */
> +static inline void reverse_endianness(u8 *dest, size_t sz, u8 *src)

This feels a bit fake-generic and doesn't fit that well into what you want
it to do in the code actually. What about:

u32 drm_hdcp2_seq_num_to_u32(u8 seq_num[HDCP_2_2_SEQ_NUM_LEN])
{
	return seq_num[2] | seq_num[1] << 8 | seq_num[0] << 16;
}

(I didn't check the math).

I think this would both make the code more clearer and avoid the problem
of "shouldn't we make sure this works for everything and put it into
linux/string.h?"

Again I'll check the actual hdcp2 flow later on.

In general I'm kinda not happy with the shim midlayer design, but I wasn't
happy with that with hdcp1 either, and hdcp2 is just an extension, and I
don't really have a better idea either so seems all good to me.
-Daniel

> +{
> +	u32 index;
> +
> +	if (!sz || sz > sizeof(u32))
> +		return;
> +	for (index = 0; index < sz; index++)
> +		dest[sz - index - 1] = src[index];
> +}
> +
>  #endif
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx