Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 17 Sep 2022 12:39:35 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        =?UTF-8?Q?T=C4=B3l_Coosemans?= <tijl@freebsd.org>
Cc:        Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>,  "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org
Subject:   Re: git: 9758dd3de1cd - main - stand: Allocate bootinfo rather than have it be static
Message-ID:  <CANCZdfq=0sMgy%2B=O=64QywMwP=4v9B5ZXmNOc_9ZHyN66=GZSw@mail.gmail.com>
In-Reply-To: <20220917190108.2b56bb39@FreeBSD.org>
References:  <202209161554.28GFs4aA086609@gitrepo.freebsd.org> <20220917190108.2b56bb39@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help

[-- Attachment #1 --]
On Sat, Sep 17, 2022, 11:01 AM Tijl Coosemans <tijl@freebsd.org> wrote:

> On Fri, 16 Sep 2022 15:54:04 GMT Warner Losh <imp@FreeBSD.org> wrote:
> > The branch main has been updated by imp:
> >
> > URL:
> https://cgit.FreeBSD.org/src/commit/?id=9758dd3de1cddc8271be8dd6fee69286c5c86535
> >
> > commit 9758dd3de1cddc8271be8dd6fee69286c5c86535
> > Author:     Warner Losh <imp@FreeBSD.org>
> > AuthorDate: 2022-09-16 15:09:41 +0000
> > Commit:     Warner Losh <imp@FreeBSD.org>
> > CommitDate: 2022-09-16 15:18:57 +0000
> >
> >     stand: Allocate bootinfo rather than have it be static
> >
> >     This saves 80 bytes (the new bootinfo structure was 84 bytes, and a
> >     pointer is 4 bytes). The bi_load32 code is the same size.
> >
> >     Sponsored by:           Netflix
> >     Reviewed by:            tsoome
> >     Differential Revision:  https://reviews.freebsd.org/D36575
> > ---
> >  stand/i386/libi386/bootinfo32.c | 29 +++++++++++++++--------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/stand/i386/libi386/bootinfo32.c
> b/stand/i386/libi386/bootinfo32.c
> > index 372bced917d6..37a797289f2b 100644
> > --- a/stand/i386/libi386/bootinfo32.c
> > +++ b/stand/i386/libi386/bootinfo32.c
> > @@ -42,7 +42,7 @@ __FBSDID("$FreeBSD$");
> >  #include "geliboot.h"
> >  #endif
> >
> > -static struct bootinfo  bi;
> > +static struct bootinfo  *bi;
> >
> >  /*
> >   * Load the information expected by an i386 kernel.
> > @@ -91,11 +91,12 @@ bi_load32(char *args, int *howtop, int *bootdevp,
> vm_offset_t *bip, vm_offset_t
> >      /* XXX - use a default bootdev of 0.  Is this ok??? */
> >      bootdevnr = 0;
> >
> > +    bi = calloc(sizeof(*bi), 1);
> >      switch(rootdev->dd.d_dev->dv_type) {
> >      case DEVT_CD:
> >      case DEVT_DISK:
> >       /* pass in the BIOS device number of the current disk */
> > -     bi.bi_bios_dev = bd_unit2bios(rootdev);
> > +     bi->bi_bios_dev = bd_unit2bios(rootdev);
> >       bootdevnr = bd_getdev(rootdev);
> >       break;
> >
> > @@ -172,22 +173,22 @@ bi_load32(char *args, int *howtop, int *bootdevp,
> vm_offset_t *bip, vm_offset_t
> >      /* legacy bootinfo structure */
> >      kernelname = getenv("kernelname");
> >      i386_getdev(NULL, kernelname, &kernelpath);
> > -    bi.bi_version = BOOTINFO_VERSION;
> > -    bi.bi_size = sizeof(bi);
> > -    bi.bi_memsizes_valid = 1;
> > -    bi.bi_basemem = bios_basemem / 1024;
> > -    bi.bi_extmem = bios_extmem / 1024;
> > -    bi.bi_envp = envp;
> > -    bi.bi_modulep = *modulep;
> > -    bi.bi_kernend = kernend;
> > -    bi.bi_kernelname = VTOP(kernelpath);
> > -    bi.bi_symtab = ssym;       /* XXX this is only the primary kernel
> symtab */
> > -    bi.bi_esymtab = esym;
> > +    bi->bi_version = BOOTINFO_VERSION;
> > +    bi->bi_size = sizeof(bi);
>
> sizeof(*bi) here and maybe make bi a function variable?
>

Damn. I even looked for those... :(

Warner

> +    bi->bi_memsizes_valid = 1;
> > +    bi->bi_basemem = bios_basemem / 1024;
> > +    bi->bi_extmem = bios_extmem / 1024;
> > +    bi->bi_envp = envp;
> > +    bi->bi_modulep = *modulep;
> > +    bi->bi_kernend = kernend;
> > +    bi->bi_kernelname = VTOP(kernelpath);
> > +    bi->bi_symtab = ssym;       /* XXX this is only the primary kernel
> symtab */
> > +    bi->bi_esymtab = esym;
> >
> >      /* legacy boot arguments */
> >      *howtop = howto | RB_BOOTINFO;
> >      *bootdevp = bootdevnr;
> > -    *bip = VTOP(&bi);
> > +    *bip = VTOP(bi);
> >
> >      return(0);
> >  }
>
>

[-- Attachment #2 --]
<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Sep 17, 2022, 11:01 AM Tijl Coosemans &lt;<a href="mailto:tijl@freebsd.org">tijl@freebsd.org</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, 16 Sep 2022 15:54:04 GMT Warner Losh &lt;imp@FreeBSD.org&gt; wrote:<br>
&gt; The branch main has been updated by imp:<br>
&gt; <br>
&gt; URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=9758dd3de1cddc8271be8dd6fee69286c5c86535" rel="noreferrer noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=9758dd3de1cddc8271be8dd6fee69286c5c86535</a><br>;
&gt; <br>
&gt; commit 9758dd3de1cddc8271be8dd6fee69286c5c86535<br>
&gt; Author:     Warner Losh &lt;imp@FreeBSD.org&gt;<br>
&gt; AuthorDate: 2022-09-16 15:09:41 +0000<br>
&gt; Commit:     Warner Losh &lt;imp@FreeBSD.org&gt;<br>
&gt; CommitDate: 2022-09-16 15:18:57 +0000<br>
&gt; <br>
&gt;     stand: Allocate bootinfo rather than have it be static<br>
&gt;     <br>
&gt;     This saves 80 bytes (the new bootinfo structure was 84 bytes, and a<br>
&gt;     pointer is 4 bytes). The bi_load32 code is the same size.<br>
&gt;     <br>
&gt;     Sponsored by:           Netflix<br>
&gt;     Reviewed by:            tsoome<br>
&gt;     Differential Revision:  <a href="https://reviews.freebsd.org/D36575" rel="noreferrer noreferrer" target="_blank">https://reviews.freebsd.org/D36575</a><br>;
&gt; ---<br>
&gt;  stand/i386/libi386/bootinfo32.c | 29 +++++++++++++++--------------<br>
&gt;  1 file changed, 15 insertions(+), 14 deletions(-)<br>
&gt; <br>
&gt; diff --git a/stand/i386/libi386/bootinfo32.c b/stand/i386/libi386/bootinfo32.c<br>
&gt; index 372bced917d6..37a797289f2b 100644<br>
&gt; --- a/stand/i386/libi386/bootinfo32.c<br>
&gt; +++ b/stand/i386/libi386/bootinfo32.c<br>
&gt; @@ -42,7 +42,7 @@ __FBSDID(&quot;$FreeBSD$&quot;);<br>
&gt;  #include &quot;geliboot.h&quot;<br>
&gt;  #endif<br>
&gt;  <br>
&gt; -static struct bootinfo  bi;<br>
&gt; +static struct bootinfo  *bi;<br>
&gt;  <br>
&gt;  /*<br>
&gt;   * Load the information expected by an i386 kernel.<br>
&gt; @@ -91,11 +91,12 @@ bi_load32(char *args, int *howtop, int *bootdevp, vm_offset_t *bip, vm_offset_t<br>
&gt;      /* XXX - use a default bootdev of 0.  Is this ok??? */<br>
&gt;      bootdevnr = 0;<br>
&gt;  <br>
&gt; +    bi = calloc(sizeof(*bi), 1);<br>
&gt;      switch(rootdev-&gt;dd.d_dev-&gt;dv_type) {<br>
&gt;      case DEVT_CD:<br>
&gt;      case DEVT_DISK:<br>
&gt;       /* pass in the BIOS device number of the current disk */<br>
&gt; -     bi.bi_bios_dev = bd_unit2bios(rootdev);<br>
&gt; +     bi-&gt;bi_bios_dev = bd_unit2bios(rootdev);<br>
&gt;       bootdevnr = bd_getdev(rootdev);<br>
&gt;       break;<br>
&gt;  <br>
&gt; @@ -172,22 +173,22 @@ bi_load32(char *args, int *howtop, int *bootdevp, vm_offset_t *bip, vm_offset_t<br>
&gt;      /* legacy bootinfo structure */<br>
&gt;      kernelname = getenv(&quot;kernelname&quot;);<br>
&gt;      i386_getdev(NULL, kernelname, &amp;kernelpath);<br>
&gt; -    bi.bi_version = BOOTINFO_VERSION;<br>
&gt; -    bi.bi_size = sizeof(bi);<br>
&gt; -    bi.bi_memsizes_valid = 1;<br>
&gt; -    bi.bi_basemem = bios_basemem / 1024;<br>
&gt; -    bi.bi_extmem = bios_extmem / 1024;<br>
&gt; -    bi.bi_envp = envp;<br>
&gt; -    bi.bi_modulep = *modulep;<br>
&gt; -    bi.bi_kernend = kernend;<br>
&gt; -    bi.bi_kernelname = VTOP(kernelpath);<br>
&gt; -    bi.bi_symtab = ssym;       /* XXX this is only the primary kernel symtab */<br>
&gt; -    bi.bi_esymtab = esym;<br>
&gt; +    bi-&gt;bi_version = BOOTINFO_VERSION;<br>
&gt; +    bi-&gt;bi_size = sizeof(bi);<br>
<br>
sizeof(*bi) here and maybe make bi a function variable?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Damn. I even looked for those... :(</div><div dir="auto"><br></div><div dir="auto">Warner </div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
&gt; +    bi-&gt;bi_memsizes_valid = 1;<br>
&gt; +    bi-&gt;bi_basemem = bios_basemem / 1024;<br>
&gt; +    bi-&gt;bi_extmem = bios_extmem / 1024;<br>
&gt; +    bi-&gt;bi_envp = envp;<br>
&gt; +    bi-&gt;bi_modulep = *modulep;<br>
&gt; +    bi-&gt;bi_kernend = kernend;<br>
&gt; +    bi-&gt;bi_kernelname = VTOP(kernelpath);<br>
&gt; +    bi-&gt;bi_symtab = ssym;       /* XXX this is only the primary kernel symtab */<br>
&gt; +    bi-&gt;bi_esymtab = esym;<br>
&gt;  <br>
&gt;      /* legacy boot arguments */<br>
&gt;      *howtop = howto | RB_BOOTINFO;<br>
&gt;      *bootdevp = bootdevnr;<br>
&gt; -    *bip = VTOP(&amp;bi);<br>
&gt; +    *bip = VTOP(bi);<br>
&gt;  <br>
&gt;      return(0);<br>
&gt;  }<br>
<br>
</blockquote></div></div></div>

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfq=0sMgy%2B=O=64QywMwP=4v9B5ZXmNOc_9ZHyN66=GZSw>