1
0
mirror of https://review.coreboot.org/flashrom.git synced 2025-04-26 22:52:34 +02:00

internal: Move laptop_ok into board_cfg

Due to how internal is structured around chipset_flash_enable()
entry we need to prepare a crafted programmer_cfg that contains
a board_enable substructure with data derived from the board_enable
subsystem. While this is certainly not perfection, it does make
clear the relationships between board_enable into chipset_flash_enable
and subsequently the overall internal programmer initialisation
in a RAII fashion at the type level over closure upon global
state that is impossible to reason about.

Also flip predicate in report_nonwl_laptop_detected() and
return early with the trivial base-case.

TEST=`$ sudo ./flashrom -p internal --flash-name`.

Change-Id: I459215253845c2af73262943ce91a36464e9eb06
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/73456
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
This commit is contained in:
Edward O'Callaghan 2023-03-06 11:25:52 +11:00 committed by Edward O'Callaghan
parent 3ed016f08f
commit 67b5526d5c
5 changed files with 45 additions and 44 deletions

View File

@ -2293,7 +2293,7 @@ static int p2_not_a_laptop(struct board_cfg *cfg)
static int p2_whitelist_laptop(struct board_cfg *cfg)
{
cfg->is_laptop = 1;
g_laptop_ok = true;
cfg->laptop_ok = true;
msg_pdbg("Whitelisted laptop detected.\n");
return 0;
}

View File

@ -826,7 +826,7 @@ static int enable_flash_ich_spi(const struct programmer_cfg *cfg, struct pci_dev
/* Suppress unknown laptop warning if we booted from SPI. */
if (boot_buses & BUS_SPI)
g_laptop_ok = true;
cfg->bcfg->laptop_ok = true;
return 0;
}
@ -971,7 +971,7 @@ static int enable_flash_pch100_or_c620(const struct programmer_cfg *cfg,
/* Suppress unknown laptop warning if we booted from SPI. */
if (!ret && (boot_buses & BUS_SPI))
g_laptop_ok = true;
cfg->bcfg->laptop_ok = true;
_freepci_ret:
pci_free_dev(spi_dev);
@ -1087,7 +1087,7 @@ static int enable_flash_silvermont(const struct programmer_cfg *cfg, struct pci_
/* Suppress unknown laptop warning if we booted from SPI. */
if (boot_buses & BUS_SPI)
g_laptop_ok = true;
cfg->bcfg->laptop_ok = true;
return 0;
}
@ -1676,7 +1676,7 @@ static int enable_flash_mcp6x_7x(const struct programmer_cfg *cfg, struct pci_de
/* Suppress unknown laptop warning if we booted from SPI. */
if (!ret && want_spi)
g_laptop_ok = true;
cfg->bcfg->laptop_ok = true;
return ret;
}

View File

@ -65,10 +65,6 @@ static struct shutdown_func_data {
*/
static bool may_register_shutdown = false;
struct programmer_cfg {
char *params;
};
/* Register a function to be executed on programmer shutdown.
* The advantage over atexit() is that you can supply a void pointer which will
* be used as parameter to the registered function upon programmer shutdown.

View File

@ -30,7 +30,11 @@ enum programmer_type {
USB,
OTHER,
};
struct programmer_cfg;
struct board_cfg;
struct programmer_cfg {
char *params;
struct board_cfg *bcfg;
};
struct dev_entry {
uint16_t vendor_id;
@ -162,6 +166,7 @@ enum board_match_phase {
struct board_cfg {
int is_laptop;
bool laptop_ok;
};
struct board_match {
@ -267,7 +272,6 @@ extern int superio_count;
#endif
#if CONFIG_INTERNAL == 1
extern bool g_laptop_ok;
extern bool force_boardmismatch;
void probe_superio(void);
int register_superio(struct superio s);

View File

@ -27,8 +27,6 @@
#include "hwaccess_x86_io.h"
#endif
bool g_laptop_ok = false;
bool force_boardmismatch = false;
enum chipbustype internal_buses_supported = BUS_NONE;
@ -107,33 +105,37 @@ static int get_params(const struct programmer_cfg *cfg,
return 0;
}
static void report_nonwl_laptop_detected(int is_laptop, bool laptop_ok)
static void report_nonwl_laptop_detected(const struct board_cfg *bcfg)
{
if (is_laptop && !laptop_ok) {
msg_pinfo("========================================================================\n");
if (is_laptop == 1) {
msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n"
"internal buses have been disabled for safety reasons.\n\n");
} else {
msg_pinfo("You may be running flashrom on an unknown laptop. We could not\n"
"detect this for sure because your vendor has not set up the SMBIOS\n"
"tables correctly. Some internal buses have been disabled for\n"
"safety reasons. You can enforce using all buses by adding\n"
" -p internal:laptop=this_is_not_a_laptop\n"
"to the command line, but please read the following warning if you\n"
"are not sure.\n\n");
}
msg_perr("Laptops, notebooks and netbooks are difficult to support and we\n"
"recommend to use the vendor flashing utility. The embedded controller\n"
"(EC) in these machines often interacts badly with flashing.\n"
"See the manpage and https://flashrom.org/Laptops for details.\n\n"
"If flash is shared with the EC, erase is guaranteed to brick your laptop\n"
"and write may brick your laptop.\n"
"Read and probe may irritate your EC and cause fan failure, backlight\n"
"failure and sudden poweroff.\n"
"You have been warned.\n"
"========================================================================\n");
const int is_laptop = bcfg->is_laptop;
const bool laptop_ok = bcfg->laptop_ok;
if (!is_laptop || laptop_ok)
return;
msg_pinfo("========================================================================\n");
if (is_laptop == 1) {
msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n"
"internal buses have been disabled for safety reasons.\n\n");
} else {
msg_pinfo("You may be running flashrom on an unknown laptop. We could not\n"
"detect this for sure because your vendor has not set up the SMBIOS\n"
"tables correctly. Some internal buses have been disabled for\n"
"safety reasons. You can enforce using all buses by adding\n"
" -p internal:laptop=this_is_not_a_laptop\n"
"to the command line, but please read the following warning if you\n"
"are not sure.\n\n");
}
msg_perr("Laptops, notebooks and netbooks are difficult to support and we\n"
"recommend to use the vendor flashing utility. The embedded controller\n"
"(EC) in these machines often interacts badly with flashing.\n"
"See the manpage and https://flashrom.org/Laptops for details.\n\n"
"If flash is shared with the EC, erase is guaranteed to brick your laptop\n"
"and write may brick your laptop.\n"
"Read and probe may irritate your EC and cause fan failure, backlight\n"
"failure and sudden poweroff.\n"
"You have been warned.\n"
"========================================================================\n");
}
static int internal_init(const struct programmer_cfg *cfg)
@ -157,9 +159,6 @@ static int internal_init(const struct programmer_cfg *cfg)
if (ret)
return ret;
/* Unconditionally reset global state from previous operation. */
g_laptop_ok = false;
/* Default to Parallel/LPC/FWH flash devices. If a known host controller
* is found, the host controller init routine sets the
* internal_buses_supported bitfield.
@ -228,13 +227,15 @@ static int internal_init(const struct programmer_cfg *cfg)
* this isn't a laptop. Board-enables may override this,
* non-legacy buses (SPI and opaque atm) are probed anyway.
*/
if (bcfg.is_laptop && !(g_laptop_ok || force_laptop || (not_a_laptop && bcfg.is_laptop == 2)))
if (bcfg.is_laptop && !(bcfg.laptop_ok || force_laptop || (not_a_laptop && bcfg.is_laptop == 2)))
internal_buses_supported = BUS_NONE;
/* try to enable it. Failure IS an option, since not all motherboards
* really need this to be done, etc., etc.
*/
ret = chipset_flash_enable(cfg);
struct programmer_cfg icfg = *cfg;
icfg.bcfg = &bcfg;
ret = chipset_flash_enable(&icfg);
if (ret == -2) {
msg_perr("WARNING: No chipset found. Flash detection "
"will most likely fail.\n");
@ -258,7 +259,7 @@ static int internal_init(const struct programmer_cfg *cfg)
internal_par_init(internal_buses_supported);
/* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */
report_nonwl_laptop_detected(bcfg.is_laptop, g_laptop_ok);
report_nonwl_laptop_detected(&bcfg);
ret = 0;