On Monday, October 24, 2011 12:25:09 pm Andriy Gapon wrote: > on 24/10/2011 18:33 John Baldwin said the following: > > On Monday, October 24, 2011 9:47:42 am Andriy Gapon wrote: > >> on 24/10/2011 16:41 John Baldwin said the following: > >>> On Sunday, October 23, 2011 1:57:59 pm Andriy Gapon wrote: > [snip] > >>>> I found a document that suggests a possibility of BIOS writing more > >>>> bytes to the > >>>> array than its current size of 0x42: > >>>> http://www.t13.org/documents/UploadedDocuments/docs2008/e08134r1-BIOS_Enhanced_Disk_Drive_Services_4.0.pdf > >>>> > >>>> Of course, the size of the array is passed to BIOS at the start of the > >>>> array and > >>>> so a _non-buggy_ BIOS should not write beyond the array, but we live in a > >>>> non-perfect world. > >>> > >>> Hmm, I think we've had to do a similar workaround in the past for a > >>> different > >>> BIOS call (SMAP maybe?). However, I do have one request, can we declare > >>> an > >>> actual structure instead of a silly char array? Then we can remove the > >>> weird > >>> casts with offsets into it, etc. Having an actual struct would be far > >>> more > >>> readable and less bug-prone. > >>> > >> > >> I am all for this. > >> Unfortunately. ENOTIME to do this properly at the moment. > > > > Ugh, it looks like in EDD 4.0 they silently expanded the path field to 16 > > bytes > > instead of 8 as in EDD 3.0 which is why the HP BIOS is probably writing an > > extra > > four bytes. > > Yes, that's exactly what I meant above, but probably wasn't clear about that. > > > Ah, so the deal is that the device path bit is variable length and the > > caller is > > supposed to set the length on input which we aren't doing. However, we > > don't > > care about the device path stuff anyway, so we can set a smaller input > > value. > > > > Perhaps try http://www.freebsd.org/~jhb/patches/edd_params.patch > > > > > +struct edd_params { > > + uint16_t len; > > + uint16_t flags; > > + uint32_t cylinders; > > + uint32_t heads; > > + uint32_t sectors_per_track; > > + uint64_t sectors; > > + uint32_t sector_size; > > sector_size should be uint16_t, I think. Ditto for edd_params_v3 and > edd_params_v4. > sizeof(struct edd_params) should be 30, judging from the specs.
Oops, yeah. > > + uint16_t edd_params_seg; > > + uint16_t edd_params_off; > > +}; > > Perhaps the structures should be declared __packed (if only just in case)? > > Also, perhaps edd_params_v3 and edd_params_v4 should inherit edd_params in > some > "smarter" way to avoid verbatim duplicates. Yeah, probably so. We will probably never even use them anyway (just as we don't use edd_packet_v3 even though we could probably make use of it to avoid some bounce buffering in the loader). > Other than these issues the patch looks great! > Perhaps later we could do detailed definitions for things like interface > paths for > various cases, etc. I doubt we will ever use them. -- John Baldwin _______________________________________________ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
