On Fri, Jul 03, 2015 at 06:35:55PM +0300, Ville Syrjälä wrote: > On Fri, Jul 03, 2015 at 04:25:15PM +0100, Damien Lespiau wrote: > > On Fri, Jul 03, 2015 at 06:21:58PM +0300, Ville Syrjälä wrote: > > > > In the old VBT spec I have, each child_dev_config is supposed to have > > > > only 33 bytes. But in this patch you're increasing it to 38. I believe > > > > this is what's causing the errors I see when I boot my BDW. > > > > > > > > Are you sure they increased the VBT's ChildDevInfo to more than 33 > > > > bytes? I don't have access to your VBT spec right now, so I can't do a > > > > proper review or a suggestion on how to fix the problem. > > > > > > The size depends on the version. On BSW last I looked it was maybe 36 or > > > 37 bytes. > > > > > > I'm not sure we want to change common_child_dev_config since it's meant > > > to be some kind of common stuff. So maybe add another member into the > > > union for this stuff. Of course that still incrases the size of the union > > > so other changes are required to make it work. > > > > > > Fortunately we copy these things out from the VBT, so I think just > > > making sure the memcpy() doesn't try to copy too much and fixing the size > > > check should be all that's needed. That would leave any extra we didn't > > > fill with the copy zeroed, and if having it zeroed isn't good enough > > > we can of course sprinkle more version checks around. > > > > > > And I guess we should still check that the size is at least 33 bytes > > > since that was the official size for the longest time. > > > > So child_dev_size lies to us? > > No, it's correct. But we require it to be at least as big as our > union child_device_config, which the patch changes from 33 to 37 > bytes. Tbh I'd like us to be more paranoid about vbt, i.e. if they change struct sizes I think we should have an if ladder (checking vbt versions/platforms) to make sure that our expectation of the size matches reality in all cases. Ofc that should be a separate patch to start this series. Given the state of vbt revisions handling and documentation I don't think we can opt for too much safety checks here, sadly. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
