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

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.

> +	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.

Other than these issues the patch looks great!
Perhaps later we could do detailed definitions for things like interface paths for
various cases, etc.

-- 
Andriy Gapon