From 044f15584125158c867f581004551b7d4e82b9ac Mon Sep 17 00:00:00 2001 From: Jett Rink Date: Tue, 19 Nov 2019 09:50:46 -0700 Subject: usbc: fix storm tracker overflow issue If there is no USB-C interrupt activity for 2^31 microseconds, then there are more than ALERT_STORM_MAX_COUNT events within 2^31 microsecond (instead of ALERT_STORM_INTERVAL), then the interrupt storm would incorrectly detect a storm and disable the port due to incorrect math regarding 32-bit overflow. BRANCH=octopus and all branches with original storm detection (CL:1650484) BUG=b:144369187 TEST=unit test in CL Change-Id: I90b888ac092f81d151538d6018771fb32f8e9c39 Signed-off-by: Jett Rink Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1925668 Commit-Queue: Denis Brockus Reviewed-by: Denis Brockus --- common/usb_pd_protocol.c | 15 ++++--- test/build.mk | 2 + test/test_config.h | 10 +++++ test/usb_pd_int.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++ test/usb_pd_int.mocklist | 8 ++++ test/usb_pd_int.tasklist | 11 +++++ 6 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 test/usb_pd_int.c create mode 100644 test/usb_pd_int.mocklist create mode 100644 test/usb_pd_int.tasklist diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c index 641909cbb9..f85efa6717 100644 --- a/common/usb_pd_protocol.c +++ b/common/usb_pd_protocol.c @@ -2744,7 +2744,7 @@ void pd_interrupt_handler_task(void *p) const int port_mask = (PD_STATUS_TCPC_ALERT_0 << port); struct { int count; - uint32_t time; + timestamp_t time; } storm_tracker[CONFIG_USB_PD_PORT_MAX_COUNT] = {}; ASSERT(port >= 0 && port < CONFIG_USB_PD_PORT_MAX_COUNT); @@ -2768,14 +2768,17 @@ void pd_interrupt_handler_task(void *p) */ while ((tcpc_get_alert_status() & port_mask) && pd_is_port_enabled(port)) { - uint32_t now; + timestamp_t now; tcpc_alert(port); - now = get_time().le.lo; - if (time_after(now, storm_tracker[port].time)) { - storm_tracker[port].time = - now + ALERT_STORM_INTERVAL; + now = get_time(); + if (timestamp_expired( + storm_tracker[port].time, &now)) { + /* Reset timer into future */ + storm_tracker[port].time.val = + now.val + ALERT_STORM_INTERVAL; + /* * Start at 1 since we are processing * an interrupt now diff --git a/test/build.mk b/test/build.mk index 7a0b80f04a..4284374df0 100644 --- a/test/build.mk +++ b/test/build.mk @@ -69,6 +69,7 @@ test-list-host += thermal test-list-host += timer_dos test-list-host += uptime test-list-host += usb_common +test-list-host += usb_pd_int test-list-host += usb_pd test-list-host += usb_pd_giveback test-list-host += usb_pd_rev30 @@ -146,6 +147,7 @@ timer_calib-y=timer_calib.o timer_dos-y=timer_dos.o uptime-y=uptime.o usb_common-y=usb_common_test.o +usb_pd_int-y=usb_pd_int.o usb_pd-y=usb_pd.o usb_pd_giveback-y=usb_pd.o usb_pd_rev30-y=usb_pd.o diff --git a/test/test_config.h b/test/test_config.h index fdea993e65..5c9b1ed3f7 100644 --- a/test/test_config.h +++ b/test/test_config.h @@ -351,6 +351,16 @@ int ncp15wb_calculate_temp(uint16_t adc); #undef CONFIG_USB_PE_SM #endif +#ifdef TEST_USB_PD_INT +#define CONFIG_USB_POWER_DELIVERY +#define CONFIG_USB_PD_DUAL_ROLE +#define CONFIG_USB_PD_PORT_MAX_COUNT 1 +#define CONFIG_USB_PD_TCPC +#define CONFIG_USB_PD_TCPM_STUB +#define CONFIG_SHA256 +#define CONFIG_SW_CRC +#endif + #if defined(TEST_USB_PD) || defined(TEST_USB_PD_GIVEBACK) || \ defined(TEST_USB_PD_REV30) #define CONFIG_USB_POWER_DELIVERY diff --git a/test/usb_pd_int.c b/test/usb_pd_int.c new file mode 100644 index 0000000000..b1701c5f96 --- /dev/null +++ b/test/usb_pd_int.c @@ -0,0 +1,105 @@ +/* Copyright 2019 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + * + * Test USB-PD interrupt task. + */ +#include "task.h" +#include "test_util.h" +#include "mock/tcpc_mock.h" +#include "mock/timer_mock.h" +#include "mock/usb_mux_mock.h" + +#define PORT0 0 + +/* Install Mock TCPC and MUX drivers */ +const struct tcpc_config_t tcpc_config[CONFIG_USB_PD_PORT_MAX_COUNT] = { + { + .drv = &mock_tcpc_driver, + }, +}; + +struct usb_mux usb_muxes[CONFIG_USB_PD_PORT_MAX_COUNT] = { + { + .driver = &mock_usb_mux_driver, + } +}; + +static int deferred_resume_called; +void pd_deferred_resume(int port) +{ + deferred_resume_called = 1; +} + +static int num_events; +uint16_t tcpc_get_alert_status(void) +{ + if (--num_events > 0) + return PD_STATUS_TCPC_ALERT_0; + else + return 0; +} + +test_static int test_storm_not_triggered(void) +{ + num_events = 100; + deferred_resume_called = 0; + schedule_deferred_pd_interrupt(PORT0); + task_wait_event(SECOND); + TEST_EQ(deferred_resume_called, 0, "%d"); + + return EC_SUCCESS; +} + +test_static int test_storm_triggered(void) +{ + num_events = 1000; + deferred_resume_called = 0; + schedule_deferred_pd_interrupt(PORT0); + task_wait_event(SECOND); + TEST_EQ(deferred_resume_called, 1, "%d"); + + return EC_SUCCESS; +} + +test_static int test_storm_not_triggered_for_32bit_overflow(void) +{ + int i; + timestamp_t time; + + /* Ensure the MSB is 1 for overflow comparison tests */ + time.val = 0xff000000; + force_time(time); + + /* + * 100 events every second for 10 seconds should never trigger + * a shutdown call. + */ + for (i = 0; i < 10; ++i) { + num_events = 100; + deferred_resume_called = 0; + schedule_deferred_pd_interrupt(PORT0); + task_wait_event(SECOND); + + TEST_EQ(deferred_resume_called, 0, "%d"); + } + + return EC_SUCCESS; +} + +void before_test(void) +{ + pd_set_suspend(PORT0, 0); +} + +void run_test(void) +{ + /* Let tasks settle down */ + task_wait_event(MINUTE); + + RUN_TEST(test_storm_not_triggered); + RUN_TEST(test_storm_triggered); + RUN_TEST(test_storm_not_triggered_for_32bit_overflow); + + test_print_result(); +} diff --git a/test/usb_pd_int.mocklist b/test/usb_pd_int.mocklist new file mode 100644 index 0000000000..71c2e2cee9 --- /dev/null +++ b/test/usb_pd_int.mocklist @@ -0,0 +1,8 @@ +/* Copyright 2019 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + + #define CONFIG_TEST_MOCK_LIST \ + MOCK(USB_MUX) \ + MOCK(TCPC) diff --git a/test/usb_pd_int.tasklist b/test/usb_pd_int.tasklist new file mode 100644 index 0000000000..3487d55dc7 --- /dev/null +++ b/test/usb_pd_int.tasklist @@ -0,0 +1,11 @@ +/* Copyright 2019 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +/** + * See CONFIG_TASK_LIST in config.h for details. + */ +#define CONFIG_TEST_TASK_LIST \ + TASK_TEST(PD_C0, pd_task, NULL, LARGER_TASK_STACK_SIZE) \ + TASK_TEST(PD_INT_C0, pd_interrupt_handler_task, 0, LARGER_TASK_STACK_SIZE) -- cgit v1.2.1