coreboot-gerrit November 2016

coreboot-gerrit@coreboot.org

1 participants
1666 discussions

Patch set updated for coreboot: vboot: No vboot verification when Chrome EC disabled
by Naresh Solanki 07 Nov '16

07 Nov '16

Naresh Solanki (naresh.solanki(a)intel.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17246 -gerrit commit 2b32198badb3bbdb69622bee02420410c2884163 Author: Naresh G Solanki <naresh.solanki(a)intel.com> Date: Sun Nov 6 12:51:14 2016 +0530 vboot: No vboot verification when Chrome EC disabled When Chrome EC is disabled, vboot hash is not available during S3 resume. Hence disable vboot verification when Chrome EC is not available. Change-Id: I665f5f0e2e53da7b735de30443d323572d8a1a9c Signed-off-by: Naresh G Solanki <naresh.solanki(a)intel.com> --- src/vboot/vboot_logic.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/vboot/vboot_logic.c b/src/vboot/vboot_logic.c index bc6ab48..41896e1 100644 --- a/src/vboot/vboot_logic.c +++ b/src/vboot/vboot_logic.c @@ -119,6 +119,13 @@ static int handle_digest_result(void *slot_hash, size_t slot_hash_sz) int is_resume; /* + * Chrome EC is the only support for vboot_save_hash()/vboot_retrieve_hash(), if Chrome EC + * is not enabled then return. + */ + if (!IS_ENABLED(CONFIG_EC_GOOGLE_CHROMEEC)) + return 0; + + /* * Nothing to do since resuming on the platform doesn't require * vboot verification again. */

1 0

Patch set updated for coreboot: [NEEDS TESTING] lenovo/T400: correct GPIO for hybrid driver
by Arthur Heymans 07 Nov '16

07 Nov '16

Arthur Heymans (arthur(a)aheymans.xyz) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16354 -gerrit commit abaa9fc393d6873f8a7b92a542e6677b03fbc4be Author: Arthur Heymans <arthur(a)aheymans.xyz> Date: Tue Aug 30 13:42:33 2016 +0200 [NEEDS TESTING] lenovo/T400: correct GPIO for hybrid driver Currently the hybrid driver uses GPIO 52 to configure mux. This is not the right GPIO on the T400 according to the schematics "MALIBU-3 EXT". It should be GPIO22. The polarities on the gpio's are also set differently on Lenovo T400. On the Lenovo T400 HIGH on GPIO22 means enable. Previously also gpio 49 (GFX_PWR_EN) and 19 (BKLT_CTRL_SEL), were configured for hybrid graphics to work on T400, but this was reverted in 14d1a93e: "Revert mainboard/lenovo/t400: Add initial hybrid graphics support". This patch sets those gpios correctly again for the Lenovo T400. This also adds a IDG disable op, since it is used here to uncleam the VGA from the IGD. Change-Id: I3167303abeb6b3711d53508c61a340d03b1e050a Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- src/drivers/lenovo/Kconfig | 5 +++++ src/drivers/lenovo/hybrid_graphics.c | 19 +++++++++++++++++-- src/mainboard/lenovo/t400/Kconfig | 17 +++++++++++++++++ src/mainboard/lenovo/t400/romstage.c | 2 +- src/northbridge/intel/gm45/gma.c | 13 +++++++++++++ 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/drivers/lenovo/Kconfig b/src/drivers/lenovo/Kconfig index 38b86da..4c62de1 100644 --- a/src/drivers/lenovo/Kconfig +++ b/src/drivers/lenovo/Kconfig @@ -39,3 +39,8 @@ config HYBRID_GRAPHICS_GPIO_NUM help Set a default GPIO that sets the panel LVDS signal routing to integrated or discrete GPU. + +config ENABLE_DISCRETE_IS_HIGH + depends on DRIVERS_LENOVO_HYBRID_GRAPHICS + bool + default n diff --git a/src/drivers/lenovo/hybrid_graphics.c b/src/drivers/lenovo/hybrid_graphics.c index 9b46646..1560eb7 100644 --- a/src/drivers/lenovo/hybrid_graphics.c +++ b/src/drivers/lenovo/hybrid_graphics.c @@ -43,9 +43,18 @@ static void hybrid_graphics_disable_peg(struct device *dev) { struct device *peg_dev; + u8 disable_lvl = CONFIG_ENABLE_DISCRETE_IS_HIGH ? + GPIO_LEVEL_LOW : GPIO_LEVEL_HIGH; /* connect LVDS interface to iGPU */ - set_gpio(CONFIG_HYBRID_GRAPHICS_GPIO_NUM, GPIO_LEVEL_HIGH); + set_gpio(CONFIG_HYBRID_GRAPHICS_GPIO_NUM, disable_lvl); + /* configure BKLT_CTL */ + if (CONFIG_BKLT_CTRL_SEL_GPIO) + set_gpio(CONFIG_BKLT_CTRL_SEL_GPIO, disable_lvl); + /* configure GFX_PWR_EN_GPIO */ + if (CONFIG_GFX_PWR_EN_GPIO) + set_gpio(CONFIG_GFX_PWR_EN_GPIO, disable_lvl); + printk(BIOS_DEBUG, "Hybrid graphics: Switching panel to integrated GPU.\n"); dev->enabled = 0; @@ -62,13 +71,19 @@ static void hybrid_graphics_disable_peg(struct device *dev) static void hybrid_graphics_enable_peg(struct device *dev) { u8 hybrid_graphics_mode; + u8 enable_lvl = CONFIG_ENABLE_DISCRETE_IS_HIGH ? + GPIO_LEVEL_HIGH : GPIO_LEVEL_LOW; hybrid_graphics_mode = HYBRID_GRAPHICS_INTEGRATED; get_option(&hybrid_graphics_mode, "hybrid_graphics_mode"); if (hybrid_graphics_mode == HYBRID_GRAPHICS_DISCRETE) { /* connect LVDS interface to dGPU */ - set_gpio(CONFIG_HYBRID_GRAPHICS_GPIO_NUM, GPIO_LEVEL_LOW); + set_gpio(CONFIG_HYBRID_GRAPHICS_GPIO_NUM, enable_lvl); + if (CONFIG_BKLT_CTRL_SEL_GPIO) + set_gpio(CONFIG_BKLT_CTRL_SEL_GPIO, enable_lvl); + if (CONFIG_GFX_PWR_EN_GPIO) + set_gpio(CONFIG_GFX_PWR_EN_GPIO, enable_lvl); printk(BIOS_DEBUG, "Hybrid graphics: Switching panel to discrete GPU.\n"); dev->enabled = 1; diff --git a/src/mainboard/lenovo/t400/Kconfig b/src/mainboard/lenovo/t400/Kconfig index 194e38c..81b2837 100644 --- a/src/mainboard/lenovo/t400/Kconfig +++ b/src/mainboard/lenovo/t400/Kconfig @@ -61,4 +61,21 @@ config SEABIOS_PS2_TIMEOUT depends on PAYLOAD_SEABIOS default 3000 +config HYBRID_GRAPHICS_GPIO_NUM + int + default 22 + +config BKLT_CTRL_SEL_GPIO + int + default 19 + +config GFX_PWR_EN_GPIO + int + default 49 + +config ENABLE_DISCRETE_IS_HIGH + bool + default y + + endif # BOARD_LENOVO_T400 diff --git a/src/mainboard/lenovo/t400/romstage.c b/src/mainboard/lenovo/t400/romstage.c index bba53d1..7307f8f 100644 --- a/src/mainboard/lenovo/t400/romstage.c +++ b/src/mainboard/lenovo/t400/romstage.c @@ -101,7 +101,7 @@ void mainboard_romstage_entry(unsigned long bist) sysinfo.spd_map[0] = 0x50; sysinfo.spd_map[2] = 0x51; sysinfo.enable_igd = 1; - sysinfo.enable_peg = 0; + sysinfo.enable_peg = 1; get_gmch_info(&sysinfo); raminit(&sysinfo, s3resume); diff --git a/src/northbridge/intel/gm45/gma.c b/src/northbridge/intel/gm45/gma.c index 8938197..a42a7e3 100644 --- a/src/northbridge/intel/gm45/gma.c +++ b/src/northbridge/intel/gm45/gma.c @@ -736,6 +736,18 @@ static struct pci_operations gma_pci_ops = { .set_subsystem = gma_set_subsystem, }; +static void gma_func0_disable(struct device *dev) +{ + u16 reg16; + device_t dev_host = dev_find_slot(0, PCI_DEVFN(0, 0)); + + reg16 = pci_read_config16(dev_host, D0F0_GGC); + reg16 |= (1 << 1); /* disable VGA decode */ + pci_write_config16(dev_host, D0F0_GGC, reg16); + + dev->enabled = 0; +} + static struct device_operations gma_func0_ops = { .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources, @@ -744,6 +756,7 @@ static struct device_operations gma_func0_ops = { .init = gma_func0_init, .scan_bus = 0, .enable = 0, + .disable = gma_func0_disable, .ops_pci = &gma_pci_ops, };

1 0

New patch to review for coreboot: soc/intel/apollolake: Provide chipset value for ifdtool
by Andrey Petrov 07 Nov '16

07 Nov '16

Andrey Petrov (andrey.petrov(a)intel.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17259 -gerrit commit ec194087f669171839cf9ea14981b64233d2f236 Author: Andrey Petrov <andrey.petrov(a)intel.com> Date: Sun Nov 6 23:43:57 2016 -0800 soc/intel/apollolake: Provide chipset value for ifdtool Change-Id: I99454a9fca3b677a389bcaf7693ff4e18415c9ad Signed-off-by: Andrey Petrov <andrey.petrov(a)intel.com> --- src/soc/intel/apollolake/Kconfig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 187214a..62bc761 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -260,4 +260,8 @@ config SMM_RESERVED_SIZE hex default 0x100000 +config IFD_USE_CHIPSET_QUIRK + string "Add chipset-specific quirks in ifdtool" + default "aplk" + endif

1 0

New patch to review for coreboot: southbridge/intel: Pass platform-specific quirks to ifdtool
by Andrey Petrov 07 Nov '16

07 Nov '16

Andrey Petrov (andrey.petrov(a)intel.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17258 -gerrit commit 87305fd038acecda541b52cbac77c523207c25cf Author: Andrey Petrov <andrey.petrov(a)intel.com> Date: Sun Nov 6 23:43:16 2016 -0800 southbridge/intel: Pass platform-specific quirks to ifdtool Since IFD format is floating, ifdtool needs to know what specific chipset it is dealing with. Add -p <chipset> argument to ifdtool invocations. Change-Id: I4fd1783b5d994617912aedcf17adc2a98c97227b Signed-off-by: Andrey Petrov <andrey.petrov(a)intel.com> --- src/southbridge/intel/common/firmware/Makefile.inc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/southbridge/intel/common/firmware/Makefile.inc b/src/southbridge/intel/common/firmware/Makefile.inc index fa28ccf..a3f1571 100644 --- a/src/southbridge/intel/common/firmware/Makefile.inc +++ b/src/southbridge/intel/common/firmware/Makefile.inc @@ -33,6 +33,7 @@ IFD_SECTIONS := $(addprefix -b ,$(CONFIG_IFD_BIOS_SECTION:"%"=%)) \ $(addprefix -p ,$(CONFIG_IFD_PLATFORM_SECTION:"%"=%)) else IFD_BIN_PATH := $(CONFIG_IFD_BIN_PATH) +IFDTOOL_CHIPSET_QUIRKS := -p $(CONFIG_IFD_USE_CHIPSET_QUIRK) endif add_intel_firmware: $(obj)/coreboot.pre $(IFDTOOL) $(IFDFAKE) @@ -57,23 +58,27 @@ endif ifeq ($(CONFIG_HAVE_GBE_BIN),y) printf " IFDTOOL gbe.bin -> coreboot.pre\n" $(objutil)/ifdtool/ifdtool \ + $(IFDTOOL_CHIPSET_QUIRKS) \ -i GbE:$(CONFIG_GBE_BIN_PATH) \ $(obj)/coreboot.pre mv $(obj)/coreboot.pre.new $(obj)/coreboot.pre endif ifeq ($(CONFIG_LOCK_MANAGEMENT_ENGINE),y) printf " IFDTOOL Locking Management Engine\n" - $(objutil)/ifdtool/ifdtool -l $(obj)/coreboot.pre + $(objutil)/ifdtool/ifdtool \ + $(IFDTOOL_CHIPSET_QUIRKS) -l $(obj)/coreboot.pre mv $(obj)/coreboot.pre.new $(obj)/coreboot.pre else ifneq ($(CONFIG_BUILD_WITH_FAKE_IFD),y) printf " IFDTOOL Unlocking Management Engine\n" - $(objutil)/ifdtool/ifdtool -u $(obj)/coreboot.pre + $(objutil)/ifdtool/ifdtool \ + $(IFDTOOL_CHIPSET_QUIRKS) -u $(obj)/coreboot.pre mv $(obj)/coreboot.pre.new $(obj)/coreboot.pre endif ifeq ($(CONFIG_EM100),y) printf " IFDTOOL Setting EM100 mode\n" - $(objutil)/ifdtool/ifdtool --em100 $(obj)/coreboot.pre + $(objutil)/ifdtool/ifdtool \ + $(IFDTOOL_CHIPSET_QUIRKS) --em100 $(obj)/coreboot.pre mv $(obj)/coreboot.pre.new $(obj)/coreboot.pre endif

1 0

Patch set updated for coreboot: ifdtool: Add option to specify platform (-p) quirks
by Andrey Petrov 07 Nov '16

07 Nov '16

Andrey Petrov (andrey.petrov(a)intel.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17202 -gerrit commit 6867ee09272bf8eaa9374ff79e4add19f328eafb Author: Andrey Petrov <andrey.petrov(a)intel.com> Date: Mon Oct 31 19:31:54 2016 -0700 ifdtool: Add option to specify platform (-p) quirks Apollo Lake uses yet another descriptor format where only two masters are used: CPU/BIOS and CSE/TXE. CSE stores data in a region number 5 that has not been used previously and CPU must not write it. Add quirk (-p aplk) that locks descriptor according to recommended values. BUG=chrome-os-partner:58974 TEST=ifdtool -p aplk -l bios.bin; ifdtool -d bios.bin.new. Make sure FLMSTR1 and FLMSTR2 are set correctly. unlock with -l and make sure FLMSTRs are restored. Change-Id: I3f33372bef3ff75d0e34030694c79cd07d5540de Signed-off-by: Andrey Petrov <andrey.petrov(a)intel.com> --- util/ifdtool/ifdtool.c | 51 +++++++++++++++++++++++++++++++++++++------------- util/ifdtool/ifdtool.h | 4 ++++ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c index 74588e1..3d08a2d 100644 --- a/util/ifdtool/ifdtool.c +++ b/util/ifdtool/ifdtool.c @@ -30,6 +30,7 @@ static int ifd_version; static int max_regions = 0; static int selected_chip = 0; +static int platform = -1; static const struct region_name region_names[MAX_REGIONS] = { { "Flash Descriptor", "fd" }, @@ -832,18 +833,31 @@ static void lock_descriptor(char *filename, char *image, int size) fmba->flmstr3 = 0x118; } - /* CPU/BIOS can read descriptor, BIOS, and GbE. */ - fmba->flmstr1 |= 0xb << rd_shift; - /* CPU/BIOS can write BIOS and GbE. */ - fmba->flmstr1 |= 0xa << wr_shift; - /* ME can read descriptor, ME, and GbE. */ - fmba->flmstr2 |= 0xd << rd_shift; - /* ME can write ME and GbE. */ - fmba->flmstr2 |= 0xc << wr_shift; - /* GbE can write only GbE. */ - fmba->flmstr3 |= 0x8 << rd_shift; - /* GbE can read only GbE. */ - fmba->flmstr3 |= 0x8 << wr_shift; + switch (platform) { + case PLATFORM_APOLLOLAKE: + /* CPU/BIOS can read descriptor and BIOS */ + fmba->flmstr1 |= 0x3 << rd_shift; + /* CPU/BIOS can write BIOS */ + fmba->flmstr1 |= 0x2 << wr_shift; + /* TXE can read descriptor, BIOS, TXE and Device Expansion */ + fmba->flmstr2 |= 0x27 << rd_shift; + /* TXE can write TXE and Device Expansion */ + fmba->flmstr2 |= 0x24 << wr_shift; + break; + default: + /* CPU/BIOS can read descriptor, BIOS, and GbE. */ + fmba->flmstr1 |= 0xb << rd_shift; + /* CPU/BIOS can write BIOS and GbE. */ + fmba->flmstr1 |= 0xa << wr_shift; + /* ME can read descriptor, ME, and GbE. */ + fmba->flmstr2 |= 0xd << rd_shift; + /* ME can write ME and GbE. */ + fmba->flmstr2 |= 0xc << wr_shift; + /* GbE can write only GbE. */ + fmba->flmstr3 |= 0x8 << rd_shift; + /* GbE can read only GbE. */ + fmba->flmstr3 |= 0x8 << wr_shift; + } write_image(filename, image, size); } @@ -1140,6 +1154,8 @@ static void print_usage(const char *name) " Dual Output Fast Read Support\n" " -l | --lock Lock firmware descriptor and ME region\n" " -u | --unlock Unlock firmware descriptor and ME region\n" + " -p | --platform Add platform-specific quirks\n" + " aplk - Apollo Lake\n" " -v | --version: print the version\n" " -h | --help: print this help\n\n" "<region> is one of Descriptor, BIOS, ME, GbE, Platform\n" @@ -1171,10 +1187,11 @@ int main(int argc, char *argv[]) {"unlock", 0, NULL, 'u'}, {"version", 0, NULL, 'v'}, {"help", 0, NULL, 'h'}, + {"platform", 0, NULL, 'p'}, {0, 0, 0, 0} }; - while ((opt = getopt_long(argc, argv, "df:D:C:xi:n:s:eluvh?", + while ((opt = getopt_long(argc, argv, "df:D:C:xi:n:s:p:eluvh?", long_options, &option_index)) != EOF) { switch (opt) { case 'd': @@ -1325,6 +1342,14 @@ int main(int argc, char *argv[]) exit(EXIT_FAILURE); } break; + case 'p': + if (!strcmp(optarg, "aplk")) { + platform = PLATFORM_APOLLOLAKE; + } else { + fprintf(stderr, "Unknown platform: %s\n", optarg); + exit(EXIT_FAILURE); + } + break; case 'v': print_version(); exit(EXIT_SUCCESS); diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h index 7878712..fe98d6a 100644 --- a/util/ifdtool/ifdtool.h +++ b/util/ifdtool/ifdtool.h @@ -21,6 +21,10 @@ enum ifd_version { IFD_VERSION_2, }; +enum platform { + PLATFORM_APOLLOLAKE +}; + #define LAYOUT_LINELEN 80 enum spi_frequency {

1 0

Patch set updated for coreboot: riscv: start to use the configstring functions
by Ronald G. Minnich 07 Nov '16

07 Nov '16

Ronald G. Minnich (rminnich(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17256 -gerrit commit cbe64ee963f90e3963654cdefeacdd28f55e695f Author: Ronald G. Minnich <rminnich(a)gmail.com> Date: Sun Nov 6 20:54:20 2016 -0800 riscv: start to use the configstring functions These functions will allow us to remove hardcodes, as long as we can verify the qemu and lowrisc targets implement the configstring correctly. Hence, for the most part, we'll start with mainboard changes first. Change-Id: I7dd3f553d3e61f1c49752fb04402b134fdfdf979 Signed-off-by: Ronald G. Minnich <rminnich(a)gmail.com> --- src/arch/riscv/Makefile.inc | 9 ++++++--- src/arch/riscv/boot.c | 8 ++++---- src/mainboard/emulation/spike-riscv/romstage.c | 7 +++++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/arch/riscv/Makefile.inc b/src/arch/riscv/Makefile.inc index 1fe8f7c..c68fd1e 100644 --- a/src/arch/riscv/Makefile.inc +++ b/src/arch/riscv/Makefile.inc @@ -43,7 +43,8 @@ bootblock-y += \ $(top)/src/lib/memcmp.c \ $(top)/src/lib/memcpy.c \ $(top)/src/lib/memmove.c \ - $(top)/src/lib/memset.c + $(top)/src/lib/memset.c \ + $(top)/src/commonlib/configstring.c $(objcbfs)/bootblock.debug: $$(bootblock-objs) @printf " LINK $(subst $(obj)/,,$(@))\n" @@ -69,7 +70,8 @@ romstage-y += \ $(top)/src/lib/memcmp.c \ $(top)/src/lib/memcpy.c \ $(top)/src/lib/memmove.c \ - $(top)/src/lib/memset.c + $(top)/src/lib/memset.c \ + $(top)/src/commonlib/configstring.c romstage-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c @@ -102,7 +104,8 @@ ramstage-y += \ $(top)/src/lib/memcmp.c \ $(top)/src/lib/memcpy.c \ $(top)/src/lib/memmove.c \ - $(top)/src/lib/memset.c + $(top)/src/lib/memset.c \ + $(top)/src/commonlib/configstring.c $(eval $(call create_class_compiler,rmodules,riscv)) diff --git a/src/arch/riscv/boot.c b/src/arch/riscv/boot.c index 0fa4d5f..9483c48 100644 --- a/src/arch/riscv/boot.c +++ b/src/arch/riscv/boot.c @@ -18,19 +18,19 @@ #include <arch/encoding.h> #include <rules.h> #include <console/console.h> +#include <commonlib/configstring.h> void arch_prog_run(struct prog *prog) { void (*doit)(void *) = prog_entry(prog); void riscvpayload(const char *configstring, void *payload); - uint32_t addr = *(uint32_t *)CONFIG_ARCH_CONFIGSTRING_RISCV; - const char *configstring = (const char *)(uintptr_t)addr; + const char *config = configstring(); if (ENV_RAMSTAGE && prog_type(prog) == PROG_PAYLOAD) { - printk(BIOS_SPEW, "Config string: '%s'\n", configstring); + printk(BIOS_SPEW, "Config string: '%s'\n", config); initVirtualMemory(); printk(BIOS_SPEW, "OK, let's go\n"); - riscvpayload(configstring, doit); + riscvpayload(config, doit); } doit(prog_entry_arg(prog)); diff --git a/src/mainboard/emulation/spike-riscv/romstage.c b/src/mainboard/emulation/spike-riscv/romstage.c index b6314ccd..1317b00 100644 --- a/src/mainboard/emulation/spike-riscv/romstage.c +++ b/src/mainboard/emulation/spike-riscv/romstage.c @@ -15,9 +15,16 @@ #include <console/console.h> #include <program_loading.h> +#include <string.h> +#include <commonlib/configstring.h> void main(void) { + uintptr_t base; + size_t size; + console_init(); + query_mem(configstring(), &base, &size); + printk(BIOS_SPEW, "0x%lx bytes of memory at 0x%llx\n", size, base); run_ramstage(); }

1 0

Patch set updated for coreboot: riscv: map first 2GiB of physical memory at the top of virtual address space
by Ronald G. Minnich 07 Nov '16

07 Nov '16

Ronald G. Minnich (rminnich(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17233 -gerrit commit 051344c46114ce4906edff89e6b5dc6930b91f17 Author: Ronald G. Minnich <rminnich(a)gmail.com> Date: Fri Nov 4 11:27:25 2016 -0700 riscv: map first 2GiB of physical memory at the top of virtual address space We were mapping physical to virtual 1:1, which makes no sense; most kernels want to start at high negative address space. Doing it this way ensures that we're following the intent of the RISCV designers, i.e that virtual addresses are correct at the start; and that the kernel does not have to unmap the low physical addresses once it starts. Change-Id: I8c577ad885891b4c33c20077995abbd90f355f1c Signed-off-by: Ronald G. Minnich <rminnich(a)gmail.com> --- src/arch/riscv/virtual_memory.c | 43 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/src/arch/riscv/virtual_memory.c b/src/arch/riscv/virtual_memory.c index 999d73c..7091f02 100644 --- a/src/arch/riscv/virtual_memory.c +++ b/src/arch/riscv/virtual_memory.c @@ -168,12 +168,40 @@ void init_vm(uintptr_t virtMemStart, uintptr_t physMemStart, pte_t *sbi_pt) // IO space. root_pt[0] = pte_create(0, PTE_W|PTE_R, 0); - root_pt[1] = pte_create(0x40000000>>RISCV_PGSHIFT, + root_pt[1] = pte_create(0x40000000 >> RISCV_PGSHIFT, PTE_W|PTE_R, 0); // Start of RAM - root_pt[2] = pte_create(0x80000000>>RISCV_PGSHIFT, - PTE_W|PTE_R, 0); + // + // A RISCV goal is that Machine or Hypervisor modes be able + // start a Supervisor (kernel) at the C entry point. The + // intent is that firmware creates the virtual memory maps for + // the kernel to use. + // + // Question: what is the virtual address of physical memory + // for a kernel? Hey, glad you asked! In most kernels out + // there, physical memory is mapped at the top of the virtual + // address space. The express intent of the RISCV designers + // (I asked them) is that we be able to enter a kernel at + // main() in C, with no assembly needed. + // + // This is a neat idea that may not quite work out. For + // example: where's the stack go? Once you start in + // Supervisor mode, you are using virtual addresses set up by + // coreboot. So you can't use the coreboot stack, it's mapped + // to low physical addresses. You *should* not use the + // coreboot stack, your kernel may need it in a different + // place. So the kernel needs to change the stack, in ... a + // bit of startup assembly code, what else? Oh well. Map + // physical to high virtual. It's the most common way to + // go. Kernels that want to put kernel virtual addresses based + // at 0 will need to fixup page tables in ... a bit of startup + // assembly code :-) + // + // TODO: use a constant for the number of PTEs per page, when + // (hopefully) newer RISCV toolchains define that constant. + root_pt[0x1ff] = pte_create(0x80000000 >> RISCV_PGSHIFT, + PTE_R | PTE_W | PTE_X, 0); mb(); root_page_table = root_pt; uintptr_t ptbr = ((uintptr_t) root_pt) >> RISCV_PGSHIFT; @@ -198,9 +226,14 @@ void initVirtualMemory(void) { } // TODO: Figure out how to grab this from cbfs + // N.B. We used to map phsyical from 0x81000000, + // but since kernels need to be able to see the page tables + // created by firmware, we're going to map from start of RAM. + // All this is subject to change as we learn more. Much + // about RISCV is still in flux. printk(BIOS_DEBUG, "Initializing virtual memory...\n"); - uintptr_t physicalStart = 0x81000000; - uintptr_t virtualStart = 0xffffffff81000000; + uintptr_t physicalStart = 0x80000000; + uintptr_t virtualStart = 0xffffffff80000000; init_vm(virtualStart, physicalStart, (pte_t *)_pagetables); mb(); flush_tlb();

1 0

Patch set updated for coreboot: riscv: bring in the configstring parsing functions from UCB
by Ronald G. Minnich 07 Nov '16

07 Nov '16

Ronald G. Minnich (rminnich(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17255 -gerrit commit cf30fad5069648e8bb7f7e5b313b98f39c9d5956 Author: Ronald G. Minnich <rminnich(a)gmail.com> Date: Sun Nov 6 20:51:18 2016 -0800 riscv: bring in the configstring parsing functions from UCB I've tested this with commits to come later. This is from the lowrisc bbl distribution, 87588c4, with fixes so it builds. Change-Id: I87893638872259c94d6972e1971578b633155e7e Signed-off-by: Ronald G. Minnich <rminnich(a)gmail.com> --- src/commonlib/configstring.c | 54 +++++++ src/commonlib/include/commonlib/configstring.h | 209 +++++++++++++++++++++++++ 2 files changed, 263 insertions(+) diff --git a/src/commonlib/configstring.c b/src/commonlib/configstring.c new file mode 100644 index 0000000..40beb16 --- /dev/null +++ b/src/commonlib/configstring.c @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2013-2016, The Regents of the University of California + *(Regents). + * All Rights Reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the Regents nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * IN NO EVENT SHALL REGENTS BE LIABLE TO ANY PARTY FOR DIRECT, INDIRECT, + * SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING LOST PROFITS, + * ARISING + * OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN IF REGENTS HAS + * BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * REGENTS SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE. THE SOFTWARE AND ACCOMPANYING DOCUMENTATION, IF ANY, PROVIDED + * HEREUNDER IS PROVIDED "AS IS". REGENTS HAS NO OBLIGATION TO PROVIDE + * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. + */ + +#include <config.h> +#include <assert.h> +#include <string.h> +#include <commonlib/configstring.h> + +void query_mem(const char *config_string, uintptr_t *base, size_t *size) +{ + query_result res = query_config_string(config_string, "ram{0{addr"); + *base = get_uint(res); + res = query_config_string(config_string, "ram{0{size"); + *size = get_uint(res); +} + +/* query_rtc returns the physical address of the rtc. */ +void query_rtc(const char *config_string, uintptr_t *mtime) +{ + query_result res = query_config_string(config_string, "rtc{addr"); + *mtime = (uintptr_t)get_uint(res); +} + +const char *configstring(void) +{ + uint32_t addr = *(uint32_t *)CONFIG_ARCH_CONFIGSTRING_RISCV; + return (const char *)(uintptr_t)addr; +} diff --git a/src/commonlib/include/commonlib/configstring.h b/src/commonlib/include/commonlib/configstring.h new file mode 100644 index 0000000..8aacbda --- /dev/null +++ b/src/commonlib/include/commonlib/configstring.h @@ -0,0 +1,209 @@ +/* + * Copyright (c) 2013-2016, The Regents of the University of California + *(Regents). + * All Rights Reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the Regents nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * IN NO EVENT SHALL REGENTS BE LIABLE TO ANY PARTY FOR DIRECT, INDIRECT, + * SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING LOST PROFITS, + * ARISING + * OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN IF REGENTS HAS + * BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * REGENTS SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE. THE SOFTWARE AND ACCOMPANYING DOCUMENTATION, IF ANY, PROVIDED + * HEREUNDER IS PROVIDED "AS IS". REGENTS HAS NO OBLIGATION TO PROVIDE + * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. + */ + +#ifndef RISCV_CONFIG_STRING_H +#define RISCV_CONFIG_STRING_H + +#include <stddef.h> +#include <stdint.h> + +static const char *skip_whitespace(const char *str) +{ + while (*str && *str <= ' ') + str++; + return str; +} + +static const char *skip_string(const char *str) +{ + while (*str && *str++ != '"') + ; + return str; +} + +static int is_hex(char ch) +{ + return (ch >= '0' && ch <= '9') || (ch >= 'a' && ch <= 'f') || + (ch >= 'A' && ch <= 'F'); +} + +static int parse_hex(char ch) +{ + return (ch >= '0' && ch <= '9') ? ch - '0' : (ch >= 'a' && ch <= 'f') + ? ch - 'a' + 10 + : ch - 'A' + 10; +} + +static const char *skip_key(const char *str) +{ + while (*str >= 35 && *str <= 122 && *str != ';') + str++; + return str; +} + +typedef struct { + const char *start; + const char *end; +} query_result; + +static inline query_result query_config_string(const char *str, const char *k) +{ + size_t ksize = 0; + while (k[ksize] && k[ksize] != '{') + ksize++; + int last = !k[ksize]; + + query_result res = {0, 0}; + while (1) { + const char *key_start = str = skip_whitespace(str); + const char *key_end = str = skip_key(str); + int match = (size_t)(key_end - key_start) == ksize; + if (match) + for (size_t i = 0; i < ksize; i++) + if (key_start[i] != k[i]) + match = 0; + const char *value_start = str = skip_whitespace(str); + while (*str != ';') { + if (!*str) { + return res; + } else if (*str == '"') { + str = skip_string(str + 1); + } else if (*str == '{') { + const char *search_key = + match && !last ? k + ksize + 1 : ""; + query_result inner_res = + query_config_string(str + 1, search_key); + if (inner_res.start) + return inner_res; + str = inner_res.end + 1; + } else { + str = skip_key(str); + } + str = skip_whitespace(str); + } + res.end = str; + if (match && last) { + res.start = value_start; + return res; + } + str = skip_whitespace(str + 1); + if (*str == '}') { + res.end = str; + return res; + } + } +} + +static void parse_string(query_result r, char *buf) +{ + if (r.start < r.end) { + if (*r.start == '"') { + for (const char *p = r.start + 1; + p < r.end && *p != '"'; p++) { + char ch = p[0]; + if (ch == '\\' && p[1] == 'x' && is_hex(p[2])) { + ch = parse_hex(p[2]); + if (is_hex(p[3])) { + ch = + (ch << 4) + parse_hex(p[3]); + p++; + } + p += 2; + } + *buf++ = ch; + } + } else { + for (const char *p = r.start; p < r.end && *p > ' '; + p++) + *buf++ = *p; + } + } + *buf = 0; +} + +static uint64_t __get_uint_hex(const char *s) +{ + uint64_t res = 0; + while (*s) { + if (is_hex(*s)) + res = (res << 4) + parse_hex(*s); + else if (*s != '_') + break; + s++; + } + return res; +} + +static uint64_t __get_uint_dec(const char *s) +{ + uint64_t res = 0; + while (*s) { + if (*s >= '0' && *s <= '9') + res = res * 10 + (*s - '0'); + else + break; + s++; + } + return res; +} + +static uint64_t __get_uint(const char *s) +{ + if (s[0] == '0' && s[1] == 'x') + return __get_uint_hex(s + 2); + return __get_uint_dec(s); +} + +// This is nasty. But the original code used a much nastier thing, +// an array declared as an automatic, which triggered gcc warnings +// as it was sized by the res.end - res.start. We'll need a better +// solution, long term. Malloc is probably not it. +static char name[1024]; +static inline uint64_t get_uint(query_result res) +{ + uint64_t v; + parse_string(res, name); + v = __get_uint(name); + return v; +} + +static inline int64_t get_sint(query_result res) +{ + int64_t v; + parse_string(res, name); + if (name[0] == '-') + return -__get_uint(name + 1); + v = __get_uint(name); + return v; +} + +const char *configstring(void); +void query_mem(const char *config_string, uintptr_t *base, size_t *size); +void query_rtc(const char *config_string, uintptr_t *mtime); +#endif

1 0

Patch set updated for coreboot: riscv: change payload() to pass the config string pointer as arg0
by Ronald G. Minnich 07 Nov '16

07 Nov '16

Ronald G. Minnich (rminnich(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17254 -gerrit commit 7fade81566bc26c920ee8ca86028e5657ddfe9de Author: Ronald G. Minnich <rminnich(a)gmail.com> Date: Sun Nov 6 16:13:52 2016 -0800 riscv: change payload() to pass the config string pointer as arg0 The riscv 1.9 standard defines a textual config string to be passed to kernels and hypervisors. A pointer to this string is available at a platform-dependent location, default 0x100c on all extant platforms. Define a new config variable, ARCH_CONFIGSTRING_RISCV, to contain this location and change payload() arguments so arg0 is this pointer and arg1 is the pointer to the payload. Change-Id: I3be7f1712accf2d726704e4c970f22749d3c3f36 Signed-off-by: Ronald G. Minnich <rminnich(a)gmail.com> --- src/arch/riscv/Kconfig | 5 +++++ src/arch/riscv/boot.c | 7 +++++-- src/arch/riscv/payload.S | 4 +++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/arch/riscv/Kconfig b/src/arch/riscv/Kconfig index a30cb70..7c43da4 100644 --- a/src/arch/riscv/Kconfig +++ b/src/arch/riscv/Kconfig @@ -19,3 +19,8 @@ config ARCH_ROMSTAGE_RISCV config ARCH_RAMSTAGE_RISCV bool default n + +config ARCH_CONFIGSTRING_RISCV + hex "Location of pointer to RISCV config string" + default 0x100c + depends on ARCH_RISCV diff --git a/src/arch/riscv/boot.c b/src/arch/riscv/boot.c index ff1844e..0fa4d5f 100644 --- a/src/arch/riscv/boot.c +++ b/src/arch/riscv/boot.c @@ -22,12 +22,15 @@ void arch_prog_run(struct prog *prog) { void (*doit)(void *) = prog_entry(prog); - void riscvpayload(void *); + void riscvpayload(const char *configstring, void *payload); + uint32_t addr = *(uint32_t *)CONFIG_ARCH_CONFIGSTRING_RISCV; + const char *configstring = (const char *)(uintptr_t)addr; if (ENV_RAMSTAGE && prog_type(prog) == PROG_PAYLOAD) { + printk(BIOS_SPEW, "Config string: '%s'\n", configstring); initVirtualMemory(); printk(BIOS_SPEW, "OK, let's go\n"); - riscvpayload(doit); + riscvpayload(configstring, doit); } doit(prog_entry_arg(prog)); diff --git a/src/arch/riscv/payload.S b/src/arch/riscv/payload.S index 3261a80..e50a589 100644 --- a/src/arch/riscv/payload.S +++ b/src/arch/riscv/payload.S @@ -11,10 +11,12 @@ * GNU General Public License for more details. */ +// "return" to a payload pointed to by a1 with +// a pointer to the config string in a0. .global riscvpayload riscvpayload: /* Jump to a0 in S-mode */ - mv t0,a0 + mv t0,a1 csrw mepc, t0 csrr t0, mstatus li t1, ~(3<<11)

1 0

Patch merged into coreboot/master: riscv: Unify SBI call implementations under arch/riscv/
by gerrit＠coreboot.org 07 Nov '16

07 Nov '16

the following patch was just integrated into master: commit 99f2f113ec397dd042dcaa23c47123f3def19ebc Author: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Date: Fri Oct 28 00:25:02 2016 +0200 riscv: Unify SBI call implementations under arch/riscv/ Note that currently, traps are only handled by the trap handler installed in the bootblock. The romstage and ramstage don't override it. TEST=Booted emulation/spike-qemu and lowrisc/nexys4ddr with a linux payload. It worked as much as before (Linux didn't boot, but it made some successful SBI calls) Change-Id: Icce96ab3f41ae0f34bd86e30f9ff17c30317854e Signed-off-by: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Reviewed-on: https://review.coreboot.org/17057 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> Reviewed-by: Ronald G. Minnich <rminnich(a)gmail.com> See https://review.coreboot.org/17057 for details. -gerrit

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2016