Re: [PATCH 2/6] cleanup multiboot

[Samuel Thibault]

Luca Dariz, le sam. 05 févr. 2022 18:51:25 +0100, a ecrit:
> * use _raw_ structs where we refer to the bootloader-provided data
> * remove unused structures
> * fix 64 bit boot
> 
> Signed-off-by: Luca Dariz <luca@orpolo.org>

Applied, thanks!

> ---
>  Makefrag.am                        |   1 -
>  i386/i386at/model_dep.c            |  23 +++---
>  i386/include/mach/i386/multiboot.h | 108 +----------------------------
>  include/mach/multiboot.h           |  82 ----------------------
>  kern/bootstrap.c                   |  20 +++++-
>  5 files changed, 31 insertions(+), 203 deletions(-)
>  delete mode 100644 include/mach/multiboot.h
> 
> diff --git a/Makefrag.am b/Makefrag.am
> index fef1e000..6e74697e 100644
> --- a/Makefrag.am
> +++ b/Makefrag.am
> @@ -404,7 +404,6 @@ include_mach_HEADERS = \
>       include/mach/message.h \
>       include/mach/mig_errors.h \
>       include/mach/msg_type.h \
> -     include/mach/multiboot.h \
>       include/mach/notify.h \
>       include/mach/pc_sample.h \
>       include/mach/policy.h \
> diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c
> index 21a36bf2..b2a22a42 100644
> --- a/i386/i386at/model_dep.c
> +++ b/i386/i386at/model_dep.c
> @@ -122,7 +122,7 @@ unsigned long *pfn_list = (void*) PFN_LIST;
>  unsigned long la_shift = VM_MIN_KERNEL_ADDRESS;
>  #endif
>  #else        /* MACH_XEN */
> -struct multiboot_info boot_info;
> +struct multiboot_raw_info boot_info;
>  #endif       /* MACH_XEN */
>  
>  /* Command line supplied to kernel.  */
> @@ -403,7 +403,7 @@ i386at_init(void)
>       }
>  
>       if (boot_info.flags & MULTIBOOT_MODS && boot_info.mods_count) {
> -             struct multiboot_module *m;
> +             struct multiboot_raw_module *m;
>               int i;
>  
>               if (! init_alloc_aligned(
> @@ -591,13 +591,14 @@ void c_boot_entry(vm_offset_t bi)
>        * so that the symbol table's memory won't be stomped on.
>        */
>       if ((boot_info.flags & MULTIBOOT_AOUT_SYMS)
> -         && boot_info.syms.a.addr)
> +         && boot_info.shdr_addr)
>       {
>               vm_size_t symtab_size, strtab_size;
>  
> -             kern_sym_start = (vm_offset_t)phystokv(boot_info.syms.a.addr);
> -             symtab_size = (vm_offset_t)phystokv(boot_info.syms.a.tabsize);
> -             strtab_size = (vm_offset_t)phystokv(boot_info.syms.a.strsize);
> +                /* For simplicity we just use a simple boot_info_raw 
> structure for elf */
> +             kern_sym_start = (vm_offset_t)phystokv(boot_info.shdr_addr);
> +             symtab_size = (vm_offset_t)phystokv(boot_info.shdr_num);
> +             strtab_size = (vm_offset_t)phystokv(boot_info.shdr_size);
>               kern_sym_end = kern_sym_start + 4 + symtab_size + strtab_size;
>  
>               printf("kernel symbol table at %08lx-%08lx (%ld,%ld)\n",
> @@ -606,12 +607,12 @@ void c_boot_entry(vm_offset_t bi)
>       }
>  
>       if ((boot_info.flags & MULTIBOOT_ELF_SHDR)
> -         && boot_info.syms.e.num)
> +         && boot_info.shdr_num)
>       {
> -             elf_shdr_num = boot_info.syms.e.num;
> -             elf_shdr_size = boot_info.syms.e.size;
> -             elf_shdr_addr = (vm_offset_t)phystokv(boot_info.syms.e.addr);
> -             elf_shdr_shndx = boot_info.syms.e.shndx;
> +             elf_shdr_num = boot_info.shdr_num;
> +             elf_shdr_size = boot_info.shdr_size;
> +             elf_shdr_addr = (vm_offset_t)phystokv(boot_info.shdr_addr);
> +             elf_shdr_shndx = boot_info.shdr_strndx;
>  
>               printf("ELF section header table at %08lx\n", elf_shdr_addr);
>       }
> diff --git a/i386/include/mach/i386/multiboot.h 
> b/i386/include/mach/i386/multiboot.h
> index 5a532576..40522d96 100644
> --- a/i386/include/mach/i386/multiboot.h
> +++ b/i386/include/mach/i386/multiboot.h
> @@ -25,31 +25,6 @@
>  
>  #include <mach/machine/vm_types.h>
>  
> -/* For a.out kernel boot images, the following header must appear
> -   somewhere in the first 8192 bytes of the kernel image file.  */
> -struct multiboot_header
> -{
> -     /* Must be MULTIBOOT_MAGIC */
> -     unsigned                magic;
> -
> -     /* Feature flags - see below.  */
> -     unsigned                flags;
> -
> -     /*
> -      * Checksum
> -      *
> -      * The above fields plus this one must equal 0 mod 2^32.
> -      */
> -     unsigned                checksum;
> -
> -     /* These are only valid if MULTIBOOT_AOUT_KLUDGE is set.  */
> -     vm_offset_t             header_addr;
> -     vm_offset_t             load_addr;
> -     vm_offset_t             load_end_addr;
> -     vm_offset_t             bss_end_addr;
> -     vm_offset_t             entry;
> -};
> -
>  /* The entire multiboot_header must be contained
>     within the first MULTIBOOT_SEARCH bytes of the kernel image.  */
>  #define MULTIBOOT_SEARCH     8192
> @@ -78,61 +53,7 @@ struct multiboot_header
>     that the multiboot method is being used */
>  #define MULTIBOOT_VALID         0x2badb002
>  
> -/* The boot loader passes this data structure to the kernel in
> -   register EBX on entry.  */
> -struct multiboot_info
> -{
> -     /* These flags indicate which parts of the multiboot_info are valid;
> -        see below for the actual flag bit definitions.  */
> -     unsigned                flags;
> -
> -     /* Lower/Upper memory installed in the machine.
> -        Valid only if MULTIBOOT_MEMORY is set in flags word above.  */
> -     vm_size_t               mem_lower;
> -     vm_size_t               mem_upper;
> -
> -     /* BIOS disk device the kernel was loaded from.
> -        Valid only if MULTIBOOT_BOOT_DEVICE is set in flags word above.  */
> -     unsigned char           boot_device[4];
> -
> -     /* Command-line for the OS kernel: a null-terminated ASCII string.
> -        Valid only if MULTIBOOT_CMDLINE is set in flags word above.  */
> -     vm_offset_t             cmdline;
> -
> -     /* List of boot modules loaded with the kernel.
> -        Valid only if MULTIBOOT_MODS is set in flags word above.  */
> -     unsigned                mods_count;
> -     vm_offset_t             mods_addr;
> -
> -     /* Symbol information for a.out or ELF executables. */
> -     union
> -     {
> -       struct
> -       {
> -         /* a.out symbol information valid only if MULTIBOOT_AOUT_SYMS
> -            is set in flags word above.  */
> -         vm_size_t           tabsize;
> -         vm_size_t           strsize;
> -         vm_offset_t         addr;
> -         unsigned            reserved;
> -       } a;
> -
> -       struct
> -       {
> -         /* ELF section header information valid only if
> -            MULTIBOOT_ELF_SHDR is set in flags word above.  */
> -         unsigned            num;
> -         vm_size_t           size;
> -         vm_offset_t         addr;
> -         unsigned            shndx;
> -       } e;
> -     } syms;
> -
> -     /* Memory map buffer.
> -        Valid only if MULTIBOOT_MEM_MAP is set in flags word above.  */
> -     vm_size_t               mmap_count;
> -     vm_offset_t             mmap_addr;
> -};
> +
>  
>  #define MULTIBOOT_MEMORY     0x00000001
>  #define MULTIBOOT_BOOT_DEVICE        0x00000002
> @@ -175,33 +96,6 @@ struct multiboot32_module
>  };
>  #endif
>  
> -
> -/* The mmap_addr field above contains the physical address of the first
> -   of the AddrRangeDesc structure.  "size" represents the size of the
> -   rest of the structure and optional padding.  The offset to the beginning
> -   of the next structure is therefore "size + 4".  */
> -struct AddrRangeDesc
> -{
> -  unsigned long size;
> -  unsigned long BaseAddrLow;
> -  unsigned long BaseAddrHigh;
> -  unsigned long LengthLow;
> -  unsigned long LengthHigh;
> -  unsigned long Type;
> -
> -  /* unspecified optional padding... */
> -};
> -
> -struct multiboot_mmap
> -{
> -  unsigned long size;
> -  unsigned long long BaseAddr;
> -  unsigned long long Length;
> -  unsigned long Type;
> -
> -  /* unspecified optional padding... */
> -};
> -
>  /* usable memory "Type", all others are reserved.  */
>  #define MB_ARD_MEMORY       1
>  
> diff --git a/include/mach/multiboot.h b/include/mach/multiboot.h
> deleted file mode 100644
> index b23df4a4..00000000
> --- a/include/mach/multiboot.h
> +++ /dev/null
> @@ -1,82 +0,0 @@
> -/*
> - * Copyright (c) 1995-1994 The University of Utah and
> - * the Computer Systems Laboratory at the University of Utah (CSL).
> - * All rights reserved.
> - *
> - * Permission to use, copy, modify and distribute this software is hereby
> - * granted provided that (1) source code retains these copyright, permission,
> - * and disclaimer notices, and (2) redistributions including binaries
> - * reproduce the notices in supporting documentation, and (3) all advertising
> - * materials mentioning features or use of this software display the 
> following
> - * acknowledgement: ``This product includes software developed by the
> - * Computer Systems Laboratory at the University of Utah.''
> - *
> - * THE UNIVERSITY OF UTAH AND CSL ALLOW FREE USE OF THIS SOFTWARE IN ITS "AS
> - * IS" CONDITION.  THE UNIVERSITY OF UTAH AND CSL DISCLAIM ANY LIABILITY OF
> - * ANY KIND FOR ANY DAMAGES WHATSOEVER RESULTING FROM THE USE OF THIS 
> SOFTWARE.
> - *
> - * CSL requests users of this software to return to csl-dist@cs.utah.edu any
> - * improvements that they make and grant CSL redistribution rights.
> - *
> - *      Author: Bryan Ford, University of Utah CSL
> - */
> -#ifndef _MACH_MULTIBOOT_H_
> -#define _MACH_MULTIBOOT_H_
> -
> -#include <mach/machine/vm_types.h>
> -#include <mach/machine/multiboot.h>
> -
> -struct multiboot_region
> -{
> -     vm_offset_t             start;
> -     vm_offset_t             end;
> -};
> -
> -struct multiboot_rlist
> -{
> -     int                     count;
> -     vm_offset_t             regions;
> -};
> -
> -struct multiboot_module
> -{
> -     /* Location and size of the module.  */
> -     struct multiboot_region region;
> -
> -     /* Command-line associated with this boot module:
> -        a null-terminated ASCII string.
> -        Both start and end are 0 if there is no command line.
> -        The end pointer points at least one byte past the terminating null.  
> */
> -     struct multiboot_region cmdline;
> -
> -     /* Reserved; boot loader must initialize to zero.  */
> -     natural_t               pad[4];
> -};
> -
> -struct multiboot_info
> -{
> -     /* List of available physical memory regions.
> -        Can (and probably does) include the memory containing
> -        the kernel, boot modules, this structure, etc.  */
> -     struct multiboot_rlist  avail;
> -
> -     /* Physical memory region occupied by things the boot loader set up
> -        and the OS shouldn't clobber at least until it's all done 
> initializing itself.
> -        This includes the kernel image, boot modules, these structures,
> -        initial processor tables, etc.  */
> -     struct multiboot_rlist  occupied;
> -
> -     /* Command-line for the OS kernel: a null-terminated ASCII string.
> -        Both start and end are 0 if there is no command line.
> -        The end pointer points at least one byte past the terminating null.  
> */
> -     struct multiboot_region cmdline;
> -
> -     /* Secondary boot modules loaded with this kernel image.  */
> -     int                     nmods;
> -     vm_offset_t             mods;
> -
> -     /* Reserved; boot loader must initialize to zero.  */
> -     natural_t               pad[4];
> -};
> -
> -#endif       /* _MACH_MULTIBOOT_H_ */
> diff --git a/kern/bootstrap.c b/kern/bootstrap.c
> index 60e1ad58..60648c9d 100644
> --- a/kern/bootstrap.c
> +++ b/kern/bootstrap.c
> @@ -70,7 +70,7 @@
>  #include <mach/xen.h>
>  extern struct start_info boot_info;  /* XXX put this in a header! */
>  #else        /* MACH_XEN */
> -extern struct multiboot_info boot_info;      /* XXX put this in a header! */
> +extern struct multiboot_raw_info boot_info;  /* XXX put this in a header! */
>  #endif       /* MACH_XEN */
>  #endif
>  
> @@ -155,9 +155,25 @@ void bootstrap_create(void)
>    boot_info.mods_count = n;
>    boot_info.flags |= MULTIBOOT_MODS;
>  #else        /* MACH_XEN */
> +#ifdef __x86_64__
> +  struct multiboot_raw_module *bmods32 = ((struct multiboot_raw_module *)
> +                                          phystokv(boot_info.mods_addr));
> +  struct multiboot_module *bmods=NULL;
> +  if (bmods32)
> +    {
> +      int i;
> +      bmods = alloca(boot_info.mods_count * sizeof(*bmods));
> +      for (i=0; i<boot_info.mods_count; i++)
> +        {
> +          bmods[i].mod_start = bmods32[i].mod_start;
> +          bmods[i].mod_end = bmods32[i].mod_end;
> +          bmods[i].string = bmods32[i].string;
> +        }
> +    }
> +#else
>    struct multiboot_module *bmods = ((struct multiboot_module *)
>                                   phystokv(boot_info.mods_addr));
> -
> +#endif
>  #endif       /* MACH_XEN */
>    if (!(boot_info.flags & MULTIBOOT_MODS)
>        || (boot_info.mods_count == 0))
> -- 
> 2.30.2
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.