Re: Fresh installed Freebsd 9 don't boot from hd

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