coreboot-gerrit December 2013

coreboot-gerrit@coreboot.org

1 participants
1377 discussions

Patch merged into coreboot/master: aa197a8 ibexpeak: ensure config compatibility with bd82x6x
by gerrit＠coreboot.org 03 Dec '13

03 Dec '13

the following patch was just integrated into master: commit aa197a881538c32167938946d934bffe2e6cbfc5 Author: Vladimir Serbinenko <phcoder(a)gmail.com> Date: Tue Nov 26 01:16:20 2013 +0100 ibexpeak: ensure config compatibility with bd82x6x Ibexpeak shares few files with bd82x6x. In order for it to work correctly their config structures from chip.h must match, so include bd82x6x/chip.h in ibexpeak/chip.h Change-Id: Ib56b311b8af04f4e4803d1834724680f604901cd Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> See http://review.coreboot.org/4277 for details. -gerrit

1 0

Patch set updated for coreboot: b2794c2 cbfs: fix issues with word size and endianness.
by Ronald G. Minnich 03 Dec '13

03 Dec '13

Ronald G. Minnich (rminnich(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4262 -gerrit commit b2794c20b8a034f8d620ff9990109b98ff1bb920 Author: Ronald G. Minnich <rminnich(a)google.com> Date: Fri Nov 22 09:28:48 2013 -0800 cbfs: fix issues with word size and endianness. Add XDR functions and use them to convert the ELF headers to native headers, using the Elf64 structs to ensure we accomodate all word sizes. Also, use these XDR functions for output. This may seem overly complex but it turned out to be much the easiest way to do this. Note that the basic elf parsing function in cbfs-mkstage.c now works over all ELF files, for all architectures, endian, and word size combinations. At the same time, the basic elf parsing in cbfs-mkstage.c is a loop that has no architecture-specific conditionals. It may seem odd to plan for stages on a cpu with different architectures for each stage, but we're already starting to see this in the ARM world. Also, had we finished the PPC905 port, we would have had to deal with this exact problem 5 years ago. Change-Id: I3457b1782f27cb930a2101d1c53f58e2488ed8a1 Signed-off-by: Ronald G. Minnich <rminnich(a)google.com> --- util/cbfstool/Makefile | 2 +- util/cbfstool/Makefile.inc | 1 + util/cbfstool/cbfs-mkstage.c | 331 ++++++++++++++++++++++++++++++++++--------- util/cbfstool/common.h | 11 ++ util/cbfstool/xdr.c | 141 ++++++++++++++++++ 5 files changed, 419 insertions(+), 67 deletions(-) diff --git a/util/cbfstool/Makefile b/util/cbfstool/Makefile index 3aa5edc..281d75c 100644 --- a/util/cbfstool/Makefile +++ b/util/cbfstool/Makefile @@ -7,7 +7,7 @@ CFLAGS += -D_7ZIP_ST BINARY:=$(obj)/cbfstool COMMON:=cbfstool.o common.o cbfs_image.o compress.o fit.o -COMMON+=cbfs-mkstage.o cbfs-mkpayload.o +COMMON+=cbfs-mkstage.o cbfs-mkpayload.o xdr.o # LZMA COMMON+=lzma/lzma.o COMMON+=lzma/C/LzFind.o lzma/C/LzmaDec.o lzma/C/LzmaEnc.o diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc index fc120f3..4270d75 100644 --- a/util/cbfstool/Makefile.inc +++ b/util/cbfstool/Makefile.inc @@ -5,6 +5,7 @@ cbfsobj += compress.o cbfsobj += cbfs_image.o cbfsobj += cbfs-mkstage.o cbfsobj += cbfs-mkpayload.o +cbfsobj += xdr.o cbfsobj += fit.o # LZMA cbfsobj += lzma.o diff --git a/util/cbfstool/cbfs-mkstage.c b/util/cbfstool/cbfs-mkstage.c index dfc93c2..6dd2775 100644 --- a/util/cbfstool/cbfs-mkstage.c +++ b/util/cbfstool/cbfs-mkstage.c @@ -28,90 +28,277 @@ #include "cbfs.h" #include "elf.h" -static unsigned int idemp(unsigned int x) +/* + * Short form: this is complicated, but we've tried making it simple + * and we keep hitting problems with our ELF parsing. + * + * The ELF parsing situation has always been a bit tricky. In fact, + * we (and most others) have been getting it wrong in small ways for + * years. Recently this has caused real trouble for the ARM V8 build. + * In this file we attempt to finally get it right for all variations + * of endian-ness and word size and target architectures and + * architectures we might get run on. Phew!. To do this we borrow a + * page from the FreeBSD NFS xdr model (see elf_ehdr and elf_phdr), + * the Plan 9 endianness functions (see xdr.c), and Go interfaces (see + * how we use buffer structs in this file). This ends up being a bit + * wordy at the lowest level, but greatly simplifies the elf parsing + * code and removes a common source of bugs, namely, forgetting to + * flip type endianness when referencing a struct member. + * + * ELF files can have four combinations of data layout: 32/64, and + * big/little endian. Further, to add to the fun, depending on the + * word size, the size of the ELF structs varies. The coreboot SELF + * format is simpler in theory: it's supposed to be always BE, and the + * various struct members allow room for growth: the entry point is + * always 64 bits, for example, so the size of a SELF struct is + * constant, regardless of target architecture word size. Hence, we + * need to do some transformation of the ELF files. + * + * A given architecture, realistically, only supports one of the four + * combinations at a time as the 'native' format. Hence, our code has + * been sprinkled with every variation of [nh]to[hn][sll] over the + * years. We've never quite gotten it all right, however, and a quick + * pass over this code revealed another bug. It's all worked because, + * until now, all the working platforms that had CBFS were 32 LE. Even then, + * however, bugs crept in: we recently realized that we're not + * transforming the entry point to big format when we store into the + * SELF image. + * + * The problem is essentially an XDR operation: + * we have something in a foreign format and need to transform it. + * It's most like XDR because: + * 1) the byte order can be wrong + * 2) the word size can be wrong + * 3) the size of elements in the stream depends on the value + * of other elements in the stream + * it's not like XDR because: + * 1) the byte order can be right + * 2) the word size can be right + * 3) the struct members are all on a natural alignment + * + * Hence, this new approach. + * To cover word size issues, we *always* transform the two structs we care about, + * the file header and program header, into a native struct in the 64 bit format: + * [32,little] -> [Elf64_Ehdr, Elf64_Phdr] + * [64,little] -> [Elf64_Ehdr, Elf64_Phdr] + * [32,big] -> [Elf64_Ehdr, Elf64_Phdr] + * [64,big] -> [Elf64_Ehdr, Elf64_Phdr] + * Then we just use those structs, and all the need for inline ntoh* goes away, + * as well as all the chances for error. + * This works because all the SELF structs have fields large enough for + * the largest ELF 64 struct members, and all the Elf64 struct members + * are at least large enough for all ELF 32 struct members. + * We end up with one function to do all our ELF parsing, and two functions + * to transform the headers. For the put case, we also have + * XDR functions, and hopefully we'll never again spend 5 years with the + * wrong endian-ness on an output value :-) + * This should work for all word sizes and endianness we hope to target. + * I *really* don't want to be here for 128 bit addresses. + * + * The parse functions are called with a pointer to an input buffer + * struct. One might ask: are there enough bytes in the input buffer? + * We know there need to be at *least* sizeof(Elf32_Ehdr) + + * sizeof(Elf32_Phdr) bytes. Realistically, there has to be some data + * too. If we start to worry, though we have not in the past, we + * might apply the simple test: the input buffer needs to be at least + * sizeof(Elf64_Ehdr) + sizeof(Elf64_Phdr) bytes because, even if it's + * ELF 32, there's got to be *some* data! This is not theoretically + * accurate but it is actually good enough in practice. It allows the + * header transformation code to ignore the possibility of underrun. + * + * We also must accomodate different ELF files, and hence formats, + * in the same cbfs invocation. We might load a 64-bit payload + * on a 32-bit machine; we might even have a mixed armv7/armv8 + * SOC or even a system with an x86/ARM! + * + * A possibly problematic (though unlikely to be so) assumption + * is that we expect the BIOS to remain in the lowest 32 bits + * of the physical address space. Since ARMV8 has standardized + * on that, and x86_64 also has, this seems a safe assumption. + * + * To repeat, ELF structs are different sizes because ELF struct + * members are different sizes, depending on values in the ELF file + * header. For this we use the functions defined in xdr.c, which + * consume bytes, convert the endianness, and advance the data pointer + * in the buffer struct. + */ + +/* Get the ident array, so we can figure out + * endian-ness, word size, and in future other useful + * parameters + */ +static void +elf_eident(struct buffer *input, Elf64_Ehdr *ehdr) { - return x; + memmove(ehdr->e_ident, input->data, sizeof(ehdr->e_ident)); + input->data += sizeof(ehdr->e_ident); + input->size -= sizeof(ehdr->e_ident); } -/* This is a wrapper around the swab32() macro to make it - * usable for the current implementation of parse_elf_to_stage() - */ -static unsigned int swap32(unsigned int x) +static void +elf_ehdr(struct buffer *input, Elf64_Ehdr *ehdr, struct xdr *xdr, int bit64) { - return swab32(x); + ehdr->e_type = xdr->get16(input); + ehdr->e_machine = xdr->get16(input); + ehdr->e_version = xdr->get32(input); + if (bit64){ + ehdr->e_entry = xdr->get64(input); + ehdr->e_phoff = xdr->get64(input); + ehdr->e_shoff = xdr->get64(input); + } else { + ehdr->e_entry = xdr->get32(input); + ehdr->e_phoff = xdr->get32(input); + ehdr->e_shoff = xdr->get32(input); + } + ehdr->e_flags = xdr->get32(input); + ehdr->e_ehsize = xdr->get16(input); + ehdr->e_phentsize = xdr->get16(input); + ehdr->e_phnum = xdr->get16(input); + ehdr->e_shentsize = xdr->get16(input); + ehdr->e_shnum = xdr->get16(input); + ehdr->e_shstrndx = xdr->get16(input); } -unsigned int (*elf32_to_native) (unsigned int) = idemp; +void +elf_phdr(struct buffer *pinput, Elf64_Phdr *phdr, int entsize, struct xdr *xdr, int bit64) +{ + /* + * The entsize need not be sizeof(*phdr). + * Hence, it is easier to keep a copy of the input, + * as the xdr functions may not advance the input + * pointer the full entsize; rather than get tricky + * we just advance it below. + */ + struct buffer input = *pinput; + if (bit64){ + phdr->p_type = xdr->get32(&input); + phdr->p_flags = xdr->get32(&input); + phdr->p_offset = xdr->get64(&input); + phdr->p_vaddr = xdr->get64(&input); + phdr->p_paddr = xdr->get64(&input); + phdr->p_filesz = xdr->get64(&input); + phdr->p_memsz = xdr->get64(&input); + phdr->p_align = xdr->get64(&input); + } else { + phdr->p_type = xdr->get32(&input); + phdr->p_offset = xdr->get32(&input); + phdr->p_vaddr = xdr->get32(&input); + phdr->p_paddr = xdr->get32(&input); + phdr->p_filesz = xdr->get32(&input); + phdr->p_memsz = xdr->get32(&input); + phdr->p_flags = xdr->get32(&input); + phdr->p_align = xdr->get32(&input); + } + pinput->size -= entsize; + pinput->data += entsize; +} + +/* Get the headers from the buffer. + * Return -1 in the event of an error. + */ +int elf_headers(const struct buffer *pinput, Elf64_Ehdr *ehdr, Elf64_Phdr **pphdr) +{ + int i; + struct xdr *xdr = &xdr_le; + int bit64 = 0; + struct buffer input = *(struct buffer *)pinput; + struct buffer phdr_buf; + Elf64_Phdr *phdr; + + if (!iself((unsigned char *)pinput->data)) { + ERROR("The stage file is not in ELF format!\n"); + return -1; + } + + elf_eident(&input, ehdr); + bit64 = ehdr->e_ident[EI_CLASS] == ELFCLASS64; + /* Assume LE unless we are sure otherwise. + * We're not going to take on the task of + * fully validating the ELF file. That way + * lies madness. + */ + if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) + xdr = &xdr_be; + + elf_ehdr(&input, ehdr, xdr, bit64); -/* returns size of result, or -1 if error */ + if (pinput->size < ehdr->e_phoff){ + ERROR("The program header offset is greater than " + "the remaining file size." + "%ld bytes left, program header offset is %ld \n", + pinput->size, ehdr->e_phoff); + return -1; + } + /* cons up an input buffer for the headers. + * Note that the program headers can be anywhere, + * per the ELF spec, You'd be surprised how many ELF + * readers miss this little detail. + */ + phdr_buf.data = &pinput->data[ehdr->e_phoff]; + phdr_buf.size = ehdr->e_phentsize * ehdr->e_phnum; + if (phdr_buf.size > (pinput->size - ehdr->e_phoff)){ + ERROR("The file is not large enough for the program headers." + "%ld bytes left, %ld bytes of headers\n", + pinput->size - ehdr->e_phoff, phdr_buf.size); + return -1; + } + /* gather up all the phdrs. + * We do them all at once because there is more + * than one loop over all the phdrs. + */ + phdr = calloc(sizeof(*phdr), ehdr->e_phnum); + for (i = 0; i < ehdr->e_phnum; i++) + elf_phdr(&phdr_buf, &phdr[i], ehdr->e_phentsize, xdr, bit64); + *pphdr = phdr; + return 0; +} +/* returns size of result, or -1 if error. + * Note that, with the new code, this function + * works for all elf files, not just the restricted set. + */ int parse_elf_to_stage(const struct buffer *input, struct buffer *output, comp_algo algo, uint32_t *location) { - Elf32_Phdr *phdr; - Elf32_Ehdr *ehdr = (Elf32_Ehdr *)input->data; - char *header, *buffer; + Elf64_Phdr *phdr; + Elf64_Ehdr ehdr; + char *buffer; + struct buffer outheader; int headers; - int i; - struct cbfs_stage *stage; - unsigned int data_start, data_end, mem_end; - - int elf_bigendian = 0; + int i, outlen; + uint32_t data_start, data_end, mem_end; comp_func_ptr compress = compression_function(algo); if (!compress) return -1; DEBUG("start: parse_elf_to_stage(location=0x%x)\n", *location); - if (!iself((unsigned char *)input->data)) { - ERROR("The stage file is not in ELF format!\n"); - return -1; - } - // The tool may work in architecture-independent way. - if (arch != CBFS_ARCHITECTURE_UNKNOWN && - !((ehdr->e_machine == EM_ARM) && (arch == CBFS_ARCHITECTURE_ARMV7)) && - !((ehdr->e_machine == EM_386) && (arch == CBFS_ARCHITECTURE_X86))) { - ERROR("The stage file has the wrong architecture\n"); + if (elf_headers(input, &ehdr, &phdr) < 0) return -1; - } - - if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) { - elf_bigendian = 1; - } - if (elf_bigendian != is_big_endian()) { - elf32_to_native = swap32; - } - - headers = ehdr->e_phnum; - header = (char *)ehdr; - phdr = (Elf32_Phdr *) & header[elf32_to_native(ehdr->e_phoff)]; + headers = ehdr.e_phnum; - /* Now, regular headers - we only care about PT_LOAD headers, - * because thats what we're actually going to load - */ - - data_start = 0xFFFFFFFF; + data_start = ~0; data_end = 0; mem_end = 0; for (i = 0; i < headers; i++) { unsigned int start, mend, rend; - if (elf32_to_native(phdr[i].p_type) != PT_LOAD) + if (phdr[i].p_type != PT_LOAD) continue; /* Empty segments are never interesting */ - if (elf32_to_native(phdr[i].p_memsz) == 0) + if (phdr[i].p_memsz == 0) continue; /* BSS */ - start = elf32_to_native(phdr[i].p_paddr); + start = phdr[i].p_paddr; - mend = start + elf32_to_native(phdr[i].p_memsz); - rend = start + elf32_to_native(phdr[i].p_filesz); + mend = start + phdr[i].p_memsz; + rend = start + phdr[i].p_filesz; if (start < data_start) data_start = start; @@ -128,8 +315,9 @@ int parse_elf_to_stage(const struct buffer *input, struct buffer *output, } if (data_end <= data_start) { - ERROR("data ends before it starts. Make sure the " - "ELF file is correct and resides in ROM space.\n"); + ERROR("data ends (%08lx) before it starts(%08lx). Make sure the " + "ELF file is correct and resides in ROM space.\n", + (unsigned long)data_end, (unsigned long)data_start); exit(1); } @@ -146,25 +334,25 @@ int parse_elf_to_stage(const struct buffer *input, struct buffer *output, for (i = 0; i < headers; i++) { unsigned int l_start, l_offset = 0; - if (elf32_to_native(phdr[i].p_type) != PT_LOAD) + if (phdr[i].p_type != PT_LOAD) continue; - if (elf32_to_native(phdr[i].p_memsz) == 0) + if (phdr[i].p_memsz == 0) continue; - l_start = elf32_to_native(phdr[i].p_paddr); + l_start = phdr[i].p_paddr; if (l_start < *location) { l_offset = *location - l_start; l_start = *location; } memcpy(buffer + (l_start - data_start), - &header[elf32_to_native(phdr[i].p_offset)+l_offset], - elf32_to_native(phdr[i].p_filesz)-l_offset); + &input->data[phdr[i].p_offset + l_offset], + phdr[i].p_filesz - l_offset); } /* Now make the output buffer */ - if (buffer_create(output, sizeof(*stage) + data_end - data_start, + if (buffer_create(output, sizeof(struct cbfs_stage) + data_end - data_start, input->name) != 0) { ERROR("Unable to allocate memory: %m\n"); free(buffer); @@ -172,19 +360,30 @@ int parse_elf_to_stage(const struct buffer *input, struct buffer *output, } memset(output->data, 0, output->size); - stage = (struct cbfs_stage *)output->data; - - stage->load = data_start; /* FIXME: htonll */ - stage->memlen = mem_end - data_start; - stage->compression = algo; - stage->entry = ehdr->e_entry; /* FIXME: htonll */ - - compress(buffer, data_end - data_start, (output->data + sizeof(*stage)), - (int *)&stage->len); + /* Compress the data, at which point we'll know information + * to fill out the header. This seems backward but it works because + * - the output header is a known size (not always true in many xdr's) + * - we do need to know the compressed output size first + */ + compress(buffer, data_end - data_start, (output->data + sizeof(struct cbfs_stage)), + &outlen); free(buffer); + /* Set up for output marshaling. */ + outheader.data = output->data; + outheader.size = 0; + /* N.B. The original plan was that SELF data was B.E. + * but: this is all L.E. + * Maybe we should just change the spec. + */ + xdr_le.put32(&outheader, algo); + xdr_le.put64(&outheader, ehdr.e_entry); + xdr_le.put64(&outheader, data_start); + xdr_le.put32(&outheader, outlen); + xdr_le.put32(&outheader, mem_end - data_start); + if (*location) *location -= sizeof(struct cbfs_stage); - output->size = sizeof(*stage) + stage->len; + output->size = sizeof(struct cbfs_stage) + outlen; return 0; } diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h index 6e12fcb..c68f0ff 100644 --- a/util/cbfstool/common.h +++ b/util/cbfstool/common.h @@ -131,6 +131,17 @@ uint32_t cbfs_find_location(const char *romfile, uint32_t filesize, void print_supported_filetypes(void); +/* xdr.c */ +struct xdr { + uint16_t (*get16)(struct buffer *input); + uint32_t (*get32)(struct buffer *input); + uint64_t (*get64)(struct buffer *input); + void (*put16)(struct buffer *input, uint16_t val); + void (*put32)(struct buffer *input, uint32_t val); + void (*put64)(struct buffer *input, uint64_t val); +}; + +extern struct xdr xdr_le, xdr_be; #define ARRAY_SIZE(a) (int)(sizeof(a) / sizeof((a)[0])) #endif diff --git a/util/cbfstool/xdr.c b/util/cbfstool/xdr.c new file mode 100644 index 0000000..1659b46 --- /dev/null +++ b/util/cbfstool/xdr.c @@ -0,0 +1,141 @@ + /* + * cbfstool, CLI utility for CBFS file manipulation + * + * Copyright 2013 Google Inc. + * + * 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; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA + */ + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <ctype.h> +#include <unistd.h> +#include <stdint.h> +#include "common.h" + +/* The assumption in all this code is that we're given a pointer to enough data. + * Hence, we do not check for underflow. + */ +uint8_t get8(struct buffer *input) +{ + uint8_t ret = *input->data++; + input->size--; + return ret; +} + +uint16_t get16be(struct buffer *input) +{ + uint16_t ret; + ret = get8(input)<<8; + ret |= get8(input); + return ret; +} + +uint32_t get32be(struct buffer *input) +{ + uint32_t ret; + ret = get16be(input)<<16; + ret |= get16be(input); + return ret; +} + +uint64_t get64be(struct buffer *input) +{ + uint64_t ret; + ret = get32be(input); + ret <<= 32; + ret |= get32be(input); + return ret; +} + +void put8(struct buffer *input, uint8_t val) +{ + input->data[input->size] = val; + input->size++; +} + +void put16be(struct buffer *input, uint16_t val) +{ + put8(input, val>>8); + put8(input, val); +} + +void put32be(struct buffer *input, uint32_t val) +{ + put16be(input, val>>16); + put16be(input, val); +} + +void put64be(struct buffer *input, uint64_t val) +{ + put32be(input, val>>32); + put32be(input, val); +} + +uint16_t get16le(struct buffer *input) +{ + uint16_t ret; + ret = get8(input); + ret |= get8(input) << 8; + return ret; +} + +uint32_t get32le(struct buffer *input) +{ + uint32_t ret; + ret = get16le(input); + ret |= get16le(input) << 16; + return ret; +} + +uint64_t get64le(struct buffer *input) +{ + uint64_t ret; + uint32_t low; + low = get32le(input); + ret = get32le(input); + ret <<= 32; + ret |= low; + return ret; +} + +void put16le(struct buffer *input, uint16_t val) +{ + put8(input, val); + put8(input, val>>8); +} + +void put32le(struct buffer *input, uint32_t val) +{ + put16le(input, val); + put16le(input, val>>16); +} + +void put64le(struct buffer *input, uint64_t val) +{ + put32le(input, val); + put32le(input, val>>32); +} + +struct xdr xdr_be = { + get16le, get32le, get64le, + put16be, put32be, put64be +}; + +struct xdr xdr_le = { + get16le, get32le, get64le, + put16le, put32le, put64le +}; +

1 0

Patch set updated for coreboot: ead8e61 AMD Kabini: Add ACPI sleep/wakeup calls for southbridge
by Zheng Bao 03 Dec '13

03 Dec '13

Zheng Bao (zheng.bao(a)amd.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4260 -gerrit commit ead8e61745ec63566238feb6db8364edfdb8661f Author: Zheng Bao <fishbaozi(a)gmail.com> Date: Thu Nov 21 16:11:40 2013 +0800 AMD Kabini: Add ACPI sleep/wakeup calls for southbridge The AML code of PTS and WAK for southbridge are in UINT8 AlibSsdtKB[], Proc/GNB/Modules/GnbInitKB/AlibSsdtKB.h. It was integrated into SSDT though it was called by nobody. The source ASL was provided by AGESA for reference, but it has been scrubbed when it was ported to Coreboot. Without the calls, Olive Hill can not wake up if it boots Windows. Both amd/olivehill and asrock/imb-a180 have been validated. Change-Id: Ia7bba29904dbd6f33fdb08bf88bb499005ef561b Signed-off-by: Zheng Bao <zheng.bao(a)amd.com> Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- src/mainboard/amd/olivehill/acpi/sleep.asl | 6 ++++++ src/mainboard/asrock/imb-a180/acpi/sleep.asl | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/mainboard/amd/olivehill/acpi/sleep.asl b/src/mainboard/amd/olivehill/acpi/sleep.asl index dd0d9a4..fc04ecc 100644 --- a/src/mainboard/amd/olivehill/acpi/sleep.asl +++ b/src/mainboard/amd/olivehill/acpi/sleep.asl @@ -36,6 +36,10 @@ Name(WKST,Package(){Zero, Zero}) * the ACPI driver. This method cannot modify the configuration or power * state of any device in the system. */ + +External(\_SB.APTS, MethodObj) +External(\_SB.AWAK, MethodObj) + Method(_PTS, 1) { /* DBGO("\\_PTS\n") */ /* DBGO("From S0 to S") */ @@ -46,6 +50,7 @@ Method(_PTS, 1) { Store(0, Index(WKST,0)) Store(0, Index(WKST,1)) Store(7, UPWS) + \_SB.APTS(Arg0) } /* End Method(\_PTS) */ /* @@ -84,6 +89,7 @@ Method(\_WAK, 1) { /* DBGO("From S") */ /* DBGO(Arg0) */ /* DBGO(" to S0\n") */ + \_SB.AWAK(Arg0) Return(WKST) } /* End Method(\_WAK) */ diff --git a/src/mainboard/asrock/imb-a180/acpi/sleep.asl b/src/mainboard/asrock/imb-a180/acpi/sleep.asl index dd0d9a4..fc04ecc 100644 --- a/src/mainboard/asrock/imb-a180/acpi/sleep.asl +++ b/src/mainboard/asrock/imb-a180/acpi/sleep.asl @@ -36,6 +36,10 @@ Name(WKST,Package(){Zero, Zero}) * the ACPI driver. This method cannot modify the configuration or power * state of any device in the system. */ + +External(\_SB.APTS, MethodObj) +External(\_SB.AWAK, MethodObj) + Method(_PTS, 1) { /* DBGO("\\_PTS\n") */ /* DBGO("From S0 to S") */ @@ -46,6 +50,7 @@ Method(_PTS, 1) { Store(0, Index(WKST,0)) Store(0, Index(WKST,1)) Store(7, UPWS) + \_SB.APTS(Arg0) } /* End Method(\_PTS) */ /* @@ -84,6 +89,7 @@ Method(\_WAK, 1) { /* DBGO("From S") */ /* DBGO(Arg0) */ /* DBGO(" to S0\n") */ + \_SB.AWAK(Arg0) Return(WKST) } /* End Method(\_WAK) */

1 0

Patch set updated for coreboot: 344b0e5 AMD Kabini: fix issue 'S3 fails to suspend after wake up from USB keyboard'
by Zheng Bao 03 Dec '13

03 Dec '13

Zheng Bao (zheng.bao(a)amd.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4261 -gerrit commit 344b0e578b8e9edae2aca0011c74b760fb59aa6d Author: Zheng Bao <fishbaozi(a)gmail.com> Date: Thu Nov 21 16:12:27 2013 +0800 AMD Kabini: fix issue 'S3 fails to suspend after wake up from USB keyboard' Propagated from http://review.coreboot.org/3347 http://review.coreboot.org/3374 The cause of this issue is: USB devices use bit 11(0x0b) of GP0_STS represents S3 wake up event, but this bit is not clear after wake up. So OS thinks there is a wake up signal and wake up immediately. Both amd/olivehill and asrock/imb-a180 have been validated. Change-Id: I7c26cb07bcd2e62bb792809b67314e5155c6adf6 Signed-off-by: Zheng Bao <zheng.bao(a)amd.com> Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- src/mainboard/amd/olivehill/acpi/mainboard.asl | 7 +++++++ src/mainboard/amd/olivehill/acpi/sleep.asl | 2 ++ src/mainboard/asrock/imb-a180/acpi/mainboard.asl | 7 +++++++ src/mainboard/asrock/imb-a180/acpi/sleep.asl | 2 ++ 4 files changed, 18 insertions(+) diff --git a/src/mainboard/amd/olivehill/acpi/mainboard.asl b/src/mainboard/amd/olivehill/acpi/mainboard.asl index 5487b79..4045143 100644 --- a/src/mainboard/amd/olivehill/acpi/mainboard.asl +++ b/src/mainboard/amd/olivehill/acpi/mainboard.asl @@ -32,3 +32,10 @@ Name(SSFG, 0x0D) /* S1 support: bit 0, S2 Support: bit 1, etc. S0 & S5 assumed Name(OSTP, 3) /* Assume nothing. WinXp = 1, Vista = 2, Linux = 3, WinCE = 4 */ Name(OSV, Ones) /* Assume nothing */ Name(PMOD, One) /* Assume APIC */ + +/* AcpiGpe0Blk */ +OperationRegion(GP0B, SystemMemory, 0xfed80814, 0x04) + Field(GP0B, ByteAcc, NoLock, Preserve) { + , 11, + USBS, 1, +} diff --git a/src/mainboard/amd/olivehill/acpi/sleep.asl b/src/mainboard/amd/olivehill/acpi/sleep.asl index fc04ecc..2fc2efe 100644 --- a/src/mainboard/amd/olivehill/acpi/sleep.asl +++ b/src/mainboard/amd/olivehill/acpi/sleep.asl @@ -89,6 +89,8 @@ Method(\_WAK, 1) { /* DBGO("From S") */ /* DBGO(Arg0) */ /* DBGO(" to S0\n") */ + Store(1,USBS) + \_SB.AWAK(Arg0) Return(WKST) diff --git a/src/mainboard/asrock/imb-a180/acpi/mainboard.asl b/src/mainboard/asrock/imb-a180/acpi/mainboard.asl index 5487b79..4045143 100644 --- a/src/mainboard/asrock/imb-a180/acpi/mainboard.asl +++ b/src/mainboard/asrock/imb-a180/acpi/mainboard.asl @@ -32,3 +32,10 @@ Name(SSFG, 0x0D) /* S1 support: bit 0, S2 Support: bit 1, etc. S0 & S5 assumed Name(OSTP, 3) /* Assume nothing. WinXp = 1, Vista = 2, Linux = 3, WinCE = 4 */ Name(OSV, Ones) /* Assume nothing */ Name(PMOD, One) /* Assume APIC */ + +/* AcpiGpe0Blk */ +OperationRegion(GP0B, SystemMemory, 0xfed80814, 0x04) + Field(GP0B, ByteAcc, NoLock, Preserve) { + , 11, + USBS, 1, +} diff --git a/src/mainboard/asrock/imb-a180/acpi/sleep.asl b/src/mainboard/asrock/imb-a180/acpi/sleep.asl index fc04ecc..2fc2efe 100644 --- a/src/mainboard/asrock/imb-a180/acpi/sleep.asl +++ b/src/mainboard/asrock/imb-a180/acpi/sleep.asl @@ -89,6 +89,8 @@ Method(\_WAK, 1) { /* DBGO("From S") */ /* DBGO(Arg0) */ /* DBGO(" to S0\n") */ + Store(1,USBS) + \_SB.AWAK(Arg0) Return(WKST)

1 0

Patch set updated for coreboot: b0d37cd AMD IMC AGESA: Access the data in stack by correct length
by Zheng Bao 03 Dec '13

03 Dec '13

Zheng Bao (zheng.bao(a)amd.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4297 -gerrit commit b0d37cd2569ad870eecf4303691989cbd0856adc Author: Zheng Bao <fishbaozi(a)gmail.com> Date: Tue Dec 3 10:37:57 2013 +0800 AMD IMC AGESA: Access the data in stack by correct length The bug is hard to find. We were adding the feature of fan control. We met some strange things which could not be explained. Like, sometimes adding printk let the error disappear. Then we traced the code by hardware debug tool (HDT). It turned out the data in stack was overwritten. The values of AccessWidthxx are { AccessWidth8 = 1, AccessWidth16, AccessWidth32,} For the case of AccessWidth8, we only need to access the index/data once. But ReadECmsg and WriteECmsg did the loop twice, 1 more time than they are supposed to do. The data in stack next to "Value" would be overwritten. For all the cases, the code should be OpFlag = OpFlag & 0x7f; switch (OpFlag) { case 1: /* AccessWidth8 */ OpFlag = 0;break; case 2: /* AccessWidth16 */ OpFlag = 1;break; case 3: /* AccessWidth32 */ OpFlag = 3;break; case 4: /* AccessWidth64 */ OpFlag = 7;break; default: error; } Actually, the caller only takes AccessWidth8 as the parameter. We can ignore other cases for now. That is an AGESA bug. AMD's AGESA team own this code. I presume let them decide the proper way to fix that. Before that, I change the code as little as possible to make it run without crash. Change-Id: I566f74c242ce93f4569eedf69ca07d2fb7fb368d Signed-off-by: Zheng Bao <zheng.bao(a)amd.com> Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c | 4 ++-- src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c | 4 ++-- src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c index 1a3f7dd..30aa59f 100644 --- a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c +++ b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c @@ -55,7 +55,7 @@ WriteECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) OpFlag = 0x03; for (Index = 0; Index <= OpFlag; Index++) { @@ -77,7 +77,7 @@ ReadECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) OpFlag = 0x03; for (Index = 0; Index <= OpFlag; Index++) { diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c index a451c41..0069970 100644 --- a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c +++ b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c @@ -55,7 +55,7 @@ WriteECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; } @@ -79,7 +79,7 @@ ReadECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; } diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c index 05f5727..5bef58e 100644 --- a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c +++ b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c @@ -66,7 +66,7 @@ WriteECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; } @@ -101,7 +101,7 @@ ReadECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; }

1 0

Patch set updated for coreboot: 3a8618c AMD IMC AGESA: Access the data in stack by correct length
by Zheng Bao 03 Dec '13

03 Dec '13

Zheng Bao (zheng.bao(a)amd.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4297 -gerrit commit 3a8618cae49b88cd8350aa5bfccb99ca0954a469 Author: Zheng Bao <fishbaozi(a)gmail.com> Date: Tue Dec 3 10:00:18 2013 +0800 AMD IMC AGESA: Access the data in stack by correct length The bug is hard to find. We were adding the feature of fan control. We met some strange things which could not be explained. Like, sometimes adding printk let the error disappear. Then we traced the code by hardware debug tool (HDT). It turned out the data in stack was overwritten. The values of AccessWidthxx are { AccessWidth8 = 1, AccessWidth16, AccessWidth32,} For the case of AccessWidth8, we only need to access the index/data once. But ReadECmsg and WriteECmsg did the loop twice, 1 more time than they are supposed to do. The data in stack next to "Value" will be overwritten. For all the cases, the code should be OpFlag = OpFlag & 0x7f; switch (OpFlag) { case 1: /* AccessWidth8 */ OpFlag = 0;break; case 2: /* AccessWidth16 */ OpFlag = 1;break; case 3: /* AccessWidth32 */ OpFlag = 3;break; case 4: /* AccessWidth64 */ OpFlag = 7;break; default: error; } Actually, the caller only takes AccessWidth8 as the parameter. We can ignore other cases for now. That is an AGESA bug. AMD's AGESA team own this code. I presume let them decide the proper way to fix that. Before that, I change the code as little as possible to make it run without crash. Change-Id: I566f74c242ce93f4569eedf69ca07d2fb7fb368d Signed-off-by: Zheng Bao <zheng.bao(a)amd.com> Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c | 4 ++-- src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c | 4 ++-- src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c index 1a3f7dd..30aa59f 100644 --- a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c +++ b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c @@ -55,7 +55,7 @@ WriteECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) OpFlag = 0x03; for (Index = 0; Index <= OpFlag; Index++) { @@ -77,7 +77,7 @@ ReadECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) OpFlag = 0x03; for (Index = 0; Index <= OpFlag; Index++) { diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c index a451c41..0069970 100644 --- a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c +++ b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c @@ -55,7 +55,7 @@ WriteECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; } @@ -79,7 +79,7 @@ ReadECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; } diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c index 05f5727..5bef58e 100644 --- a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c +++ b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c @@ -66,7 +66,7 @@ WriteECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; } @@ -101,7 +101,7 @@ ReadECmsg ( { UINT8 Index; - OpFlag = OpFlag & 0x7f; + OpFlag = (OpFlag - 1) & 0x7f; if (OpFlag == 0x02) { OpFlag = 0x03; }

1 0

Patch merged into coreboot/master: 4055697 Drop obsolete CONSOLE_LOGBUF
by gerrit＠coreboot.org 03 Dec '13

03 Dec '13

the following patch was just integrated into master: commit 40556974c47ea5883e69dccaca27c4831dcb89cc Author: Stefan Reinauer <reinauer(a)chromium.org> Date: Wed Jun 19 13:47:46 2013 -0700 Drop obsolete CONSOLE_LOGBUF This was used by Ron 13ys ago and was never used again ever since. Change-Id: I8ae8a570d67fa0b34b17c9e3709845687f73c724 Signed-off-by: Stefan Reinauer <reinauer(a)google.com> Reviewed-on: https://gerrit.chromium.org/gerrit/59320 Reviewed-by: Stefan Reinauer <reinauer(a)chromium.org> Tested-by: Stefan Reinauer <reinauer(a)chromium.org> Commit-Queue: Stefan Reinauer <reinauer(a)chromium.org> See http://review.coreboot.org/4256 for details. -gerrit

1 0

Patch merged into coreboot/master: 5f6afca libpayload: ARM: Don't leave alignment checking on after the exception test
by gerrit＠coreboot.org 03 Dec '13

03 Dec '13

the following patch was just integrated into master: commit 5f6afca48cc72055a81a5c2230101616c5f14db2 Author: Gabe Black <gabeblack(a)google.com> Date: Wed Jun 19 20:48:06 2013 -0700 libpayload: ARM: Don't leave alignment checking on after the exception test Currently, the exception handling code on ARM in libpayload turns on alignment checks as an easy way to generate an exception for testing purposes. It was leaving it on which disabled unaligned accesses for other, unlreated code running later. This change adjusts the code so the original value of the alignment bit is restored after the test exception. Built and booted into depthcharge on pit with an unaligned accesses added after the call to exception_init in the depthcharge's main. Before this change, the access caused an exception. After this change, the access completed successfully. Change-Id: If92cab3cc8eabca7c5b0560ce88a8796a27fe3b2 Signed-off-by: Gabe Black <gabeblack(a)google.com> Reviewed-on: https://gerrit.chromium.org/gerrit/59372 Reviewed-by: Stefan Reinauer <reinauer(a)google.com> Commit-Queue: Gabe Black <gabeblack(a)chromium.org> Tested-by: Gabe Black <gabeblack(a)chromium.org> See http://review.coreboot.org/4255 for details. -gerrit

1 0

Patch merged into coreboot/master: 5031909 libpayload: Make the region to scan for the cb tables configurable.
by gerrit＠coreboot.org 03 Dec '13

03 Dec '13

the following patch was just integrated into master: commit 50319097fc5df130a5060cf1cd5bbaa0eb71eac0 Author: Gabe Black <gabeblack(a)google.com> Date: Wed Jun 19 04:54:38 2013 -0700 libpayload: Make the region to scan for the cb tables configurable. The address range to scan for the coreboot tables varies from machine to machine based on the range memory occupies on the SOC being booted and on the amount of memory installed on the machine. To make libpayload work on different ARM systems with different needs, this change makes the region to scan configurable. In the future, we might want to come up with a more automatic mechanism like on x86, although there's less consistency on ARM as far as what ranges are even memory in the first place. Change-Id: Ib50efe25a6152171b0fbd0e324dbc5e89c527d6e Signed-off-by: Gabe Black <gabeblack(a)google.com> Reviewed-on: https://gerrit.chromium.org/gerrit/59242 Reviewed-by: Gabe Black <gabeblack(a)chromium.org> Tested-by: Gabe Black <gabeblack(a)chromium.org> Commit-Queue: Gabe Black <gabeblack(a)chromium.org> See http://review.coreboot.org/4254 for details. -gerrit

1 0

Patch merged into coreboot/master: 24c773c lynxpoint: Fix LPT-LP PME_B0 bit offset in ACPI _PRW objects
by gerrit＠coreboot.org 03 Dec '13

03 Dec '13

the following patch was just integrated into master: commit 24c773c6e93965886eb0491b7a1b38c8b5a2aeec Author: Duncan Laurie <dlaurie(a)chromium.org> Date: Wed Jun 19 10:49:29 2013 -0700 lynxpoint: Fix LPT-LP PME_B0 bit offset in ACPI _PRW objects LynxPoint-LP has a lot of GPEs and the "default" set has been moved to register 4 starting at bit offset 96. This means that PME_B0 bit in GPE0_EN/GPE0_STS is now bit 109 in LPT-LP but still bit 13 in LPT-H. suspend on falco and wake from usb 4 | 2013-06-19 10:49:17 | ACPI Enter | S3 5 | 2013-06-19 10:49:22 | ACPI Wake | S3 6 | 2013-06-19 10:49:22 | Wake Source | Internal PME | 0 Change-Id: I443cd4d17796888debed70c0bda27ae09accd09b Signed-off-by: Duncan Laurie <dlaurie(a)chromium.org> Reviewed-on: https://gerrit.chromium.org/gerrit/59265 Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> See http://review.coreboot.org/4253 for details. -gerrit

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2013