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: > >> on 23/10/2011 18:27 Dennis Koegel said the following: > >>> On Fri, Oct 21, 2011 at 04:33:38PM -0400, John Baldwin wrote: > >>>> Working offline with Dennis, we found that changing the CFLAGS in > >>>> sys/boot/i386/gptboot/Makefile from "-O1" to "-Os -mrtd" (partially reverting > >>>> an earlier commit) fixed gptboot. The next test for someone to do would be to > >>>> try just adding "-mrtd" and leaving "-O1" as-is to see if that fixes it. > >>> > >>> More test results: > >>> > >>> gcc -Os -fno-guess-branch-probability -fomit-frame-pointer -fno-unit-at-a-time \ > >>> -mno-align-long-strings -mrtd [from before r225530]: Boots OK > >>> gcc -Os -mrtd: Boots OK > >>> gcc -O1 -mrtd: Fails > >>> gcc -O1: Fails > >>> gcc -O0: Fails > >>> gcc -Os: Boots OK > >>> > >>> clang -O1: Fails > >>> clang -Os: Fails > >>> clang -Oz: Fails > >>> > >>> I've put some printf()s into gpt{,boot}.c to trace where the reboot is > >>> triggered. It appears to be in drvsize() (called from gptread()). OTOH > >>> the debug output may have changed where the problem occurs, I don't > >>> know about that. > >>> > >>> With 9.0R drawing near, CFLAGS should be s/-O1/-Os/, until we can figure > >>> out what happens. But as for why gcc's magic -Os is required and clang's > >>> output doesn't work at all, I'm clueless. > >> > >> Thank you for your very valuable analysis! > >> I looked at a difference in assembly code of the drvsize function produced by > >> gcc -Os and by gcc -O1. One thing that was immediately obvious is that gcc > >> places the params array and the sectors variable in a different order for > >> different options. One idea is that if BIOS actually writes beyond the end of > >> the array, then in one case it could be harmless (overwrites the sector > >> variable), but in the other case it could be more harmful. > >> 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. 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 -- John BaldwinReceived on Mon Oct 24 2011 - 13:50:29 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:19 UTC