On Wed, 8 Nov 2006, Tyler Trafford wrote: > On Wed, Nov 08, 2006 at 02:09:36PM -0500, Robert Hardy wrote: > > On Tue, 7 Nov 2006, Tyler Trafford wrote: > >> On Nov 7, 2006, at 3:44 PM, Robert Hardy wrote: > >>> The most important modification is the code now no longer blindly > >>> writes all the data fw->size returns if it makes no sense to! > >> > >> Maybe I misunderstand, but it seems to write at most the requested > >> size, not fw->size. > > > > The point is you are missing here is that the circa 0.8.0 > > load_fw_direct only writes at most the requested "size" if "size" is > > properly set. I've seen many cases where "size" was larger than > > "fw->size", and both values were incorrect. In those cases the code > > blindly read and wrote "fw->size" worth of data, usually blowing the > > system out of the water in the process. > > Ah, when the firmware is too small (or read incompletely).
No, perhaps an example will help. Take the case where this was loading: v4l-cx2341x-init.mpg firmware (size=1048576 bytes;fw->size=376836 bytes) In this case the writel loop in load_fw_direct in the 0.8.0 branch would properly read and write it's way through the first 152*1024 bytes of data and then proceed to erroneously keep reading 221188 bytes of whatever data happened to be in memory after the firmware and overwrite that same amount memory at the destination, causing unpredictable results. > > Here are the "size" and "fw->size" values across a few reboots to > > illustrate what I'm talking about: > > > v4l-cx2341x-init.mpg firmware (size=1048576 bytes;fw->size=376836 bytes) > > I don't even know where this size value is coming from, the correct > value is hardcoded. (Not to mention that fw->size is showing the size > of the previous file read.) The first two calls to load_fw_direct are, correctly IMHO, hard coding size to be IVTV_FIRM_IMAGE_SIZE (or 256x1024 bytes.) The third call to load_fw_direct, was passing data[2] which was in theory seemed to be coming out of some decoder mailbox which, given the results, can obviously have garbage in it... Hard coding the third call too prevents this kind of problem. > > v4l-cx2341x-dec.fw firmware (size=262144 bytes;fw->size=262144 bytes) > > v4l-cx2341x-init.mpg firmware (size=155648 bytes;fw->size=262144 bytes) > > Same here with the previous fw->size being returned. It's like there > needs to be some locking around the firmware retrieval code or something... No question there is something foobar with fw->size but the bottom line is we can't and really shouldn't be trusting the contents of that memory location for determining the ranges for our memcpy/memcpy_toio or writel/iowrite32 loop. Looking at a larger data set I don't think it is as simple as fw->size sometimes returning the previous firmware's fw->size. Regards, Rob -- ---------------------"Happiness is understanding."---------------------- Robert Hardy, B.Eng Computer Systems C.E.O. Webcon Inc. rhardy <at> webcon <dot> ca GPG Key available (613) 276-7327 _______________________________________________ ivtv-devel mailing list [email protected] http://ivtvdriver.org/mailman/listinfo/ivtv-devel
