[coreboot] Fwd: [PATCH v3 1/1] libpayload: add support for finding and parsing Coreboot BIOS tables
Philip Prindeville
philipp_subx at redfish-solutions.com
Mon Aug 22 02:37:02 CEST 2011
Since libpayload seems to be ownerless (but still active), I'm copying the 5 top committers of the last 100 commits.
Plus Andres, who is knowledgeable about the Geode platforms (many of which use Coreboot).
On 8/19/11 4:23 PM, Andrew Morton wrote:
> On Thu, 18 Aug 2011 20:55:56 -0700
> Philip Prindeville <philipp at redfish-solutions.com> wrote:
>
>> Hi Andrew,
>>
>> Any chance this could make it in for 3.2?
>
> Perhaps. But I'm not on the linux-geode list (few people are!) and I
> don't like to merge privately-sent patches.
>
> So please do a full resend, ccing myself and linux-kernel and then
> we'll see what happens.
>
>
> mini-review follows:
>
>>
>>
>> -------- Original Message --------
>> Subject: [PATCH v2 1/1] libpayload: add support for finding and parsing Coreboot BIOS tables
>> Date: Wed, 17 Aug 2011 23:11:00 -0700
>> From: Philip Prindeville <philipp_subx at redfish-solutions.com>
>> To: linux-geode at lists.infradead.org, Patrick Georgi <patrick.georgi at secunet.com>
>> CC: Andres Salomon <dilinger at queued.net>
>>
>> Many experimental and small design house SBC's use Coreboot; here we add linux support for it. Platform drivers on generic processors such as Geode or C7 may interrogate Coreboot for the vendor and model.
>>
>> Removed the conditional compilation and non-kernel parts.
>>
>> Split into architecture indpendent and machine-specific parts as per Andres' comment.
>>
>> Signed-off-by: Philip Prindeville <philipp at redfish-solutions.com>
>> Acked-by: Patrick Georgi <patrick.georgi at secunet.com>
>> ---
>> Taken from the coreboot project, largely unmodified.
>>
>
> Given that it was taken from coreboot, are you sure that we have all
> the signed-off-by:s? It seems that you're the primary/sole author so I
> guess "yes".
I'll add that when I post to linux-kernel after all review comments have been addressed.
> The changelog is way too terse. I don't actually know what coreboot
> *is*, and if you don't tell me, who will?
I'll add that for the final review.
> What does the code do? How does it work? What interfaces does it
> offer and how does client code use them? etc.
>
> Should there be an associated documentation file?
>
> None of the exported interfaces are documented at all. This makes
> review harder and less effective and makes the code harder and less
> reliable to use.
Yeah, unfortunately that's how it is at the source, too (the coreboot project).
The upstream documentation is a little thin.
http://www.coreboot.org/
http://www.coreboot.org/Libpayload
but I can include a pointer to it.
> diff --git a/include/linux/libpayload/coreboot_tables.h b/include/linux/libpayload/coreboot_tables.h
> new file mode 100644
> index 0000000..ab1a652
> --- /dev/null
> +++ b/include/linux/libpayload/coreboot_tables.h
> @@ -0,0 +1,233 @@
> +/*
> + * This file is part of the libpayload project.
> + *
> + * Copyright (C) 2008 Advanced Micro Devices, Inc.
> + * Copyright (C) 2011 Philip Prindeville, Redfish Solutions, LLC.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#ifndef _COREBOOT_TABLES_H
> +#define _COREBOOT_TABLES_H
> +
> +#include <asm-generic/int-ll64.h>
> +
> +struct cbuint64 {
> + u32 lo;
> + u32 hi;
> +};
>
> I assume that all the structures in this file are based on some
> coreboot spec? Without that spec, the implementation is unreviewable!
> If such a spec is available, please prominently describe how the
> reviewer can obtain it, within this file.
It might be documented, but the Doxygen stuff seems to be faulting... I get a stack exception when I go to:
http://qa.coreboot.org/docs/doxygen/
>
> + u8 reserved_mask_pos;
> + u8 reserved_mask_size;
> +};
> +
> +#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
> +struct cb_cmos_option_table {
> + u32 tag;
> + u32 size;
> + u32 header_length;
> +};
> +
> +#define CB_TAG_OPTION 0x00c9
> +#define CMOS_MAX_NAME_LENGTH 32
> +struct cb_cmos_entries {
> + u32 tag;
> + u32 size;
> + u32 bit;
> + u32 length;
> + u32 config;
> + u32 config_id;
> + u8 name[CMOS_MAX_NAME_LENGTH];
> +};
> +
> +
> +#define CB_TAG_OPTION_ENUM 0x00ca
> +#define CMOS_MAX_TEXT_LENGTH 32
> +struct cb_cmos_enums {
> + u32 tag;
> + u32 size;
> + u32 config_id;
> + u32 value;
> + u8 text[CMOS_MAX_TEXT_LENGTH];
> +};
> +
> +#define CB_TAG_OPTION_DEFAULTS 0x00cb
> +#define CMOS_IMAGE_BUFFER_SIZE 128
> +struct cb_cmos_defaults {
> + u32 tag;
> + u32 size;
> + u32 name_length;
> + u8 name[CMOS_MAX_NAME_LENGTH];
> + u8 default_set[CMOS_IMAGE_BUFFER_SIZE];
> +};
> +
> +#define CB_TAG_OPTION_CHECKSUM 0x00cc
> +#define CHECKSUM_NONE 0
> +#define CHECKSUM_PCBIOS 1
> +struct cb_cmos_checksum {
> + u32 tag;
> + u32 size;
> + u32 range_start;
> + u32 range_end;
> + u32 location;
> + u32 type;
> +};
> +
> +/* Helpful macros */
> +
> +#define MEM_RANGE_COUNT(_rec) \
> + (((_rec)->size - sizeof(*(_rec))) / sizeof((_rec)->map[0]))
> +
> +#define MEM_RANGE_PTR(_rec, _idx) \
> + (((u8 *) (_rec)) + sizeof(*(_rec)) \
> + + (sizeof((_rec)->map[0]) * (_idx)))
> +
> +#define MB_VENDOR_STRING(_mb) \
> + (((unsigned char *) ((_mb)->strings)) + (_mb)->vendor_idx)
> +
> +#define MB_PART_STRING(_mb) \
> + (((unsigned char *) ((_mb)->strings)) + (_mb)->part_number_idx)
> +
> +#define UNPACK_CB64(_in) \
> + ( (((u64) _in.hi) << 32) | _in.lo )
>
> If at all possible, these should be implemented as (lower-cased) C
> functions, not as macros. Because
The first two can't be, since they are polymorphous (i.e. the "sizeof()" needs to work directly on the typeof() the argument it's getting).
> - for some reason, people are more likely to document functions than
> macros.
>
> - C functions provide argument typechecking
>
> - C functions are more readable
>
> - C functions can prevent unused-var warnings in obscure situations
>
> - C functions don't cause bugs when called using an expression with
> side-effects. The above macros are in fact buggy if called with, for
> example, UNPACK_CB64(*foo++).
>
> - C functions don't need vast amounts of parenthesis around argumetns
> to prevent compilation errors. UNPACK_CB64() lacks these, for example.
Done for the remainder.
> +extern int get_coreboot_info(struct sysinfo_t *info);
> +extern int cb_parse_header(void *addr, void *end, struct sysinfo_t *info);
> +
> +#endif
> diff --git a/include/linux/libpayload/libpayload.h b/include/linux/libpayload/libpayload.h
> new file mode 100644
> index 0000000..e2a4939
> --- /dev/null
> +++ b/include/linux/libpayload/libpayload.h
> @@ -0,0 +1,48 @@
> +/*
> + * This file is part of the libpayload project.
> + *
> + * Copyright (C) 2008 Advanced Micro Devices, Inc.
> + * Copyright (C) 2011 Philip Prindeville, Redfish Solutions, LLC.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#ifndef _LIBPAYLOAD_H
> +#define _LIBPAYLOAD_H
> +
> +#include <linux/types.h>
> +#include <linux/unistd.h>
> +#include <linux/string.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +#include <linux/mm.h>
> +#include <linux/highmem.h>
> +
> +#include <asm/io.h>
> +#include <asm/page.h>
> +#include <asm/checksum.h>
> +
> +#define ipchksum(start, len) ip_compute_csum(start, len)
> +
> +#endif
>
> I think this file needs to go away altogether. In the kernel we don't
> like "#include <everything.h>" header files. Just explicitly list all
> the needed includes directly, within each and every .c file. Nice and
> simple.
Done.
> The ipchksum() macros hould be removed - jsut edit the code to call
> ip_compute_csum().
I used a convenience function for this: cb_checksum().
> Is ip_compute_csum() available without CONFIG_NET? Seems to be "yes"
> for x86, I don't know if that's true for other architectures, should
> they developer coreboot support.
For now, non-x86 platforms aren't supported. Worst case, we add a K-select or K-depends-on for it.
>>
>> ...
>> +struct sysinfo_t {
>
> The _t should only be used in rare circumstances, for typedefs. This
> should be "struct sysinfo". But we already have one of those. I'd
> suggest a rename to "struct coreboot_sysinfo".
Fixed. Called cb_sysinfo.
> + unsigned int cpu_khz;
> + unsigned short ser_ioport;
> + unsigned long ser_base; // for mmapped serial
> +
> + int n_memranges;
> +
> + struct memrange {
> + unsigned long long base;
> + unsigned long long size;
> + unsigned int type;
> + } memrange[SYSINFO_MAX_MEM_RANGES];
> +
> + struct cb_cmos_option_table *option_table;
> + u32 cmos_range_start;
> + u32 cmos_range_end;
> + u32 cmos_checksum_location;
> +
> + struct cb_framebuffer *framebuffer;
> +
> + unsigned long *mbtable; /** Pointer to the multiboot table */
> +
> + struct cb_header *header;
> + struct cb_mainboard *mainboard;
> +};
> +
> +extern struct sysinfo_t lib_sysinfo;
> +extern int lib_get_sysinfo(void);
>
> All globally exported symbols from code such as this should identify
> what subsystem they belong to. So all these things should start with
> "coreboot_", please.
Done as "cb_".
> Or "payload". This subsystem seems to have two main identifiers:
> coreboot and payload. This is confusing. I prefer "coreboot", because
> "payload" is terribly vague and could refer to anything at all. Looky:
>
> z:/usr/src/linux-3.1-rc2> grep -r payload . | wc -l
> 5368
>
> How do you distinguish this subsystem from all that?
>
> +#endif
> +
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 6c695ff..172c721 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -91,6 +91,8 @@ config AUDIT_GENERIC
> depends on AUDIT && !AUDIT_ARCH
> default y
>
> +source "lib/libpayload/Kconfig"
> +
> #
> # compression support is select'ed if needed
> #
> diff --git a/lib/Makefile b/lib/Makefile
> index 15f7bb6..5af01ee 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_BCH) += bch.o
> obj-$(CONFIG_LZO_COMPRESS) += lzo/
> obj-$(CONFIG_LZO_DECOMPRESS) += lzo/
> obj-$(CONFIG_XZ_DEC) += xz/
> +obj-$(CONFIG_LIBPAYLOAD) += libpayload/
> obj-$(CONFIG_RAID6_PQ) += raid6/
>
> lib-$(CONFIG_DECOMPRESS_GZIP) += decompress_inflate.o
> diff --git a/lib/libpayload/Kconfig b/lib/libpayload/Kconfig
> new file mode 100644
> index 0000000..89c0fd8
> --- /dev/null
> +++ b/lib/libpayload/Kconfig
> @@ -0,0 +1,5 @@
> +config LIBPAYLOAD
> + bool "Libpayload support"
> + depends on X86
> + help
> + Libpayload support for locating coreboot tables.
> diff --git a/lib/libpayload/Makefile b/lib/libpayload/Makefile
> new file mode 100644
> index 0000000..d65f5d4
> --- /dev/null
> +++ b/lib/libpayload/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_LIBPAYLOAD) += coreboot.o sysinfo.o coreboot_x86.o
> diff --git a/lib/libpayload/coreboot.c b/lib/libpayload/coreboot.c
> new file mode 100644
> index 0000000..e1937e6
> --- /dev/null
> +++ b/lib/libpayload/coreboot.c
> @@ -0,0 +1,213 @@
> +/*
> + * This file is part of the libpayload project.
> + *
> + * Copyright (C) 2008 Advanced Micro Devices, Inc.
> + * Copyright (C) 2009 coresystems GmbH
> + * Copyright (C) 2011 Philip Prindeville, Redfish Solutions, LLC.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#include <linux/libpayload/libpayload.h>
> +#include <linux/libpayload/sysinfo.h>
> +#include <linux/libpayload/coreboot_tables.h>
> +
> +#include <linux/highmem.h>
> +#include <asm/page.h>
> +
> +/*
> + * Some of this is x86 specific, and the rest of it is generic. Right now,
> + * since we only support x86, we'll avoid trying to make lots of infrastructure
> + * we don't need. If in the future, we want to use coreboot on some other
> + * architecture, then take out the generic parsing code and move it elsewhere.
> + */
> +
> +/* === Parsing code === */
> +/* This is the generic parsing code. */
> +
> +#define page_base(addr) ((typeof(addr)) (((uintptr_t)(addr) & PAGE_MASK)))
>
> Remove, use PAGE_ALIGN
Can't. PAGE_ALIGN() rounds *up* to the end of the page. I need the base page address.
> +#define page_offset(addr) (((uintptr_t)(addr) & ~PAGE_MASK))
>
> There might be an existing helper for this also. It should at least be
> written in C so people can't do
>
> struct foo bar;
> page_offset(bar);
Done.
> +static void cb_parse_memory(unsigned char *ptr, struct sysinfo_t *info)
>
> Documentation...
I'll defer this to upstream. I'm trying to keep my code from becoming too divergent from what's upstream so it stays relatively simple to port bugfixes.
Patrick: can we merge some of these changes back into coreboot/payloads/libpayload/arch/x86 ?
> +{
> + struct cb_memory *mem = (struct cb_memory *)ptr;
> + int count = MEM_RANGE_COUNT(mem);
> + int i;
> +
> + if (count > SYSINFO_MAX_MEM_RANGES)
> + count = SYSINFO_MAX_MEM_RANGES;
> +
> + info->n_memranges = 0;
> +
> + for (i = 0; i < count; i++) {
> + struct cb_memory_range *range =
> + (struct cb_memory_range *)MEM_RANGE_PTR(mem, i);
>
> Arrange for the MEM_RANGE_PTR() repalcement to return the correct type,
> or void* then lose the typecast.
Done.
> + info->memrange[info->n_memranges].base =
> + UNPACK_CB64(range->start);
> +
> + info->memrange[info->n_memranges].size =
> + UNPACK_CB64(range->size);
> +
> + info->memrange[info->n_memranges].type = range->type;
> +
> + info->n_memranges++;
> + }
> +}
> +
> +static void cb_parse_serial(unsigned char *ptr, struct sysinfo_t *info)
> +{
> + struct cb_serial *ser = (struct cb_serial *)ptr;
> + info->ser_ioport = ser->ioport;
> +}
>
> Make the arg void* and lose the typecast?
Done.
> +static void cb_parse_optiontable(unsigned char *ptr, struct sysinfo_t *info)
> +{
> + info->option_table = (struct cb_cmos_option_table *)ptr;
> +}
>
> Ditto.
Done.
> +static void cb_parse_checksum(unsigned char *ptr, struct sysinfo_t *info)
> +{
> + struct cb_cmos_checksum *cmos_cksum = (struct cb_cmos_checksum *)ptr;
> + info->cmos_range_start = cmos_cksum->range_start;
> + info->cmos_range_end = cmos_cksum->range_end;
> + info->cmos_checksum_location = cmos_cksum->location;
> +}
> +
> +static void cb_parse_framebuffer(unsigned char *ptr, struct sysinfo_t *info)
> +{
> + info->framebuffer = (struct cb_framebuffer *)ptr;
> +}
>
> More.
Done.
> +int cb_parse_header(void *addr, void *end, struct sysinfo_t *info)
> +{
> + struct cb_header *header;
> + unsigned char *ptr = addr;
>
> make this void* then remove lots of typecasts.
Done.
> + void *forward;
> + int i;
> +
> + for (i = 0; (void *)ptr < end; i += 16, ptr += 16) {
> + header = (struct cb_header *)ptr;
> + if (!strncmp((const char *)header->signature, "LBIO", 4))
> + break;
> + }
> +
> + /* We walked the entire space and didn't find anything. */
> + if ((void *)ptr >= end)
> + return -1;
> +
> + printk(KERN_DEBUG "coreboot: signature %.4s at %p\n",
> + header->signature, header);
> +
> + if (!header->table_bytes)
> + return 0;
> +
> + /* Make sure the checksums match. */
> + if (ipchksum(header, sizeof(*header)) != 0)
> + return -1;
> +
> + printk(KERN_DEBUG "coreboot: header checksum verified\n");
> +
> + if (ipchksum((char *)header + sizeof(*header),
> + header->table_bytes) != header->table_checksum)
> + return -1;
> +
> + printk(KERN_DEBUG "coreboot: table checksum verified\n");
> +
> + info->header = header;
> +
> + /* Now, walk the tables. */
> + ptr += header->header_bytes;
> +
> + printk(KERN_DEBUG "coreboot: table start %p\n", ptr);
> +
> + for (i = 0; i < header->table_entries; i++) {
> + struct cb_record *rec = (struct cb_record *)ptr;
> +
> + printk(KERN_DEBUG "coreboot: entry[%u] @ %p type %04x\n",
> + i, rec, rec->tag);
> +
> + /* We only care about a few tags here (maybe more later). */
> + switch (rec->tag) {
> + case CB_TAG_FORWARD:
> + forward = (void *)(uintptr_t)((struct cb_forward *)rec)->forward;
>
> omigod. Your task is to get all the types sorted out and , make this
> sort of thing vanish, please.
Hard to do... "forward" is a 64-bit quantity, so it needs to be down-converted to the same width as your native pointer size, then cast back to a void *.
Otherwise, we'll get compilation warnings which I hate to see... they're sloppy.
> + printk(KERN_DEBUG "coreboot: forward to %p\n",
> + forward);
> +
> + if (phys_to_virt((phys_addr_t) forward) >= high_memory)
> + addr = ioremap((phys_addr_t) page_base(forward), 0x1000)
> + + page_offset(forward);
> + else
> + addr = phys_to_virt((phys_addr_t) forward);
> + /* we will leave the pages mapped */
> + return cb_parse_header(addr, addr + 0x1000, info);
> +
> + continue;
>
> All the playing with highmem internals is a bit obscure. I can't tell
> what it's up to. Code comments, please.
Done.
>
> + case CB_TAG_MEMORY:
> + cb_parse_memory(ptr, info);
> + break;
> + case CB_TAG_SERIAL:
> + cb_parse_serial(ptr, info);
> + break;
> + case CB_TAG_CMOS_OPTION_TABLE:
> + cb_parse_optiontable(ptr, info);
> + break;
> + case CB_TAG_OPTION_CHECKSUM:
> + cb_parse_checksum(ptr, info);
> + break;
> + // FIXME we should warn on serial if coreboot set up a
> + // framebuffer buf the payload does not know about it.
>
> Please run the entire patch through scripts/checkpatch.pl.
Done.
>
> + case CB_TAG_FRAMEBUFFER:
> + cb_parse_framebuffer(ptr, info);
> + break;
> + case CB_TAG_MAINBOARD:
> + info->mainboard = (struct cb_mainboard *)ptr;
> + break;
> + case CB_TAG_VERSION:
> + case CB_TAG_EXTRA_VERSION:
> + case CB_TAG_BUILD:
> + case CB_TAG_COMPILE_TIME:
> + case CB_TAG_COMPILE_BY:
> + case CB_TAG_COMPILE_HOST:
> + case CB_TAG_COMPILE_DOMAIN:
> + case CB_TAG_COMPILER:
> + case CB_TAG_LINKER:
> + case CB_TAG_ASSEMBLER:
> + printk(KERN_DEBUG "cb_parse_string: [%04x] %s\n",
> + rec->tag, ((struct cb_string *)rec)->string);
> + break;
> + }
> +
> + ptr += rec->size;
> +
> + if ((void *)ptr >= end)
> + return -1;
> + }
> +
> + printk(KERN_DEBUG "coreboot: success (%u entries)!\n",
> + header->table_entries);
> +
> + return 1;
> +}
> +
> diff --git a/lib/libpayload/coreboot_x86.c b/lib/libpayload/coreboot_x86.c
> new file mode 100644
> index 0000000..fa68041
> --- /dev/null
> +++ b/lib/libpayload/coreboot_x86.c
> @@ -0,0 +1,54 @@
> +/*
> + * This file is part of the libpayload project.
> + *
> + * Copyright (C) 2008 Advanced Micro Devices, Inc.
> + * Copyright (C) 2009 coresystems GmbH
> + * Copyright (C) 2011 Philip Prindeville, Redfish Solutions, LLC.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#include <linux/libpayload/libpayload.h>
> +#include <linux/libpayload/sysinfo.h>
> +#include <linux/libpayload/coreboot_tables.h>
> +
> +#include <linux/highmem.h>
> +#include <asm/page.h>
> +
> +/* == Architecture specific == */
> +/* This is the x86 specific stuff. */
> +
> +int get_coreboot_info(struct sysinfo_t *info)
>
> The globally-visible symbol should start with the subsystem identifier.
> coreboot_get_info() would suit.
Done.
>
> ALso, needs documentation. *What* info?
>
> +{
> + int ret;
> + void *base = phys_to_virt(0x00000000);
> +
> + ret = cb_parse_header(base, base + 0x1000, info);
> + if (ret != 1) {
> + base = phys_to_virt(0x000f0000);
> + ret = cb_parse_header(base, base + 0x1000, info);
> + }
> +
> + return (ret == 1) ? 0 : -1;
> +}
>
>
> diff --git a/lib/libpayload/sysinfo.c b/lib/libpayload/sysinfo.c
> new file mode 100644
> index 0000000..21c22f9
> --- /dev/null
> +++ b/lib/libpayload/sysinfo.c
> @@ -0,0 +1,68 @@
> +/*
> + * This file is part of the libpayload project.
> + *
> + * Copyright (C) 2008 Advanced Micro Devices, Inc.
> + * Copyright (C) 2011 Philip Prindeville, Redfish Solutions, LLC.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#include <linux/libpayload/libpayload.h>
> +#include <linux/libpayload/sysinfo.h>
> +#include <linux/libpayload/coreboot_tables.h>
> +
> +/**
> + * This is a global structure that is used through the library - we set it
> + * up initially with some dummy values - hopefully they will be overridden.
> + */
> +struct sysinfo_t lib_sysinfo = {
> + .ser_ioport = 0x3f8,
> +};
> +EXPORT_SYMBOL(lib_sysinfo);
>
> Again, poor naming choices. coreboot_sysinfo_lib, perhaps.
Done.
>
> +int lib_get_sysinfo(void)
> +{
> + int ret;
> +
> + /* Get information from the coreboot tables,
> + * if they exist */
> +
> + ret = get_coreboot_info(&lib_sysinfo);
> +
> + if (!lib_sysinfo.n_memranges) {
> + /* If we can't get a good memory range, use the default. */
> + lib_sysinfo.n_memranges = 2;
> +
> + lib_sysinfo.memrange[0].base = 0;
> + lib_sysinfo.memrange[0].size = 640 * 1024;
> + lib_sysinfo.memrange[0].type = CB_MEM_RAM;
> +
> + lib_sysinfo.memrange[1].base = 1024 * 1024;
> + lib_sysinfo.memrange[1].size = 31 * 1024 * 1024;
> + lib_sysinfo.memrange[1].type = CB_MEM_RAM;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(lib_get_sysinfo);
>
> coreboot_sysinfo_lib_get?
>
Done.
Attached.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: libpayload.patch
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20110821/5f0e008d/attachment.ksh>
More information about the coreboot
mailing list