Re: git: 86077f4fd110 - main - stand: use globals for the kernel and module types

Reply: Warner Losh : "Re: git: 86077f4fd110 - main - stand: use globals for the kernel and module types"
In reply to: Warner Losh : "git: 86077f4fd110 - main - stand: use globals for the kernel and module types"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Jessica Clarke <jrtc27_at_freebsd.org>
Date: Fri, 24 Jan 2025 21:54:58 UTC
On 24 Jan 2025, at 21:37, Warner Losh <imp@FreeBSD.org> wrote:
> 
> The branch main has been updated by imp:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=86077f4fd11070518a6d04eee7fdb93cbbfb1b52
> 
> commit 86077f4fd11070518a6d04eee7fdb93cbbfb1b52
> Author:     Ahmad Khalifa <ahmadkhalifa570@gmail.com>
> AuthorDate: 2024-08-24 15:16:09 +0000
> Commit:     Warner Losh <imp@FreeBSD.org>
> CommitDate: 2025-01-24 21:29:39 +0000
> 
>    stand: use globals for the kernel and module types

As with the kernel, please just use the macros directly.

Jess

>    Reviewed by: imp, kib
>    Pull Request: https://github.com/freebsd/freebsd-src/pull/1394
> ---
> stand/common/gfx_fb.c                        |  6 ++++--
> stand/common/load_elf.c                      | 12 +++++-------
> stand/common/load_elf_obj.c                  |  8 +++-----
> stand/common/metadata.c                      |  2 +-
> stand/common/modinfo.c                       |  5 +++++
> stand/common/modinfo.h                       |  5 +++++
> stand/efi/loader/arch/amd64/multiboot2.c     |  9 +++++----
> stand/efi/loader/bootinfo.c                  |  2 +-
> stand/i386/libi386/bootinfo32.c              |  2 +-
> stand/i386/libi386/bootinfo64.c              |  2 +-
> stand/i386/libi386/multiboot.c               |  9 +++++----
> stand/powerpc/ofw/elf_freebsd.c              |  2 +-
> stand/powerpc/ofw/ppc64_elf_freebsd.c        |  2 +-
> stand/uboot/arch/powerpc/ppc64_elf_freebsd.c |  2 +-
> stand/uboot/elf_freebsd.c                    |  2 +-
> stand/userboot/userboot/bootinfo32.c         |  2 +-
> stand/userboot/userboot/bootinfo64.c         |  2 +-
> 17 files changed, 42 insertions(+), 32 deletions(-)
> 
> diff --git a/stand/common/gfx_fb.c b/stand/common/gfx_fb.c
> index 9bda0e7139a9..63036f3c07f6 100644
> --- a/stand/common/gfx_fb.c
> +++ b/stand/common/gfx_fb.c
> @@ -102,6 +102,8 @@
> #include <vbe.h>
> #endif
> 
> +#include "modinfo.h"
> +
> /* VGA text mode does use bold font. */
> #if !defined(VGA_8X16_FONT)
> #define VGA_8X16_FONT "/boot/fonts/8x16b.fnt"
> @@ -2982,7 +2984,7 @@ build_font_module(vm_offset_t addr)
> 
> fi.fi_checksum = -checksum;
> 
> - fp = file_findfile(NULL, "elf kernel");
> + fp = file_findfile(NULL, md_kerntype);
> if (fp == NULL)
> panic("can't find kernel file");
> 
> @@ -3024,7 +3026,7 @@ build_splash_module(vm_offset_t addr)
> return (addr);
> }
> 
> - fp = file_findfile(NULL, "elf kernel");
> + fp = file_findfile(NULL, md_kerntype);
> if (fp == NULL)
> panic("can't find kernel file");
> 
> diff --git a/stand/common/load_elf.c b/stand/common/load_elf.c
> index d3775b9f0017..e19aefa121e7 100644
> --- a/stand/common/load_elf.c
> +++ b/stand/common/load_elf.c
> @@ -37,6 +37,7 @@
> #include <sys/link_elf.h>
> 
> #include "bootstrap.h"
> +#include "modinfo.h"
> 
> #define COPYOUT(s,d,l) archsw.arch_copyout((vm_offset_t)(s), d, l)
> 
> @@ -89,9 +90,6 @@ static int __elfN(parse_modmetadata)(struct preloaded_file *mp, elf_file_t ef,
> static symaddr_fn __elfN(symaddr);
> static char *fake_modname(const char *name);
> 
> -const char *__elfN(kerneltype) = "elf kernel";
> -const char *__elfN(moduletype) = "elf module";
> -
> uint64_t __elfN(relocation_offset) = 0;
> 
> #ifdef __powerpc__
> @@ -384,7 +382,7 @@ __elfN(loadfile_raw)(char *filename, uint64_t dest,
> /*
> * Check to see what sort of module we are.
> */
> - kfp = file_findfile(NULL, __elfN(kerneltype));
> + kfp = file_findfile(NULL, md_kerntype);
> #ifdef __powerpc__
> /*
> * Kernels can be ET_DYN, so just assume the first loaded object is the
> @@ -435,7 +433,7 @@ __elfN(loadfile_raw)(char *filename, uint64_t dest,
> err = EPERM;
> goto oerr;
> }
> - if (strcmp(__elfN(kerneltype), kfp->f_type)) {
> + if (strcmp(md_kerntype, kfp->f_type)) {
> printf("elf" __XSTRING(__ELF_WORD_SIZE)
> "_loadfile: can't load module with kernel type '%s'\n",
>    kfp->f_type);
> @@ -470,9 +468,9 @@ __elfN(loadfile_raw)(char *filename, uint64_t dest,
> fp->f_name = strdup(filename);
> if (multiboot == 0)
> fp->f_type = strdup(ef.kernel ?
> -    __elfN(kerneltype) : __elfN(moduletype));
> +    md_kerntype : md_modtype);
> else
> - fp->f_type = strdup("elf multiboot kernel");
> + fp->f_type = strdup(md_kerntype_mb);
> 
> if (module_verbose >= MODULE_VERBOSE_FULL) {
> if (ef.kernel)
> diff --git a/stand/common/load_elf_obj.c b/stand/common/load_elf_obj.c
> index 9ff1d238b8c8..1e07828dd8ac 100644
> --- a/stand/common/load_elf_obj.c
> +++ b/stand/common/load_elf_obj.c
> @@ -37,6 +37,7 @@
> #include <sys/link_elf.h>
> 
> #include "bootstrap.h"
> +#include "modinfo.h"
> 
> #define COPYOUT(s,d,l) archsw.arch_copyout((vm_offset_t)(s), d, l)
> 
> @@ -77,9 +78,6 @@ static int __elfN(obj_parse_modmetadata)(struct preloaded_file *mp,
>     elf_file_t ef);
> static Elf_Addr __elfN(obj_symaddr)(struct elf_file *ef, Elf_Size symidx);
> 
> -const char *__elfN(obj_kerneltype) = "elf kernel";
> -const char *__elfN(obj_moduletype) = "elf obj module";
> -
> /*
>  * Attempt to load the file (file) as an ELF module.  It will be stored at
>  * (dest), and a pointer to a module structure describing the loaded object
> @@ -154,7 +152,7 @@ __elfN(obj_loadfile)(char *filename, uint64_t dest,
> }
> #endif
> 
> - kfp = file_findfile(NULL, __elfN(obj_kerneltype));
> + kfp = file_findfile(NULL, md_kerntype);
> if (kfp == NULL) {
> printf("elf" __XSTRING(__ELF_WORD_SIZE)
>    "_obj_loadfile: can't load module before kernel\n");
> @@ -178,7 +176,7 @@ __elfN(obj_loadfile)(char *filename, uint64_t dest,
> goto out;
> }
> fp->f_name = strdup(filename);
> - fp->f_type = strdup(__elfN(obj_moduletype));
> + fp->f_type = strdup(md_modtype_obj);
> 
> if (module_verbose > MODULE_VERBOSE_SILENT)
> printf("%s ", filename);
> diff --git a/stand/common/metadata.c b/stand/common/metadata.c
> index 8962763061dc..22df6f175791 100644
> --- a/stand/common/metadata.c
> +++ b/stand/common/metadata.c
> @@ -146,7 +146,7 @@ md_load_dual(char *args, vm_offset_t *modulep, vm_offset_t *dtb, int kern64)
> #endif
> 
>     kernend = 0;
> -    kfp = file_findfile(NULL, "elf kernel");
> +    kfp = file_findfile(NULL, md_kerntype);
>     if (kfp == NULL)
> panic("can't find kernel file");
>     file_addmetadata(kfp, MODINFOMD_HOWTO, sizeof howto, &howto);
> diff --git a/stand/common/modinfo.c b/stand/common/modinfo.c
> index 381bd9dfd719..d00548c91c57 100644
> --- a/stand/common/modinfo.c
> +++ b/stand/common/modinfo.c
> @@ -109,6 +109,11 @@
> 
> #define MOD_ALIGN(l) roundup(l, align)
> 
> +const char md_modtype[] = MODTYPE;
> +const char md_kerntype[] = KERNTYPE;
> +const char md_modtype_obj[] = MODTYPE_OBJ;
> +const char md_kerntype_mb[] = KERNTYPE_MB;
> +
> vm_offset_t
> md_copymodules(vm_offset_t addr, bool kern64)
> {
> diff --git a/stand/common/modinfo.h b/stand/common/modinfo.h
> index 967367beeeb0..d26129089fb6 100644
> --- a/stand/common/modinfo.h
> +++ b/stand/common/modinfo.h
> @@ -6,6 +6,11 @@
> #ifndef COMMON_MODINFO_H
> #define COMMON_MODINFO_H
> 
> +extern const char md_modtype[];
> +extern const char md_kerntype[];
> +extern const char md_modtype_obj[];
> +extern const char md_kerntype_mb[];
> +
> int md_load(char *args, vm_offset_t *modulep, vm_offset_t *dtb);
> int md_load64(char *args, vm_offset_t *modulep, vm_offset_t *dtb);
> 
> diff --git a/stand/efi/loader/arch/amd64/multiboot2.c b/stand/efi/loader/arch/amd64/multiboot2.c
> index d09b01fce1fc..eb7362293406 100644
> --- a/stand/efi/loader/arch/amd64/multiboot2.c
> +++ b/stand/efi/loader/arch/amd64/multiboot2.c
> @@ -51,6 +51,7 @@
> #include "bootstrap.h"
> #include "multiboot2.h"
> #include "loader_efi.h"
> +#include "modinfo.h"
> 
> extern int elf32_loadfile_raw(char *filename, uint64_t dest,
>     struct preloaded_file **result, int multiboot);
> @@ -436,7 +437,7 @@ exec(struct preloaded_file *fp)
> *  module 0                 module 1
> */
> 
> - fp = file_findfile(NULL, "elf kernel");
> + fp = file_findfile(NULL, md_kerntype);
> if (fp == NULL) {
> printf("No FreeBSD kernel provided, aborting\n");
> error = EINVAL;
> @@ -498,7 +499,7 @@ obj_loadfile(char *filename, uint64_t dest, struct preloaded_file **result)
> int error;
> 
> /* See if there's a multiboot kernel loaded */
> - mfp = file_findfile(NULL, "elf multiboot kernel");
> + mfp = file_findfile(NULL, md_kerntype_mb);
> if (mfp == NULL)
> return (EFTYPE);
> 
> @@ -506,14 +507,14 @@ obj_loadfile(char *filename, uint64_t dest, struct preloaded_file **result)
> * We have a multiboot kernel loaded, see if there's a FreeBSD
> * kernel loaded also.
> */
> - kfp = file_findfile(NULL, "elf kernel");
> + kfp = file_findfile(NULL, md_kerntype);
> if (kfp == NULL) {
> /*
> * No kernel loaded, this must be it. The kernel has to
> * be loaded as a raw file, it will be processed by
> * Xen and correctly loaded as an ELF file.
> */
> - rfp = file_loadraw(filename, "elf kernel", 0);
> + rfp = file_loadraw(filename, md_kerntype, 0);
> if (rfp == NULL) {
> printf(
> "Unable to load %s as a multiboot payload kernel\n",
> diff --git a/stand/efi/loader/bootinfo.c b/stand/efi/loader/bootinfo.c
> index b7d4070c2ffb..3e74a9228b5e 100644
> --- a/stand/efi/loader/bootinfo.c
> +++ b/stand/efi/loader/bootinfo.c
> @@ -420,7 +420,7 @@ bi_load(char *args, vm_offset_t *modulep, vm_offset_t *kernendp, bool exit_bs)
> addr += roundup(dtb_size, PAGE_SIZE);
> #endif
> 
> - kfp = file_findfile(NULL, "elf kernel");
> + kfp = file_findfile(NULL, md_kerntype);
> if (kfp == NULL)
> panic("can't find kernel file");
> kernend = 0; /* fill it in later */
> diff --git a/stand/i386/libi386/bootinfo32.c b/stand/i386/libi386/bootinfo32.c
> index df715e547795..37b227b913bd 100644
> --- a/stand/i386/libi386/bootinfo32.c
> +++ b/stand/i386/libi386/bootinfo32.c
> @@ -129,7 +129,7 @@ bi_load32(char *args, int *howtop, int *bootdevp, vm_offset_t *bip, vm_offset_t
>     /* pad to a page boundary */
>     addr = roundup(addr, PAGE_SIZE);
> 
> -    kfp = file_findfile(NULL, "elf kernel");
> +    kfp = file_findfile(NULL, md_kerntype);
>     if (kfp == NULL)
> panic("can't find kernel file");
>     kernend = 0; /* fill it in later */
> diff --git a/stand/i386/libi386/bootinfo64.c b/stand/i386/libi386/bootinfo64.c
> index 4731b10325fe..f7181dcd599f 100644
> --- a/stand/i386/libi386/bootinfo64.c
> +++ b/stand/i386/libi386/bootinfo64.c
> @@ -143,7 +143,7 @@ bi_load64(char *args, vm_offset_t *modulep,
>     /* place the metadata before anything */
>     module = *modulep = addr;
> 
> -    kfp = file_findfile(NULL, "elf kernel");
> +    kfp = file_findfile(NULL, md_kerntype);
>     if (kfp == NULL)
> panic("can't find kernel file");
>     kernend = 0; /* fill it in later */
> diff --git a/stand/i386/libi386/multiboot.c b/stand/i386/libi386/multiboot.c
> index b69895de9706..e2bd44fe83f5 100644
> --- a/stand/i386/libi386/multiboot.c
> +++ b/stand/i386/libi386/multiboot.c
> @@ -48,6 +48,7 @@
> #include "bootstrap.h"
> #include "multiboot.h"
> #include "libi386.h"
> +#include "modinfo.h"
> #include <btxv86.h>
> 
> #define MULTIBOOT_SUPPORTED_FLAGS \
> @@ -256,7 +257,7 @@ multiboot_exec(struct preloaded_file *fp)
> *  module 0                 module 1
> */
> 
> - fp = file_findfile(NULL, "elf kernel");
> + fp = file_findfile(NULL, md_kerntype);
> if (fp == NULL) {
> printf("No FreeBSD kernel provided, aborting\n");
> error = EINVAL;
> @@ -324,7 +325,7 @@ multiboot_obj_loadfile(char *filename, uint64_t dest,
> int error, mod_num;
> 
> /* See if there's a multiboot kernel loaded */
> - mfp = file_findfile(NULL, "elf multiboot kernel");
> + mfp = file_findfile(NULL, md_kerntype_mb);
> if (mfp == NULL)
> return (EFTYPE);
> 
> @@ -332,14 +333,14 @@ multiboot_obj_loadfile(char *filename, uint64_t dest,
> * We have a multiboot kernel loaded, see if there's a FreeBSD
> * kernel loaded also.
> */
> - kfp = file_findfile(NULL, "elf kernel");
> + kfp = file_findfile(NULL, md_kerntype);
> if (kfp == NULL) {
> /*
> * No kernel loaded, this must be it. The kernel has to
> * be loaded as a raw file, it will be processed by
> * Xen and correctly loaded as an ELF file.
> */
> - rfp = file_loadraw(filename, "elf kernel", 0);
> + rfp = file_loadraw(filename, md_kerntype, 0);
> if (rfp == NULL) {
> printf(
> "Unable to load %s as a multiboot payload kernel\n",
> diff --git a/stand/powerpc/ofw/elf_freebsd.c b/stand/powerpc/ofw/elf_freebsd.c
> index 21ab834f76fa..4d34fa18c5dd 100644
> --- a/stand/powerpc/ofw/elf_freebsd.c
> +++ b/stand/powerpc/ofw/elf_freebsd.c
> @@ -58,7 +58,7 @@ __elfN(ofw_loadfile)(char *filename, uint64_t dest,
> * No need to sync the icache for modules: this will
> * be done by the kernel after relocation.
> */
> - if (!strcmp((*result)->f_type, "elf kernel"))
> + if (!strcmp((*result)->f_type, md_kerntype))
> __syncicache((void *) (*result)->f_addr, (*result)->f_size);
> #endif
> return (0);
> diff --git a/stand/powerpc/ofw/ppc64_elf_freebsd.c b/stand/powerpc/ofw/ppc64_elf_freebsd.c
> index e0518abe2283..bc68d129f353 100644
> --- a/stand/powerpc/ofw/ppc64_elf_freebsd.c
> +++ b/stand/powerpc/ofw/ppc64_elf_freebsd.c
> @@ -57,7 +57,7 @@ ppc64_ofw_elf_loadfile(char *filename, uint64_t dest,
> * No need to sync the icache for modules: this will
> * be done by the kernel after relocation.
> */
> - if (!strcmp((*result)->f_type, "elf kernel"))
> + if (!strcmp((*result)->f_type, md_kerntype))
> __syncicache((void *) (*result)->f_addr, (*result)->f_size);
> return (0);
> }
> diff --git a/stand/uboot/arch/powerpc/ppc64_elf_freebsd.c b/stand/uboot/arch/powerpc/ppc64_elf_freebsd.c
> index 291fe6b944fb..e500b862de2e 100644
> --- a/stand/uboot/arch/powerpc/ppc64_elf_freebsd.c
> +++ b/stand/uboot/arch/powerpc/ppc64_elf_freebsd.c
> @@ -53,7 +53,7 @@ ppc64_uboot_elf_loadfile(char *filename, uint64_t dest,
> * No need to sync the icache for modules: this will
> * be done by the kernel after relocation.
> */
> - if (!strcmp((*result)->f_type, "elf kernel"))
> + if (!strcmp((*result)->f_type, md_kerntype))
> __syncicache((void *) (*result)->f_addr, (*result)->f_size);
> return (0);
> }
> diff --git a/stand/uboot/elf_freebsd.c b/stand/uboot/elf_freebsd.c
> index 6c764d143881..3b1bdc542538 100644
> --- a/stand/uboot/elf_freebsd.c
> +++ b/stand/uboot/elf_freebsd.c
> @@ -53,7 +53,7 @@ __elfN(uboot_load)(char *filename, uint64_t dest,
> * No need to sync the icache for modules: this will
> * be done by the kernel after relocation.
> */
> - if (!strcmp((*result)->f_type, "elf kernel"))
> + if (!strcmp((*result)->f_type, md_kerntype))
> __syncicache((void *) (*result)->f_addr, (*result)->f_size);
> #endif
> return (0);
> diff --git a/stand/userboot/userboot/bootinfo32.c b/stand/userboot/userboot/bootinfo32.c
> index 91f1f81f1181..750574912172 100644
> --- a/stand/userboot/userboot/bootinfo32.c
> +++ b/stand/userboot/userboot/bootinfo32.c
> @@ -108,7 +108,7 @@ bi_load32(char *args, int *howtop, int *bootdevp, vm_offset_t *bip, vm_offset_t
>     /* pad to a page boundary */
>     addr = roundup(addr, PAGE_SIZE);
> 
> -    kfp = file_findfile(NULL, "elf kernel");
> +    kfp = file_findfile(NULL, md_kerntype);
>     if (kfp == NULL)
> panic("can't find kernel file");
>     kernend = 0; /* fill it in later */
> diff --git a/stand/userboot/userboot/bootinfo64.c b/stand/userboot/userboot/bootinfo64.c
> index fb9fd0fb82f1..d20202bf4fbb 100644
> --- a/stand/userboot/userboot/bootinfo64.c
> +++ b/stand/userboot/userboot/bootinfo64.c
> @@ -140,7 +140,7 @@ bi_load64(char *args, vm_offset_t *modulep, vm_offset_t *kernendp)
>     /* pad to a page boundary */
>     addr = roundup(addr, PAGE_SIZE);
> 
> -    kfp = file_findfile(NULL, "elf kernel");
> +    kfp = file_findfile(NULL, md_kerntype);
>     if (kfp == NULL)
> panic("can't find kernel file");
>     kernend = 0; /* fill it in later */