| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20101027101933.GD6797@angua.secretlab.ca>
Date: Wed, 27 Oct 2010 11:19:33 +0100
From: Grant Likely <grant.likely@...retlab.ca>
To: Andres Salomon <dilinger@...ued.net>
Cc: devicetree-discuss@...ts.ozlabs.org,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: OLPC: add OLPC device-tree support (v3)
On Fri, Oct 22, 2010 at 03:58:46PM -0700, Andres Salomon wrote:
>
> Make use of PROC_DEVICETREE to export the tree, and sparc's PROMTREE code to
> call into OLPC's Open Firmware to build the tree.
>
> v3: rename olpc_prom to olpc_dt
> - rework Kconfig entries
> - drop devtree build hook from proc, instead adding a call to x86's
> paging_init (similarly to how sparc64 does it)
> - switch allocation from using slab to alloc_bootmem. this allows
> the DT to be built earlier during boot (during setup_arch); the
> downside is that there are some 1200 bootmem reservations that are
> done during boot. Not ideal..
> - add a helper olpc_ofw_is_installed function to test for the
> existence and successful detection of OLPC's OFW.
>
> Signed-off-by: Andres Salomon <dilinger@...ued.net>
Overall this patch looks fine, but it needs to be acked from the x86
maintainers, and I'd like it to have a cycle through linux-next before
it gets merged, so that means 2.6.38 because the 2.6.37 merge window
has already been open for almost a week.
The promtree patches have been merged though.
Comments below.
g.
> ---
> arch/x86/Kconfig | 6 ++
> arch/x86/include/asm/olpc_ofw.h | 9 ++
> arch/x86/include/asm/prom.h | 1 +
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/olpc_dt.c | 165 +++++++++++++++++++++++++++++++++++++++
> arch/x86/kernel/olpc_ofw.c | 5 +
> arch/x86/mm/init_32.c | 2 +
> 7 files changed, 189 insertions(+), 0 deletions(-)
> create mode 100644 arch/x86/include/asm/prom.h
> create mode 100644 arch/x86/kernel/olpc_dt.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cea0cd9..1d57e2b 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2069,11 +2069,17 @@ config OLPC_OPENFIRMWARE
> bool "Support for OLPC's Open Firmware"
> depends on !X86_64 && !X86_PAE
> default y if OLPC
> + select OF
> help
> This option adds support for the implementation of Open Firmware
> that is used on the OLPC XO-1 Children's Machine.
> If unsure, say N here.
>
> +config OLPC_OPENFIRMWARE_DT
> + bool
> + default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
> + select OF_PROMTREE
> +
> endif # X86_32
>
> config K8_NB
> diff --git a/arch/x86/include/asm/olpc_ofw.h b/arch/x86/include/asm/olpc_ofw.h
> index 08fde47..a41250e 100644
> --- a/arch/x86/include/asm/olpc_ofw.h
> +++ b/arch/x86/include/asm/olpc_ofw.h
> @@ -8,6 +8,8 @@
>
> #ifdef CONFIG_OLPC_OPENFIRMWARE
>
> +extern bool olpc_ofw_is_installed(void);
> +
> /* run an OFW command by calling into the firmware */
> #define olpc_ofw(name, args, res) \
> __olpc_ofw((name), ARRAY_SIZE(args), args, ARRAY_SIZE(res), res)
> @@ -23,9 +25,16 @@ extern void setup_olpc_ofw_pgd(void);
>
> #else /* !CONFIG_OLPC_OPENFIRMWARE */
>
> +static inline bool olpc_ofw_is_installed(void) { return false; }
> static inline void olpc_ofw_detect(void) { }
> static inline void setup_olpc_ofw_pgd(void) { }
>
> #endif /* !CONFIG_OLPC_OPENFIRMWARE */
>
> +#ifdef CONFIG_OLPC_OPENFIRMWARE_DT
> +extern void olpc_dt_build_devicetree(void);
> +#else
> +static inline void olpc_dt_build_devicetree(void) { }
> +#endif /* CONFIG_OLPC_OPENFIRMWARE_DT */
> +
> #endif /* _ASM_X86_OLPC_OFW_H */
> diff --git a/arch/x86/include/asm/prom.h b/arch/x86/include/asm/prom.h
> new file mode 100644
> index 0000000..b4ec95f
> --- /dev/null
> +++ b/arch/x86/include/asm/prom.h
> @@ -0,0 +1 @@
> +/* dummy prom.h; here to make linux/of.h's #includes happy */
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index fedf32a..519539a 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -107,6 +107,7 @@ scx200-y += scx200_32.o
>
> obj-$(CONFIG_OLPC) += olpc.o
> obj-$(CONFIG_OLPC_OPENFIRMWARE) += olpc_ofw.o
> +obj-$(CONFIG_OLPC_OPENFIRMWARE_DT) += olpc_dt.o
olpc_ofw_dt.o perhaps?
> obj-$(CONFIG_X86_MRST) += mrst.o
>
> microcode-y := microcode_core.o
> diff --git a/arch/x86/kernel/olpc_dt.c b/arch/x86/kernel/olpc_dt.c
> new file mode 100644
> index 0000000..f660a11
> --- /dev/null
> +++ b/arch/x86/kernel/olpc_dt.c
> @@ -0,0 +1,165 @@
> +/*
> + * olpc_dt.c: OLPC-specific OFW device tree support code.
Nit: I personally prefer not to see the file name encoded in the
header block.
> + *
> + * Paul Mackerras August 1996.
> + * Copyright (C) 1996-2005 Paul Mackerras.
> + *
> + * Adapted for 64bit PowerPC by Dave Engebretsen and Peter Bergner.
> + * {engebret|bergner}@...ibm.com
> + *
> + * Adapted for sparc by David S. Miller davem@...emloft.net
> + * Adapted for x86/OLPC by Andres Salomon <dilinger@...ued.net>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/bootmem.h>
> +#include <linux/of.h>
> +#include <linux/of_pdt.h>
> +#include <asm/olpc_ofw.h>
> +
> +static phandle __init olpc_dt_getsibling(phandle node)
> +{
> + const void *args[] = { (void *)node };
> + void *res[] = { &node };
> +
> + if (node == -1)
> + return 0;
phandle is a u32, so testing against a negative value is not sane.
> +
> + if (olpc_ofw("peer", args, res) || node == -1)
> + return 0;
> +
> + return node;
> +}
> +
> +static phandle __init olpc_dt_getchild(phandle node)
> +{
> + const void *args[] = { (void *)node };
> + void *res[] = { &node };
> +
> + if (node == -1)
> + return 0;
> +
> + if (olpc_ofw("child", args, res) || node == -1) {
> + pr_err("PROM: %s: fetching child failed!\n", __func__);
> + return 0;
> + }
> +
> + return node;
> +}
> +
> +static int __init olpc_dt_getproplen(phandle node, const char *prop)
> +{
> + const void *args[] = { (void *)node, prop };
> + int len;
> + void *res[] = { &len };
> +
> + if (node == -1)
> + return -1;
> +
> + if (olpc_ofw("getproplen", args, res)) {
> + pr_err("PROM: %s: getproplen failed!\n", __func__);
> + return -1;
> + }
> +
> + return len;
> +}
> +
> +static int __init olpc_dt_getproperty(phandle node, const char *prop,
> + char *buf, int bufsize)
> +{
> + int plen;
> +
> + plen = olpc_dt_getproplen(node, prop);
> + if (plen > bufsize || plen < 1)
> + return -1;
> + else {
Nit: If there are braces on the else clause, then please use them on
the if() clause also.
> + const void *args[] = { (void *)node, prop, buf, (void *)plen };
> + void *res[] = { &plen };
> +
> + if (olpc_ofw("getprop", args, res)) {
> + pr_err("PROM: %s: getprop failed!\n", __func__);
> + return -1;
> + }
> + }
> +
> + return plen;
> +}
> +
> +static int __init olpc_dt_nextprop(phandle node, char *prev, char *buf)
> +{
> + const void *args[] = { (void *)node, prev, buf };
This looks wrong. It does not look sane to cast node as a pointer
(which won't be the same size on x86_64), and then recast it to an int
inside __olpc_ofw(). Do you know why the api is implemented in this
way?
> + int success;
> + void *res[] = { &success };
> +
> + buf[0] = '\0';
> +
> + if (node == -1)
> + return -1;
> +
> + if (olpc_ofw("nextprop", args, res) || success != 1)
> + return -1;
> +
> + return 0;
> +}
> +
> +static int __init olpc_dt_pkg2path(phandle node, char *buf,
> + const int buflen, int *len)
> +{
> + const void *args[] = { (void *)node, buf, (void *)buflen };
> + void *res[] = { len };
> +
> + if (node == -1)
> + return -1;
> +
> + if (olpc_ofw("package-to-path", args, res) || *len < 1)
> + return -1;
> +
> + return 0;
> +}
> +
> +static unsigned int prom_early_allocated __initdata;
> +
> +void * __init prom_early_alloc(unsigned long size)
> +{
> + void *res;
> +
> + res = alloc_bootmem(size);
> + if (res)
> + memset(res, 0, size);
> +
> + prom_early_allocated += size;
> +
> + return res;
> +}
> +
> +static struct of_pdt_ops prom_olpc_ops __initdata = {
> + .nextprop = olpc_dt_nextprop,
> + .getproplen = olpc_dt_getproplen,
> + .getproperty = olpc_dt_getproperty,
> + .getchild = olpc_dt_getchild,
> + .getsibling = olpc_dt_getsibling,
> + .pkg2path = olpc_dt_pkg2path,
> +};
> +
> +void __init olpc_dt_build_devicetree(void)
> +{
> + phandle root;
> +
> + if (!olpc_ofw_is_installed())
> + return;
> +
> + root = olpc_dt_getsibling(0);
> + if (root < 0) {
> + pr_err("PROM: unable to get root node from OFW!\n");
> + return;
> + }
> + of_pdt_build_devicetree(root, &prom_olpc_ops);
> +
> + pr_info("PROM DT: Built device tree with %u bytes of memory.\n",
> + prom_early_allocated);
> +}
> diff --git a/arch/x86/kernel/olpc_ofw.c b/arch/x86/kernel/olpc_ofw.c
> index 3218aa7..2176048 100644
> --- a/arch/x86/kernel/olpc_ofw.c
> +++ b/arch/x86/kernel/olpc_ofw.c
> @@ -104,3 +104,8 @@ void __init olpc_ofw_detect(void)
> (unsigned long)olpc_ofw_cif, (-start) >> 20);
> reserve_top_address(-start);
> }
> +
> +bool __init olpc_ofw_is_installed(void)
> +{
> + return olpc_ofw_cif != NULL;
> +}
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index bca7909..41bf85f 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -44,6 +44,7 @@
> #include <asm/bugs.h>
> #include <asm/tlb.h>
> #include <asm/tlbflush.h>
> +#include <asm/olpc_ofw.h>
> #include <asm/pgalloc.h>
> #include <asm/sections.h>
> #include <asm/paravirt.h>
> @@ -833,6 +834,7 @@ void __init paging_init(void)
> /*
> * NOTE: at this point the bootmem allocator is fully available.
> */
> + olpc_dt_build_devicetree();
> sparse_init();
> zone_sizes_init();
> }
> --
> 1.5.6.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists