summaryrefslogtreecommitdiff
path: root/src/soc/intel/skylake/me.c
AgeCommit message (Collapse)Author
2020-04-06soc/intel/skylake: Use SPDX for GPL-2.0-only filesAngel Pons
Done with sed and God Lines. Only done for C-like code for now. Change-Id: I7354edb15ca9cbe181739bc2a148f16bb85ab118 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/40218 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Singer <felixsinger@posteo.net>
2020-03-18soc: Remove copyright noticesPatrick Georgi
They're listed in AUTHORS and often incorrect anyway, for example: - What's a "Copyright $year-present"? - Which incarnation of Google (Inc, LLC, ...) is the current copyright holder? - People sometimes have their editor auto-add themselves to files even though they only deleted stuff - Or they let the editor automatically update the copyright year, because why not? - Who is the copyright holder "The coreboot project Authors"? - Or "Generated Code"? Sidestep all these issues by simply not putting these notices in individual files, let's list all copyright holders in AUTHORS instead and use the git history to deal with the rest. Change-Id: I4c110f60b764c97fab2a29f6f04680196f156da5 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/39610 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: David Hendricks <david.hendricks@gmail.com>
2020-03-02soc/intel/{common, skl, cnl, apl}: Move print_me_fw_version() to CSE libSridhar Siricilla
Move print_me_fw_version(), remove print_me_version/dump_me_version from cnl/skl/apl and make changes to call print_me_version() which is defined in the CSE lib. TEST=Verified on hatch, soraka and bobba. Change-Id: I7567fac100b14dc207b7fc6060e7a064fb05caf6 Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/39010 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
2020-02-17soc/intel/skl: Rename me_hfs union into me_hfstsSridhar Siricilla
Rename below union tags for consistency: me_hfs2 -> me_hfsts2 me_hfs3 -> me_hfsts3 me_hfs6 -> me_hfsts6 TEST=Verified on Soraka Change-Id: Ibb53e6a5f2b95021f86b3e42e100b711b7d6e64e Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/38797 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-02-09soc/intel/{cnl,icl,skl,tgl,common}: Make changes to ↵Sridhar Siricilla
send_heci_reset_req_message() Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. 5. Rename send_heci_reset_req_message() to cse_request_global_reset(). Test=Verified on hatch board. Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37584 Reviewed-by: Furquan Shaikh <furquan@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-12-19src/soc/intel: Remove unused <stdlib.h>Elyes HAOUAS
Change-Id: I71a5a6c3748d5a3910970bfb1ec3d7ecd3184cfd Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/33686 Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-10-20src: Remove unused 'include <string.h>'Elyes HAOUAS
Change-Id: I2a94c3b6282e9915fd2b8136b124740c8a7b774c Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/36082 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2019-09-25soc/intel/(apl,skl,cnl,common): Move mkhi_hdr structure definition to commonSridhar Siricilla
Removed mkhi_hdr structure definition from multiple SOCs, and moved to common. TEST=Built code for Hatch, apollolake boards. Change-Id: Ifeba0ed4d98975049179d1b47fb22c06a927dc29 Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/35545 Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Subrata Banik <subrata.banik@intel.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-09-16src/soc/intel/{common,cnl,skl,icl}: Move global reset req function to commonSridhar Siricilla
send_heci_reset_req_message() is defined in multiple places, hence move it to common code. TEST=Verified on CMLRVP/Hatch/Soraka/Bobba/Dragon Egg boards. Change-Id: I691fc0610356ef1f64ffa7cc4fe7a39b1344cc16 Signed-off-by: Rizwan Qureshi <rizwan.qureshi@intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/35228 Reviewed-by: V Sowmya <v.sowmya@intel.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-09-12src/soc/intel/common/block/cse: Make hfsts1 common & add helper functionsSridhar Siricilla
Host FW status 1 (FWSTS1/HFSTS1) register definition is common across SoCs, hence move it to common. Also add below helper function, * wait_cse_sec_override_mode() - Polls ME status for "HECI_OP_MODE_SEC_OVERRIDE". It's a special CSE mode, the mode ensures CSE does not trigger any spi cycles to CSE region. * set_host_ready() - Clears reset state from host CSR. TEST=Verified CSE recover mode on CML RVP & Hatch board Change-Id: Id5c12b7abdb27c38af74ea6ee568b42ec74bcb3c Signed-off-by: Rizwan Qureshi <rizwan.qureshi@intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/35226 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aamir Bohra <aamir.bohra@intel.com>
2019-09-11soc/intel/common/block/cse: Move me_read_config32() to common codeSridhar Siricilla
me_read_config32() is defined in multiple places, move it to common location. Also, this function is usually used for reading HFSTS registers, hence move the HFSTS register definitions to common location. Also add a funtion to check if the CSE device has been enabled in the devicetree and it is visible on the bus. This API can be used by the caller to check before initiating any HECI communication. TEST=Verified reading HFSTS registers on CML RVP & Hatch board Change-Id: Icdbfb6b30a007d469b5e018a313c14586addb130 Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Signed-off-by: Rizwan Qureshi <rizwan.qureshi@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/35225 Reviewed-by: Subrata Banik <subrata.banik@intel.com> Reviewed-by: Aamir Bohra <aamir.bohra@intel.com> Reviewed-by: V Sowmya <v.sowmya@intel.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-03-08coreboot: Replace all IS_ENABLED(CONFIG_XXX) with CONFIG(XXX)Julius Werner
This patch is a raw application of find src/ -type f | xargs sed -i -e 's/IS_ENABLED\s*(CONFIG_/CONFIG(/g' Change-Id: I6262d6d5c23cabe23c242b4f38d446b74fe16b88 Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31774 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2019-03-04arch/io.h: Drop unnecessary includeKyösti Mälkki
Change-Id: I91158452680586ac676ea11c8589062880a31f91 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/31692 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-03-01device/pci: Fix PCI accessor headersKyösti Mälkki
PCI config accessors are no longer indirectly included from <arch/io.h> use <device/pci_ops.h> instead. Change-Id: I2adf46430a33bc52ef69d1bf7dca4655fc8475bd Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/31675 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2019-01-09soc/intel: Clean mess around UART_DEBUGNico Huber
Everything is wrong here, the Kconfig symbols are only the tip of the iceberg. Based on Kconfig prompts the SoC code performed pad configu- rations! I don't see why the person who configures coreboot should have the board schematics at hand. As a mitigation, we remove the prompts for UART_DEBUG, which is renamed to INTEL_LPSS_UART_FOR_CONSOLE (because the former didn't really say what it's about), and for UART_FOR_CONSOLE in case the former is selec- ted. Change-Id: Ibe2ed3cab0bb04bb23989c22da45299f088c758b Signed-off-by: Nico Huber <nico.h@gmx.de> Reviewed-on: https://review.coreboot.org/c/29573 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
2018-12-19soc: Remove useless include <device/pci_ids.h>Elyes HAOUAS
Change-Id: Idef8c556ac8c05c5e2047a38629422544392cd62 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/30200 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2018-10-08Move compiler.h to commonlibNico Huber
Its spreading copies got out of sync. And as it is not a standard header but used in commonlib code, it belongs into commonlib. While we are at it, always include it via GCC's `-include` switch. Some Windows and BSD quirk handling went into the util copies. We always guard from redefinitions now to prevent further issues. Change-Id: I850414e6db1d799dce71ff2dc044e6a000ad2552 Signed-off-by: Nico Huber <nico.h@gmx.de> Reviewed-on: https://review.coreboot.org/28927 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2018-07-12soc/intel/skylake: add a space in printing ME FPF statusPratik Prajapati
This is cosmetic change Before: ME: Power Management Event : Clean global reset ME: Progress Phase State : Unknown phase: 0x08 state: 0x10 ME: Power Down Mitigation : NO ME: FPF status : fused After: ME: Power Management Event : Clean Moff->Mx wake ME: Progress Phase State : Unknown phase: 0x08 state: 0x10 ME: Power Down Mitigation : NO ME: FPF status : fused Change-Id: I15c02045d0f94fdb3f4a028585cad488d4ac9aa6 Signed-off-by: Pratik Prajapati <pratikkumar.v.prajapati@intel.com> Reviewed-on: https://review.coreboot.org/27246 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2018-07-09src/soc: Use "foo *bar" instead of "foo* bar"Elyes HAOUAS
Change-Id: I21680354f33916b7b4d913f51a842b5d6c2ecef3 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/27408 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2018-03-30soc/intel/skylake: Protect me_progress_rom_values array boundaryKane Chen
me_progress_rom_values array provides detailed information maps to ME HFSTS2 register value. There is a chance that ME status value might be over the size of me_progress_rom_values. This commit adds a check before access the array. BUG=b:77247550 Change-Id: I5de569c62b94b0595d3d3ea254f50e312e8c11a4 Signed-off-by: Kane Chen <kane.chen@intel.com> Reviewed-on: https://review.coreboot.org/25425 Reviewed-by: Kevin Chiu <Kevin.Chiu@quantatw.com> Reviewed-by: Shelley Chen <shchen@google.com> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Marc Jones <marc@marcjonesconsulting.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-03-27soc/intel/skylake: Do a heci_reset before reading ME firmware versionFurquan Shaikh
This change adds a call to heci_reset before attempting to read ME firmware version. This is important to ensure that both ME and BIOS are in sync. BUG=b:76167737 BRANCH=poppy TEST=Verfied that ME firmware version read does not fail on first boot after power failure (i.e. removing battery and AC power). Change-Id: Ib6b39c398d2e1177b087352a4acb8bcf5a9897d6 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/25362 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Duncan Laurie <dlaurie@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2018-02-28soc/intel/skylake: Add support to print ME versionFurquan Shaikh
This change adds a boot state callback to print ME version after DEV_ENABLE is complete. Information is printed only if UART_DEBUG is enabled because talking to ME to get the firmware version adds ~1 second to boot time. TEST=Verified on Soraka that ME version printed is correct. Change-Id: I360d5d7420950d5aa255df08be6d7123621b87a8 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/23857 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Julien Viard de Galbert <jviarddegalbert@online.net> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-15soc/intel/skylake: Make use of common CSE code for skylakeSubrata Banik
TEST=Ensures global reset could able to reset system. Change-Id: I11ce1812a5a0aa2da6b414555374460d606e220e Signed-off-by: Subrata Banik <subrata.banik@intel.com> Reviewed-on: https://review.coreboot.org/22395 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-10-22skylake/me: Add debug output of HFST registersYouness Alaoui
The ME status is the interpretation of the status registers, but having the actual status registers printed is important and it doesn't hurt to show them. Change-Id: I6ef3401b36fedfa8aed14f4a62bdbec3d8c6d446 Signed-off-by: Youness Alaoui <youness.alaoui@puri.sm> Reviewed-on: https://review.coreboot.org/21960 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2017-07-28soc/intel/skylake: Remove incorrect ME PG-status printBarnali Sarkar
As per discussion with CSME team, ME is NOT using PCI Config Space register HFSTS2 Bit 10 to update ME power-gated status. ME goes to CM0-PG state after ME device becomes idle after Bit 2 of MMIO register offset 0x800 (D0i3 Control - HECI1_D0I3C) is being set. And to retrieve the PG status of ME, one should read from the PWRMBASE+offset 0x590 (which should give the value 0xF9) and PWRMBASE+offset 0x594 (which should give the value 0xFF). But, also it needs some time for the ME FW to go to idle state and reflect these values in PWRMBASE registers after D0i3 bit is being set. This does not happen instantly. So, in coreboot, if we read the ME PG state in finalize.c, which happens just after FSP Notify phase, where actually ME D0i3 bit is set, we do not read the correct PG state values (i.e, 0xF9 and 0xFF). But, once it boots to Kernel, if we read those same registers through iotool mmio_read32 command, we get correct values. So, removing the ME PG state prints from coreboot, since it is actually showing wrong information, although ME Power Gating is successful. Change-Id: Idd31a9803b4c9db7d4bb8bbec5374583a8df0c41 Signed-off-by: Barnali Sarkar <barnali.sarkar@intel.com> Reviewed-on: https://review.coreboot.org/20172 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-07-25soc/intel/skylake: soc/intel/skylake: Initialize struct member to 0Subrata Banik
As per GCC 7.1 compiler struct reset_reply is considered as uninitialized inside send_heci_reset_message function. Change-Id: Ide53a9267dfba1a00263ada1d7016a48ecb9aad8 Signed-off-by: Subrata Banik <subrata.banik@intel.com> Reviewed-on: https://review.coreboot.org/20739 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2017-07-13Rename __attribute__((packed)) --> __packedStefan Reinauer
Also unify __attribute__ ((..)) to __attribute__((..)) and handle ((__packed__)) like ((packed)) Change-Id: Ie60a51c3fa92b5009724a5b7c2932e361bf3490c Signed-off-by: Stefan Reinauer <stefan.reinauer@coreboot.org> Reviewed-on: https://review.coreboot.org/15921 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-05-22soc/intel/skylake: Display FPF status of CSMEPratik Prajapati
Field Programmable Fuses (FPF) status maintained by CSME in bits 30:31 of FWSTS6 for Skylake and Kabylake. FPF committed means CSME has blown the fuses. Change-Id: If63c7874e6c894749df8100426faca0ad432384b Signed-off-by: Pratik Prajapati <pratikkumar.v.prajapati@intel.com> Reviewed-on: https://review.coreboot.org/19747 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com>
2017-03-28soc/pci_devs.h: Use consistent naming in soc/pci_devs.hSubrata Banik
This patch to make common PCI device name between APL and SKL. Change-Id: I5e4c7502e9678c0a367e9c7a96cf848d5b24f68e Signed-off-by: Barnali Sarkar <barnali.sarkar@intel.com> Signed-off-by: Subrata Banik <subrata.banik@intel.com> Reviewed-on: https://review.coreboot.org/18576 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Martin Roth <martinroth@google.com>
2017-03-17soc/intel/skylake: Fix remaining issues detected by checkpatchLee Leahy
Fix the following errors and warnings detected by checkpatch.pl: ERROR: code indent should use tabs where possible ERROR: Macros with complex values should be enclosed in parentheses ERROR: "foo * bar" should be "foo *bar" ERROR: space required before the open parenthesis '(' ERROR: spaces required around that '=' (ctx:VxW) WARNING: space prohibited between function name and open parenthesis '(' WARNING: storage class should be at the beginning of the declaration WARNING: char * array declaration might be better as static const WARNING: please, no space before tabs WARNING: braces {} are not necessary for single statement blocks WARNING: else is not generally useful after a break or return WARNING: static const char * array should probably be static const char * const TEST=Build for glados Change-Id: Ic14ca3abd193cfe257504a55ab6b74782b26bf6d Signed-off-by: Lee Leahy <Leroy.P.Leahy@intel.com> Reviewed-on: https://review.coreboot.org/18868 Tested-by: build bot (Jenkins) Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
2017-03-17soc/intel/skylake: Wrap lines at 80 columnsLee Leahy
Fix the following warning detected by checkpatch: WARNING: line over 80 characters TEST=Build for glados Change-Id: I79341f46ca06ac052f987975ccaf975470d27806 Signed-off-by: Lee Leahy <Leroy.P.Leahy@intel.com> Reviewed-on: https://review.coreboot.org/18867 Tested-by: build bot (Jenkins) Reviewed-by: Lee Leahy <leroy.p.leahy@intel.com>
2016-10-16soc/intel/skylake: Implement Global Reset MEI messageSubrata Banik
As per ME BWG, there are two mechanism to generate a Global Reset (resets both host and Intel ME), one is through CF9h IO write of 6h or Eh with "CF9h Global Reset" (CF9GR) bit set, PMC PCI offset ACh[20]. Another is to issue the Global Reset MEI message. Because any attempts to cause global reset without synchronizing the two sides might cause unwanted side effects, such as unwritten flash data that will get destroyed if the host were to cause a global reset without informing Intel ME firmware, the recommended method is to send a Global Reset MEI message when the following conditions are met: The PCH chipset firmware just needs to complete the Intel ME Interface #1 initialization and check the Intel ME HFSTS state if Intel ME is not in ERROR state and is accepting MEI commands then firmware should be able to use Global Reset MEI message to trigger global reset. Furthermore, if Intel ME is in ERROR state, BIOS can use I/O 0xCF9 write of 0x06 or 0x0E command with PCH ETR3 register bit [20] to perform the global reset. BUG=none BRANCH=none TEST=Verified Global Reset MEI message is able to perform platform global issue in ME good state. Change-Id: If326a137eeadaa695668b76b84c510e12c546024 Signed-off-by: Subrata Banik <subrata.banik@intel.com> Signed-off-by: Rizwan Qureshi <rizwan.qureshi@intel.com> Reviewed-on: https://review.coreboot.org/16902 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2015-07-16soc/intel: Add Skylake SOC supportLee Leahy
Add the files to support the Skylake SOC. Matches chromium tree at 927026db BRANCH=none BUG=None TEST=Build and run on a Skylake platform Change-Id: I80248f7e47eaf13b52e3c7ff951eb1976edbaa15 Signed-off-by: Lee Leahy <leroy.p.leahy@intel.com> Reviewed-on: http://review.coreboot.org/10341 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2015-07-16soc/intel/skylake: Use Broadwell as comparision base for Skylake SOCLee Leahy
Use the Broadwell implementation as the comparison base for Skylake. BRANCH=none BUG=None TEST=None Change-Id: I22eb55ea89eb0d6883f98e4c72a6d243e819e6d8 Signed-off-by: Lee Leahy <leroy.p.leahy@intel.com> Reviewed-on: http://review.coreboot.org/10340 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Georgi <pgeorgi@google.com>