on 07/05/2012 16:53 John Baldwin said the following: > On Saturday, May 05, 2012 5:53:07 am Andriy Gapon wrote: [snip] >> The new patchset: http://people.freebsd.org/~avg/zfsboot.patches.7.diff > > Looks great, thanks! A few replies below:
Here's a followup patch for the suggestions: http://people.freebsd.org/~avg/bootargs.followup.diff I will merge it into the main patch. What do you think about the -LOCORE- change that Bruce inspired? >>>> - Add a CTASSERT() in loader/main.c that BI_SIZE == sizeof(struct bootinfo) >>> >>> I have added a definition of CTASSERT to boostrap.h as it was not available >>> for >>> sys/boot and there were two local definitions of the macro in individual >>> files. >>> >>> However the assertion would fail right now. >>> The backward-compatible value of BI_SIZE (72 == 0x48) covers only part of >>> the >>> fields in struct bootinfo, those up to the following comment: >>> /* Items below only from advanced bootloader */ >>> >>> I am a little bit hesitant: should I increase BI_SIZE to cover the whole >>> struct >>> bootinfo or should I compare BI_SIZE to offsetof bi_kernend? > > Actually, we should probably be reading the 'bi_size' field and not using a > BI_SIZE > constant at all? Done in the above patch. > Looks like only the non-functional EFI boot loader doesn't set bi_size (and > it should > just be fixed to do so since it needs to pass new fields in anyway). > >>> I've decided to define ARGADJ in the new common header, then I've had to >>> rename >>> btxcsu.s to .S, so that the preprocessing is executed for it. > > Ok. Maybe add one comment to the bootargs.h head to explain that the > 'bootargs' > struct starts at ARGOFF and can grow up, while struct bootinfo is copied such > that > it's end is at the top of the argument area and grows down. Will do. > Also, at some point we could use a genassym.c file ala the kernel builds to > generate > some of the constants in bootargs.h instead (e.g. the offsets of fields within > structures, and BA_SIZE, though we probably want to ensure that BA_SIZE never > changes). The genassym approach sounds good, but, indeed - later :) Thank you. -- Andriy Gapon _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
