Re: [PATCH 07/10] drm/i915: Allow 27 bytes child_dev for VBT <109

Jani Nikula
2015-12-16T08:58:12Z

On Mon, 14 Dec 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My 85x has VBT version 108 which has a child dev size of 27 bytes.
> Let's allow that without printing an error.
>
> We still want to reject the actual parsin since for that we need
> the child device size to be at least 33 bytes. So we should still
> check for that, but let's make it print a debug message only instead
> of an error.
>
> While at it, toss in a BUILD_BUG_ON() to verify our struct
> old_child_dev_config is in fact 33 bytes.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This is scary stuff, but

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

as far as the code goes. I don't have the history books to check this
against.

> ---
> drivers/gpu/drm/i915/intel_bios.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 070470fe9a91..770b825dabc0 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1089,7 +1089,10 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
> return;
> }
> - if (bdb->version < 195) {
> + if (bdb->version < 109) {
> + expected_size = 27;
> + } else if (bdb->version < 195) {
> + BUILD_BUG_ON(sizeof(struct old_child_dev_config) != 33);
> expected_size = sizeof(struct old_child_dev_config);
> } else if (bdb->version == 195) {
> expected_size = 37;
> @@ -1102,18 +1105,18 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> bdb->version, expected_size);
> }
>
> - /* The legacy sized child device config is the minimum we need. */
> - if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
> - DRM_ERROR("Child device config size %u is too small.\n",
> - p_defs->child_dev_size);
> - return;
> - }
> -
> /* Flag an error for unexpected size, but continue anyway. */
> if (p_defs->child_dev_size != expected_size)
> DRM_ERROR("Unexpected child device config size %u (expected %u for VBT version %u)\n",
> p_defs->child_dev_size, expected_size, bdb->version);
>
> + /* The legacy sized child device config is the minimum we need. */
> + if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
> + DRM_DEBUG_KMS("Child device config size %u is too small.\n",
> + p_defs->child_dev_size);
> + return;
> + }
> +
> /* get the block size of general definitions */
> block_size = get_blocksize(p_defs);
> /* get the number of child device */

--
Jani Nikula, Intel Open Source Technology Center