Re: [coreboot] Multiboot

[Jordan Crouse]

On 22/09/08 23:03 +0200, Robert Millan wrote:
> Hi,
>
> Here's a new version of the patch.  Changes relative to the last one:
> Add "static" keyword to run_address_multiboot() and remove its header
> declaration.

Ack - the code should be defined where it is used - don't make things
too complex.
> Add "edx" to clobber list.  Together with eax and ecx which are already
> in input list, this makes the call able to return for any payload that
> complies with "cdecl" calling convention.

libpayload will use cdecl, as should the other payloads.  This is a
legitimate restriction to put on the payloads, i think.

Acked-by: Jordan Crouse

I'm slightly tired of the back and forth on this patch - so I'll give
the opponents a chance to discussion the technical merits of the patch,
otherwise I'm going to check it in and let cleanups happen as they may.

Jordan
> Signed-off-by: Robert Millan
> Index: include/arch/x86/multiboot.h
>
> --- include/arch/x86/multiboot.h	(revision 0)
> +++ include/arch/x86/multiboot.h	(revision 0)
> @@ -0,0 +1,179 @@
> +/* multiboot.h - multiboot header file. */
> +/*
> Copyright (C) 2003  Free Software Foundation, Inc.
> Copyright (C) 2008  Robert Millan
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, see http://www.gnu.org/licenses/.
> */
>
> +#ifndef MULTIBOOT_H
> +#define MULTIBOOT_H
>
> +#include <types.h>
> +typedef u16 uint16_t;
> +typedef u32 uint32_t;
> +typedef u64 uint64_t;
>
> +/* How many bytes from the start of the file we search for the header.  */
> +#define MB_SEARCH                 8192
>
> +/* The magic field should contain this.  */
> +#define MB_MAGIC                  0x1BADB002
>
> +/* This should be in %eax.  */
> +#define MB_MAGIC2                 0x2BADB002
>
> +/* The bits in the required part of flags field we don't support.  */
> +#define MB_UNSUPPORTED            0x0000fffc
>
> +/* Alignment of multiboot modules.  */
> +#define MB_MOD_ALIGN              0x00001000
>
> +/*
> Flags set in the 'flags' member of the multiboot header.
> */
>
> +/* Align all boot modules on i386 page (4KB) boundaries.  */
> +#define MB_PAGE_ALIGN		0x00000001
>
> +/* Must pass memory information to OS.  */
> +#define MB_MEMORY_INFO		0x00000002
>
> +/* Must pass video information to OS.  */
> +#define MB_VIDEO_MODE		0x00000004
>
> +/* This flag indicates the use of the address fields in the header.  */
> +#define MB_AOUT_KLUDGE		0x00010000
>
> +/*
> Flags to be set in the 'flags' member of the multiboot info structure.
> */
>
> +/* is there basic lower/upper memory information? */
> +#define MB_INFO_MEMORY		0x00000001
> +/* is there a boot device set? */
> +#define MB_INFO_BOOTDEV		0x00000002
> +/* is the command-line defined? */
> +#define MB_INFO_CMDLINE		0x00000004
> +/* are there modules to do something with? */
> +#define MB_INFO_MODS		0x00000008
>
> +/* These next two are mutually exclusive */
>
> +/* is there a symbol table loaded? */
> +#define MB_INFO_AOUT_SYMS		0x00000010
> +/* is there an ELF section header table? */
> +#define MB_INFO_ELF_SHDR		0x00000020
>
> +/* is there a full memory map? */
> +#define MB_INFO_MEM_MAP		0x00000040
>
> +/* Is there drive info?  */
> +#define MB_INFO_DRIVE_INFO		0x00000080
>
> +/* Is there a config table?  */
> +#define MB_INFO_CONFIG_TABLE	0x00000100
>
> +/* Is there a boot loader name?  */
> +#define MB_INFO_BOOT_LOADER_NAME	0x00000200
>
> +/* Is there a APM table?  */
> +#define MB_INFO_APM_TABLE		0x00000400
>
> +/* Is there video information?  */
> +#define MB_INFO_VIDEO_INFO		0x00000800
>
> +struct multiboot_header
> +{
> /* Must be MB_MAGIC - see above.  */
> uint32_t magic;
> /* Feature flags.  */
> uint32_t flags;
> /* The above fields plus this one must equal 0 mod 2^32. */
> uint32_t checksum;
> /* These are only valid if MB_AOUT_KLUDGE is set.  */
> uint32_t header_addr;
> uint32_t load_addr;
> uint32_t load_end_addr;
> uint32_t bss_end_addr;
> uint32_t entry_addr;
> /* These are only valid if MB_VIDEO_MODE is set.  */
> uint32_t mode_type;
> uint32_t width;
> uint32_t height;
> uint32_t depth;
>
> +};
>
> +struct multiboot_info
> +{
> /* Multiboot info version number */
> uint32_t flags;
> /* Available memory from BIOS */
> uint32_t mem_lower;
> uint32_t mem_upper;
> /* "root" partition */
> uint32_t boot_device;
> /* Kernel command line */
> uint32_t cmdline;
> /* Boot-Module list */
> uint32_t mods_count;
> uint32_t mods_addr;
> uint32_t syms[4];
> /* Memory Mapping buffer */
> uint32_t mmap_length;
> uint32_t mmap_addr;
> /* Drive Info buffer */
> uint32_t drives_length;
> uint32_t drives_addr;
> /* ROM configuration table */
> uint32_t config_table;
> /* Boot Loader Name */
> uint32_t boot_loader_name;
> /* APM table */
> uint32_t apm_table;
> /* Video */
> uint32_t vbe_control_info;
> uint32_t vbe_mode_info;
> uint16_t vbe_mode;
> uint16_t vbe_interface_seg;
> uint16_t vbe_interface_off;
> uint16_t vbe_interface_len;
>
> +};
>
> +struct multiboot_mmap_entry
> +{
> uint32_t size;
> uint64_t addr;
> uint64_t len;
> uint32_t type;
>
> +} __attribute__((packed));
>
> +unsigned long write_multiboot_info(unsigned long, unsigned long, unsigned long, unsigned long);
>
> +#endif
> Index: arch/x86/archtables.c
> ===================================================================
> --- arch/x86/archtables.c	(revision 867)
> +++ arch/x86/archtables.c	(working copy)
> @@ -24,6 +24,7 @@
>  #include <console.h>
>  #include <string.h>
>  #include <tables.h>
> +#include <multiboot.h>
>  //#include <cpu/cpu.h>
>  //#include <pirq_routing.h>
>  //#include <smp/mpspec.h>
> @@ -78,6 +79,14 @@
>
> post_code(POST_STAGE2_ARCH_WRITE_TABLES_ENTER);
> /* The Multiboot information structure must be in 0xf0000 */
> rom_table_end = write_multiboot_info(
> 	      low_table_start, low_table_end,
> 	      rom_table_start, rom_table_end);
> /* FIXME: is this alignment needed for PIRQ table? */
> rom_table_end = (rom_table_end + 1023) & ~1023;
> /* This table must be betweeen 0xf0000 & 0x100000 */
>  /* we need to make a decision: create empty functions
> in .h files if the cpp variable is undefined, or #ifdef?
> Index: arch/x86/multiboot.c
>
> --- arch/x86/multiboot.c	(revision 0)
> +++ arch/x86/multiboot.c	(revision 0)
> @@ -0,0 +1,70 @@
> +/*
> support for Multiboot payloads
> Copyright (C) 2008 Robert Millan
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, see http://www.gnu.org/licenses/.
> */
>
> +#include <multiboot.h>
> +#include <string.h>
> +#include <device/resource.h>
> +#include <console.h>
>
> +static struct multiboot_mmap_entry *mb_mem;
>
> +static void build_mb_mem_range(void *gp, struct device *dev, struct resource *res)
> +{
> mb_mem->addr = res->base;
> mb_mem->len = res->size;
> mb_mem->type = 1;
> mb_mem->size = sizeof(*mb_mem) - sizeof(mb_mem->size);
> mb_mem++;
>
> +}
>
> +unsigned long write_multiboot_info(
> unsigned long low_table_start, unsigned long low_table_end,
> unsigned long rom_table_start, unsigned long rom_table_end)
>
> +{
> struct multiboot_info *mbi = rom_table_end;
> memset(mbi, 0, sizeof(*mbi));
> rom_table_end += sizeof(*mbi);
> mbi->mmap_addr = (u32) rom_table_end;
> mb_mem = rom_table_end;
> /* free regions */
> search_global_resources( IORESOURCE_MEM | IORESOURCE_CACHEABLE,
> IORESOURCE_MEM | IORESOURCE_CACHEABLE, build_mb_mem_range, NULL);
> /* reserved regions */
> mb_mem->addr = low_table_start;
> mb_mem->len = low_table_end - low_table_start;
> mb_mem->type = 2;
> mb_mem->size = sizeof(*mb_mem) - sizeof(mb_mem->size);
> mb_mem++;
> mb_mem->addr = rom_table_start;
> mb_mem->len = rom_table_end - rom_table_start;
> mb_mem->type = 2;
> mb_mem->size = sizeof(*mb_mem) - sizeof(mb_mem->size);
> mb_mem++;
> mbi->mmap_length = ((u32) mb_mem) - mbi->mmap_addr;
> mbi->flags |= MB_INFO_MEM_MAP;
> printk(BIOS_INFO, "Multiboot Information structure has been written.\n");
> return mb_mem;
>
> +}
> Index: arch/x86/stage1.c
> ===================================================================
> --- arch/x86/stage1.c	(revision 867)
> +++ arch/x86/stage1.c	(working copy)
> @@ -28,6 +28,7 @@
>  #include <lib.h>
>  #include <mc146818rtc.h>
>  #include <cpu.h>
> +#include <multiboot.h>
>
> #ifdef CONFIG_PAYLOAD_ELF_LOADER
>  /* ah, well, what a mess! This is a hard code. FIX ME but how? 
> @@ -139,6 +140,14 @@
>  }
>  #endif /* CONFIG_PAYLOAD_ELF_LOADER */
>
> +static int run_address_multiboot(void *f)
> +{
> int ret;
> __asm__ __volatile__ ("call *%3" : "=a" (ret) : "a" (MB_MAGIC2), "b" (0xf0000), "c" (f) : "edx");
> return ret;
>
> +}
>
> /**
> This function is called from assembler code with its argument on the
> stack. Force the compiler to generate always correct code for this case.
>
> @@ -259,7 +268,7 @@
>    if (entry != (void*)-1) {
>    	/* Final coreboot call before handing off to the payload. */
>    	mainboard_pre_payload();
> run_address(entry);
> run_address_multiboot(entry);
> } else {
> die("FATAL: No usable payload found.\n");
> }
> Index: arch/x86/Makefile
>
> --- arch/x86/Makefile	(revision 867)
> +++ arch/x86/Makefile	(working copy)
> @@ -188,7 +188,7 @@
>  STAGE2_LIB_SRC       = stage2.c clog2.c mem.c tables.c delay.c \
>    	       compute_ip_checksum.c string.c
>
> -STAGE2_ARCH_X86_SRC  = archtables.c coreboot_table.c udelay_io.c
> +STAGE2_ARCH_X86_SRC  = archtables.c coreboot_table.c multiboot.c udelay_io.c
>  STAGE2_ARCH_X86_SRC += pci_ops_auto.c
>  STAGE2_ARCH_X86_SRC += keyboard.c i8259.c isa-dma.c