Re: [Intel-gfx] [PATCH v1] drm/i915/bdb: Fix version check

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 21 Sep 2021, Radosław Biernacki <rad@semihalf.com> wrote:
> - dropping stable
>
> ...
>
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/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];
>>

-- 
Jani Nikula, Intel Open Source Graphics Center