On Fri, 18 Mar 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Thu, Mar 17, 2022 at 09:02:46PM +0200, Jani Nikula wrote: >> On Thu, 17 Mar 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: >> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> > >> > 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@xxxxxxxxxxxxxxx> >> >> 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. That pretty much aligns with what my direction has been with the child devices. So gradually start throwing away stuff from i915->vbt, and instead have some accessors on the opaque list of bdb block entries. I guess it's going to be a lot of functions, but at least their names can be self-documenting and they can handle the VBT version differences cleanly. > >> 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. Agreed. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
