On Wed, Feb 12, 2020 at 03:59:38PM +0530, Ramalingam C wrote:
> As we are not using the sysfs infrastructure anymore, link to it is
> removed. And global srm data and mutex to protect it are removed,
> with required handling at revocation check function.
> 
> v2:
>   srm_data is dropped and few more comments are addressed.
> v3:
>   ptr passing around is fixed with functional testing.
> v4:
>   fix htmldoc [lkp]
> 
> Signed-off-by: Ramalingam C <ramalinga...@intel.com>

I think we could probably improve things even further, but this is a good start.
Thanks for doing this!


Reviewed-by: Sean Paul <s...@poorly.run>


> Suggested-by: Sean Paul <seanp...@chromium.org>
> ---
>  drivers/gpu/drm/drm_hdcp.c     | 158 ++++++++++++---------------------
>  drivers/gpu/drm/drm_internal.h |   4 -
>  drivers/gpu/drm/drm_sysfs.c    |   2 -
>  include/drm/drm_hdcp.h         |   4 +-
>  4 files changed, 61 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index 9191633a3c43..7f386adcf872 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -23,14 +23,6 @@
>  
>  #include "drm_internal.h"
>  
> -static struct hdcp_srm {
> -     u32 revoked_ksv_cnt;
> -     u8 *revoked_ksv_list;
> -
> -     /* Mutex to protect above struct member */
> -     struct mutex mutex;
> -} *srm_data;
> -
>  static inline void drm_hdcp_print_ksv(const u8 *ksv)
>  {
>       DRM_DEBUG("\t%#02x, %#02x, %#02x, %#02x, %#02x\n",
> @@ -60,11 +52,11 @@ static u32 drm_hdcp_get_revoked_ksv_count(const u8 *buf, 
> u32 vrls_length)
>       return ksv_count;
>  }
>  
> -static u32 drm_hdcp_get_revoked_ksvs(const u8 *buf, u8 *revoked_ksv_list,
> +static u32 drm_hdcp_get_revoked_ksvs(const u8 *buf, u8 **revoked_ksv_list,
>                                    u32 vrls_length)
>  {
> -     u32 parsed_bytes = 0, ksv_count = 0;
>       u32 vrl_ksv_cnt, vrl_ksv_sz, vrl_idx = 0;
> +     u32 parsed_bytes = 0, ksv_count = 0;
>  
>       do {
>               vrl_ksv_cnt = *buf;
> @@ -74,10 +66,10 @@ static u32 drm_hdcp_get_revoked_ksvs(const u8 *buf, u8 
> *revoked_ksv_list,
>  
>               DRM_DEBUG("vrl: %d, Revoked KSVs: %d\n", vrl_idx++,
>                         vrl_ksv_cnt);
> -             memcpy(revoked_ksv_list, buf, vrl_ksv_sz);
> +             memcpy((*revoked_ksv_list) + (ksv_count * DRM_HDCP_KSV_LEN),
> +                    buf, vrl_ksv_sz);
>  
>               ksv_count += vrl_ksv_cnt;
> -             revoked_ksv_list += vrl_ksv_sz;
>               buf += vrl_ksv_sz;
>  
>               parsed_bytes += (vrl_ksv_sz + 1);
> @@ -91,7 +83,8 @@ static inline u32 get_vrl_length(const u8 *buf)
>       return drm_hdcp_be24_to_cpu(buf);
>  }
>  
> -static int drm_hdcp_parse_hdcp1_srm(const u8 *buf, size_t count)
> +static int drm_hdcp_parse_hdcp1_srm(const u8 *buf, size_t count,
> +                                 u8 **revoked_ksv_list, u32 *revoked_ksv_cnt)
>  {
>       struct hdcp_srm_header *header;
>       u32 vrl_length, ksv_count;
> @@ -131,29 +124,28 @@ static int drm_hdcp_parse_hdcp1_srm(const u8 *buf, 
> size_t count)
>       ksv_count = drm_hdcp_get_revoked_ksv_count(buf, vrl_length);
>       if (!ksv_count) {
>               DRM_DEBUG("Revoked KSV count is 0\n");
> -             return count;
> +             return 0;
>       }
>  
> -     kfree(srm_data->revoked_ksv_list);
> -     srm_data->revoked_ksv_list = kcalloc(ksv_count, DRM_HDCP_KSV_LEN,
> -                                          GFP_KERNEL);
> -     if (!srm_data->revoked_ksv_list) {
> +     *revoked_ksv_list = kcalloc(ksv_count, DRM_HDCP_KSV_LEN, GFP_KERNEL);
> +     if (!*revoked_ksv_list) {
>               DRM_ERROR("Out of Memory\n");
>               return -ENOMEM;
>       }
>  
> -     if (drm_hdcp_get_revoked_ksvs(buf, srm_data->revoked_ksv_list,
> +     if (drm_hdcp_get_revoked_ksvs(buf, revoked_ksv_list,
>                                     vrl_length) != ksv_count) {
> -             srm_data->revoked_ksv_cnt = 0;
> -             kfree(srm_data->revoked_ksv_list);
> +             *revoked_ksv_cnt = 0;
> +             kfree(*revoked_ksv_list);
>               return -EINVAL;
>       }
>  
> -     srm_data->revoked_ksv_cnt = ksv_count;
> -     return count;
> +     *revoked_ksv_cnt = ksv_count;
> +     return 0;
>  }
>  
> -static int drm_hdcp_parse_hdcp2_srm(const u8 *buf, size_t count)
> +static int drm_hdcp_parse_hdcp2_srm(const u8 *buf, size_t count,
> +                                 u8 **revoked_ksv_list, u32 *revoked_ksv_cnt)
>  {
>       struct hdcp_srm_header *header;
>       u32 vrl_length, ksv_count, ksv_sz;
> @@ -195,13 +187,11 @@ static int drm_hdcp_parse_hdcp2_srm(const u8 *buf, 
> size_t count)
>       ksv_count = (*buf << 2) | DRM_HDCP_2_KSV_COUNT_2_LSBITS(*(buf + 1));
>       if (!ksv_count) {
>               DRM_DEBUG("Revoked KSV count is 0\n");
> -             return count;
> +             return 0;
>       }
>  
> -     kfree(srm_data->revoked_ksv_list);
> -     srm_data->revoked_ksv_list = kcalloc(ksv_count, DRM_HDCP_KSV_LEN,
> -                                          GFP_KERNEL);
> -     if (!srm_data->revoked_ksv_list) {
> +     *revoked_ksv_list = kcalloc(ksv_count, DRM_HDCP_KSV_LEN, GFP_KERNEL);
> +     if (!*revoked_ksv_list) {
>               DRM_ERROR("Out of Memory\n");
>               return -ENOMEM;
>       }
> @@ -210,10 +200,10 @@ static int drm_hdcp_parse_hdcp2_srm(const u8 *buf, 
> size_t count)
>       buf += DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ;
>  
>       DRM_DEBUG("Revoked KSVs: %d\n", ksv_count);
> -     memcpy(srm_data->revoked_ksv_list, buf, ksv_sz);
> +     memcpy(*revoked_ksv_list, buf, ksv_sz);
>  
> -     srm_data->revoked_ksv_cnt = ksv_count;
> -     return count;
> +     *revoked_ksv_cnt = ksv_count;
> +     return 0;
>  }
>  
>  static inline bool is_srm_version_hdcp1(const u8 *buf)
> @@ -226,22 +216,27 @@ static inline bool is_srm_version_hdcp2(const u8 *buf)
>       return *buf == (u8)(DRM_HDCP_2_SRM_ID << 4 | DRM_HDCP_2_INDICATOR);
>  }
>  
> -static void drm_hdcp_srm_update(const u8 *buf, size_t count)
> +static int drm_hdcp_srm_update(const u8 *buf, size_t count,
> +                            u8 **revoked_ksv_list, u32 *revoked_ksv_cnt)
>  {
>       if (count < sizeof(struct hdcp_srm_header))
> -             return;
> +             return -EINVAL;
>  
>       if (is_srm_version_hdcp1(buf))
> -             drm_hdcp_parse_hdcp1_srm(buf, count);
> +             return drm_hdcp_parse_hdcp1_srm(buf, count, revoked_ksv_list,
> +                                             revoked_ksv_cnt);
>       else if (is_srm_version_hdcp2(buf))
> -             drm_hdcp_parse_hdcp2_srm(buf, count);
> +             return drm_hdcp_parse_hdcp2_srm(buf, count, revoked_ksv_list,
> +                                             revoked_ksv_cnt);
> +     else
> +             return -EINVAL;
>  }
>  
> -static void drm_hdcp_request_srm(struct drm_device *drm_dev)
> +static int drm_hdcp_request_srm(struct drm_device *drm_dev,
> +                             u8 **revoked_ksv_list, u32 *revoked_ksv_cnt)
>  {
>       char fw_name[36] = "display_hdcp_srm.bin";
>       const struct firmware *fw;
> -
>       int ret;
>  
>       ret = request_firmware_direct(&fw, (const char *)fw_name,
> @@ -250,10 +245,12 @@ static void drm_hdcp_request_srm(struct drm_device 
> *drm_dev)
>               goto exit;
>  
>       if (fw->size && fw->data)
> -             drm_hdcp_srm_update(fw->data, fw->size);
> +             ret = drm_hdcp_srm_update(fw->data, fw->size, revoked_ksv_list,
> +                                       revoked_ksv_cnt);
>  
>  exit:
>       release_firmware(fw);
> +     return ret;
>  }
>  
>  /**
> @@ -279,71 +276,34 @@ static void drm_hdcp_request_srm(struct drm_device 
> *drm_dev)
>   * 
> https://www.digital-cp.com/sites/default/files/specifications/HDCP%20on%20HDMI%20Specification%20Rev2_2_Final1.pdf
>   *
>   * Returns:
> - * TRUE on any of the KSV is revoked, else FALSE.
> + * Count of the revoked KSVs or -ve error number incase of the failure.
>   */
> -bool drm_hdcp_check_ksvs_revoked(struct drm_device *drm_dev, u8 *ksvs,
> -                              u32 ksv_count)
> +int 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;
> +     u32 revoked_ksv_cnt = 0, i, j;
> +     u8 *revoked_ksv_list = NULL;
> +     int ret = 0;
> +
> +     ret = drm_hdcp_request_srm(drm_dev, &revoked_ksv_list,
> +                                &revoked_ksv_cnt);
> +
> +     /* revoked_ksv_cnt will be zero when above function failed */
> +     for (i = 0; i < revoked_ksv_cnt; i++)
> +             for  (j = 0; j < ksv_count; j++)
> +                     if (!memcmp(&ksvs[j * DRM_HDCP_KSV_LEN],
> +                                 &revoked_ksv_list[i * DRM_HDCP_KSV_LEN],
> +                                 DRM_HDCP_KSV_LEN)) {
> +                             DRM_DEBUG("Revoked KSV is ");
> +                             drm_hdcp_print_ksv(&ksvs[j * DRM_HDCP_KSV_LEN]);
> +                             ret++;
> +                     }
> +
> +     kfree(revoked_ksv_list);
> +     return ret;
>  }
>  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);
> -     }
> -}
> -
>  static struct drm_prop_enum_list drm_cp_enum_list[] = {
>       { DRM_MODE_CONTENT_PROTECTION_UNDESIRED, "Undesired" },
>       { DRM_MODE_CONTENT_PROTECTION_DESIRED, "Desired" },
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index aeec2e68d772..5714a78365ac 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -236,7 +236,3 @@ 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 dd2bc85f43cc..2e83c3d72af9 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -85,7 +85,6 @@ int drm_sysfs_init(void)
>       }
>  
>       drm_class->devnode = drm_devnode;
> -     drm_setup_hdcp_srm(drm_class);
>       return 0;
>  }
>  
> @@ -98,7 +97,6 @@ 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 06a11202a097..d512089b873f 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -288,8 +288,8 @@ struct hdcp_srm_header {
>  struct drm_device;
>  struct drm_connector;
>  
> -bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
> -                              u8 *ksvs, u32 ksv_count);
> +int drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
> +                             u8 *ksvs, u32 ksv_count);
>  int drm_connector_attach_content_protection_property(
>               struct drm_connector *connector, bool hdcp_content_type);
>  void drm_hdcp_update_content_protection(struct drm_connector *connector,
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to