On Wed, Feb 20, 2019 at 4:31 PM Waldemar Kozaczuk <[email protected]>
wrote:

> This patch makes all necessary changes to OSv boot logic
> to make it bootable on firecracker.
>
> Please note that until we add virtio-mmio support
> we can only boot ramfs images on firecracker like so:
>
> ./scripts/build fs=ramfs image=native-example
>

I comitted this patch already, but have some minor comments below.

One overarching comment I had while reading this was that it is getting
hard - if it wasn't already hard enough -
to follow why all these alternative entry functions, option parsing, etc.,
exist. You added some comments explaining
some of the things, which is good, but I wonder if we shouldn't try to
restructure our source code to make this
separation even clearer, so that alternative code paths and their weird
definitions and dependencies appear
in separate files.

For example, it appears that now, "entry64", which is now mentioned in the
ld scripts as the entry point, exists
to adhere to some "vmlinuz" (?) standard which is needed by firecracker and
(?) qemu -kernel, but not for
traditional "disk images". The same vmlinuz standard dictactes
(apparently?) in that case where the command
line options are, and other stuff, and a lot of bizarre definitions
included below but not needed for other boot types.
Could we have this stuff in separate source files, e.g., vmlinuz.S (if the
appropriate name is "vmlinuz"...) and
vmlinuz.cc (or some other split of boot types into different files) so it
will be clearer what is specific support for this
standard?

This sort of reorganization is not critical or urgent at all. I just think
that if we'll ever want to do it, we need to do it while this material
is still fresh in someone's mind...


> Signed-off-by: Waldemar Kozaczuk <[email protected]>
> ---
>  arch/x64/arch-setup.cc | 93 ++++++++++++++++++++++++++++++++++++++++++
>  arch/x64/boot.S        | 76 ++++++++++++++++++++++++----------
>  arch/x64/boot16.S      |  4 +-
>  arch/x64/loader.ld     |  8 +++-
>  loader.cc              |  5 +++
>  5 files changed, 161 insertions(+), 25 deletions(-)
>
> diff --git a/arch/x64/arch-setup.cc b/arch/x64/arch-setup.cc
> index 50500662..6db2f77c 100644
> --- a/arch/x64/arch-setup.cc
> +++ b/arch/x64/arch-setup.cc
> @@ -117,6 +117,96 @@ extern size_t elf_size;
>  extern void* elf_start;
>  extern boot_time_chart boot_time;
>
> +// Because entry64 replaces start32 as a new entry of loader.elf we need
> a way
> +// to place address of start32 so that boot16 know where to jump to. We
> achieve
> +// it by placing address of start32 at the known offset at memory
> +// as defined by section .start32_address in loader.ld
> +extern "C" void start32();
> +void * __attribute__((section (".start32_address"))) start32_address =
> reinterpret_cast<void*>(&start32);
>
+
> +#define OSV_MULTI_BOOT_INFO_ADDR      0x1000
> +#define OSV_E820_TABLE_ADDR           0x2000
> +
> +//
> +// Instead of defining full boot_params and setup_header structs as in
> +// Linux source code, we define only handful of offsets pointing the
> fields
> +// we need to read from there. For details please this chunk of Linux
> code -
> +//
> https://github.com/torvalds/linux/blob/b6839ef26e549de68c10359d45163b0cfb031183/arch/x86/include/uapi/asm/bootparam.h#L151-L198
> +#define
> <https://github.com/torvalds/linux/blob/b6839ef26e549de68c10359d45163b0cfb031183/arch/x86/include/uapi/asm/bootparam.h#L151-L198+%23define>
> LINUX_KERNEL_BOOT_FLAG_MAGIC  0xaa55
> +#define LINUX_KERNEL_HDR_MAGIC        0x53726448 // "HdrS"
> +
> +#define SETUP_HEADER_OFFSET  0x1f1   // look at bootparam.h in linux
> +#define SETUP_HEADER_FIELD_VAL(boot_params, offset, field_type) \
> +    (*static_cast<field_type*>(boot_params + SETUP_HEADER_OFFSET +
> offset))
> +
> +#define BOOT_FLAG_OFFSET     sizeof(u8) + 4 * sizeof(u16) + sizeof(u32)
> +#define HDR_MAGIC_OFFSET     sizeof(u8) + 6 * sizeof(u16) + sizeof(u32)
> +
> +#define E820_ENTRIES_OFFSET  0x1e8   // look at bootparam.h in linux
> +#define E820_TABLE_OFFSET    0x2d0   // look at bootparam.h in linux
> +
> +#define CMD_LINE_PTR_OFFSET  sizeof(u8) * 5 + sizeof(u16) * 11 +
> sizeof(u32) * 7
> +
> +struct linux_e820ent {
> +    u64 addr;
> +    u64 size;
> +    u32 type;
> +} __attribute__((packed));
> +
> +// When OSv kernel gets booted directly as 64-bit ELF (loader.elf) as it
> is
> +// the case on firecracker we need a way to extract all necessary
> information
> +// about available memory and command line. This information is provided
> +// the struct boot_params (see details above) placed in memory at the
> address
> +// specified in the RSI register.
> +// The following extract_linux_boot_params() function is called from
> +// entry64 in boot.S and verifies OSV was indeed boot as Linux and
> +// copies memory and cmdline information into OSv multiboot struct.
> +// Please see https://www.kernel.org/doc/Documentation/x86/boot.txt for
> details
> +// of Linux boot protocol. Bear in mind that OSv implements very narrow
> specific
> +// subset of the protocol as assumed by firecracker.
> +extern "C" void extract_linux_boot_params(void *boot_params)
> +{   //
> +    // Verify we are being booted as Linux 64-bit ELF kernel
> +    assert( SETUP_HEADER_FIELD_VAL(boot_params, BOOT_FLAG_OFFSET, u16) ==
> LINUX_KERNEL_BOOT_FLAG_MAGIC);
> +    assert( SETUP_HEADER_FIELD_VAL(boot_params, HDR_MAGIC_OFFSET, u32) ==
> LINUX_KERNEL_HDR_MAGIC);
> +
> +    // Set location of multiboot info struct at arbitrary place in lower
> memory
> +    // to copy to (happens to be the same as in boot16.S)
> +    osv_multiboot_info_type* mb_info =
> reinterpret_cast<osv_multiboot_info_type*>(OSV_MULTI_BOOT_INFO_ADDR);
> +
> +    // Copy command line pointer from boot params
> +    mb_info->mb.cmdline = SETUP_HEADER_FIELD_VAL(boot_params,
> CMD_LINE_PTR_OFFSET, u32);
> +
> +    // Copy e820 information from boot params
> +    mb_info->mb.mmap_length = 0;
> +    mb_info->mb.mmap_addr = OSV_E820_TABLE_ADDR;
> +
> +    struct linux_e820ent *source_e820_table = static_cast<struct
> linux_e820ent *>(boot_params + E820_TABLE_OFFSET);
> +    struct e820ent *dest_e820_table = reinterpret_cast<struct e820ent
> *>(mb_info->mb.mmap_addr);
> +
> +    u8 en820_entries = *static_cast<u8*>(boot_params +
> E820_ENTRIES_OFFSET);
> +    for (int e820_index = 0; e820_index < en820_entries; e820_index++) {
> +        dest_e820_table[e820_index].ent_size = 20;
> +        dest_e820_table[e820_index].type =
> source_e820_table[e820_index].type;
> +        dest_e820_table[e820_index].addr =
> source_e820_table[e820_index].addr;
> +        dest_e820_table[e820_index].size =
> source_e820_table[e820_index].size;
> +        mb_info->mb.mmap_length += sizeof(e820ent);
> +    }
> +
> +    auto now = processor::ticks();
> +    u32 now_high = (u32)(now >> 32);
> +    u32 now_low = (u32)now;
> +
> +    mb_info->tsc_init_hi = now_high;
> +    mb_info->tsc_init = now_low;
> +
> +    mb_info->tsc_disk_done_hi = now_high;
> +    mb_info->tsc_disk_done = now_low;
> +
> +    mb_info->tsc_uncompress_done_hi = now_high;
> +    mb_info->tsc_uncompress_done = now_low;
> +}
> +
>  void arch_setup_free_memory()
>  {
>      static ulong edata;
> @@ -272,6 +362,9 @@ void arch_init_drivers()
>      boot_time.event("pvpanic done");
>
>      // Enumerate PCI devices
> +    //TODO: Add boot option --pci=off to skip PCI enumeration
> +    // as on some platforms like firecracker enumerating "empty" bus
> +    // takes up to 10msyhuuuuu
>

Would be even nicer to not have an option, but also not take 10ms to
recognize there is no PCI. Why does it take so long?

     pci::pci_device_enumeration();
>      boot_time.event("pci enumerated");
>
> diff --git a/arch/x64/boot.S b/arch/x64/boot.S
> index c4b97e2d..9277dffd 100644
> --- a/arch/x64/boot.S
> +++ b/arch/x64/boot.S
> @@ -5,6 +5,40 @@
>
>  #include "processor-flags.h"
>
> +#define BOOT_CR0 ( X86_CR0_PE \
> +                 | X86_CR0_WP \
> +                 | X86_CR0_PG )
> +
> +#define BOOT_CR4 ( X86_CR4_DE         \
> +                 | X86_CR4_PSE        \
> +                 | X86_CR4_PAE        \
> +                 | X86_CR4_PGE        \
> +                 | X86_CR4_PCE        \
> +                 | X86_CR4_OSFXSR     \
> +                 | X86_CR4_OSXMMEXCPT )
> +
> +.macro setup_64bit_long_mode
> +    // Enable PAE (Physical Address Extension) - ability to address 64GB
> +    // TODO: Add more comments to processor-flags.h what each flag does
> +    mov $BOOT_CR4, %eax
> +    mov %eax, %cr4
> +
> +    // Set root of a page table in cr3
> +    lea ident_pt_l4, %eax
> +    mov %eax, %cr3
>
+
> +    // Set long mode?
> +    mov $0xc0000080, %ecx // EFER MSR number
> +    mov $0x00000900, %eax // Set LME = 1
> +    xor %edx, %edx
> +    wrmsr // Write contents of EDX:EAX (0:to Model Specific Register
> specified by ECX register
> +
> +    // Activate paging and ...?
> +    // TODO: Add more comments to processor-flags.h what each flag does
> +    mov $BOOT_CR0, %eax
> +    mov %eax, %cr0
> +.endm
> +
>  .text
>  .code32
>
> @@ -49,18 +83,6 @@ init_stack_top = .
>
>  .text
>
> -#define BOOT_CR0 ( X86_CR0_PE \
> -                 | X86_CR0_WP \
> -                 | X86_CR0_PG )
> -
> -#define BOOT_CR4 ( X86_CR4_DE         \
> -                 | X86_CR4_PSE        \
> -                 | X86_CR4_PAE        \
> -                 | X86_CR4_PGE        \
> -                 | X86_CR4_PCE        \
> -                 | X86_CR4_OSFXSR     \
> -                 | X86_CR4_OSXMMEXCPT )
> -
>  .globl start32
>  start32:
>      # boot16.S set %eax to ELF start address, we'll use it later
> @@ -75,16 +97,7 @@ start32:
>      ljmp $0x18, $1f
>  1:
>      and $~7, %esp
> -    mov $BOOT_CR4, %eax
> -    mov %eax, %cr4
> -    lea ident_pt_l4, %eax
> -    mov %eax, %cr3
> -    mov $0xc0000080, %ecx
> -    mov $0x00000900, %eax
> -    xor %edx, %edx
> -    wrmsr
> -    mov $BOOT_CR0, %eax
> -    mov %eax, %cr0
> +    setup_64bit_long_mode
>      ljmpl $8, $start64
>  .code64
>  .global start64
> @@ -181,3 +194,22 @@ smpboot64:
>      jnz 1b
>      lea 4096(%rax), %rsp
>      call smp_main
> +
> +.text
> +.code64
> +.global entry64
> +entry64

+# The address of boot_params structed is passed in RSI
> +# register so pass it to extract_linux_boot_params fuction
> +# which will extract cmdline and memory information and verify
> +# that loader.elf was indeed called as Linux 64-bit vmlinux ELF
> +    mov %rsi, %rdi
> +    call extract_linux_boot_params
> +
> +# Even though we are in 64-bit long mode we need to reset
> +# page tables and other CPU settings the way OSv expects it
> +    setup_64bit_long_mode
> +
> +    mov $OSV_KERNEL_BASE, %rbp
> +    mov $0x1000, %rbx
> +    jmp start64
> diff --git a/arch/x64/boot16.S b/arch/x64/boot16.S
> index d053a3f4..6d5527d3 100644
> --- a/arch/x64/boot16.S
> +++ b/arch/x64/boot16.S
> @@ -12,7 +12,7 @@ target =  OSV_LZKERNEL_BASE
>  elf_entry_point_header_offset = 0x18
>  lzentry = target+elf_entry_point_header_offset
>  loader = OSV_KERNEL_BASE
> -entry = loader+elf_entry_point_header_offset
> +start32 = loader+0x800
>
>  mb_info = 0x1000
>  // OSv information lays in the end of this struct, which is 88 bytes in
> size.
> @@ -168,7 +168,7 @@ done_e820:
>      mov %edx, mb_uncompress_hi
>      mov $loader, %eax
>      mov $mb_info, %ebx
> -    call *entry
> +    call *start32
>
>  .org 0x1b8
>  .byte 0x56, 0x53, 0x4F, 0, 0, 0
> diff --git a/arch/x64/loader.ld b/arch/x64/loader.ld
> index efe78d52..6bb4b861 100644
> --- a/arch/x64/loader.ld
> +++ b/arch/x64/loader.ld
> @@ -14,7 +14,13 @@ SECTIONS
>          *
>          * We can't export the ELF header base as a symbol, because ld
>          * insists on moving stuff around if we do.
> +        *
> +        * Place address of start32 routine at predefined offset in memory
>          */
> +    . = OSV_KERNEL_BASE + 0x800;
>

I assume this is fine because the ELF header is much shorter than 0x800?

+    .start32_address : {
> +        *(.start32_address)
> +    }
>      . = OSV_KERNEL_BASE + 0x1000;
>      .dynamic : { *(.dynamic) } :dynamic :text
>      .text : {
> @@ -108,4 +114,4 @@ PHDRS {
>         eh_frame PT_GNU_EH_FRAME;
>         note PT_NOTE;
>  }
> -ENTRY(start32);
> +ENTRY(entry64);
> diff --git a/loader.cc b/loader.cc
> index c29bf4c4..019e8761 100644
> --- a/loader.cc
> +++ b/loader.cc
> @@ -55,6 +55,7 @@
>  #include "drivers/null.hh"
>
>  #include "libc/network/__dns.hh"
> +#include <processor.hh>
>
>  using namespace osv;
>  using namespace osv::clock::literals;
> @@ -426,6 +427,10 @@ void* do_main_thread(void *_main_args)
>      }
>
>      boot_time.event("Total time");
> +    // Some hypervisors like firecracker when booting OSv
> +    // look for this write to this port as a signal of end of
> +    // boot time.
> +    processor::outb(123, 0x3f0);
>

I would love to one day understand what the heck this is. Port 0x3f0 used
to belong to the first floppy disk drive, and reading from it provides the
first byte of status. I don't understand what writing decimal 123 is to it
is supposed to do, and am curious where you found this thing in the Linux
or firecracker source code.

>
>      if (opt_bootchart) {
>          boot_time.print_chart();
> --
> 2.19.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to