Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 16 Feb 2011 22:41:26 +0000
From:      Alexander Best <arundel@freebsd.org>
To:        Warner Losh <imp@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r218745 - head/sys/boot/i386/boot2
Message-ID:  <20110216224126.GA47777@freebsd.org>
In-Reply-To: <20110216221712.GA44796@freebsd.org>
References:  <201102161805.p1GI5ABX078768@svn.freebsd.org> <20110216221014.GA43296@freebsd.org> <20110216221712.GA44796@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed Feb 16 11, Alexander Best wrote:
> On Wed Feb 16 11, Alexander Best wrote:
> > On Wed Feb 16 11, Warner Losh wrote:
> > > Author: imp
> > > Date: Wed Feb 16 18:05:10 2011
> > > New Revision: 218745
> > > URL: http://svn.freebsd.org/changeset/base/218745
> > > 
> > > Log:
> > >   Remove reading of symbols from a.out loaded files.  Since we are tight
> > >   on space for clang and a.out support is only needed for /boot/loader,
> > >   they are excess bytes that serve no useful purpose other than to
> > >   support really old kernels (FreeBSD < 3.2 or so).  Prefer clang
> > >   support over support for these old kernels and remove this code.  We
> > >   gain about 100 bytes of space this way.
> > 
> > i think without this code uint32_t x serves no purpose any longer:
> > 
> > /usr/git-freebsd-head/sys/boot/i386/boot2/boot2.c:322:20: warning: unused variable 'x' [-Wunused-variable]
> >     uint32_t addr, x;
> >                    ^
> 
> also due to
> 
> /usr/git-freebsd-head/sys/boot/i386/boot2/boot2.c:631:8: warning: cast from 'caddr_t' (aka 'char *') to 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Wcast-align]
>         t1 = *(uint32_t *)PTOV(0x46c);
>               ^~~~~~~~~~~~~~~~~~~~~~~
> 
> i think t0 and t1 can be turned into uint8_t's and PTOV(0x46c); can be casted
> to (uint8_t *), instead of (uint32_t *).

with this additional change the code fits when compiled with clang:

diff --git a/sys/boot/i386/boot2/sio.S b/sys/boot/i386/boot2/sio.S
index 7b8e9eb..d745129 100644
--- a/sys/boot/i386/boot2/sio.S
+++ b/sys/boot/i386/boot2/sio.S
@@ -29,11 +29,11 @@
 sio_init:      movw $SIO_PRT+0x3,%dx           # Data format reg
                movb $SIO_FMT|0x80,%al          # Set format
                outb %al,(%dx)                  #  and DLAB
-               pushl %edx                      # Save
+               pushb %dl                       # Save
                subb $0x3,%dl                   # Divisor latch reg
                movl 0x8(%esp),%eax             # Set
                outw %ax,(%dx)                  #  BPS
-               popl %edx                       # Restore
+               popb %dl                        # Restore
                movb $SIO_FMT,%al               # Clear
                outb %al,(%dx)                  #  DLAB
                incl %edx                       # Modem control reg

...since we're only modifying %dl in subb $0x3,%dl, we don't need to push/pop
a 32 bit value, but only 8 bits.

cheers.
alex

> 
> cheers.
> alex
> 
> > 
> > cheers.
> > alex
> > 
> > >   
> > >   Reviewed by:	rdivacky@
> > > 
> > > Modified:
> > >   head/sys/boot/i386/boot2/boot2.c
> > > 
> > > Modified: head/sys/boot/i386/boot2/boot2.c
> > > ==============================================================================
> > > --- head/sys/boot/i386/boot2/boot2.c	Wed Feb 16 17:50:21 2011	(r218744)
> > > +++ head/sys/boot/i386/boot2/boot2.c	Wed Feb 16 18:05:10 2011	(r218745)
> > > @@ -347,23 +347,6 @@ load(void)
> > >  	p += roundup2(hdr.ex.a_text, PAGE_SIZE);
> > >  	if (xfsread(ino, p, hdr.ex.a_data))
> > >  	    return;
> > > -	p += hdr.ex.a_data + roundup2(hdr.ex.a_bss, PAGE_SIZE);
> > > -	bootinfo.bi_symtab = VTOP(p);
> > > -	*(uint32_t*)p = hdr.ex.a_syms;
> > > -	p += sizeof(hdr.ex.a_syms);
> > > -	if (hdr.ex.a_syms) {
> > > -	    if (xfsread(ino, p, hdr.ex.a_syms))
> > > -		return;
> > > -	    p += hdr.ex.a_syms;
> > > -	    if (xfsread(ino, p, sizeof(int)))
> > > -		return;
> > > -	    x = *(uint32_t *)p;
> > > -	    p += sizeof(int);
> > > -	    x -= sizeof(int);
> > > -	    if (xfsread(ino, p, x))
> > > -		return;
> > > -	    p += x;
> > > -	}
> > >      } else {
> > >  	fs_off = hdr.eh.e_phoff;
> > >  	for (j = i = 0; i < hdr.eh.e_phnum && j < 2; i++) {
> > > @@ -395,8 +378,8 @@ load(void)
> > >  	    }
> > >  	}
> > >  	addr = hdr.eh.e_entry & 0xffffff;
> > > +	bootinfo.bi_esymtab = VTOP(p);
> > >      }
> > > -    bootinfo.bi_esymtab = VTOP(p);
> > >      bootinfo.bi_kernelname = VTOP(kname);
> > >      bootinfo.bi_bios_dev = dsk.drive;
> > >      __exec((caddr_t)addr, RB_BOOTINFO | (opts & RBX_MASK),
> > 
> > -- 
> > a13x
> 
> -- 
> a13x

-- 
a13x



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110216224126.GA47777>