From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 179E9C433EF for ; Tue, 21 Sep 2021 10:27:43 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D0EF261038 for ; Tue, 21 Sep 2021 10:27:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D0EF261038 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9CD116E935; Tue, 21 Sep 2021 10:27:41 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id EA0F36E935; Tue, 21 Sep 2021 10:27:39 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10113"; a="286998067" X-IronPort-AV: E=Sophos;i="5.85,310,1624345200"; d="scan'208";a="286998067" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2021 03:27:39 -0700 X-IronPort-AV: E=Sophos;i="5.85,310,1624345200"; d="scan'208";a="549430025" Received: from unknown (HELO localhost) ([10.251.218.108]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2021 03:27:35 -0700 From: Jani Nikula To: =?utf-8?Q?Rados=C5=82aw?= Biernacki , "Souza\, Jose" Cc: "Lee\, Shawn C" , "lma\@semihalf.com" , "dri-devel\@lists.freedesktop.org" , "joonas.lahtinen\@linux.intel.com" , "intel-gfx\@lists.freedesktop.org" , "upstream\@semihalf.com" Subject: Re: [PATCH v1] drm/i915/bdb: Fix version check In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20210920141101.194959-1-lma@semihalf.com> <051f4a37e178d11c6dbcd05b5d6be28731cd7302.camel@intel.com> Date: Tue, 21 Sep 2021 13:27:32 +0300 Message-ID: <87bl4mte97.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, 21 Sep 2021, Rados=C5=82aw Biernacki wrote: > - dropping stable > > ... > >> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/g= pu/drm/i915/display/intel_vbt_defs.h >> > index 330077c2e588..fff456bf8783 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> > @@ -814,6 +814,11 @@ struct lfp_brightness_level { >> > u16 reserved; >> > } __packed; >> > >> > +/* >> > + * Changing struct bdb_lfp_backlight_data might affect its >> > + * size comparation to the value hold in BDB. >> > + * (e.g. in parse_lfp_backlight()) >> > + */ >> >> This is true for all the blocks so I don't think we need this comment. > > Lack of such comment was probable cause of this overlook. > As this is an example of the consequence (bricking platforms dependent > on mentioned conditions) IMO we need some comment here, or this will > probably happen again. The whole file is full of __packed structs with the sole purpose of parsing VBT data in memory. People are generally well aware of the consequences of changing the size, and this is the only such mistake I can recall. BR, Jani. > > >> >> > struct bdb_lfp_backlight_data { >> > u8 entry_size; >> > struct lfp_backlight_data_entry data[16]; >> --=20 Jani Nikula, Intel Open Source Graphics Center From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 035AEC433EF for ; Tue, 21 Sep 2021 10:27:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BC9C461038 for ; Tue, 21 Sep 2021 10:27:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BC9C461038 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4449D6E949; Tue, 21 Sep 2021 10:27:42 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id EA0F36E935; Tue, 21 Sep 2021 10:27:39 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10113"; a="286998067" X-IronPort-AV: E=Sophos;i="5.85,310,1624345200"; d="scan'208";a="286998067" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2021 03:27:39 -0700 X-IronPort-AV: E=Sophos;i="5.85,310,1624345200"; d="scan'208";a="549430025" Received: from unknown (HELO localhost) ([10.251.218.108]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2021 03:27:35 -0700 From: Jani Nikula To: =?utf-8?Q?Rados=C5=82aw?= Biernacki , "Souza\, Jose" Cc: "Lee\, Shawn C" , "lma\@semihalf.com" , "dri-devel\@lists.freedesktop.org" , "joonas.lahtinen\@linux.intel.com" , "intel-gfx\@lists.freedesktop.org" , "upstream\@semihalf.com" In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20210920141101.194959-1-lma@semihalf.com> <051f4a37e178d11c6dbcd05b5d6be28731cd7302.camel@intel.com> Date: Tue, 21 Sep 2021 13:27:32 +0300 Message-ID: <87bl4mte97.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Intel-gfx] [PATCH v1] drm/i915/bdb: Fix version check X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, 21 Sep 2021, Rados=C5=82aw Biernacki wrote: > - dropping stable > > ... > >> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/g= pu/drm/i915/display/intel_vbt_defs.h >> > index 330077c2e588..fff456bf8783 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> > @@ -814,6 +814,11 @@ struct lfp_brightness_level { >> > u16 reserved; >> > } __packed; >> > >> > +/* >> > + * Changing struct bdb_lfp_backlight_data might affect its >> > + * size comparation to the value hold in BDB. >> > + * (e.g. in parse_lfp_backlight()) >> > + */ >> >> This is true for all the blocks so I don't think we need this comment. > > Lack of such comment was probable cause of this overlook. > As this is an example of the consequence (bricking platforms dependent > on mentioned conditions) IMO we need some comment here, or this will > probably happen again. The whole file is full of __packed structs with the sole purpose of parsing VBT data in memory. People are generally well aware of the consequences of changing the size, and this is the only such mistake I can recall. BR, Jani. > > >> >> > struct bdb_lfp_backlight_data { >> > u8 entry_size; >> > struct lfp_backlight_data_entry data[16]; >> --=20 Jani Nikula, Intel Open Source Graphics Center