On Thursday, May 03, 2012 11:23:51 am Andriy Gapon wrote:
> on 03/05/2012 18:02 Andriy Gapon said the following:
> >
> > Here's the latest version of the patches:
> > http://people.freebsd.org/~avg/zfsboot.patches.4.diff
>
> I've found a couple of problems in the previous version, so here's another
> one:
> http://people.freebsd.org/~avg/zfsboot.patches.5.diff
> The important change is in the first patch (__exec args).
A few comments/suggestions on the args bits:
- Please move the type of the 'kargs' struct into the new header and
give it a type name. Don't add the LOADER_ZFS_SUPPORT #ifdef's
however or the new size field (see below).
- Add #ifndef LOCORE guards to the new header around the structure so
it can be used in assembly as well as C.
- Move BI_SIZE and ARGOFF into the header as constants.
- Add a CTASSERT() in loader/main.c that BI_SIZE == sizeof(struct bootinfo)
- Add a KA_SIZE for sizeof(struct kargs) with a CTASSERT. Use it in this
instruction:
+ addl 0x1c(%esp),%ecx # Add size of variable args
in place of the 0x1c
- Don't add the new 'size' field to the end of the 'struct kargs'. Instead
add a comment saying that if KARGS_FLAGS_EXT is set, then a structure will
be present at (kargs + 1) that has a int size as its first member. Maybe
create a 'struct kargs_ext' that looks like this:
struct kargs_ext {
uint32_t size;
char data[0];
};
- Use 'zargs = (struct zfs_boot_args)(kargs + 1)' in loader/main.c.
- Change the ARGADJ line in btxcsu to be this:
.set ARGADJ,0x1000 - ARGOFF
- If you are adventurous, you could even add a new constant ARGSPACE or some
such with an appropriate comment in the new header that you use to replace
the 0x1000 in btx.S and in the definition of ARGADJ.
Hope that isn't too much, but I do think this will help make things even more
understandable.
--
John Baldwin
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[email protected]"
