CBI: Report CBI in user feedback |
|||||||||||||||||||
Issue descriptionCBI stands for Cros Board Info. We should include it in feedback reports for debugging.
,
Jun 4 2018
[0] As integer: 3 (0x3) As binary: 03 02 [1] As integer: 1 (0x1) As binary: 01 [2] As integer: 2 (0x2) As binary: 02
,
Jun 4 2018
and those represent ... ? every piece of info in feedback reports needs clear documentation as to what it is and its purpose and PII impact. this doesn't look like PII, but still should be explained.
,
Jun 4 2018
Here are currently defined CBI types: [0] Board Version - Monotonically increasing integer assigned to each hardware build. [1] OEM ID - Integer assigned to OEMs. Used to distinguish each variant. [2] SKU ID - Integer assigned to SKUs. Used to distinguish hardware features (CPU, RAM size, stylus, etc.)
,
Jun 4 2018
sounds fine. can you link to source repos/files too ?
,
Jun 4 2018
Nami: https://chrome-internal.googlesource.com/chromeos/overlays/overlay-nami-private/+/master/chromeos-base/chromeos-firmware-nami/chromeos-firmware-nami-9999.ebuild Fizz: https://chrome-internal.googlesource.com/chromeos/overlays/overlay-fizz-private/+/master/chromeos-base/chromeos-firmware-fizz/chromeos-firmware-fizz-9999.ebuild Script to retrieve CBI: https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/userfeedback/scripts/cbi_info ectool, EC firmware, cbi-util: https://chromium.googlesource.com/chromiumos/platform/ec/+/master CBI Design doc: https://docs.google.com/document/d/1SCfav8coDd1HQA5q0w_1gpDrq-uqtlHDzggIqHHndPg/
,
Jun 5 2018
Is this something we can back-port to M67 and M68?
,
Jun 5 2018
In general new features don't get picked back to release branches, only bug fixes do. For 67 we are almost certainly too late as it is nearly stable, for 68 we are on the verge of beta and as such I would prefer not to merge a bunch of new stuff unless we really need it. What issue are we trying to solve in 68 that would require these patches?
,
Jun 5 2018
CBI is critical on Fizz as it's used to select the max current and voltage. Having it in user feedbacks would certainly improve our diagnosing capability for issues like https://b.corp.google.com/issues/80482240.
,
Jun 6 2018
What specific CLs need picked to make this possible in 68?
,
Jun 6 2018
https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1085548. This CL has been mysteriously merged without Gerrit knowing it. https://bugs.chromium.org/p/gerrit/issues/detail?id=9176
,
Jun 6 2018
Yea that review is borked. If this has been verified, we don't think it is high risk, and it is just adding that script file we can do it, consider it merge approved for 68. For 67 we are already stable so I don't think we want to merge it there.
,
Jun 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/018a78598cdbcb919317ec8b02f716cec336c6b6 commit 018a78598cdbcb919317ec8b02f716cec336c6b6 Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Wed Jun 06 19:11:33 2018 userfeedback: Report CBI in user feedback CBI stands for Cros Board Info. This patch makes user feedback reports contain CBI. BUG=chromium:849399 TEST=Run cbi_info on Fizz and verify the ouput is expected. Change-Id: I7b8cbf05592d5b235d055b1f950e4db5a73678b6 Reviewed-on: https://chromium-review.googlesource.com/1089457 Commit-Queue: Daisuke Nojiri <dnojiri@chromium.org> Tested-by: Daisuke Nojiri <dnojiri@chromium.org> Reviewed-by: Bernie Thompson <bhthompson@chromium.org> [modify] https://crrev.com/018a78598cdbcb919317ec8b02f716cec336c6b6/debugd/src/log_tool.cc [add] https://crrev.com/018a78598cdbcb919317ec8b02f716cec336c6b6/userfeedback/scripts/cbi_info
,
Jun 6 2018
,
Jun 11 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 15 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 19 2018
The script fails due to the errors shown below. The script just runs 'ectool cbi get x' (x=0, 1, 2). cbi_info=<multiline> ---------- START ---------- Cannot stat /run/lock. Trying fallback directory: /tmp Error getting I/O privilege: Operation not permitted Cannot find I2C adapter Unable to establish host communication Couldn't find EC /usr/share/userfeedback/scripts/cbi_info: ERROR: Failed to read type 0 Cannot stat /run/lock. Trying fallback directory: /tmp Error getting I/O privilege: Operation not permitted Cannot find I2C adapter Unable to establish host communication Couldn't find EC /usr/share/userfeedback/scripts/cbi_info: ERROR: Failed to read type 1 Cannot stat /run/lock. Trying fallback directory: /tmp Error getting I/O privilege: Operation not permitted Cannot find I2C adapter Unable to establish host communication Couldn't find EC /usr/share/userfeedback/scripts/cbi_info: ERROR: Failed to read type 2 [0] [1] [2]
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/3d832eea6f82094feb86958f35cd967beaed49c4 commit 3d832eea6f82094feb86958f35cd967beaed49c4 Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Fri Jun 22 02:56:29 2018 userfeedback: Run cbi_info script as root Since ectool requires root to access /run/lock, this change makes cbi_info to run as root. Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org> BUG=chromium:849399 TEST=cros_workon_make --board=fizz debugd --test Change-Id: I61aaee0a9f69e7d57fd62320d575f9852efed06d Reviewed-on: https://chromium-review.googlesource.com/1106710 Commit-Ready: Daisuke Nojiri <dnojiri@chromium.org> Tested-by: Daisuke Nojiri <dnojiri@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/3d832eea6f82094feb86958f35cd967beaed49c4/debugd/src/log_tool.cc
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/3b43d6942e6bd6ead69c0165cc6f09d94c80f866 commit 3b43d6942e6bd6ead69c0165cc6f09d94c80f866 Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Tue Aug 07 17:30:14 2018 userfeedback: Run cbi_info script as root Since ectool requires root to access /run/lock, this change makes cbi_info to run as root. Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org> BUG=chromium:849399 TEST=cros_workon_make --board=fizz debugd --test Change-Id: I61aaee0a9f69e7d57fd62320d575f9852efed06d Reviewed-on: https://chromium-review.googlesource.com/1108699 Reviewed-by: Bernie Thompson <bhthompson@chromium.org> Commit-Queue: Daisuke Nojiri <dnojiri@chromium.org> Tested-by: Daisuke Nojiri <dnojiri@chromium.org> [modify] https://crrev.com/3b43d6942e6bd6ead69c0165cc6f09d94c80f866/debugd/src/log_tool.cc
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/f8b85a76e6971951b54c0b25a1e10708274541b8 commit f8b85a76e6971951b54c0b25a1e10708274541b8 Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Tue Aug 07 17:59:43 2018 userfeedback: Run cbi_info script as root Since ectool requires root to access /run/lock, this change makes cbi_info to run as root. Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org> BUG=chromium:849399 TEST=cros_workon_make --board=fizz debugd --test Change-Id: I61aaee0a9f69e7d57fd62320d575f9852efed06d Reviewed-on: https://chromium-review.googlesource.com/1108699 Reviewed-by: Bernie Thompson <bhthompson@chromium.org> Commit-Queue: Daisuke Nojiri <dnojiri@chromium.org> Tested-by: Daisuke Nojiri <dnojiri@chromium.org> (cherry picked from commit 3b43d6942e6bd6ead69c0165cc6f09d94c80f866) Reviewed-on: https://chromium-review.googlesource.com/1165685 Reviewed-by: YH Lin <yueherngl@chromium.org> Commit-Queue: YH Lin <yueherngl@chromium.org> Tested-by: YH Lin <yueherngl@chromium.org> [modify] https://crrev.com/f8b85a76e6971951b54c0b25a1e10708274541b8/debugd/src/log_tool.cc
,
Aug 20
I still see: cbi_info=<multiline> ---------- START ---------- Cannot stat /run/lock. Trying fallback directory: /tmp Error getting I/O privilege: Operation not permitted Cannot find I2C adapter Unable to establish host communication Couldn't find EC /usr/share/userfeedback/scripts/cbi_info: ERROR: Failed to read type 0 Cannot stat /run/lock. Trying fallback directory: /tmp Error getting I/O privilege: Operation not permitted Cannot find I2C adapter Unable to establish host communication Couldn't find EC /usr/share/userfeedback/scripts/cbi_info: ERROR: Failed to read type 1 Cannot stat /run/lock. Trying fallback directory: /tmp Error getting I/O privilege: Operation not permitted Cannot find I2C adapter Unable to establish host communication Couldn't find EC /usr/share/userfeedback/scripts/cbi_info: ERROR: Failed to read type 2 [0] [1] [2] ---------- END ----------
,
Oct 2
This issue hasn't been updated in the last 6 weeks, so removing its merge approval label. Please re-request a merge if needed. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 15
Other ectool invocations from debugd are also failing for the same lock acquisition failure. The lock acquisition originates from b/35509002. mosys & flashrom use the same lock.` I think the bug is caused by debugd running in minijail where /run/lock isn't mounted. Shall we bind or mount /run/lock in debugd's minijail?
,
Dec 20
,
Jan 2
What processes have access to /run/lock?
,
Jan 2
usually /run/lock is 1777 so any process can grab a lock i've asked multiple times now why a lock is necessary in the first place and have yet to receive an answer
,
Jan 2
The lock acquisition originates from b/35509002. The problem seems LPC I/O access isn't serialized. So, we added a big mutex (/run/lock) to fix the bug. It looks we already have a dev driver in cros kernel 4.4 and the tools (ectool, flashrom, and mosys) support it on some platforms. So, it could be a matter of adding a dev interface to x86 (or forcing tools to use a dev interface).
,
Jan 2
Ok, I verified mosys, ectool, and flashrom all use the dev interface by default (on Nami). Shall we just drop /run/lock acquisition from ectool (if it can probe /dev/cros_ec successfully) , then?
,
Jan 3
That sounds better than giving access to /run/lock, yes. Thanks!
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/43c253367f789605a28f781ca2f0759e363fc941 commit 43c253367f789605a28f781ca2f0759e363fc941 Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Fri Jan 04 20:46:37 2019 cbi_info: Give kRoot to group access This patch gives kRoot to group access for cbi_info. Without it, ectool fails to open /dev/cros_ec. $ generate_logs ... feedback/cbi_info ... $ cat feedback/cbi_info [0] As integer: 1 (0x1) As binary: 01 02 [1] As integer: 3 (0x3) As binary: 03 [2] As integer: 103 (0x67) As binary: 67 3a BUG=chromium:849399 TEST=See above. Change-Id: I574c2d0dbb927b64de2f20b18e30fccb0cc6a2b7 Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1394243 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> [modify] https://crrev.com/43c253367f789605a28f781ca2f0759e363fc941/debugd/src/log_tool.cc
,
Jan 6
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/aed008f87c3c880edecf7608ab24eaa4bee1bc46 commit aed008f87c3c880edecf7608ab24eaa4bee1bc46 Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Sun Jan 06 04:08:41 2019 ectool: Don't acquire lock when dev interface is used The /dev/cros_ec interface has a built-in mutex, thus we do not need to use /run/lock to arbitrate access since we can assume other tools (mosys, flashrom) also use the dev interface. $ generate_logs ... feedback/cbi_info ... $ cat feedback/cbi_info [0] As integer: 1 (0x1) As binary: 01 02 [1] As integer: 3 (0x3) As binary: 03 [2] As integer: 103 (0x67) As binary: 67 3a Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org> BUG=chromium:849399 BRANCH=none TEST=Verify 'ectool version' runs on Nami. Verify lock is acquired when '--interface=lpc' is specified. Verify debugd can run cbi_info. Change-Id: Id94317472917a974218bb137bda11fe5618a4b88 Reviewed-on: https://chromium-review.googlesource.com/1393729 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Daisuke Nojiri <dnojiri@chromium.org> Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/aed008f87c3c880edecf7608ab24eaa4bee1bc46/util/ec_sb_firmware_update.c [modify] https://crrev.com/aed008f87c3c880edecf7608ab24eaa4bee1bc46/util/comm-host.h [modify] https://crrev.com/aed008f87c3c880edecf7608ab24eaa4bee1bc46/util/ectool.c [modify] https://crrev.com/aed008f87c3c880edecf7608ab24eaa4bee1bc46/util/comm-host.c
,
Jan 7
,
Jan 7
The bug is marked as P3 or Feature. It should not be merged as M72 is in beta. Please contact the approriate milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 7
,
Jan 7
This feature has been released but ins't working. So, this merge request is for a bug fix.
,
Jan 7
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 7
,
Jan 7
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/cfdb3c0cf1d134c98a3c0d2ec74a6607cd06e671 commit cfdb3c0cf1d134c98a3c0d2ec74a6607cd06e671 Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Mon Jan 07 21:19:36 2019 cbi_info: Give kRoot to group access This patch gives kRoot to group access for cbi_info. Without it, ectool fails to open /dev/cros_ec. $ generate_logs ... feedback/cbi_info ... $ cat feedback/cbi_info [0] As integer: 1 (0x1) As binary: 01 02 [1] As integer: 3 (0x3) As binary: 03 [2] As integer: 103 (0x67) As binary: 67 3a BUG=chromium:849399 TEST=See above. Change-Id: I574c2d0dbb927b64de2f20b18e30fccb0cc6a2b7 Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/1394258 [modify] https://crrev.com/cfdb3c0cf1d134c98a3c0d2ec74a6607cd06e671/debugd/src/log_tool.cc
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/4b72e2d82c27beae0536bec84bef2a7e5b541b5e commit 4b72e2d82c27beae0536bec84bef2a7e5b541b5e Author: Daisuke Nojiri <dnojiri@chromium.org> Date: Mon Jan 07 21:19:36 2019 ectool: Don't acquire lock when dev interface is used The /dev/cros_ec interface has a built-in mutex, thus we do not need to use /run/lock to arbitrate access since we can assume other tools (mosys, flashrom) also use the dev interface. $ generate_logs ... feedback/cbi_info ... $ cat feedback/cbi_info [0] As integer: 1 (0x1) As binary: 01 02 [1] As integer: 3 (0x3) As binary: 03 [2] As integer: 103 (0x67) As binary: 67 3a Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org> BUG=chromium:849399 BRANCH=none TEST=Verify 'ectool version' runs on Nami. Verify lock is acquired when '--interface=lpc' is specified. Verify debugd can run cbi_info. Change-Id: Id94317472917a974218bb137bda11fe5618a4b88 Reviewed-on: https://chromium-review.googlesource.com/1393729 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Daisuke Nojiri <dnojiri@chromium.org> Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> (cherry picked from commit aed008f87c3c880edecf7608ab24eaa4bee1bc46) Reviewed-on: https://chromium-review.googlesource.com/c/1398342 Commit-Queue: Daisuke Nojiri <dnojiri@chromium.org> Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org> [modify] https://crrev.com/4b72e2d82c27beae0536bec84bef2a7e5b541b5e/util/ec_sb_firmware_update.c [modify] https://crrev.com/4b72e2d82c27beae0536bec84bef2a7e5b541b5e/util/comm-host.h [modify] https://crrev.com/4b72e2d82c27beae0536bec84bef2a7e5b541b5e/util/ectool.c [modify] https://crrev.com/4b72e2d82c27beae0536bec84bef2a7e5b541b5e/util/comm-host.c |
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by vapier@chromium.org
, Jun 4 2018