svn commit: r214457 - in head/sys: amd64/amd64 conf i386/i386
x86/x86
John Baldwin
jhb at freebsd.org
Thu Oct 28 18:15:52 UTC 2010
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
I would not do this. What I meant is that amd64 and i386 should be
changed to both use X86_BUS_SPACE_* and {AMD64,I386}_BUS_SPACE_* should be
retired. This would involve changing the bus space code itself slightly,
but it could perhaps be shared as a result. Alternatively as bde@ notes
we could just drop the MD prefix entirely and have BUS_SPACE_* instead.
However, I wouldn't make any changes to just nexus.c for the BUS_SPACE_*
stuff.
> @@ -668,20 +666,20 @@
> 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");
> + kmdp = preload_search_by_type(ELF_KERN_STR);
> smapbase = (struct bios_smap *)preload_search_info(kmdp,
> MODINFO_METADATA | MODINFOMD_SMAP);
> smapsize = *((u_int32_t *)smapbase - 1);
It would be nice if ELF_KERN_STR could be autogenerated as something like:
"elf ## __ELF_WORD_SIZE ## kernel" instead of needing an #ifdef.
> @@ -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);
> /*
> * We use the dump_avail[] array rather than phys_avail[] for
> * the memory map as phys_avail[] contains holes for kernel
> @@ -724,7 +717,7 @@
> */
> for (i = 0, p = dump_avail; p[1] != 0; i++, p += 2) {
> rid = i;
> -#ifdef PAE
> +#if !defined(__amd64__) && defined(PAE)
> /*
> * Resources use long's to track resources, so we can't
> * include memory regions above 4GB.
No need for this bit, PAE is never true on amd64, so it can just be used
directly in x86 code.
> @@ -743,7 +736,6 @@
> }
> return (0);
> }
> -#endif
>
> static device_method_t ram_methods[] = {
> /* Device interface */
>
--
John Baldwin
More information about the svn-src-all
mailing list