<p style="white-space: pre-wrap; word-wrap: break-word;">Hmm... hmmm... how much of my yak would you be willing to shave? ;)</p><p style="white-space: pre-wrap; word-wrap: break-word;">I mean, we can take this, and it will work (you'll need a little depthcharge change as well, to call dt_set_mac_addresses()). But this code and the whole pass-MACs-through-coreboot-table mechanism is sort of deprecated, because we now have a VPD driver in depthcharge. You could really implement everything we need for this solely in depthcharge. (I've also never been a big fan of most of this code, honestly...)</p><p style="white-space: pre-wrap; word-wrap: break-word;">The other problem is that even though this can match multiple template strings, it was only ever used with one group per device. Cheza is the first one that will have both WiFi and Bluetooth MACs now, and for those this mechanism becomes very ugly. If you look at how it is matched (e.g. depthcharge/src/board/gale/board.c:mac_maps[]), things are only assigned by index, there is no matching back to the original VPD name. That means that you'll just have to happen to know that e.g. the first two MACs are WiFi MACs and the next three are Bluetooth MACs. It also means that an attacker with temporary root access can permanently hose your bluetooth by sneaking a couple of wifi_mac2, wifi_mac3 keys into your RW_VPD, in a way that recovery will not normally fix.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think it would be much better to design a new mechanism for Cheza that lives entirely within depthcharge and gets rid of all the weird design quirks of this legacy crap. I imagine something where you just do this in depthcharge/src/cheza/board.c:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> static const VpdDeviceTreeMap vpd_dt_map[] = {<br>   { "wifi_mac0", { "path_to", "WiFi_MAC0", "DTS_node", NULL } },<br>   { "wifi_mac1", { "path_to", "WiFi_MAC1", "DTS_node", NULL } },<br>   { "bluetooth_mac0", { "path_to", "BT_MAC0", "DTS_node", NULL } },<br> }</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> static int board_setup(void) {<br>   ...<br>   install_vpd_dt_fixup(&vpd_dt_map, sizeof(vpd_dt_map));<br>   ...<br> }</pre><p style="white-space: pre-wrap; word-wrap: break-word;">And then you have a fixup function that just looks up a single VPD entry directly (and you don't need all this "increase number to search for all entries" crap), runs depthcharge's vpd_gets() to find it, decode it and throw it in the DT if found.</p><p><a href="https://review.coreboot.org/28711">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://review.coreboot.org/28711">change 28711</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/28711"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ib290fba2504f6ed80c3a95e24627ae5922e608af </div>
<div style="display:none"> Gerrit-Change-Number: 28711 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Matthias Kaehlcke <mka@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Brian Norris <briannorris@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Matthias Kaehlcke <mka@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 21 Sep 2018 23:39:49 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-HasLabels: No </div>