coreboot-gerrit February 2019

coreboot-gerrit@coreboot.org

1 participants
1249 discussions

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by Julius Werner (Code Review) 11 Feb '19

11 Feb '19

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 ) Change subject: security/vboot: Add measured boot mode ...................................................................... Patch Set 47: (7 comments) https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.h File src/security/vboot/vboot_crtm.h: https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.h@23 PS47, Line 23: #if IS_ENABLED(CONFIG_CHROMEOS) This isn't helping... PCRs 0 and 1 are extended by vboot itself, regardless of whether you're using CONFIG_CHROMEOS. You need to make this branch the default for everyone. (That seems way less confusing than having two possible configs anyway.) https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.h@69 PS47, Line 69: // e.g. CMOS, NVRAM... nit: I'm not sure if we really said we'd allow C99 comments in coreboot now, but even if so probably best to not mix the two within the same file. https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.h@70 PS47, Line 70: #define TPM_WHITELISTED_PCR 23 nit: Is there a good reason these are all so far apart? Why not just use RO = 8, RW = 9, payload = 10, unknown = 11 and whitelisted = 12 or something like that? https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c: https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.c@104 PS47, Line 104: } nit: You essentially have this part three times now, so it probably makes sense to factor it out into a measure_stage(cbfs_path, pcr) function. https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.c@116 PS47, Line 116: if (IS_ENABLED(CONFIG_VBOOT_STARTS_IN_ROMSTAGE)) Rather than doing this, I think it would be cleaner to put a if (!vboot_logic_executed()) return 0; at the top of this file (otherwise, you have the same problem with a separate verstage). https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_crtm.c@142 PS47, Line 142: return -1; Should this print some error as well? https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_logic.c File src/security/vboot/vboot_logic.c: https://review.coreboot.org/#/c/29547/47/src/security/vboot/vboot_logic.c@3… PS47, Line 319: | I assume this is supposed to be an & ? (Also, maybe this would thematically fit better in extend_pcrs()?) -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 47 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 21:28:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[rampayload]: WIP: Port libpayload to riscv
by Philipp Hug (Code Review) 11 Feb '19

11 Feb '19

Philipp Hug has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/28606 ) Change subject: WIP: Port libpayload to riscv ...................................................................... Abandoned Closed in favor of 31356 -- To view, visit https://review.coreboot.org/c/coreboot/+/28606 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: rampayload Gerrit-Change-Id: I91df02069a0f8fd8771f73de0e866e9cea05cded Gerrit-Change-Number: 28606 Gerrit-PatchSet: 7 Gerrit-Owner: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Change in ...coreboot[master]: device/pci_ops: Apply some symmetry in headers
by Kyösti Mälkki (Code Review) 11 Feb '19

11 Feb '19

Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31306 ) Change subject: device/pci_ops: Apply some symmetry in headers ...................................................................... device/pci_ops: Apply some symmetry in headers To make PCI driver side arch-agnostic, function declarations have to be in symmetrical header file locations. From the driver side, the correct file to include is now <device/pci_ops.h> Change-Id: I8076a4867fd7472beaae0a021dcf0d9c7c905871 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/31306 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- M src/arch/x86/include/arch/io.h M src/arch/x86/include/arch/pci_io_cfg.h M src/arch/x86/include/arch/pci_ops.h M src/device/pci_early.c M src/include/device/pci_mmio_cfg.h 5 files changed, 91 insertions(+), 58 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Aaron Durbin: Looks good to me, approved diff --git a/src/arch/x86/include/arch/io.h b/src/arch/x86/include/arch/io.h index a2ba776..20338e0 100644 --- a/src/arch/x86/include/arch/io.h +++ b/src/arch/x86/include/arch/io.h @@ -203,67 +203,14 @@ } #endif +/* FIXME: We should avoid this indirect include. Also this has to + * appear here after all MMIO and IO read/write functions. */ +#include <arch/pci_ops.h> + #ifdef __SIMPLE_DEVICE__ #define PNP_DEV(PORT, FUNC) (((PORT) << 8) | (FUNC)) -#include <arch/pci_io_cfg.h> -#include <device/pci_mmio_cfg.h> - -static __always_inline -uint8_t pci_read_config8(pci_devfn_t dev, unsigned int where) -{ - if (IS_ENABLED(CONFIG_MMCONF_SUPPORT)) - return pci_mmio_read_config8(dev, where); - else - return pci_io_read_config8(dev, where); -} - -static __always_inline -uint16_t pci_read_config16(pci_devfn_t dev, unsigned int where) -{ - if (IS_ENABLED(CONFIG_MMCONF_SUPPORT)) - return pci_mmio_read_config16(dev, where); - else - return pci_io_read_config16(dev, where); -} - -static __always_inline -uint32_t pci_read_config32(pci_devfn_t dev, unsigned int where) -{ - if (IS_ENABLED(CONFIG_MMCONF_SUPPORT)) - return pci_mmio_read_config32(dev, where); - else - return pci_io_read_config32(dev, where); -} - -static __always_inline -void pci_write_config8(pci_devfn_t dev, unsigned int where, uint8_t value) -{ - if (IS_ENABLED(CONFIG_MMCONF_SUPPORT)) - pci_mmio_write_config8(dev, where, value); - else - pci_io_write_config8(dev, where, value); -} - -static __always_inline -void pci_write_config16(pci_devfn_t dev, unsigned int where, uint16_t value) -{ - if (IS_ENABLED(CONFIG_MMCONF_SUPPORT)) - pci_mmio_write_config16(dev, where, value); - else - pci_io_write_config16(dev, where, value); -} - -static __always_inline -void pci_write_config32(pci_devfn_t dev, unsigned int where, uint32_t value) -{ - if (IS_ENABLED(CONFIG_MMCONF_SUPPORT)) - pci_mmio_write_config32(dev, where, value); - else - pci_io_write_config32(dev, where, value); -} - /* Generic functions for pnp devices */ static __always_inline void pnp_write_config( pnp_devfn_t dev, uint8_t reg, uint8_t value) diff --git a/src/arch/x86/include/arch/pci_io_cfg.h b/src/arch/x86/include/arch/pci_io_cfg.h index ddc62ed..d02e640 100644 --- a/src/arch/x86/include/arch/pci_io_cfg.h +++ b/src/arch/x86/include/arch/pci_io_cfg.h @@ -14,7 +14,9 @@ #ifndef _PCI_IO_CFG_H #define _PCI_IO_CFG_H +#include <stdint.h> #include <arch/io.h> +#include <device/pci_type.h> static __always_inline unsigned int pci_io_encode_addr(pci_devfn_t dev, unsigned int where) @@ -75,4 +77,44 @@ outl(value, 0xCFC); } +#if !IS_ENABLED(CONFIG_MMCONF_SUPPORT) +#ifdef __SIMPLE_DEVICE__ +static __always_inline +uint8_t pci_read_config8(pci_devfn_t dev, unsigned int where) +{ + return pci_io_read_config8(dev, where); +} + +static __always_inline +uint16_t pci_read_config16(pci_devfn_t dev, unsigned int where) +{ + return pci_io_read_config16(dev, where); +} + +static __always_inline +uint32_t pci_read_config32(pci_devfn_t dev, unsigned int where) +{ + return pci_io_read_config32(dev, where); +} + +static __always_inline +void pci_write_config8(pci_devfn_t dev, unsigned int where, uint8_t value) +{ + pci_io_write_config8(dev, where, value); +} + +static __always_inline +void pci_write_config16(pci_devfn_t dev, unsigned int where, uint16_t value) +{ + pci_io_write_config16(dev, where, value); +} + +static __always_inline +void pci_write_config32(pci_devfn_t dev, unsigned int where, uint32_t value) +{ + pci_io_write_config32(dev, where, value); +} +#endif /* __SIMPLE_DEVICE__ */ +#endif + #endif /* _PCI_IO_CFG_H */ diff --git a/src/arch/x86/include/arch/pci_ops.h b/src/arch/x86/include/arch/pci_ops.h index 3f1515e..67633f4 100644 --- a/src/arch/x86/include/arch/pci_ops.h +++ b/src/arch/x86/include/arch/pci_ops.h @@ -14,6 +14,9 @@ #ifndef ARCH_I386_PCI_OPS_H #define ARCH_I386_PCI_OPS_H +#include <arch/pci_io_cfg.h> +#include <device/pci_mmio_cfg.h> + #ifndef __SIMPLE_DEVICE__ extern const struct pci_bus_operations pci_cf8_conf1; diff --git a/src/device/pci_early.c b/src/device/pci_early.c index ea2ebd5..5a1fb22 100644 --- a/src/device/pci_early.c +++ b/src/device/pci_early.c @@ -15,7 +15,6 @@ #define __SIMPLE_DEVICE__ -#include <arch/io.h> #include <device/pci.h> #include <device/pci_def.h> #include <device/pci_ops.h> diff --git a/src/include/device/pci_mmio_cfg.h b/src/include/device/pci_mmio_cfg.h index 2e2c19a..0545001 100644 --- a/src/include/device/pci_mmio_cfg.h +++ b/src/include/device/pci_mmio_cfg.h @@ -70,4 +70,46 @@ write32(addr, value); } +#if IS_ENABLED(CONFIG_MMCONF_SUPPORT) + +#ifdef __SIMPLE_DEVICE__ +static __always_inline +uint8_t pci_read_config8(pci_devfn_t dev, unsigned int where) +{ + return pci_mmio_read_config8(dev, where); +} + +static __always_inline +uint16_t pci_read_config16(pci_devfn_t dev, unsigned int where) +{ + return pci_mmio_read_config16(dev, where); +} + +static __always_inline +uint32_t pci_read_config32(pci_devfn_t dev, unsigned int where) +{ + return pci_mmio_read_config32(dev, where); +} + +static __always_inline +void pci_write_config8(pci_devfn_t dev, unsigned int where, uint8_t value) +{ + pci_mmio_write_config8(dev, where, value); +} + +static __always_inline +void pci_write_config16(pci_devfn_t dev, unsigned int where, uint16_t value) +{ + pci_mmio_write_config16(dev, where, value); +} + +static __always_inline +void pci_write_config32(pci_devfn_t dev, unsigned int where, uint32_t value) +{ + pci_mmio_write_config32(dev, where, value); +} +#endif /* __SIMPLE_DEVICE__ */ + +#endif + #endif /* _PCI_MMIO_CFG_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/31306 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8076a4867fd7472beaae0a021dcf0d9c7c905871 Gerrit-Change-Number: 31306 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in ...coreboot[master]: device/pci_ops: Move common pci_mmio_cfg.h
by Kyösti Mälkki (Code Review) 11 Feb '19

11 Feb '19

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31305 Change subject: device/pci_ops: Move common pci_mmio_cfg.h ...................................................................... device/pci_ops: Move common pci_mmio_cfg.h It is expected that method of accessing PCI configuration register space via memory-mapped region is arch-agnostic. Change-Id: Ide6baa00d611953aeb324be0d3561f464395c5eb Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/arch/x86/include/arch/io.h R src/include/device/pci_mmio_cfg.h 2 files changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/31305/1 diff --git a/src/arch/x86/include/arch/io.h b/src/arch/x86/include/arch/io.h index 4b4a178..a2ba776 100644 --- a/src/arch/x86/include/arch/io.h +++ b/src/arch/x86/include/arch/io.h @@ -208,7 +208,7 @@ #define PNP_DEV(PORT, FUNC) (((PORT) << 8) | (FUNC)) #include <arch/pci_io_cfg.h> -#include <arch/pci_mmio_cfg.h> +#include <device/pci_mmio_cfg.h> static __always_inline uint8_t pci_read_config8(pci_devfn_t dev, unsigned int where) diff --git a/src/arch/x86/include/arch/pci_mmio_cfg.h b/src/include/device/pci_mmio_cfg.h similarity index 97% rename from src/arch/x86/include/arch/pci_mmio_cfg.h rename to src/include/device/pci_mmio_cfg.h index c660ed5..2e2c19a 100644 --- a/src/arch/x86/include/arch/pci_mmio_cfg.h +++ b/src/include/device/pci_mmio_cfg.h @@ -16,7 +16,9 @@ #ifndef _PCI_MMIO_CFG_H #define _PCI_MMIO_CFG_H +#include <stdint.h> #include <arch/io.h> +#include <device/pci_type.h> #define DEFAULT_PCIEXBAR CONFIG_MMCONF_BASE_ADDRESS -- To view, visit https://review.coreboot.org/c/coreboot/+/31305 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide6baa00d611953aeb324be0d3561f464395c5eb Gerrit-Change-Number: 31305 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

4 4

Change in ...coreboot[master]: cpu/x86/mtrr: Fix shift into sign-bit in C
by Nico Huber (Code Review) 11 Feb '19

11 Feb '19

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31319 Change subject: cpu/x86/mtrr: Fix shift into sign-bit in C ...................................................................... cpu/x86/mtrr: Fix shift into sign-bit in C In C, the calculations are done using an `int` type. When shifting left in _FROM_4G_TOP(), we shift the result into its sign bit which is undefined behaviour in C. We cannot cast to an unsigned here and neither use an integer suffix if we want to stay most portable and compatible to assemblers. So we have to treat C and assembly inde- pendently. Alternatively, we could let the preprocessor do the job. But only with a lot of `if` and `elif` as we can't let it evaluate arith- metic expressions directly. Change-Id: I67ce5873e1ba35468022ebab1a3a67669d282e1e Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/arch/x86/postcar_loader.c M src/cpu/intel/car/non-evict/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/intel/fsp_model_406dx/bootblock.c M src/cpu/intel/haswell/bootblock.c M src/cpu/intel/haswell/romstage.c M src/cpu/intel/model_2065x/bootblock.c M src/cpu/intel/model_206ax/bootblock.c M src/drivers/intel/fsp1_0/cache_as_ram.inc M src/include/cpu/x86/mtrr.h M src/northbridge/intel/i440bx/ram_calc.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/broadwell/bootblock/cpu.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/fsp_baytrail/bootblock/bootblock.c 15 files changed, 20 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/31319/1 diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index 141e8d2..e53b15b 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -107,7 +107,7 @@ { if (!IS_ENABLED(CONFIG_BOOT_DEVICE_MEMORY_MAPPED)) return; - postcar_frame_add_mtrr(pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, type); + postcar_frame_add_mtrr(pcf, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, type); } void *postcar_commit_mtrrs(struct postcar_frame *pcf) diff --git a/src/cpu/intel/car/non-evict/cache_as_ram.S b/src/cpu/intel/car/non-evict/cache_as_ram.S index 7788a2d..6378f68 100644 --- a/src/cpu/intel/car/non-evict/cache_as_ram.S +++ b/src/cpu/intel/car/non-evict/cache_as_ram.S @@ -118,7 +118,7 @@ /* Enable cache for our code in Flash because we do XIP here */ movl $MTRR_PHYS_BASE(1), %ecx xorl %edx, %edx - movl $(CACHE_ROM_BASE | MTRR_TYPE_WRPROT), %eax + movl $(CACHE_ROM_BASE_ASM | MTRR_TYPE_WRPROT), %eax wrmsr movl $MTRR_PHYS_MASK(1), %ecx diff --git a/src/cpu/intel/car/p4-netburst/cache_as_ram.S b/src/cpu/intel/car/p4-netburst/cache_as_ram.S index 9d50582..7324619 100644 --- a/src/cpu/intel/car/p4-netburst/cache_as_ram.S +++ b/src/cpu/intel/car/p4-netburst/cache_as_ram.S @@ -304,7 +304,7 @@ /* Cache the whole rom to fetch microcode updates */ movl $MTRR_PHYS_BASE(1), %ecx xorl %edx, %edx - movl $(CACHE_ROM_BASE | MTRR_TYPE_WRPROT), %eax + movl $(CACHE_ROM_BASE_ASM | MTRR_TYPE_WRPROT), %eax wrmsr movl $MTRR_PHYS_MASK(1), %ecx diff --git a/src/cpu/intel/fsp_model_406dx/bootblock.c b/src/cpu/intel/fsp_model_406dx/bootblock.c index 13c5d0f..0a388ed 100644 --- a/src/cpu/intel/fsp_model_406dx/bootblock.c +++ b/src/cpu/intel/fsp_model_406dx/bootblock.c @@ -61,7 +61,7 @@ msr_t msr; disable_cache(); - set_var_mtrr(1, CACHE_ROM_BASE, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); + set_var_mtrr(1, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); enable_cache(); /* Enable Variable MTRRs */ diff --git a/src/cpu/intel/haswell/bootblock.c b/src/cpu/intel/haswell/bootblock.c index 3a306b5..48edc21 100644 --- a/src/cpu/intel/haswell/bootblock.c +++ b/src/cpu/intel/haswell/bootblock.c @@ -50,7 +50,7 @@ msr_t msr; disable_cache(); - set_var_mtrr(1, CACHE_ROM_BASE, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); + set_var_mtrr(1, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); enable_cache(); /* Enable Variable MTRRs */ diff --git a/src/cpu/intel/haswell/romstage.c b/src/cpu/intel/haswell/romstage.c index ff729d6..9b9f156 100644 --- a/src/cpu/intel/haswell/romstage.c +++ b/src/cpu/intel/haswell/romstage.c @@ -55,7 +55,7 @@ if (postcar_frame_init(&pcf, ROMSTAGE_RAM_STACK_SIZE)) die("Unable to initialize postcar frame.\n"); /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, + postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ diff --git a/src/cpu/intel/model_2065x/bootblock.c b/src/cpu/intel/model_2065x/bootblock.c index ed528d1..b017c50 100644 --- a/src/cpu/intel/model_2065x/bootblock.c +++ b/src/cpu/intel/model_2065x/bootblock.c @@ -50,7 +50,7 @@ msr_t msr; disable_cache(); - set_var_mtrr(1, CACHE_ROM_BASE, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); + set_var_mtrr(1, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); enable_cache(); /* Enable Variable MTRRs */ diff --git a/src/cpu/intel/model_206ax/bootblock.c b/src/cpu/intel/model_206ax/bootblock.c index 670b097..9cf51be 100644 --- a/src/cpu/intel/model_206ax/bootblock.c +++ b/src/cpu/intel/model_206ax/bootblock.c @@ -52,7 +52,7 @@ msr_t msr; disable_cache(); - set_var_mtrr(1, CACHE_ROM_BASE, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); + set_var_mtrr(1, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); enable_cache(); /* Enable Variable MTRRs */ diff --git a/src/drivers/intel/fsp1_0/cache_as_ram.inc b/src/drivers/intel/fsp1_0/cache_as_ram.inc index 8ac3595..3477374 100644 --- a/src/drivers/intel/fsp1_0/cache_as_ram.inc +++ b/src/drivers/intel/fsp1_0/cache_as_ram.inc @@ -112,8 +112,8 @@ CAR_init_params: .long dummy_microcode .long 0 - .long CACHE_ROM_BASE /* Firmware Location */ - .long CACHE_ROM_SIZE /* Total Firmware Length */ + .long CACHE_ROM_BASE_ASM /* Firmware Location */ + .long CACHE_ROM_SIZE /* Total Firmware Length */ CAR_init_stack: .long CAR_init_done diff --git a/src/include/cpu/x86/mtrr.h b/src/include/cpu/x86/mtrr.h index eb7d78d..f1e4ecf 100644 --- a/src/include/cpu/x86/mtrr.h +++ b/src/include/cpu/x86/mtrr.h @@ -150,7 +150,7 @@ #define _ALIGN_DOWN_POW2(x) ((x) & ~_POW2_MASK(x)) /* Calculate `4GiB - x` (e.g. absolute address for offset from 4GiB) */ -#define _FROM_4G_TOP(x) (((1 << 20) - ((x) >> 12)) << 12) +#define _FROM_4G_TOP(one, x) (((one << 20) - ((x) >> 12)) << 12) /* At the end of romstage, low RAM 0..CACHE_TM_RAMTOP may be set * as write-back cacheable to speed up ramstage decompression. @@ -164,7 +164,7 @@ /* For ROM caching, generally, try to use the next power of 2. */ #define OPTIMAL_CACHE_ROM_SIZE _ALIGN_UP_POW2(CONFIG_ROM_SIZE) -#define OPTIMAL_CACHE_ROM_BASE _FROM_4G_TOP(OPTIMAL_CACHE_ROM_SIZE) +#define OPTIMAL_CACHE_ROM_BASE _FROM_4G_TOP(1, OPTIMAL_CACHE_ROM_SIZE) #if (OPTIMAL_CACHE_ROM_SIZE < CONFIG_ROM_SIZE) || \ (OPTIMAL_CACHE_ROM_SIZE >= (2 * CONFIG_ROM_SIZE)) # error "Optimal CACHE_ROM_SIZE can't be derived, _POW2_MASK needs refinement." @@ -176,7 +176,7 @@ to get a power of 2 again, for a single MTRR. */ #define CAR_END (CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE) #if CAR_END > OPTIMAL_CACHE_ROM_BASE -# define CAR_CACHE_ROM_SIZE _ALIGN_DOWN_POW2(_FROM_4G_TOP(CAR_END)) +# define CAR_CACHE_ROM_SIZE _ALIGN_DOWN_POW2(_FROM_4G_TOP(1, CAR_END)) #else # define CAR_CACHE_ROM_SIZE OPTIMAL_CACHE_ROM_SIZE #endif @@ -192,6 +192,7 @@ # define CACHE_ROM_SIZE CAR_CACHE_ROM_SIZE #endif -#define CACHE_ROM_BASE _FROM_4G_TOP(CACHE_ROM_SIZE) +#define CACHE_ROM_BASE_ASM _FROM_4G_TOP(1, CACHE_ROM_SIZE) +#define CACHE_ROM_BASE_C _FROM_4G_TOP(1u, CACHE_ROM_SIZE) #endif /* CPU_X86_MTRR_H */ diff --git a/src/northbridge/intel/i440bx/ram_calc.c b/src/northbridge/intel/i440bx/ram_calc.c index 3362d93..50e0a0d 100644 --- a/src/northbridge/intel/i440bx/ram_calc.c +++ b/src/northbridge/intel/i440bx/ram_calc.c @@ -81,7 +81,7 @@ die("Unable to initialize postcar frame.\n"); /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, + postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ diff --git a/src/soc/intel/baytrail/romstage/romstage.c b/src/soc/intel/baytrail/romstage/romstage.c index 4b49906..a2bf938 100644 --- a/src/soc/intel/baytrail/romstage/romstage.c +++ b/src/soc/intel/baytrail/romstage/romstage.c @@ -249,7 +249,7 @@ if (postcar_frame_init(&pcf, ROMSTAGE_RAM_STACK_SIZE)) die("Unable to initialize postcar frame.\n"); /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, + postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ diff --git a/src/soc/intel/broadwell/bootblock/cpu.c b/src/soc/intel/broadwell/bootblock/cpu.c index 7508bc2..2d56f32 100644 --- a/src/soc/intel/broadwell/bootblock/cpu.c +++ b/src/soc/intel/broadwell/bootblock/cpu.c @@ -42,7 +42,7 @@ disable_cache(); /* Why only top 4MiB ? */ - set_var_mtrr(1, CACHE_ROM_BASE, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); + set_var_mtrr(1, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); enable_cache(); /* Enable Variable MTRRs */ diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c index b89d948..add0b47 100644 --- a/src/soc/intel/broadwell/romstage/romstage.c +++ b/src/soc/intel/broadwell/romstage/romstage.c @@ -47,7 +47,7 @@ if (postcar_frame_init(&pcf, ROMSTAGE_RAM_STACK_SIZE)) die("Unable to initialize postcar frame.\n"); /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, + postcar_frame_add_mtrr(&pcf, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ diff --git a/src/soc/intel/fsp_baytrail/bootblock/bootblock.c b/src/soc/intel/fsp_baytrail/bootblock/bootblock.c index 8ce0a1d..2b44804 100644 --- a/src/soc/intel/fsp_baytrail/bootblock/bootblock.c +++ b/src/soc/intel/fsp_baytrail/bootblock/bootblock.c @@ -70,7 +70,7 @@ msr_t msr; disable_cache(); - set_var_mtrr(1, CACHE_ROM_BASE, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); + set_var_mtrr(1, CACHE_ROM_BASE_C, CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); enable_cache(); /* Enable Variable MTRRs */ -- To view, visit https://review.coreboot.org/c/coreboot/+/31319 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I67ce5873e1ba35468022ebab1a3a67669d282e1e Gerrit-Change-Number: 31319 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

2 3

Change in ...coreboot[master]: soc/amd/common: Remove redundant ACPI S3 test
by Richard Spiegel (Code Review) 11 Feb '19

11 Feb '19

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27276 ) Change subject: soc/amd/common: Remove redundant ACPI S3 test ...................................................................... Patch Set 4: Code-Review+2 (1 comment) https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig: https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig@a53 PS4, Line 53: > I don't remember the details now.. […] Ok, convinced. -- To view, visit https://review.coreboot.org/c/coreboot/+/27276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c7a9ecbfcb82790e477d906a00f9749103b4045 Gerrit-Change-Number: 27276 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 20:18:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com> Comment-In-Reply-To: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/amd/common: Remove redundant ACPI S3 test
by Kyösti Mälkki (Code Review) 11 Feb '19

11 Feb '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27276 ) Change subject: soc/amd/common: Remove redundant ACPI S3 test ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig: https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig@a53 PS4, Line 53: > I'm OK with this change. No change in compile time (see the makefile). I don't remember the details now.. Was it so that Merlin Falcon (from CarrizoPI) could still do 'fastboot from MRC cache'? The code this guards (s3_resume.c) is not only for ACPI S3 so I expect to see some more changes here, i.e. the HAVE_ACPI_RESUME guard is not quite right. Also, I plan to reuse that s3_resume.c with improved stage_cache and mrc_cache code with agesa/family16 that definetly can do fastboot but is currently not so clean implementation. -- To view, visit https://review.coreboot.org/c/coreboot/+/27276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c7a9ecbfcb82790e477d906a00f9749103b4045 Gerrit-Change-Number: 27276 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 19:26:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com> Comment-In-Reply-To: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: arch/arm64: Make ARM64 specific options depend on select ARCH_ARM64
by Julius Werner (Code Review) 11 Feb '19

11 Feb '19

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31315 ) Change subject: arch/arm64: Make ARM64 specific options depend on select ARCH_ARM64 ...................................................................... Patch Set 2: I'm not really a fan of the extra boilerplate in all SoC Kconfig files. Can you explain why this is necessary? If it's just about removing some unused lines from .config, that doesn't really seem worth it. (I'd be okay with guarding all the really arch-specific options like ARM64_USE_ARCH_TIMER, but I'd prefer the stage architecture selectors like ARCH_ROMSTAGE_ARM64 to stay available without having to add more select lines to SoCs.) -- To view, visit https://review.coreboot.org/c/coreboot/+/31315 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa9cd902281e51f823717f6ea4c72e5736fefb31 Gerrit-Change-Number: 31315 Gerrit-PatchSet: 2 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-Comment-Date: Mon, 11 Feb 2019 19:20:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/amd/common: Remove redundant ACPI S3 test
by Marshall Dawson (Code Review) 11 Feb '19

11 Feb '19

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27276 ) Change subject: soc/amd/common: Remove redundant ACPI S3 test ...................................................................... Patch Set 4: Code-Review+2 (2 comments) https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig: https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig@a53 PS4, Line 53: > Why remove? True, if no S3 enabled than the linker will not add these files. […] I'm OK with this change. No change in compile time (see the makefile). https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/chip.c File src/soc/amd/stoneyridge/chip.c: https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/chip.c@158 PS4, Line 158: acpi_s3_resume_allowed() > Not sure why this more complicated test. If it's on a resume path, then certainly S3 is allowed. Richard, look at acpi_s3_resume_allowed(). This will compile to if (0 && romstage_handoff_is_()) which avoids the 2nd check with all its cbmem stuff when resume is unsupported. -- To view, visit https://review.coreboot.org/c/coreboot/+/27276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c7a9ecbfcb82790e477d906a00f9749103b4045 Gerrit-Change-Number: 27276 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 19:12:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/apl: Implement "Power State after Failure"
by Thomas Heijligen (Code Review) 11 Feb '19

11 Feb '19

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/31352 to look at the new patch set (#2). Change subject: soc/intel/apl: Implement "Power State after Failure" ...................................................................... soc/intel/apl: Implement "Power State after Failure" Change-Id: Ibf218b90088a45349c54f4b881e895bb852e88bb Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/pmc.c M src/soc/intel/common/block/fast_spi/Makefile.inc 3 files changed, 48 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/31352/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31352 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf218b90088a45349c54f4b881e895bb852e88bb Gerrit-Change-Number: 31352 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2019