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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> > > 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 > -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
