Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47016 )
Change subject: cpu/x86/smm: Add a function for the amount of support CPUs
......................................................................
cpu/x86/smm: Add a function for the amount of support CPUs
CONFIG_MAX_CPUS might be larger than the amount for which an SMI
handler is installed. So when looping over CONFIG_MAX_CPUS to fetch a
SMM save state one might access memory that does not belong to a CPU
node save state and get invalid results.
Change-Id: Ibc17602aa2817d910292c37d0a0095d3d9dc0aae
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/cpu/amd/smm/amd64_save_state.c
M src/cpu/intel/smm/em64t100_save_state.c
M src/cpu/intel/smm/em64t101_save_state.c
M src/cpu/x86/smm/legacy_save_state.c
M src/cpu/x86/smm/save_state.c
M src/cpu/x86/smm/smihandler.c
M src/cpu/x86/smm/smm_module_handler.c
M src/include/cpu/x86/smm.h
8 files changed, 20 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/47016/1
diff --git a/src/cpu/amd/smm/amd64_save_state.c b/src/cpu/amd/smm/amd64_save_state.c
index db329c0..a274fd1 100644
--- a/src/cpu/amd/smm/amd64_save_state.c
+++ b/src/cpu/amd/smm/amd64_save_state.c
@@ -84,7 +84,7 @@
int node;
u8 reg_al;
- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);
smm_io_trap = state->smm_io_trap_offset;
diff --git a/src/cpu/intel/smm/em64t100_save_state.c b/src/cpu/intel/smm/em64t100_save_state.c
index 4fd7d76..fe5f65e 100644
--- a/src/cpu/intel/smm/em64t100_save_state.c
+++ b/src/cpu/intel/smm/em64t100_save_state.c
@@ -72,7 +72,7 @@
em64t100_smm_state_save_area_t *state;
int node;
- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);
/* Check for Synchronous IO (bit0 == 1) */
diff --git a/src/cpu/intel/smm/em64t101_save_state.c b/src/cpu/intel/smm/em64t101_save_state.c
index c6aa397..70112df 100644
--- a/src/cpu/intel/smm/em64t101_save_state.c
+++ b/src/cpu/intel/smm/em64t101_save_state.c
@@ -72,7 +72,7 @@
em64t101_smm_state_save_area_t *state;
int node;
- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);
/* Check for Synchronous IO (bit0 == 1) */
diff --git a/src/cpu/x86/smm/legacy_save_state.c b/src/cpu/x86/smm/legacy_save_state.c
index 8e996ee..0b07155 100644
--- a/src/cpu/x86/smm/legacy_save_state.c
+++ b/src/cpu/x86/smm/legacy_save_state.c
@@ -71,7 +71,7 @@
legacy_smm_state_save_area_t *state;
int node;
- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);
/* Check AL against the requested command */
diff --git a/src/cpu/x86/smm/save_state.c b/src/cpu/x86/smm/save_state.c
index bb08f86..c860b03 100644
--- a/src/cpu/x86/smm/save_state.c
+++ b/src/cpu/x86/smm/save_state.c
@@ -59,7 +59,7 @@
if (init_save_state())
return -1;
- if (node > CONFIG_MAX_CPUS)
+ if (node > smm_max_cpus())
return -1;
return save_state->get_reg(reg, node, out, length);
@@ -70,7 +70,7 @@
if (init_save_state())
return -1;
- if (node > CONFIG_MAX_CPUS)
+ if (node > smm_max_cpus())
return -1;
return save_state->set_reg(reg, node, in, length);
diff --git a/src/cpu/x86/smm/smihandler.c b/src/cpu/x86/smm/smihandler.c
index 0d9131e..21b2b5e 100644
--- a/src/cpu/x86/smm/smihandler.c
+++ b/src/cpu/x86/smm/smihandler.c
@@ -136,6 +136,11 @@
return region_overlap(&r_smm, r);
}
+uint32_t smm_max_cpus(void)
+{
+ return CONFIG_MAX_CPUS;
+}
+
/**
* @brief Interrupt handler for SMI#
*
diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c
index 3ba5684..89864c4 100644
--- a/src/cpu/x86/smm/smm_module_handler.c
+++ b/src/cpu/x86/smm/smm_module_handler.c
@@ -119,6 +119,11 @@
return region_overlap(&r_smm, r) || region_overlap(&r_aseg, r);
}
+uint32_t smm_max_cpus(void)
+{
+ return MIN(smm_runtime->num_cpus, CONFIG_MAX_CPUS);
+}
+
asmlinkage void smm_handler_start(void *arg)
{
const struct smm_module_params *p;
@@ -137,7 +142,7 @@
if (smm_runtime == NULL)
smm_runtime = runtime;
- if (cpu >= CONFIG_MAX_CPUS) {
+ if (cpu >= smm_max_cpus()) {
console_init();
printk(BIOS_CRIT,
"Invalid CPU number assigned in SMM stub: %d\n", cpu);
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index 6cf6f82..8bdd695 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -98,6 +98,9 @@
* account CPUs which are configured to not save their state to RAM. */
void *smm_get_save_state(int cpu);
+/* Return the amount of CPUs the smihandler is set up to handle. */
+uint32_t smm_max_cpus(void);
+
/* Returns true if the region overlaps with the SMM */
bool smm_region_overlaps_handler(const struct region *r);
--
To view, visit https://review.coreboot.org/c/coreboot/+/47016
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibc17602aa2817d910292c37d0a0095d3d9dc0aae
Gerrit-Change-Number: 47016
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange
Evan Green has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47181 )
Change subject: mb/google/dedede/variants/drawcia: Update TSR1 passive temperature
......................................................................
mb/google/dedede/variants/drawcia: Update TSR1 passive temperature
Dogfooders get throttled easily on Meet with a TSR1 temperature of 51.
I've seen the temperature climb as high as ~60, Intel has reported
upwards of 63. Raise the limit for now until the final value is worked
out.
BUG=b:170229672
BRANCH=None
TEST=Test on dedede system with Meet call, observe no 800MHz throttling.
Signed-off-by: Evan Green <evgreen(a)chromium.org>
Change-Id: I914e3e6a3941267bb7cc99531eb71fa48a578300
---
M src/mainboard/google/dedede/variants/drawcia/overridetree.cb
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/47181/1
diff --git a/src/mainboard/google/dedede/variants/drawcia/overridetree.cb b/src/mainboard/google/dedede/variants/drawcia/overridetree.cb
index 5942121..248efa4 100644
--- a/src/mainboard/google/dedede/variants/drawcia/overridetree.cb
+++ b/src/mainboard/google/dedede/variants/drawcia/overridetree.cb
@@ -92,7 +92,7 @@
register "policies.passive" = "{
[0] = DPTF_PASSIVE(CPU, CPU, 80, 1000),
[1] = DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 70, 4000),
- [2] = DPTF_PASSIVE(CPU, TEMP_SENSOR_1, 51, 1000),
+ [2] = DPTF_PASSIVE(CPU, TEMP_SENSOR_1, 65, 1000),
[3] = DPTF_PASSIVE(CHARGER, TEMP_SENSOR_2, 75, 5000),
[4] = DPTF_PASSIVE(CPU, TEMP_SENSOR_3, 60, 1000)
}"
--
To view, visit https://review.coreboot.org/c/coreboot/+/47181
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I914e3e6a3941267bb7cc99531eb71fa48a578300
Gerrit-Change-Number: 47181
Gerrit-PatchSet: 1
Gerrit-Owner: Evan Green <evgreen(a)chromium.org>
Gerrit-MessageType: newchange
Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46730 )
Change subject: [RFC] libpayload/keyboard: Dynamically toggle controller translation
......................................................................
[RFC] libpayload/keyboard: Dynamically toggle controller translation
This is what would be necessary to detect cases where we have to
toggle the keyboard controller's scancode translation.
After setting the matching scancode set for the current translation
setting, we read it back and try to adapt the translation setting
of necessary.
It adds a lot complexity, so we'd probably want to avoid it? Chances
that we find a keyboard that can't switch scancode sets and isn't
integrated (where we could set the correct translation setting in
coreboot) seem rather low.
Change-Id: I938184875e1c4c40cdb2762e1c5b6ed9c8f82b90
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M payloads/libpayload/drivers/i8042/i8042.c
M payloads/libpayload/drivers/i8042/i8042.h
M payloads/libpayload/drivers/i8042/keyboard.c
3 files changed, 102 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/46730/1
diff --git a/payloads/libpayload/drivers/i8042/i8042.c b/payloads/libpayload/drivers/i8042/i8042.c
index 317d04b..487863e 100644
--- a/payloads/libpayload/drivers/i8042/i8042.c
+++ b/payloads/libpayload/drivers/i8042/i8042.c
@@ -317,6 +317,24 @@
}
/**
+ * Send command & data to keyboard controller.
+ *
+ * @param cmd: The command to be send.
+ * @param data: The data to be send.
+ * Returns 0 on success, -1 on failure.
+ */
+int i8042_cmd_with_data(const u8 cmd, const u8 data)
+{
+ const int ret = i8042_cmd(cmd);
+ if (ret != 0)
+ return ret;
+
+ i8042_write_data(data);
+
+ return ret;
+}
+
+/**
* Probe for keyboard controller data and queue it.
*/
static void i8042_data_poll(void)
diff --git a/payloads/libpayload/drivers/i8042/i8042.h b/payloads/libpayload/drivers/i8042/i8042.h
index 68fd149..f2dc0d0 100644
--- a/payloads/libpayload/drivers/i8042/i8042.h
+++ b/payloads/libpayload/drivers/i8042/i8042.h
@@ -29,6 +29,8 @@
#ifndef __DRIVERS_I8042_I8042_H__
#define __DRIVERS_I8042_I8042_H__
+#include <stdint.h>
+
/* Port 0x64 commands */
#define I8042_CMD_RD_CMD_BYTE 0x20
#define I8042_CMD_WR_CMD_BYTE 0x60
@@ -67,5 +69,6 @@
#define I8042_KBCMD_RESET 0xff
int i8042_cmd_with_response(u8 cmd);
+int i8042_cmd_with_data(u8 cmd, u8 data);
#endif /* __DRIVERS_I8042_I8042_H__ */
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c
index aa21b9d..c3cba9b 100644
--- a/payloads/libpayload/drivers/i8042/keyboard.c
+++ b/payloads/libpayload/drivers/i8042/keyboard.c
@@ -27,6 +27,7 @@
* SUCH DAMAGE.
*/
+#include <assert.h>
#include <stdbool.h>
#include <keycodes.h>
@@ -326,27 +327,90 @@
return ctrl_cfg < 0 || (ctrl_cfg & I8042_CMD_BYTE_XLATE);
}
-/* Set scancode set 1 */
-static bool set_scancode_set(const unsigned char set)
+static bool send_scancode_set(const unsigned char set)
{
- bool ret;
+ assert(set <= 3);
- if (set < 1 || set > 3)
+ if (!keyboard_cmd(I8042_KBCMD_SET_SCANCODE) ||
+ !keyboard_cmd(set)) {
+ printf("ERROR: Keyboard send scancode set#%u failed!\n", set);
+ return false;
+ }
+ return true;
+}
+
+static unsigned char select_scancode_set(const bool xlate)
+{
+ /*
+ * We only support scancode set 1. The controller translation
+ * would translate from set 2 to 1 for us, so we try to configure:
+ *
+ * o set 1 if the controller doesn't translate, and
+ * o set 2 if the controller does.
+ */
+ const unsigned char set = xlate ? 2 : 1;
+
+ if (!send_scancode_set(set))
+ return 2;
+
+ /*
+ * Now try to confirm the current scancode set selection.
+ */
+ if (!send_scancode_set(0)) /* retrieve current scancode set # */
+ return 2;
+
+ int actual_set = i8042_wait_read_ps2();
+ if (xlate) {
+ /* controllers translate even this answer */
+ switch (actual_set) {
+ case 0x43: actual_set = 1; break;
+ case 0x41: actual_set = 2; break;
+ case 0x3f: actual_set = 3; break;
+ }
+ }
+
+ if (actual_set < 1) /* assume success on unexpected data */
+ return set;
+
+ return actual_set;
+}
+
+static bool prepare_scancode_set(void)
+{
+ const bool xlate = controller_translates();
+
+ const unsigned char set = select_scancode_set(xlate);
+
+ if (set > 2) /* We can't do anything with scancode set #3. */
return false;
- ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE);
- if (!ret) {
- printf("ERROR: Keyboard set scancode failed!\n");
- return ret;
+ if ((set != 2 && !xlate) || (set == 2 && xlate))
+ return true;
+
+ /*
+ * In case we failed to set the correct scancode set, fall back
+ * and try to change the controller's translation setting.
+ */
+ printf("WARNING: Keyboard failed to select correct scancode set.\n");
+
+ int ctrl_cfg = i8042_cmd_with_response(I8042_CMD_RD_CMD_BYTE);
+ if (ctrl_cfg < 0) {
+ printf("ERROR: i8042_cmd RD_CMD failed!\n");
+ return false;
}
- ret = keyboard_cmd(set);
- if (!ret) {
- printf("ERROR: Keyboard scancode set#%u failed!\n", set);
- return ret;
+ if (xlate)
+ ctrl_cfg &= ~I8042_CMD_BYTE_XLATE;
+ else
+ ctrl_cfg |= I8042_CMD_BYTE_XLATE;
+
+ const int ret = i8042_cmd_with_data(I8042_CMD_WR_CMD_BYTE, ctrl_cfg);
+ if (ret < 0) {
+ printf("ERROR: i8042_cmd WR_CMD failed!\n");
+ return false;
}
- return ret;
+ return true;
}
void keyboard_init(void)
@@ -364,14 +428,10 @@
/* Enable first PS/2 port */
i8042_cmd(I8042_CMD_EN_KB);
- /*
- * We only support scancode set 1. The controller translation
- * would translate from set 2 to 1 for us, so we try to configure
- *
- * o set 1 if the controller doesn't translate, and
- * o set 2 if the controller does.
- */
- (void)set_scancode_set(controller_translates() ? 2 : 1);
+ if (!prepare_scancode_set()) {
+ printf("ERROR: Keyboard failed to configure scancode set, giving up.\n");
+ return;
+ }
/* Enable scanning */
const bool ret = keyboard_cmd(I8042_KBCMD_EN);
--
To view, visit https://review.coreboot.org/c/coreboot/+/46730
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I938184875e1c4c40cdb2762e1c5b6ed9c8f82b90
Gerrit-Change-Number: 46730
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: newchange
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47238 )
Change subject: device/pciexp: Allow ASPM on bridge devices
......................................................................
device/pciexp: Allow ASPM on bridge devices
The device acceptable latency field is only valid for 'endpoints', but not
for bridge devices. Set the maximum acceptable latency on such devices to
allow ASPM being enabled if supported on both sides.
Allows the PCIe link on bridge devices to go into L0s/L1.
This allows the package to enter a deeper sleep state when all links are idle.
WARNING: This might cause issues on PCIe bridge devices that don't properly
support ASPM. In addition it might decrease performance.
Change-Id: I277efe0bd1448ee8bff633428aa729aeedf04e28
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
M src/device/pciexp_device.c
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/47238/1
diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c
index 8d4bb98..b13318a 100644
--- a/src/device/pciexp_device.c
+++ b/src/device/pciexp_device.c
@@ -368,8 +368,20 @@
if (endp->disable_pcie_aspm)
return;
- /* Get endpoint device capabilities for acceptable limits */
- devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);
+ const uint16_t xcap = pci_read_config16(endp, endp_cap + PCI_EXP_FLAGS);
+ const uint8_t type = (xcap & PCI_EXP_FLAGS_TYPE) >> 4;
+
+ /*
+ * PCI_EXP_DEVCAP_L0S and PCI_EXP_DEVCAP_L1 are only valid for PCIe endpoints.
+ * Refer to "PCI Express Base Specification Revision 2.0" Chapter 7.8.3
+ */
+ if (type != PCI_EXP_TYPE_ENDPOINT && type != PCI_EXP_TYPE_LEG_END) {
+ /* Set no limit in acceptable latency */
+ devcap = (0x7 << 6) | (0x7 << 9);
+ } else {
+ /* Get endpoint device capabilities for acceptable limits */
+ devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);
+ }
/* Enable L0s if it is within endpoint acceptable limit */
ok_latency = (devcap & PCI_EXP_DEVCAP_L0S) >> 6;
--
To view, visit https://review.coreboot.org/c/coreboot/+/47238
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I277efe0bd1448ee8bff633428aa729aeedf04e28
Gerrit-Change-Number: 47238
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-MessageType: newchange
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47451 )
Change subject: sb/intel/common: Move xHCI SMI callback after mainboard
......................................................................
sb/intel/common: Move xHCI SMI callback after mainboard
The only platform with xHCI is bd82x6x, and the callback merely toggles
some bits and accounts for workarounds (although the code is wrong). In
preparation for having Lynxpoint use the common SMI handler code, place
the xHCI callback after the mainboard's, which seems to be safe to do.
Change-Id: I85305ef588747bddfd414c4344c207a00a5aa8d0
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/southbridge/intel/common/smihandler.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/47451/1
diff --git a/src/southbridge/intel/common/smihandler.c b/src/southbridge/intel/common/smihandler.c
index 7610aa1..5312285 100644
--- a/src/southbridge/intel/common/smihandler.c
+++ b/src/southbridge/intel/common/smihandler.c
@@ -118,11 +118,11 @@
printk(BIOS_SPEW, "SMI#: SLP = 0x%08x\n", reg32);
slp_typ = acpi_sleep_from_pm1(reg32);
- southbridge_smm_xhci_sleep(slp_typ);
-
/* Do any mainboard sleep handling */
mainboard_smi_sleep(slp_typ);
+ southbridge_smm_xhci_sleep(slp_typ);
+
/* Log S3, S4, and S5 entry */
if (slp_typ >= ACPI_S3)
elog_gsmi_add_event_byte(ELOG_TYPE_ACPI_ENTER, slp_typ);
--
To view, visit https://review.coreboot.org/c/coreboot/+/47451
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I85305ef588747bddfd414c4344c207a00a5aa8d0
Gerrit-Change-Number: 47451
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newchange