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

Attilio Rao attilio at freebsd.org
Fri Oct 29 00:25:40 UTC 2010


2010/10/28 John Baldwin <jhb at freebsd.org>:
> On Thursday, October 28, 2010 4:28:12 pm Attilio Rao wrote:
>> 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).
>
> Oh, that needs to be fixed then.  It can be NULL on i386 with an old loader
> (or on a really old machine without an SMAP).  The amd64 nexus code could
> assume it would never be NULL, but i386 cannot.
>
> It should probably more closely match what i386 does during the memory probe
> which is:
>
>        kmdp = search("elf kernel");
>        if (kmdp == NULL)
>                kmdp = search("elfXX kernel");
>        if (kmdp != NULL)
>                smapbase = preload_search(...);
>        else
>                smapbase = NULL;
>        if (smapbase != NULL) {
>                for (smap = ...) {
>                }
>
>                return (0);
>        }
>
>        /* fall through to old i386 code using phys_avail[] */

This further patch should address your concerns about nexus.c:
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,13 +85,15 @@
 #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        BUS_SPACE_IO    AMD64_BUS_SPACE_IO
+#define        BUS_SPACE_MEM   AMD64_BUS_SPACE_MEM
 #else
-#define        RMAN_BUS_SPACE_IO       I386_BUS_SPACE_IO
-#define        RMAN_BUS_SPACE_MEM      I386_BUS_SPACE_MEM
+#define        BUS_SPACE_IO    I386_BUS_SPACE_IO
+#define        BUS_SPACE_MEM   I386_BUS_SPACE_MEM
 #endif

+#define        ELF_KERN_STR    ("elf"__XSTRING(__ELF_WORD_SIZE)" kernel")
+
 static MALLOC_DEFINE(M_NEXUSDEV, "nexusdev", "Nexus device");

 #define DEVTONX(dev)   ((struct nexus_device *)device_get_ivars(dev))
@@ -435,7 +433,7 @@
 #else
                rman_set_bushandle(r, rman_get_start(r));
 #endif
-               rman_set_bustag(r, RMAN_BUS_SPACE_IO);
+               rman_set_bustag(r, BUS_SPACE_IO);
                break;
        case SYS_RES_MEMORY:
 #ifdef PC98
@@ -446,7 +444,7 @@
 #endif
                vaddr = pmap_mapdev(rman_get_start(r), rman_get_size(r));
                rman_set_virtual(r, vaddr);
-               rman_set_bustag(r, RMAN_BUS_SPACE_MEM);
+               rman_set_bustag(r, BUS_SPACE_MEM);
 #ifdef PC98
                /* PC-98: the type of bus_space_handle_t is the structure. */
                bh->bsh_base = (bus_addr_t) vaddr;
@@ -668,48 +666,45 @@
        return (0);
 }

-#ifdef __amd64__
 static int
 ram_attach(device_t dev)
 {
        struct bios_smap *smapbase, *smap, *smapend;
        struct resource *res;
+       vm_paddr_t *p;
        caddr_t kmdp;
        uint32_t smapsize;
-       int error, rid;
+       int error, i, rid;

        /* Retrieve the system memory map from the loader. */
        kmdp = preload_search_by_type("elf kernel");
        if (kmdp == NULL)
-               kmdp = preload_search_by_type("elf64 kernel");
-       smapbase = (struct bios_smap *)preload_search_info(kmdp,
-           MODINFO_METADATA | MODINFOMD_SMAP);
-       smapsize = *((u_int32_t *)smapbase - 1);
-       smapend = (struct bios_smap *)((uintptr_t)smapbase + smapsize);
+               kmdp = preload_search_by_type(ELF_KERN_STR);
+       if (kmdp != NULL) {
+               smapbase = (struct bios_smap *)preload_search_info(kmdp,
+                   MODINFO_METADATA | MODINFOMD_SMAP);
+               smapsize = *((u_int32_t *)smapbase - 1);
+               smapend = (struct bios_smap *)((uintptr_t)smapbase + smapsize);

-       rid = 0;
-       for (smap = smapbase; smap < smapend; smap++) {
-               if (smap->type != SMAP_TYPE_MEMORY || smap->length == 0)
-                       continue;
-               error = bus_set_resource(dev, SYS_RES_MEMORY, rid, smap->base,
-                   smap->length);
-               if (error)
-                       panic("ram_attach: resource %d failed set with %d", rid,
-                           error);
-               res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, 0);
-               if (res == NULL)
-                       panic("ram_attach: resource %d failed to attach", rid);
-               rid++;
+               rid = 0;
+               for (smap = smapbase; smap < smapend; smap++) {
+                       if (smap->type != SMAP_TYPE_MEMORY ||
+                           smap->length == 0)
+                               continue;
+                       error = bus_set_resource(dev, SYS_RES_MEMORY, rid,
+                           smap->base, smap->length);
+                       if (error)
+                       panic("ram_attach: resource %d failed set with %d",
+                                   rid, error);
+                       res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid,
+                           0);
+                       if (res == NULL)
+                       panic("ram_attach: resource %d failed to attach",
+                                   rid);
+                       rid++;
+               }
+               return (0);
        }
-       return (0);
-}
-#else
-static int
-ram_attach(device_t dev)
-{
-       struct resource *res;
-       vm_paddr_t *p;
-       int error, i, rid;

        /*
         * We use the dump_avail[] array rather than phys_avail[] for
@@ -743,7 +738,6 @@
        }
        return (0);
 }
-#endif

 static device_method_t ram_methods[] = {
        /* Device interface */


More information about the svn-src-head mailing list