Re: [PATCH v7 04/11] drm: revocation check at drm subsystem

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7644 bytes --]

On Tue,  7 May 2019 21:57:38 +0530
Ramalingam C <ramalingam.c@intel.com> wrote:

> On every hdcp revocation check request SRM is read from fw file
> /lib/firmware/display_hdcp_srm.bin
> 
> SRM table is parsed and stored at drm_hdcp.c, with functions exported
> for the services for revocation check from drivers (which
> implements the HDCP authentication)
> 
> This patch handles the HDCP1.4 and 2.2 versions of SRM table.
> 
> v2:
>   moved the uAPI to request_firmware_direct() [Daniel]
> v3:
>   kdoc added. [Daniel]
>   srm_header unified and bit field definitions are removed. [Daniel]
>   locking improved. [Daniel]
>   vrl length violation is fixed. [Daniel]
> v4:
>   s/__swab16/be16_to_cpu [Daniel]
>   be24_to_cpu is done through a global func [Daniel]
>   Unused variables are removed. [Daniel]
>   unchecked return values are dropped from static funcs [Daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Acked-by: Satyeshwar Singh <satyeshwar.singh@intel.com>
> Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
> ---
>  Documentation/gpu/drm-kms-helpers.rst |   6 +
>  drivers/gpu/drm/Makefile              |   2 +-
>  drivers/gpu/drm/drm_hdcp.c            | 333 ++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_internal.h        |   4 +
>  drivers/gpu/drm/drm_sysfs.c           |   2 +
>  include/drm/drm_hdcp.h                |  24 ++
>  6 files changed, 370 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_hdcp.c
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> index 14102ae035dc..0fe726a6ee67 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -181,6 +181,12 @@ Panel Helper Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
>     :export:
>  
> +HDCP Helper Functions Reference
> +===============================
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_hdcp.c
> +   :export:
> +
>  Display Port Helper Functions Reference
>  =======================================
>  

...

> +/**
> + * drm_hdcp_check_ksvs_revoked - Check the revoked status of the IDs
> + *
> + * @drm_dev: drm_device for which HDCP revocation check is requested
> + * @ksvs: List of KSVs (HDCP receiver IDs)
> + * @ksv_count: KSV count passed in through @ksvs
> + *
> + * This function reads the HDCP System renewability Message(SRM Table)
> + * from userspace as a firmware and parses it for the revoked HDCP
> + * KSVs(Receiver IDs) detected by DCP LLC. Once the revoked KSVs are known,
> + * revoked state of the KSVs in the list passed in by display drivers are
> + * decided and response is sent.
> + *
> + * SRM should be presented in the name of "display_hdcp_srm.bin".
> + *
> + * Returns:
> + * TRUE on any of the KSV is revoked, else FALSE.

Hi,

this does not seem to be specifying the file format. Since this file is
expected to be provided by vendors and not the driver developers AFAIU,
I think the file format counts as UAPI and needs to be explicitly and
unambiguously specified. Especially as the file or the file format are
not tied to a specific DRM driver or any driver. A searchable reference
to a particular revision of a public specification document could
suffice if such exists.

This doc comment is only kernel internal API. I would also expect UAPI
documentation for the same reason as above.

The Weston work[1] does not validate the UAPI added in this patch.


[1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/48

Thanks,
pq

> + */
> +bool drm_hdcp_check_ksvs_revoked(struct drm_device *drm_dev, u8 *ksvs,
> +				 u32 ksv_count)
> +{
> +	u32 rev_ksv_cnt, cnt, i, j;
> +	u8 *rev_ksv_list;
> +
> +	if (!srm_data)
> +		return false;
> +
> +	mutex_lock(&srm_data->mutex);
> +	drm_hdcp_request_srm(drm_dev);
> +
> +	rev_ksv_cnt = srm_data->revoked_ksv_cnt;
> +	rev_ksv_list = srm_data->revoked_ksv_list;
> +
> +	/* If the Revoked ksv list is empty */
> +	if (!rev_ksv_cnt || !rev_ksv_list) {
> +		mutex_unlock(&srm_data->mutex);
> +		return false;
> +	}
> +
> +	for  (cnt = 0; cnt < ksv_count; cnt++) {
> +		rev_ksv_list = srm_data->revoked_ksv_list;
> +		for (i = 0; i < rev_ksv_cnt; i++) {
> +			for (j = 0; j < DRM_HDCP_KSV_LEN; j++)
> +				if (ksvs[j] != rev_ksv_list[j]) {
> +					break;
> +				} else if (j == (DRM_HDCP_KSV_LEN - 1)) {
> +					DRM_DEBUG("Revoked KSV is ");
> +					drm_hdcp_print_ksv(ksvs);
> +					mutex_unlock(&srm_data->mutex);
> +					return true;
> +				}
> +			/* Move the offset to next KSV in the revoked list */
> +			rev_ksv_list += DRM_HDCP_KSV_LEN;
> +		}
> +
> +		/* Iterate to next ksv_offset */
> +		ksvs += DRM_HDCP_KSV_LEN;
> +	}
> +	mutex_unlock(&srm_data->mutex);
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(drm_hdcp_check_ksvs_revoked);
> +
> +int drm_setup_hdcp_srm(struct class *drm_class)
> +{
> +	srm_data = kzalloc(sizeof(*srm_data), GFP_KERNEL);
> +	if (!srm_data)
> +		return -ENOMEM;
> +	mutex_init(&srm_data->mutex);
> +
> +	return 0;
> +}
> +
> +void drm_teardown_hdcp_srm(struct class *drm_class)
> +{
> +	if (srm_data) {
> +		kfree(srm_data->revoked_ksv_list);
> +		kfree(srm_data);
> +	}
> +}
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index e19ac7ca602d..476a422414f6 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -201,3 +201,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>  void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
>  				const struct drm_framebuffer *fb);
>  int drm_framebuffer_debugfs_init(struct drm_minor *minor);
> +
> +/* drm_hdcp.c */
> +int drm_setup_hdcp_srm(struct class *drm_class);
> +void drm_teardown_hdcp_srm(struct class *drm_class);
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ecb7b33002bb..18b1ac442997 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -78,6 +78,7 @@ int drm_sysfs_init(void)
>  	}
>  
>  	drm_class->devnode = drm_devnode;
> +	drm_setup_hdcp_srm(drm_class);
>  	return 0;
>  }
>  
> @@ -90,6 +91,7 @@ void drm_sysfs_destroy(void)
>  {
>  	if (IS_ERR_OR_NULL(drm_class))
>  		return;
> +	drm_teardown_hdcp_srm(drm_class);
>  	class_remove_file(drm_class, &class_attr_version.attr);
>  	class_destroy(drm_class);
>  	drm_class = NULL;
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 1cc66df05a43..2f0335d0a50f 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -265,4 +265,28 @@ void drm_hdcp_cpu_to_be24(u8 seq_num[HDCP_2_2_SEQ_NUM_LEN], u32 val)
>  	seq_num[2] = val;
>  }
>  
> +#define DRM_HDCP_SRM_GEN1_MAX_BYTES		(5 * 1024)
> +#define DRM_HDCP_1_4_SRM_ID			0x8
> +#define DRM_HDCP_SRM_ID_MASK			(0xF << 4)
> +#define DRM_HDCP_1_4_VRL_LENGTH_SIZE		3
> +#define DRM_HDCP_1_4_DCP_SIG_SIZE		40
> +#define DRM_HDCP_2_SRM_ID			0x9
> +#define DRM_HDCP_2_INDICATOR			0x1
> +#define DRM_HDCP_2_INDICATOR_MASK		0xF
> +#define DRM_HDCP_2_VRL_LENGTH_SIZE		3
> +#define DRM_HDCP_2_DCP_SIG_SIZE			384
> +#define DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ	4
> +#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte)	(((byte) & 0xC) >> 6)
> +
> +struct hdcp_srm_header {
> +	u8 srm_id;
> +	u8 reserved;
> +	__be16 srm_version;
> +	u8 srm_gen_no;
> +} __packed;
> +
> +struct drm_device;
> +
> +bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
> +				 u8 *ksvs, u32 ksv_count);
>  #endif


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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