Add serial variant of pmbr

[thomas]

Summary

pmbr, the Protective Master Boot Record, is the boot sector for GPT
partitioned disks. The default version relies on the video BIOS
to output diagnostic messages.

For the benefit of headless machines, introduce a "pmbrsio" variant
which instead sends output to the first serial port. Serial port
parameters is controlled by BOOT_BOOT0_COMCONSOLE_SPEED.

Test Plan

Both modified pmbr and new pmbrsio tested under qemu with:

• valid GPT boot partition
• valid GPT table but no boot partition
• invalid GPT table.

[jhb]

I think the Makefile shuffling clutters this a bit and I would prefer leaving pmbr.S in the pmbr directory rather than moving it to boot0. I would probably just have pmbrsio's Makefile include pmbr's Makefile for now instead of reusing boot0's.

Ah, I think I see better. You need at least the BOOT_BOOT0_COMCONSOLE_SPEED bits. I would maybe create a shared Makefile that boot0 and pmbr explicitly include. Maybe something like 'sys/boot/i386/Makefile.mbr' and rename BOOT_BOOT0_COMCONSOLE_SPEED to BOOT_BIOS_COMCONSOLE_SPEED. I think that is all that would go in there though. pmbr and boot0 both happen to use 0x600 for their relocated copy, but they aren't explicitly tied to that. As such, I don't think they should be hard-bound to it.

The comment above BOOT_BOOT0_ARG is also missing "relocate". Should be "base address that we relocate the boot0 code to to run it". Even better would be to axe the double "to" though. I'll probably fix that separately.

[thomas]

Reorganize per jhb's comments

[thomas]

(please ignore the boot2.c change, it is unrelated, and was included in this diff by mistake)

[imp]

sys/boot/i386/boot2/boot2.c

439

Style nit:
/*

• Long comments */

and

/* Short comments */

But never

/* This is a comment
*/

Please use the long format.

[jhb]

sys/boot/i386/Makefile.mbr

6

I would not move this. pmbr doesn't honor it and hardcodes 0x600 in the asm (whereas boot0.S uses an address relative to 'start' and thus honors the org value set by the linker).

Instead I would change the common LDFLAGS in this file to use 'ORG'. I would leave ORG hardcoded in the pmbr and mbr Makefiles and change boot0's Makefile to do:

ORG=  ${BOOT_BOOT0_ORG:U:0x600}

below this comment.

[thomas]

sys/boot/i386/Makefile.mbr

6

I'm not sure I understand this comment. It seems to me that boot0 harcodes ORIGIN=0x600 and uses it just exactly as pmbr and mbr do.

I think we should either unconditionally hardcode ORG=0x600 in Makefile.mbr, and remove the BOOT_BOOT0_ORG knob altogether, or make sure that the value defined in the Makefile is passed in CFLAGS and adequately taken into account in all sources.

sys/boot/i386/boot2/boot2.c

439

Thanks, done in my local copy (however note that as mentioned in a previous comment, in any case I do NOT intend to commit this hunk as part of the pmbr change, it is independent, and got included here by mistake).

[jhb]

sys/boot/i386/Makefile.mbr

6

So boot0 has this code:

/*
 * Copy this code to the address it was linked for, 0x600 by default.
 */
        movw %sp,%si                    # Source
        movw $start,%di                 # Destination
        movw $0x100,%cx                 # Word count
        rep                             # Relocate
        movsw                           #  code

This uses 'start' as the destination to relocate to. Since it uses 'start' the linker will resolve this to the ORG passed in in LDFLAGS. This means that if you change BOOT_BOOT0_ORG you can actually move this around (and assuming you don't move it to a "bad" place it should still work).

Contrast this with what is used in pmbr and mbr:

            .set LOAD,0x7c00                # Load address
            .set EXEC,0x600                 # Execution address
....
#
# Relocate ourself to a lower address so that we have more room to load
# other sectors.
# 
            movw $main-EXEC+LOAD,%si        # Source
            movw $main,%di                  # Destination
            movw $SECSIZE-(main-start),%cx  # Byte count
            rep                             # Relocate
            movsb                           #  code

This only really works if the ORG passed to ld is the same as EXEC.

[thomas]

sys/boot/i386/Makefile.mbr

6

Hi Jonathan,
Sorry for taking so long to get back to you.
Wouldn't the proper solution to the issue you are pointing out be to modify mbr/pbmr to use LOAD+$main-$start as the source address instead of LOAD+$main-EXEC (which indeeds creates an implicit assumption that EXEC is set to ORG)?

[jhb]

sys/boot/i386/Makefile.mbr

6

That would work I believe, but I'm also not sure how useful it is to move ORG around. I have never heard of anyone using a non-default value. (Granted, I haven't ever heard of anyone needing that or using it for boot0 either.)

If you do change mbr/pmbr, you should use md5/sha1 to verify that in the default case the change you suggested doesn't result in any changes in the generated binary.

My preference would still be to only allow ORG to be moved for boot0 and leave pmbr/mbr alone however.

[thomas]

Note that in any case, boot0 does also hardcode its origin in the asm source, with the following comment:

* ORIGIN is the relocation address. If you change it, you also need
*      to change the value passed to the linker in the Makefile

and this value is used to compute the relative jump to main after relocation:

jmp main-LOAD+ORIGIN            # Jump to relocated code

So even for boot0, you can't just change the value in the Makefiles, you also need to update sources accordingly.