svn commit: r214457 - in head/sys: amd64/amd64 conf i386/i386 x86/x86

[Attilio Rao]

2010/10/28 John Baldwin <jhb at freebsd.org>:
> On Thursday, October 28, 2010 1:21:34 pm Attilio Rao wrote:
>> 2010/10/28 John Baldwin <jhb at freebsd.org>:
>> > On Thursday, October 28, 2010 12:31:39 pm Attilio Rao wrote:
>> >> Author: attilio
>> >> Date: Thu Oct 28 16:31:39 2010
>> >> New Revision: 214457
>> >> URL: http://svn.freebsd.org/changeset/base/214457
>> >>
>> >> Log:
>> >>   Merge nexus.c from amd64 and i386 to x86 subtree.
>> >>
>> >>   Sponsored by:       Sandvine Incorporated
>> >>   Tested by:  gianni
>> >>
>> >
>> > It would be better to merge these two routines.  The loader now passes the
>> > smap to i386 kernels as well, so ram_attach() should probably be changed to
>> > try the amd64 approach first and if that fails fall back to using the
>> > phys_avail[] array instead.
>>
>> What do you think about this patch?:
>> Index: nexus.c
>> ===================================================================
>> --- nexus.c     (revision 214457)
>> +++ nexus.c     (working copy)
>> @@ -52,9 +52,7 @@
>>  #include <sys/systm.h>
>>  #include <sys/bus.h>
>>  #include <sys/kernel.h>
>> -#ifdef __amd64__
>>  #include <sys/linker.h>
>> -#endif
>>  #include <sys/malloc.h>
>>  #include <sys/module.h>
>>  #include <machine/bus.h>
>> @@ -67,12 +65,10 @@
>>  #include <vm/pmap.h>
>>  #include <machine/pmap.h>
>>
>> -#ifdef __amd64__
>>  #include <machine/metadata.h>
>> -#include <machine/pc/bios.h>
>> -#endif
>>  #include <machine/nexusvar.h>
>>  #include <machine/resource.h>
>> +#include <machine/pc/bios.h>
>>
>>  #ifdef DEV_APIC
>>  #include "pcib_if.h"
>> @@ -89,11 +85,13 @@
>>  #include <sys/rtprio.h>
>>
>>  #ifdef __amd64__
>> -#define        RMAN_BUS_SPACE_IO       AMD64_BUS_SPACE_IO
>> -#define        RMAN_BUS_SPACE_MEM      AMD64_BUS_SPACE_MEM
>> +#define        X86_BUS_SPACE_IO        AMD64_BUS_SPACE_IO
>> +#define        X86_BUS_SPACE_MEM       AMD64_BUS_SPACE_MEM
>> +#define        ELF_KERN_STR            "elf64 kernel"
>>  #else
>> -#define        RMAN_BUS_SPACE_IO       I386_BUS_SPACE_IO
>> -#define        RMAN_BUS_SPACE_MEM      I386_BUS_SPACE_MEM
>> +#define        X86_BUS_SPACE_IO        I386_BUS_SPACE_IO
>> +#define        X86_BUS_SPACE_MEM       I386_BUS_SPACE_MEM
>> +#define        ELF_KERN_STR            "elf32 kernel"
>>  #endif
>> @@ -701,16 +699,11 @@
>>                         panic("ram_attach: resource %d failed to attach", rid);
>>                 rid++;
>>         }
>> -       return (0);
>> -}
>> -#else
>> -static int
>> -ram_attach(device_t dev)
>> -{
>> -       struct resource *res;
>> -       vm_paddr_t *p;
>> -       int error, i, rid;
>>
>> +       /* If at least one smap attached, return. */
>> +       if (rid != 0)
>> +               return (0);
>> +
>
> Perhaps this instead:
>
>        /* If we found an SMAP, return. */
>        if (smapbase != NULL)
>                return (0);

No, I don't think this check is right, smapbase will always be != NULL
(otherwise the code panics).

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein