[Intel-gfx] [PATCH 02/11] drm/i915/bios: Make copies of VBT data blocks

[Ville Syrjälä]

On Thu, Mar 17, 2022 at 09:02:46PM +0200, Jani Nikula wrote:
> On Thu, 17 Mar 2022, Ville Syrjala <ville.syrjala at linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > Make a copy of each VB data block with a guaranteed minimum
> > size. The extra (if any) will just be left zeroed.
> 
> *VBT
> 
> >
> > This means we don't have to worry about going out of bounds
> > when accessing any of the structure members. Otherwise that
> > could easliy happen if we simply get the version check wrong,
> > or if the VBT is broken/malicious.
> 
> *easily
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> The high level question is if we really want to save the copies until
> driver remove instead of just during parsing. The lifetime should be
> mentioned in the commit message, with rationale if you have some.

Sure, I'll amend the commit msg a bit.

I think we at least must move away from the "parse VBT once at
driver init" idea because that won't ever let us deal with the
panel_type=0xff->check PnP ID thing. I think I even have a few
real world VBTs in my stash which have panel_type=0xff, so it's
not just some theoretical thing.

So we must hang onto the blocks at least until we've finished the
output setup. But I'm thinking we can just keep them permanently
and even start thinking about moving away from the "parse once,
stash results somewhere" mentality. Ie. we could just parse on
demand instead.

> I was wondering about making the copies up front instead of as needed,
> but that means setting up a list for the min sizes. It would clean up
> the usage (avoids passing around any pointers to original data to the
> parsers). Then you could use just find_section(i915, BDB_XXX). Dunno.

At least if we go for the parse on demand apporach then we definitely
want to do that. Avoids having to deal with kmalloc() fails/etc. while
parsing.

-- 
Ville Syrjälä
Intel