Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool
On Saturday, May 05, 2012 5:53:07 am Andriy Gapon wrote: > on 05/05/2012 12:31 Andriy Gapon said the following: > > on 04/05/2012 18:25 John Baldwin said the following: > >> 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: > > > > John, > > > > these are excellent suggestions! Thank you! > > The new patchset: http://people.freebsd.org/~avg/zfsboot.patches.7.diff Looks great, thanks! A few replies below: > >> - 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? 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. 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). -- John Baldwin ___ 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"
Re: How does loader(8) decide where to load the kernel?
On Saturday, May 05, 2012 1:06:13 am Tim Kientzle wrote: > I have ubldr loading the ELF kernel on BeagleBone and am now > trying to untangle some of the hacks I used to get this working. > > Unfortunately, there's one area of the common loader(8) code > that I really don't understand: How does sys/boot/common/load_elf.c > determine the physical address at which to load the kernel? > > __elfN(loadfile) has the following comment: > > [The file] will be stored at (dest). > > But that's not really true. For starters, loadfile maps dest > through archsw.arch_loadaddr. (This mechanism seems > to only be used on ia64 and pc98, though the result is > later discarded on those platforms.) > > Loadfile then passes the value to loadimage which does > very strange things: > > On i386, amd64, powerpc, and arm, loadimage subtracts > the dest value from the address declared in the actual ELF > headers so that the kernel always gets loaded into low memory. > (there's some intermediate bit-twiddling I'm glossing over, but > this is the general idea). The bit twiddling is supposed to be the equivalent of subtracting KERNBASE from the load address. On both i386 and amd64, there is a direct mapping of the kernel text such that KERNBASE maps address 0, etc. By default on i386 KERNBASE is 0xc000. -- John Baldwin ___ 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"
Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool
on 05/05/2012 13:49 Bruce Evans said the following:
> On Sat, 5 May 2012, Andriy Gapon wrote:
>
>> on 04/05/2012 18:25 John Baldwin said the following:
>>> 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:
>>
>> John,
>>
>> these are excellent suggestions! Thank you!
>> Some comments:
>>> - Add #ifndef LOCORE guards to the new header around the structure so
>>> it can be used in assembly as well as C.
>>
>> Done. I have had to go into a few btx makefiles and add a necessary include
>> path and -DLOCORE to make the header usable from asm.
Bruce,
first a note that the change that we discussed affects (should affect) only BTX
code and as such only boot1/2 -> loader interface.
> Ugh, why not use genassym, as is done for all old uses of this header in
> locore.s, at least on i386 (5% of the i386 genassym.c is for this).
Can not parse 'this header' in this context. We were talking about a new header
file, so there could not be any old uses of it :-)
Probably you meant sys/i386/include/bootinfo.h ?
But, as you say later, it's probably not easy to use genassym with sys/boot
code. Not sure if it would be worth while going this path given the possible
alternatives.
>>> - Move BI_SIZE and ARGOFF into the header as constants.
>>
>> Done.
>>
>>> - Add a CTASSERT() in loader/main.c that BI_SIZE == sizeof(struct bootinfo)
>
> Ugh, BI_SIZE was already used in locore.s.
OK, but this is "the other" BI_SIZE. Maybe the name clash is not nice indeed,
though.
> It wasn't the size of the struct,
> but was the offset of the field that gives the size. No CTASSERT() was
> needed -- the size is whatever it is, as given by sizeof() on the struct
> at the time of compilation of the utility that initializes the struct.
> It was a feature that sizeof() and offsetof() can't be used in asm so they
> must be translated in genassym and no macros are needed in the header (the
> size was fully dynamic, so the asm code only needs the offsetof() values).
> Of course, you could use CTASSERT()s to check that the struct layout didn't
> get broken. The old code just assumes that the struct is packed by the
> programmer and that the arch's struct packing conventions don't change,
> so that for example BI_SIZE = offsetof(struct bootinfo, bi_size) never
> changes.
It seems that boot1/2 -> kernel interface and boo1/2 -> {btxldr, btx} -> loader
interfaces are quite independent and a bit different.
> genassym is hard to use in boot programs, but the old design was that
> boot programs shouldn't use bootinfo in asm and should just use the
> target bootinfo.h at compile time (whatever time the target is compiled).
I am not sure if it is worthwhile adapting genassym to sys/boot...
BTX code needs to know only "some size" of bootinfo. Although it doesn't look
like boot1/2 passes anything really useful to loader via bootinfo except for
bi_bios_dev. For that matter it looks like maybe only two fields from the whole
(x86) bootinfo are useful to (x86) kernel either...
> Anyway, LOCORE means "for use in locore.[sS]", so other uses of it, e.g.
> in boot programs, are bogus.
That's a good point. Maybe we should use some more generic name. Maybe there
is even some macro that is always set for .S files that we can check. Oh, thank
google, is __ASSEMBLER__ it?
It seems like couple of non-x86 headers already use this macro.
>> 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
>
> This isn't backwards compatible. BI_SIZE was decimal 48 (covers everything
> up to the bi_size field).
I meant backward compatible with the BTX code that I was changing, of course.
>> 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?
>
> Neither. BI_SIZE shouldn't exist. It defeats the bi_size field.
Using the bi_size field may be the proper solution indeed. Even if no data
beyond certain offset is ever used by loader. The planned changes to BTX code
should make using bi_size easier.
>>> Maybe
>>> create a 'struct kargs_ext' that looks like this:
>>>
>>> struct kargs_ext {
>>> uint32_t size;
>>> char data[0];
>>> };
>>
>> I've decided to skip
Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool
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"
Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool
on 07/05/2012 17:47 Andriy Gapon said the following: > on 07/05/2012 16:53 John Baldwin said the following: >> 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. Could you please check the wording and correct it or suggest alternatives? Thank you. diff --git a/sys/boot/i386/common/bootargs.h b/sys/boot/i386/common/bootargs.h index 510efdd..8bc1b32 100644 --- a/sys/boot/i386/common/bootargs.h +++ b/sys/boot/i386/common/bootargs.h @@ -29,6 +29,15 @@ #defineBF_OFF 8 /* offsetof(struct bootargs, bootflags) */ #defineBI_OFF 20 /* offsetof(struct bootargs, bootinfo) */ +/* + * We reserve some space above BTX allocated stack for the arguments + * and certain data that could hang off them. Currently only struct bootinfo + * is supported in that category. The bootinfo is placed at the top + * of the arguments area and the actual arguments are placed at ARGOFF offset + * from the top and grow towards the top. Hopefully we have enough space + * for bootinfo and the arguments to not run into each other. + * Arguments area below ARGOFF is reserved for future use. + */ #defineARGSPACE0x1000 /* total size of the BTX args area */ #defineARGOFF 0x800 /* actual args offset within the args area */ #defineARGADJ (ARGSPACE - ARGOFF) -- 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"
Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool
On Monday, May 07, 2012 10:47:05 am Andriy Gapon wrote: > 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? In general I think this looks good. I have only one suggestion. In other code (e.g. the genassym constants in the kernel) where we define constants for field offsets, we make the constant be the uppercase name of the field itself (e.g. TD_PCB for offsetof(struct thread, td_pcb)). I would rather do that here as well. In this case the field names do not have a prefix, but let's just use a BA_ prefix for members of 'bootargs'. BI_SIZE is already correct, but this would mean renaming HT_OFF to BA_HOWTO, BF_OFF to BA_BOOTFLAGS, and BI_OFF to BA_BOOTINFO. I think you can probably leave BA_SIZE as-is. > > 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 :) Yes, that can wait. I think it would not be very hard to do however. All you really need is access to sys/kern/genassym.sh and nm. -- John Baldwin ___ 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"
Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool
On Monday, May 07, 2012 11:15:53 am Andriy Gapon wrote: > on 07/05/2012 17:47 Andriy Gapon said the following: > > on 07/05/2012 16:53 John Baldwin said the following: > >> 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. > > Could you please check the wording and correct it or suggest alternatives? > Thank you. > > diff --git a/sys/boot/i386/common/bootargs.h b/sys/boot/i386/common/bootargs.h > index 510efdd..8bc1b32 100644 > --- a/sys/boot/i386/common/bootargs.h > +++ b/sys/boot/i386/common/bootargs.h > @@ -29,6 +29,15 @@ > #define BF_OFF 8 /* offsetof(struct bootargs, bootflags) > */ > #define BI_OFF 20 /* offsetof(struct bootargs, bootinfo) > */ > > +/* > + * We reserve some space above BTX allocated stack for the arguments > + * and certain data that could hang off them. Currently only struct bootinfo > + * is supported in that category. The bootinfo is placed at the top > + * of the arguments area and the actual arguments are placed at ARGOFF offset > + * from the top and grow towards the top. Hopefully we have enough space > + * for bootinfo and the arguments to not run into each other. > + * Arguments area below ARGOFF is reserved for future use. > + */ > #define ARGSPACE0x1000 /* total size of the BTX args area */ > #define ARGOFF 0x800 /* actual args offset within the args > area */ > #define ARGADJ (ARGSPACE - ARGOFF) I think this is good, thanks! -- John Baldwin ___ 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"
Re: FreeBSD on MacBook
On 05/05/12 23:58, Alfred Zhong wrote: > Hi Dear Hackers, > > I tried to install FreeBSD on my MacBook. Bascially I followed this post > online. > http://www.glenbarber.us/2011/11/12/Dual-Booting-OS-X-and-FreeBSD-9.html > > And the tragedy happened. I do remember, as the post said, typed > gpart bootcode -b /boot/pmbr -p /boot/gptboot -i 3 ada0 > > By the way, I did installed rEFIT on Mac OS > > After rebooting, I see neither my Mac boot option nor the FreeBSD boot > option. Even after holding Option key... > > The only stuff can boot is if I insert a CD (say, the FreeBSD live CD)... > > My installation was successful, but the boot loader was messed up! > > How can I fix this? > It is possible to install on a macbook (I've done it), though the process is a bit more intricate than any of the guides available suggest. Basically, you need to install an MBR and boot in legacy BIOS mode. Apple's EFI implementation does some funny things, and FreeBSD doesn't support EFI on i386 (yet, I'm actually working on adding support). I have a ZFS-only system, laid out as follows: there is an MBR with one partition, containing a BSDlabel. Inside that, there is a swap partition, and a single ZFS instance. When installing, you'll need to use dd to install the first part of zfsboot to the bsdlabel, and the remaining portion to the free space after the ZFS header (there's a guide on how to do that somewhere). -- Eric McCorkle Computer Science Ph.D Student e...@shadowsun.net signature.asc Description: OpenPGP digital signature
Re: How does loader(8) decide where to load the kernel?
On May 7, 2012, at 6:57 AM, John Baldwin wrote: > On Saturday, May 05, 2012 1:06:13 am Tim Kientzle wrote: >> I have ubldr loading the ELF kernel on BeagleBone and am now >> trying to untangle some of the hacks I used to get this working. >> >> Unfortunately, there's one area of the common loader(8) code >> that I really don't understand: How does sys/boot/common/load_elf.c >> determine the physical address at which to load the kernel? >> >> __elfN(loadfile) has the following comment: >> >> [The file] will be stored at (dest). >> >> But that's not really true. For starters, loadfile maps dest >> through archsw.arch_loadaddr. (This mechanism seems >> to only be used on ia64 and pc98, though the result is >> later discarded on those platforms.) >> >> Loadfile then passes the value to loadimage which does >> very strange things: >> >> On i386, amd64, powerpc, and arm, loadimage subtracts >> the dest value from the address declared in the actual ELF >> headers so that the kernel always gets loaded into low memory. >> (there's some intermediate bit-twiddling I'm glossing over, but >> this is the general idea). > > The bit twiddling is supposed to be the equivalent of subtracting > KERNBASE from the load address. On both i386 and amd64, there is > a direct mapping of the kernel text such that KERNBASE maps address > 0, etc. By default on i386 KERNBASE is 0xc000. Exactly my problem. This all assumes that you're loading the kernel into low memory. On the AM3358, the DRAM starts at 0x8000 on boot, so I'm trying to find a clean way to convince the loader's ELF code to put the kernel there. Tim ___ 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"
