On Mon, May 06, 2019 at 06:56:03PM +0200, Daniel Vetter wrote: > On Thu, May 02, 2019 at 06:52:56PM +0530, Ramalingam C 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] > > > > Signed-off-by: Ramalingam C <ramalinga...@intel.com> > > Suggested-by: Daniel Vetter <dan...@ffwll.ch> > > Found a few small details to polish, but looks good to me. With the > details addressed: > > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> > > We also still need an ack on the firmware blob approach from Matt Roper or > someone else at iotg I think.
+Satyeshwar Satyeshwar's probably the best person from IOTG to give the Ack since he's part of the group that needs this functionality and is involved in the userspace/compositor side as well. Matt > > Cheers, Daniel > > > --- > > Documentation/gpu/drm-kms-helpers.rst | 6 + > > drivers/gpu/drm/Makefile | 2 +- > > drivers/gpu/drm/drm_hdcp.c | 342 ++++++++++++++++++++++++++ > > drivers/gpu/drm/drm_internal.h | 4 + > > drivers/gpu/drm/drm_sysfs.c | 2 + > > include/drm/drm_hdcp.h | 24 ++ > > 6 files changed, 379 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 > > ======================================= > > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > > index 72f5036d9bfa..dd02e9dec810 100644 > > --- a/drivers/gpu/drm/Makefile > > +++ b/drivers/gpu/drm/Makefile > > @@ -17,7 +17,7 @@ drm-y := drm_auth.o drm_cache.o \ > > drm_plane.o drm_color_mgmt.o drm_print.o \ > > drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ > > drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \ > > - drm_atomic_uapi.o > > + drm_atomic_uapi.o drm_hdcp.o > > > > drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o > > drm_dma.o drm_scatter.o drm_lock.o > > drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o > > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c > > new file mode 100644 > > index 000000000000..dc0e13409221 > > --- /dev/null > > +++ b/drivers/gpu/drm/drm_hdcp.c > > @@ -0,0 +1,342 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2019 Intel Corporation. > > + * > > + * Authors: > > + * Ramalingam C <ramalinga...@intel.com> > > + */ > > + > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/gfp.h> > > +#include <linux/export.h> > > +#include <linux/slab.h> > > +#include <linux/firmware.h> > > + > > +#include <drm/drm_hdcp.h> > > +#include <drm/drm_sysfs.h> > > +#include <drm/drm_print.h> > > +#include <drm/drm_device.h> > > + > > +struct hdcp_srm { > > + u8 *srm_buf; > > Allocated, but seems to not be used. > > > + size_t received_srm_sz; > > Seems to be unused. Seems to both be leftovers from the sysfs interface. > > > + 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", > > + ksv[0], ksv[1], ksv[2], ksv[3], ksv[4]); > > +} > > + > > +static u32 drm_hdcp_get_revoked_ksv_count(const u8 *buf, u32 vrls_length) > > +{ > > + u32 parsed_bytes = 0, ksv_count = 0, vrl_ksv_cnt, vrl_sz; > > + > > + while (parsed_bytes < vrls_length) { > > + vrl_ksv_cnt = *buf; > > + ksv_count += vrl_ksv_cnt; > > + > > + vrl_sz = (vrl_ksv_cnt * DRM_HDCP_KSV_LEN) + 1; > > + buf += vrl_sz; > > + parsed_bytes += vrl_sz; > > + } > > + > > + /* > > + * When vrls are not valid, ksvs are not considered. > > + * Hence SRM will be discarded. > > + */ > > + if (parsed_bytes != vrls_length) > > + ksv_count = 0; > > + > > + return ksv_count; > > +} > > + > > +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; > > + > > + do { > > + vrl_ksv_cnt = *buf; > > + vrl_ksv_sz = vrl_ksv_cnt * DRM_HDCP_KSV_LEN; > > + > > + buf++; > > + > > + DRM_DEBUG("vrl: %d, Revoked KSVs: %d\n", vrl_idx++, > > + vrl_ksv_cnt); > > + memcpy(revoked_ksv_list, 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); > > + } while (parsed_bytes < vrls_length); > > + > > + return ksv_count; > > +} > > + > > +static inline u32 get_vrl_length(const u8 *buf) > > +{ > > + return (u32)(buf[0] << 16 | buf[1] << 8 | buf[2]); > > This is the same conversion as drm_hdcp2_seq_num_to_u32. Maybe rename that > to drm_hdcp_be24_bytes_to_cpu and use that instead of what you have hare. > Plus rename the other function to drm_hdcp_u32_to_b24_bytes for > consistency. Probably best to do that in a small prep patch. > > +} > > + > > +static int drm_hdcp_parse_hdcp1_srm(const u8 *buf, size_t count) > > +{ > > + struct hdcp_srm_header *header; > > + u32 vrl_length, ksv_count; > > + > > + if (count < (sizeof(struct hdcp_srm_header) + > > + DRM_HDCP_1_4_VRL_LENGTH_SIZE + DRM_HDCP_1_4_DCP_SIG_SIZE)) { > > + DRM_ERROR("Invalid blob length\n"); > > + return -EINVAL; > > + } > > + > > + header = (struct hdcp_srm_header *)buf; > > + DRM_DEBUG("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n", > > + header->srm_id, > > + __swab16(header->srm_version), header->srm_gen_no); > > s/__swab16/be16_to_cpu/ everywhere. Otherwise it'll stop working on big > endian cpus. Yes won't matter, but better correct, it's also better > self-documenting code. > > > > + > > + WARN_ON(header->reserved); > > + > > + buf = buf + sizeof(*header); > > + vrl_length = get_vrl_length(buf); > > + if (count < (sizeof(struct hdcp_srm_header) + vrl_length) || > > + vrl_length < (DRM_HDCP_1_4_VRL_LENGTH_SIZE + > > + DRM_HDCP_1_4_DCP_SIG_SIZE)) { > > + DRM_ERROR("Invalid blob length or vrl length\n"); > > + return -EINVAL; > > + } > > + > > + /* Length of the all vrls combined */ > > + vrl_length -= (DRM_HDCP_1_4_VRL_LENGTH_SIZE + > > + DRM_HDCP_1_4_DCP_SIG_SIZE); > > + > > + if (!vrl_length) { > > + DRM_ERROR("No vrl found\n"); > > + return -EINVAL; > > + } > > + > > + buf += DRM_HDCP_1_4_VRL_LENGTH_SIZE; > > + ksv_count = drm_hdcp_get_revoked_ksv_count(buf, vrl_length); > > + if (!ksv_count) { > > + DRM_DEBUG("Revoked KSV count is 0\n"); > > + return count; > > + } > > + > > + 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) { > > + DRM_ERROR("Out of Memory\n"); > > + return -ENOMEM; > > + } > > + > > + if (drm_hdcp_get_revoked_ksvs(buf, srm_data->revoked_ksv_list, > > + vrl_length) != ksv_count) { > > + srm_data->revoked_ksv_cnt = 0; > > + kfree(srm_data->revoked_ksv_list); > > + return -EINVAL; > > + } > > + > > + srm_data->revoked_ksv_cnt = ksv_count; > > + return count; > > +} > > + > > +static int drm_hdcp_parse_hdcp2_srm(const u8 *buf, size_t count) > > +{ > > + struct hdcp_srm_header *header; > > + u32 vrl_length, ksv_count, ksv_sz; > > + > > + if (count < (sizeof(struct hdcp_srm_header) + > > + DRM_HDCP_2_VRL_LENGTH_SIZE + DRM_HDCP_2_DCP_SIG_SIZE)) { > > + DRM_ERROR("Invalid blob length\n"); > > + return -EINVAL; > > + } > > + > > + header = (struct hdcp_srm_header *)buf; > > + DRM_DEBUG("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n", > > + header->srm_id & DRM_HDCP_SRM_ID_MASK, > > + __swab16(header->srm_version), header->srm_gen_no); > > + > > + if (header->reserved) > > + return -EINVAL; > > + > > + buf = buf + sizeof(*header); > > + vrl_length = get_vrl_length(buf); > > + > > + if (count < (sizeof(struct hdcp_srm_header) + vrl_length) || > > + vrl_length < (DRM_HDCP_2_VRL_LENGTH_SIZE + > > + DRM_HDCP_2_DCP_SIG_SIZE)) { > > + DRM_ERROR("Invalid blob length or vrl length\n"); > > + return -EINVAL; > > + } > > + > > + /* Length of the all vrls combined */ > > + vrl_length -= (DRM_HDCP_2_VRL_LENGTH_SIZE + > > + DRM_HDCP_2_DCP_SIG_SIZE); > > + > > + if (!vrl_length) { > > + DRM_ERROR("No vrl found\n"); > > + return -EINVAL; > > + } > > + > > + buf += DRM_HDCP_2_VRL_LENGTH_SIZE; > > + 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; > > + } > > + > > + 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) { > > + DRM_ERROR("Out of Memory\n"); > > + return -ENOMEM; > > + } > > + > > + ksv_sz = ksv_count * DRM_HDCP_KSV_LEN; > > + 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); > > + > > + srm_data->revoked_ksv_cnt = ksv_count; > > + return count; > > +} > > + > > +static inline bool is_srm_version_hdcp1(const u8 *buf) > > +{ > > + return *buf == (u8)(DRM_HDCP_1_4_SRM_ID << 4); > > +} > > + > > +static inline bool is_srm_version_hdcp2(const u8 *buf) > > +{ > > + return *buf == (u8)(DRM_HDCP_2_SRM_ID << 4 | DRM_HDCP_2_INDICATOR); > > +} > > + > > +static ssize_t drm_hdcp_srm_update(const u8 *buf, size_t count) > > +{ > > + if (is_srm_version_hdcp1(buf)) > > + return (ssize_t)drm_hdcp_parse_hdcp1_srm(buf, count); > > + else if (is_srm_version_hdcp2(buf)) > > + return (ssize_t)drm_hdcp_parse_hdcp2_srm(buf, count); > > + > > + return (ssize_t)-EINVAL; > > You wire return values through but then throw them away. Plus there's a > confusion between int and ssize_t. Since you have DRM_DEBUG everywhere > already and we don't care about any parsing errors anyyway I think best to > throw the return values away and use void for all these functions. > > > +} > > + > > +void drm_hdcp_request_srm(struct drm_device *drm_dev) > > +{ > > + char fw_name[36] = "display_hdcp_srm.bin"; > > + const struct firmware *fw; > > + > > + int ret; > > + > > + ret = request_firmware_direct(&fw, (const char *)fw_name, > > + drm_dev->dev); > > + if (ret < 0) > > + goto exit; > > + > > + if (fw->size && fw->data) > > + drm_hdcp_srm_update(fw->data, fw->size); > > + > > +exit: > > + release_firmware(fw); > > +} > > + > > +/** > > + * 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. > > + */ > > +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; > > + > > + srm_data->srm_buf = kcalloc(DRM_HDCP_SRM_GEN1_MAX_BYTES, > > + sizeof(u8), GFP_KERNEL); > > + if (!srm_data->srm_buf) { > > + kfree(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->srm_buf); > > + 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 f243408ecf26..292998a2497f 100644 > > --- a/include/drm/drm_hdcp.h > > +++ b/include/drm/drm_hdcp.h > > @@ -265,4 +265,28 @@ void drm_hdcp2_u32_to_seq_num(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) > > This is surprising (I'd have expected the hdcp2 indicator in the low bits) > from reading the spec. But I guess you tested this, and there's some > mumbo-jumbo about "everything is big endian", which might also mean > nibbles. Who knows. > > > +#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; > > + u16 srm_version; > > Needs to be __be16, so that sparse can validate you got all the byte > swapping correct. Would be good to double-check with sparse yourself > (otherwise 0day will do it for you, but if there's a problem better to > catch it early). > > > + u8 srm_gen_no; > > +} __packed; > > + > > +struct drm_device; > > + > > +bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev, > > + u8 *ksvs, u32 ksv_count); > > #endif > > -- > > 2.19.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > 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 -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx