svn commit: r218745 - head/sys/boot/i386/boot2

[Alexander Best]

On Wed Feb 16 11, Ronald Klop wrote:
> On Wed, 16 Feb 2011 23:41:26 +0100, Alexander Best <arundel at freebsd.org>  
> wrote:
> 
> >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.
> 
> 
> You guys are kings. :-) I heard they don't even teach assembly anymore at  
> a lot of universities.

*lol* assembly (especially i386) isn't that complicated. but you're right:
they don't teach it at university that much. usually you only get to hear
about it in classes which focus on the principals of the CPU, RAM, cache, etc.

the reason for that is quite simple: assembly languages don't get used that
much anymore. in the past they were used a lot to optimise speed, but since
computers have gotten so fast that's not needed anymore.

even for embedded platforms assembly isn't used very often. one of the reason
is also that it's incredibly hard to maintain. comprehending foreign code
(written by another programmer) is really really hard work.

so +1 for having fewer assembly code nowadays. we now have enough cpu power
and hardware ressources in general to having the luxury of letting the
computer do all the abstractions from human thinking to how computers work.

cheers.
alex

> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> >
> >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