coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: util/cbfstool: Remove legacy cbfs support
by Arthur Heymans (Code Review) 01 Jul '21

01 Jul '21

Arthur Heymans has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/56001 ) Change subject: util/cbfstool: Remove legacy cbfs support ...................................................................... util/cbfstool: Remove legacy cbfs support FMAP has been there for quite some time and the coreboot code itself cannot handle legacy images anymore so it makes sense to drop support in cbfstool itself. Change-Id: I2f34a25dae1ec1924cabe392dcd1480d7b4fbf49 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h M util/cbfstool/cbfstool.c 3 files changed, 21 insertions(+), 262 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/56001/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2f34a25dae1ec1924cabe392dcd1480d7b4fbf49 Gerrit-Change-Number: 56001 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: util/cbfstool: Remove unused pagesize parameter
by Arthur Heymans (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: Furquan Shaikh, Martin Roth, Julius Werner, Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55581 ) Change subject: util/cbfstool: Remove unused pagesize parameter ...................................................................... Patch Set 5: (1 comment) File util/cbfstool/cbfstool.c: https://review.coreboot.org/c/coreboot/+/55581/comment/f1193b14_c5cf7cce PS3, Line 545: uint32_t pagesize = 1; > > I think this reduces to: […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/55581 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib672ba8ed418b1a76e4a48951eabda6923358e7a Gerrit-Change-Number: 55581 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 01 Jul 2021 16:01:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/cbfstool: Remove legacy cbfs support
by Arthur Heymans (Code Review) 01 Jul '21

01 Jul '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56001 ) Change subject: util/cbfstool: Remove legacy cbfs support ...................................................................... util/cbfstool: Remove legacy cbfs support FMAP has been there for quite some time and the coreboot code itself cannot handle legacy images anymore so it makes sense to drop support in cbfstool itself. Change-Id: I2f34a25dae1ec1924cabe392dcd1480d7b4fbf49 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h M util/cbfstool/cbfstool.c 3 files changed, 21 insertions(+), 261 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/56001/1 diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 1fb19ba..598f697 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -241,94 +241,7 @@ capacity, ""); } -int cbfs_legacy_image_create(struct cbfs_image *image, - uint32_t architecture, - uint32_t align, - struct buffer *bootblock, - uint32_t bootblock_offset, - uint32_t header_offset, - uint32_t entries_offset) -{ - assert(image); - assert(image->buffer.data); - assert(bootblock); - - int32_t *rel_offset; - uint32_t cbfs_len; - void *header_loc; - size_t size = image->buffer.size; - - DEBUG("cbfs_image_create: bootblock=0x%x+0x%zx, " - "header=0x%x+0x%zx, entries_offset=0x%x\n", - bootblock_offset, bootblock->size, header_offset, - sizeof(image->header), entries_offset); - - DEBUG("cbfs_create_image: (real offset) bootblock=0x%x, " - "header=0x%x, entries_offset=0x%x\n", - bootblock_offset, header_offset, entries_offset); - - // Prepare bootblock - if (bootblock_offset + bootblock->size > size) { - ERROR("Bootblock (0x%x+0x%zx) exceed ROM size (0x%zx)\n", - bootblock_offset, bootblock->size, size); - return -1; - } - if (entries_offset > bootblock_offset && - entries_offset < bootblock->size) { - ERROR("Bootblock (0x%x+0x%zx) overlap CBFS data (0x%x)\n", - bootblock_offset, bootblock->size, entries_offset); - return -1; - } - memcpy(image->buffer.data + bootblock_offset, bootblock->data, - bootblock->size); - - // Prepare header - if (header_offset + sizeof(image->header) > size - sizeof(int32_t)) { - ERROR("Header (0x%x+0x%zx) exceed ROM size (0x%zx)\n", - header_offset, sizeof(image->header), size); - return -1; - } - image->header.magic = CBFS_HEADER_MAGIC; - image->header.version = CBFS_HEADER_VERSION; - image->header.romsize = size; - image->header.bootblocksize = bootblock->size; - image->header.align = align; - image->header.offset = entries_offset; - image->header.architecture = architecture; - - header_loc = (image->buffer.data + header_offset); - cbfs_put_header(header_loc, &image->header); - image->has_header = true; - - // The last 4 byte of the image contain the relative offset from the end - // of the image to the master header as a 32-bit signed integer. x86 - // relies on this also being its (memory-mapped, top-aligned) absolute - // 32-bit address by virtue of how two's complement numbers work. - assert(size % sizeof(int32_t) == 0); - rel_offset = (int32_t *)(image->buffer.data + size - sizeof(int32_t)); - *rel_offset = header_offset - size; - - // Prepare entries - if (align_up(entries_offset, align) != entries_offset) { - ERROR("Offset (0x%x) must be aligned to 0x%x.\n", - entries_offset, align); - return -1; - } - // To calculate available length, find - // e = min(bootblock, header, rel_offset) where e > entries_offset. - cbfs_len = size - sizeof(int32_t); - if (bootblock_offset > entries_offset && bootblock_offset < cbfs_len) - cbfs_len = bootblock_offset; - if (header_offset > entries_offset && header_offset < cbfs_len) - cbfs_len = header_offset; - - if (cbfs_image_create(image, cbfs_len - entries_offset)) - return -1; - return 0; -} - -int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in, - uint32_t offset) +int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in) { assert(out); assert(in); @@ -341,16 +254,14 @@ return 0; } - void *header_loc = cbfs_find_header(in->data, in->size, offset); + void *header_loc = cbfs_find_header(in->data, in->size, ~0); if (header_loc) { cbfs_get_header(&out->header, header_loc); out->has_header = true; cbfs_fix_legacy_size(out, header_loc); return 0; - } else if (offset != ~0u) { - ERROR("The -H switch is only valid on legacy images having CBFS master headers.\n"); - return 1; } + ERROR("Selected image region is not a valid CBFS.\n"); return 1; } @@ -416,7 +327,7 @@ struct cbfs_image image; memset(&image, 0, sizeof(image)); - if (cbfs_image_from_buffer(&image, region, 0)) { + if (cbfs_image_from_buffer(&image, region)) { ERROR("reading CBFS failed!\n"); return 1; } @@ -455,7 +366,7 @@ struct cbfs_image image; memset(&image, 0, sizeof(image)); - if (cbfs_image_from_buffer(&image, region, 0)) { + if (cbfs_image_from_buffer(&image, region)) { ERROR("reading CBFS failed!\n"); return 1; } diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index 664d146..36eaf34 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -39,26 +39,12 @@ * Returns 0 on success, otherwise nonzero. */ int cbfs_image_create(struct cbfs_image *image, size_t entries_size); -/* Creates an empty CBFS image by given size, and description to its content - * (bootblock, align, header location, starting offset of CBFS entries). - * The output image will contain a valid cbfs_header, with one cbfs_file - * entry with type CBFS_TYPE_NULL, with max available size. - * Only call this if you want a legacy CBFS with a master header. - * Returns 0 on success, otherwise nonzero. */ -int cbfs_legacy_image_create(struct cbfs_image *image, - uint32_t arch, - uint32_t align, - struct buffer *bootblock, - uint32_t bootblock_offset, - uint32_t header_offset, - uint32_t entries_offset); /* Constructs a cbfs_image from a buffer. The resulting image contains a shallow * copy of the buffer; releasing either one is the legal way to clean up after * both of them at once. Always produces a cbfs_image, but... * Returns 0 if it contains a valid CBFS, non-zero if it's unrecognized data. */ -int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in, - uint32_t offset); +int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in); /* Create a duplicate CBFS image. Returns 0 on success, otherwise non-zero. * Will not succeed on new-style images without a master header. */ diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index dc6a432..9828c79 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -45,7 +45,6 @@ const char *fmap; const char *region_name; const char *source_region; - const char *bootblock; const char *ignore_section; const char *ucode_region; uint64_t u64val; @@ -58,24 +57,10 @@ long long int baseaddress_input; uint32_t baseaddress_assigned; uint32_t loadaddress; - uint32_t headeroffset; - /* - * Input can be negative. It will be transformed to offset from start of region (if - * negative) and stored in baseaddress. - */ - long long int headeroffset_input; - uint32_t headeroffset_assigned; uint32_t entrypoint; uint32_t size; uint32_t alignment; uint32_t pagesize; - uint32_t cbfsoffset; - /* - * Input can be negative. It will be transformed to corresponding region offset (if - * negative) and stored in baseaddress. - */ - long long int cbfsoffset_input; - uint32_t cbfsoffset_assigned; uint32_t arch; uint32_t padding; uint32_t topswap_size; @@ -113,7 +98,6 @@ .arch = CBFS_ARCHITECTURE_UNKNOWN, .compression = CBFS_COMPRESS_NONE, .hash = VB2_HASH_INVALID, - .headeroffset = ~0, .region_name = SECTION_NAME_PRIMARY_CBFS, .u64val = -1, }; @@ -166,7 +150,7 @@ SECTION_NAME_PRIMARY_CBFS)) goto no_metadata_hash; mhc.region = SECTION_NAME_PRIMARY_CBFS; - if (cbfs_image_from_buffer(&cbfs, &buffer, param.headeroffset)) + if (cbfs_image_from_buffer(&cbfs, &buffer)) goto no_metadata_hash; bootblock = cbfs_get_entry(&cbfs, "bootblock"); if (!bootblock || ntohl(bootblock->type) != CBFS_TYPE_BOOTBLOCK) @@ -521,8 +505,7 @@ } struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; if (cbfs_get_entry(&image, param.name)) @@ -582,8 +565,7 @@ static int cbfs_add_integer_component(const char *name, uint64_t u64val, - uint32_t offset, - uint32_t headeroffset) { + uint32_t offset) { struct cbfs_image image; struct cbfs_file *header = NULL; struct buffer buffer; @@ -600,7 +582,7 @@ for (i = 0; i < 8; i++) buffer.data[i] = (u64val >> i*8) & 0xff; - if (cbfs_image_from_buffer(&image, param.image_region, headeroffset)) { + if (cbfs_image_from_buffer(&image, param.image_region)) { ERROR("Selected image region is not a CBFS.\n"); goto done; } @@ -695,8 +677,7 @@ size_t size; void *h_loc; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) { + if (cbfs_image_from_buffer(&image, param.image_region)) { ERROR("Selected image region is not a CBFS.\n"); return 1; } @@ -813,7 +794,6 @@ static int cbfs_add_component(const char *filename, const char *name, uint32_t type, - uint32_t headeroffset, convert_buffer_t convert) { size_t len_align = 0; @@ -840,7 +820,7 @@ } struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; if (cbfs_get_entry(&image, name)) { @@ -1254,7 +1234,6 @@ return cbfs_add_component(param.filename, param.name, param.type, - param.headeroffset, convert); } @@ -1275,7 +1254,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_STAGE, - param.headeroffset, cbfstool_convert_mkstage); } @@ -1284,7 +1262,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_SELF, - param.headeroffset, cbfstool_convert_mkpayload); } @@ -1303,7 +1280,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_SELF, - param.headeroffset, cbfstool_convert_mkflatpayload); } @@ -1314,9 +1290,8 @@ return 1; } return cbfs_add_integer_component(param.name, - param.u64val, - param.baseaddress, - param.headeroffset); + param.u64val, + param.baseaddress); } static int cbfs_remove(void) @@ -1327,8 +1302,7 @@ } struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; if (cbfs_remove_entry(&image, param.name) != 0) { @@ -1346,84 +1320,7 @@ memset(&image, 0, sizeof(image)); buffer_clone(&image.buffer, param.image_region); - if (param.fmap) { - if (param.arch != CBFS_ARCHITECTURE_UNKNOWN || param.size || - param.baseaddress_assigned || - param.headeroffset_assigned || - param.cbfsoffset_assigned || - param.bootblock) { - ERROR("Since -M was provided, -m, -s, -b, -o, -H, and -B should be omitted\n"); - return 1; - } - - return cbfs_image_create(&image, image.buffer.size); - } - - if (param.arch == CBFS_ARCHITECTURE_UNKNOWN) { - ERROR("You need to specify -m/--machine arch.\n"); - return 1; - } - - struct buffer bootblock; - if (!param.bootblock) { - DEBUG("-B not given, creating image without bootblock.\n"); - if (buffer_create(&bootblock, 0, "(dummy)") != 0) - return 1; - } else if (buffer_from_file(&bootblock, param.bootblock)) { - return 1; - } - - if (!param.alignment) - param.alignment = CBFS_ALIGNMENT; - - // Set default offsets. x86, as usual, needs to be a special snowflake. - if (!param.baseaddress_assigned) { - if (param.arch == CBFS_ARCHITECTURE_X86) { - // Make sure there's at least enough room for rel_offset - param.baseaddress = param.size - - MAX(bootblock.size, sizeof(int32_t)); - DEBUG("x86 -> bootblock lies at end of ROM (%#x).\n", - param.baseaddress); - } else { - param.baseaddress = 0; - DEBUG("bootblock starts at address 0x0.\n"); - } - } - if (!param.headeroffset_assigned) { - if (param.arch == CBFS_ARCHITECTURE_X86) { - param.headeroffset = param.baseaddress - - sizeof(struct cbfs_header); - DEBUG("x86 -> CBFS header before bootblock (%#x).\n", - param.headeroffset); - } else { - param.headeroffset = align_up(param.baseaddress + - bootblock.size, sizeof(uint32_t)); - DEBUG("CBFS header placed behind bootblock (%#x).\n", - param.headeroffset); - } - } - if (!param.cbfsoffset_assigned) { - if (param.arch == CBFS_ARCHITECTURE_X86) { - param.cbfsoffset = 0; - DEBUG("x86 -> CBFS entries start at address 0x0.\n"); - } else { - param.cbfsoffset = align_up(param.headeroffset + - sizeof(struct cbfs_header), - CBFS_ALIGNMENT); - DEBUG("CBFS entries start beind master header (%#x).\n", - param.cbfsoffset); - } - } - - int ret = cbfs_legacy_image_create(&image, - param.arch, - CBFS_ALIGNMENT, - &bootblock, - param.baseaddress, - param.headeroffset, - param.cbfsoffset); - buffer_delete(&bootblock); - return ret; + return cbfs_image_create(&image, image.buffer.size); } static int cbfs_layout(void) @@ -1496,8 +1393,7 @@ static int cbfs_print(void) { struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; if (param.machine_parseable) { if (verbose) @@ -1551,8 +1447,7 @@ } struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; return cbfs_export_entry(&image, param.name, param.filename, @@ -1668,7 +1563,7 @@ return 1; } - if (cbfs_image_from_buffer(&src_image, &src_buf, param.headeroffset)) + if (cbfs_image_from_buffer(&src_image, &src_buf)) return 1; return cbfs_copy_instance(&src_image, param.image_region); @@ -1677,8 +1572,7 @@ static int cbfs_compact(void) { struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; WARN("Compacting a CBFS doesn't honor alignment or fixed addresses!\n"); return cbfs_compact_instance(&image); @@ -1750,7 +1644,6 @@ static struct option long_options[] = { {"alignment", required_argument, 0, 'a' }, {"base-address", required_argument, 0, 'b' }, - {"bootblock", required_argument, 0, 'B' }, {"cmdline", required_argument, 0, 'C' }, {"compression", required_argument, 0, 'c' }, {"topswap-size", required_argument, 0, 'j' }, @@ -1764,7 +1657,6 @@ {"force", no_argument, 0, 'F' }, {"source-region", required_argument, 0, 'R' }, {"hash-algorithm",required_argument, 0, 'A' }, - {"header-offset", required_argument, 0, 'H' }, {"help", no_argument, 0, 'h' }, {"ignore-sec", required_argument, 0, 'S' }, {"initrd", required_argument, 0, 'I' }, @@ -1772,7 +1664,6 @@ {"load-address", required_argument, 0, 'l' }, {"machine", required_argument, 0, 'm' }, {"name", required_argument, 0, 'n' }, - {"offset", required_argument, 0, 'o' }, {"padding", required_argument, 0, 'p' }, {"pow2page", no_argument, 0, 'Q' }, {"ucode-region", required_argument, 0, 'q' }, @@ -1809,10 +1700,6 @@ if (param.baseaddress_assigned) ret |= get_region_offset(param.baseaddress_input, &param.baseaddress); - if (param.headeroffset_assigned) - ret |= get_region_offset(param.headeroffset_input, &param.headeroffset); - if (param.cbfsoffset_assigned) - ret |= get_region_offset(param.cbfsoffset_input, &param.cbfsoffset); return ret; } @@ -1928,9 +1815,6 @@ "Defragment CBFS image.\n" " copy -r image,regions -R source-region " "Create a copy (duplicate) cbfs instance in fmap\n" - " create -m ARCH -s size [-b bootblock offset] \\\n" - " [-o CBFS offset] [-H header offset] [-B bootblock] " - "Create a legacy ROM file with CBFS master header*\n" " create -M flashmap [-r list,of,regions,containing,cbfses] " "Create a new-style partitioned firmware image\n" " locate [-r image,regions] -f FILE -n NAME [-P page-size] \\\n" @@ -1951,7 +1835,7 @@ " expand [-r fmap-region] " "Expand CBFS to span entire region\n" "OFFSETs:\n" - " Numbers accompanying -b, -H, and -o switches* may be provided\n" + " Numbers accompanying the -b switche may be provided\n" " in two possible formats: if their value is greater than\n" " 0x80000000, they are interpreted as a top-aligned x86 memory\n" " address; otherwise, they are treated as an offset into flash.\n" @@ -2130,18 +2014,6 @@ return 1; } break; - case 'B': - param.bootblock = optarg; - break; - case 'H': - param.headeroffset_input = strtoll(optarg, &suffix, 0); - if (!*optarg || (suffix && *suffix)) { - ERROR("Invalid header offset '%s'.\n", - optarg); - return 1; - } - param.headeroffset_assigned = 1; - break; case 'a': param.alignment = strtoul(optarg, &suffix, 0); if (!*optarg || (suffix && *suffix)) { @@ -2161,15 +2033,6 @@ case 'Q': param.force_pow2_pagesize = 1; break; - case 'o': - param.cbfsoffset_input = strtoll(optarg, &suffix, 0); - if (!*optarg || (suffix && *suffix)) { - ERROR("Invalid cbfs offset '%s'.\n", - optarg); - return 1; - } - param.cbfsoffset_assigned = 1; - break; case 'f': param.filename = optarg; break; -- To view, visit https://review.coreboot.org/c/coreboot/+/56001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2f34a25dae1ec1924cabe392dcd1480d7b4fbf49 Gerrit-Change-Number: 56001 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: util/cbfstool: Remove unused pagesize parameter
by Arthur Heymans (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: Furquan Shaikh, Martin Roth, Julius Werner, Kyösti Mälkki. Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55581 to look at the new patch set (#5). Change subject: util/cbfstool: Remove unused pagesize parameter ...................................................................... util/cbfstool: Remove unused pagesize parameter Change-Id: Ib672ba8ed418b1a76e4a48951eabda6923358e7a Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M util/cbfstool/cbfstool.c 1 file changed, 5 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/55581/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/55581 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib672ba8ed418b1a76e4a48951eabda6923358e7a Gerrit-Change-Number: 55581 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
by Subrata Banik (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Nick Vaccaro, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 ) Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 ...................................................................... Patch Set 5: (1 comment) File src/soc/intel/common/block/cpu/car/cache_as_ram.S: https://review.coreboot.org/c/coreboot/+/51374/comment/1b0d2499_3ca8a267 PS5, Line 524: (1 << data_ways) > I don't see this calculation anywhere... below I see […] i guess better we accommodate this 1 << data_ways in below code as well? -- To view, visit https://review.coreboot.org/c/coreboot/+/51374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941 Gerrit-Change-Number: 51374 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Thu, 01 Jul 2021 15:46:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
by Tim Wawrzynczak (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: Furquan Shaikh, Subrata Banik, Nick Vaccaro, EricR Lai. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 ) Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 ...................................................................... Patch Set 5: (2 comments) File src/soc/intel/common/block/cpu/car/cache_as_ram.S: https://review.coreboot.org/c/coreboot/+/51374/comment/62e7d6bb_9fe7cbde PS5, Line 520: mov %ecx, %edi suggestion: `/* save number of ways to edi because rdmsr has to use ecx */` https://review.coreboot.org/c/coreboot/+/51374/comment/12fe02ae_4df7eb7a PS5, Line 524: (1 << data_ways) I don't see this calculation anywhere... below I see `SF_MASK_1 = (1 << (IA32_SF_QOS_INFO & 0x3f)) - 1` or `SF_MASK_1 = (1 << SFWayCnt) - 1)` -- To view, visit https://review.coreboot.org/c/coreboot/+/51374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941 Gerrit-Change-Number: 51374 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Thu, 01 Jul 2021 15:38:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya0: Update the FIVR configurations
by Tim Wawrzynczak (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: V Sowmya, Kedar J. Karanje, Maulik V Vaghela, vagdevi.p(a)intel.com, Vinay Kumar. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55704 ) Change subject: mb/google/brya0: Update the FIVR configurations ...................................................................... Patch Set 3: (1 comment) File src/mainboard/google/brya/variants/brya0/overridetree.cb: https://review.coreboot.org/c/coreboot/+/55704/comment/59d7e8f3_92ee6dbc PS3, Line 40: impletemented implemented -- To view, visit https://review.coreboot.org/c/coreboot/+/55704 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1c4fdb38c5c56798935b2c6627a75c3f1ac9fbef Gerrit-Change-Number: 55704 Gerrit-PatchSet: 3 Gerrit-Owner: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Kedar J. Karanje <kedar.j.karanje(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Vinay Kumar <vinay.kumar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: vagdevi.p(a)intel.com Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Kedar J. Karanje <kedar.j.karanje(a)intel.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: vagdevi.p(a)intel.com Gerrit-Attention: Vinay Kumar <vinay.kumar(a)intel.com> Gerrit-Comment-Date: Thu, 01 Jul 2021 15:30:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Correct Bus and Device of Touch Host Controller
by Subrata Banik (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: Varshit B Pandya, Maulik V Vaghela, Rizwan Qureshi, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55998 ) Change subject: soc/intel/alderlake: Correct Bus and Device of Touch Host Controller ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55998 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I41858ea156c8258ea0e7be9e2f67fb0e24144c80 Gerrit-Change-Number: 55998 Gerrit-PatchSet: 2 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 01 Jul 2021 15:29:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Correct Bus and Device of Touch Host Controller
by Tim Wawrzynczak (Code Review) 01 Jul '21

01 Jul '21

Attention is currently required from: Varshit B Pandya, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55998 ) Change subject: soc/intel/alderlake: Correct Bus and Device of Touch Host Controller ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55998 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I41858ea156c8258ea0e7be9e2f67fb0e24144c80 Gerrit-Change-Number: 55998 Gerrit-PatchSet: 2 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 01 Jul 2021 15:27:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool/cbfs-mkstage.c: Change signature of parse_elf_to_xip_stage()
by Arthur Heymans (Code Review) 01 Jul '21

01 Jul '21

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55792 ) Change subject: cbfstool/cbfs-mkstage.c: Change signature of parse_elf_to_xip_stage() ...................................................................... cbfstool/cbfs-mkstage.c: Change signature of parse_elf_to_xip_stage() The dereferced parameter is never updated so passing a copy would work too. Change-Id: Ie36f64f55d4fc7034780116c28aaed65aa304d5e Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55792 Reviewed-by: Julius Werner <jwerner(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/cbfstool/cbfs-mkstage.c M util/cbfstool/cbfstool.c M util/cbfstool/common.h 3 files changed, 4 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved diff --git a/util/cbfstool/cbfs-mkstage.c b/util/cbfstool/cbfs-mkstage.c index 68eb641..7f4a38b 100644 --- a/util/cbfstool/cbfs-mkstage.c +++ b/util/cbfstool/cbfs-mkstage.c @@ -260,7 +260,7 @@ } int parse_elf_to_xip_stage(const struct buffer *input, struct buffer *output, - uint32_t *location, const char *ignore_section, + uint32_t location, const char *ignore_section, struct cbfs_file_attr_stageheader *stageheader) { struct xip_context xipctx; @@ -310,7 +310,7 @@ /* Single loadable segment. The entire segment moves to final * location from based on virtual address of loadable segment. */ - adjustment = *location - pelf->phdr->p_vaddr; + adjustment = location - pelf->phdr->p_vaddr; DEBUG("Relocation adjustment: %08x\n", adjustment); fill_cbfs_stageheader(stageheader, diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 6133536..48c9387 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -1131,7 +1131,7 @@ **/ *offset = convert_addr_space(param.image_region, *offset); - ret = parse_elf_to_xip_stage(buffer, &output, offset, + ret = parse_elf_to_xip_stage(buffer, &output, *offset, param.ignore_section, stageheader); } else { diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h index 479fd713..1c25a66 100644 --- a/util/cbfstool/common.h +++ b/util/cbfstool/common.h @@ -178,7 +178,7 @@ struct cbfs_file_attr_stageheader *stageheader); /* location is TOP aligned. */ int parse_elf_to_xip_stage(const struct buffer *input, struct buffer *output, - uint32_t *location, const char *ignore_section, + uint32_t location, const char *ignore_section, struct cbfs_file_attr_stageheader *stageheader); void print_supported_architectures(void); -- To view, visit https://review.coreboot.org/c/coreboot/+/55792 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie36f64f55d4fc7034780116c28aaed65aa304d5e Gerrit-Change-Number: 55792 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021