<p>Tristan Corrick has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/c/coreboot/+/30079">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">sb/intel/lynxpoint/pcie.c: Add more checks for NULL pointers<br><br>If PCIe root port `n` is disabled, then `rpc.ports[n - 1]` remains NULL.<br>The existing Lynx Point systems probably don't end up dereferencing<br>NULL pointers this way. However, it might occur on a system using<br>Flexible I/O to remap PCIe root ports to other functions.<br><br>Tested on an ASRock H81M-HDS and an Acer C720 (Google Peppy). No issues<br>presented themselves.<br><br>Change-Id: I2c22fa36217766c2c4d6e8046f99989063066b16<br>Signed-off-by: Tristan Corrick <tristan@corrick.kiwi><br>---<br>M src/southbridge/intel/lynxpoint/pcie.c<br>1 file changed, 29 insertions(+), 16 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/30079/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/southbridge/intel/lynxpoint/pcie.c b/src/southbridge/intel/lynxpoint/pcie.c</span><br><span>index 231f3e7..695abf2 100644</span><br><span>--- a/src/southbridge/intel/lynxpoint/pcie.c</span><br><span>+++ b/src/southbridge/intel/lynxpoint/pcie.c</span><br><span>@@ -14,6 +14,8 @@</span><br><span> * GNU General Public License for more details.</span><br><span> */</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#include <assert.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <commonlib/helpers.h></span><br><span> #include <console/console.h></span><br><span> #include <device/device.h></span><br><span> #include <device/pci.h></span><br><span>@@ -22,6 +24,8 @@</span><br><span> #include <device/pci_ops.h></span><br><span> #include "pch.h"</span><br><span> #include <southbridge/intel/common/gpio.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <stddef.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <stdint.h></span><br><span> </span><br><span> #define MAX_NUM_ROOT_PORTS 8</span><br><span> </span><br><span>@@ -68,6 +72,16 @@</span><br><span> return PCI_FUNC(dev->path.pci.devfn) + 1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static bool is_rp_enabled(int rp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ ASSERT(rp > 0 && rp <= ARRAY_SIZE(rpc.ports));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rpc.ports[rp - 1] == NULL)</span><br><span style="color: hsl(120, 100%, 40%);">+ return false;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return rpc.ports[rp - 1]->enabled;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void root_port_config_update_gbe_port(void)</span><br><span> {</span><br><span> /* Is the Gbe Port enabled? */</span><br><span>@@ -163,7 +177,7 @@</span><br><span> /* Determine the new devfn for this port */</span><br><span> new_devfn = PCI_DEVFN(PCH_PCIE_DEV_SLOT, pci_func);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (dev->path.pci.devfn != new_devfn) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (dev && dev->path.pci.devfn != new_devfn) {</span><br><span> printk(BIOS_DEBUG,</span><br><span> "PCH: PCIe map %02x.%1x -> %02x.%1x\n",</span><br><span> PCI_SLOT(dev->path.pci.devfn),</span><br><span>@@ -188,9 +202,12 @@</span><br><span> int rp;</span><br><span> </span><br><span> dev = rpc.ports[i];</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!dev)</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> rp = root_port_number(dev);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!dev->enabled) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!is_rp_enabled(rp)) {</span><br><span> static const uint32_t high_bit = (1UL << 31);</span><br><span> </span><br><span> /* Configure shared resource clock gating. */</span><br><span>@@ -198,17 +215,13 @@</span><br><span> pci_update_config8(dev, 0xe1, 0xc3, 0x3c);</span><br><span> </span><br><span> if (!is_lp) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (rp == 1 && !rpc.ports[1]->enabled &&</span><br><span style="color: hsl(0, 100%, 40%);">- !rpc.ports[2]->enabled &&</span><br><span style="color: hsl(0, 100%, 40%);">- !rpc.ports[3]->enabled) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rp == 1 && !is_rp_enabled(2) &&</span><br><span style="color: hsl(120, 100%, 40%);">+ !is_rp_enabled(3) && !is_rp_enabled(4)) {</span><br><span> pci_update_config8(dev, 0xe2, ~1, 1);</span><br><span> pci_update_config8(dev, 0xe1, 0x7f, 0x80);</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (rp == 5 && !rpc.ports[5]->enabled &&</span><br><span style="color: hsl(0, 100%, 40%);">- (rpc.ports[6] == NULL ||</span><br><span style="color: hsl(0, 100%, 40%);">- !rpc.ports[6]->enabled) &&</span><br><span style="color: hsl(0, 100%, 40%);">- (rpc.ports[7] == NULL ||</span><br><span style="color: hsl(0, 100%, 40%);">- !rpc.ports[7]->enabled)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rp == 5 && !is_rp_enabled(6) &&</span><br><span style="color: hsl(120, 100%, 40%);">+ !is_rp_enabled(7) && !is_rp_enabled(8)) {</span><br><span> pci_update_config8(dev, 0xe2, ~1, 1);</span><br><span> pci_update_config8(dev, 0xe1, 0x7f, 0x80);</span><br><span> }</span><br><span>@@ -223,8 +236,8 @@</span><br><span> pci_update_config32(dev, 0x420, ~0, (3 << 29));</span><br><span> </span><br><span> /* Enable static clock gating. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (rp == 1 && !rpc.ports[1]->enabled &&</span><br><span style="color: hsl(0, 100%, 40%);">- !rpc.ports[2]->enabled && !rpc.ports[3]->enabled) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rp == 1 && !is_rp_enabled(2) &&</span><br><span style="color: hsl(120, 100%, 40%);">+ !is_rp_enabled(3) && !is_rp_enabled(4)) {</span><br><span> pci_update_config8(dev, 0xe2, ~1, 1);</span><br><span> pci_update_config8(dev, 0xe1, 0x7f, 0x80);</span><br><span> } else if (rp == 5 || rp == 6) {</span><br><span>@@ -258,7 +271,7 @@</span><br><span> pci_update_config8(dev, 0xe1, 0xc3, 0x3c);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!enabled_ports && is_lp)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!enabled_ports && is_lp && rpc.ports[0])</span><br><span> pci_update_config8(rpc.ports[0], 0xe1, ~(1 << 6), (1 << 6));</span><br><span> }</span><br><span> </span><br><span>@@ -267,7 +280,7 @@</span><br><span> int i;</span><br><span> </span><br><span> /* If the first root port is disabled the coalesce ports. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!rpc.ports[0]->enabled)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!is_rp_enabled(1))</span><br><span> rpc.coalesce = 1;</span><br><span> </span><br><span> /* Perform clock gating configuration. */</span><br><span>@@ -307,7 +320,7 @@</span><br><span> * function numbers. */</span><br><span> current_func = 0;</span><br><span> for (i = 0; i < rpc.num_ports; i++) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (!rpc.ports[i]->enabled)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!is_rp_enabled(i + 1))</span><br><span> continue;</span><br><span> pch_pcie_device_set_func(i, current_func);</span><br><span> current_func++;</span><br><span>@@ -315,7 +328,7 @@</span><br><span> </span><br><span> /* Allocate the disabled devices' PCI function number. */</span><br><span> for (i = 0; i < rpc.num_ports; i++) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (rpc.ports[i]->enabled)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (is_rp_enabled(i + 1))</span><br><span> continue;</span><br><span> pch_pcie_device_set_func(i, current_func);</span><br><span> current_func++;</span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/c/coreboot/+/30079">change 30079</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/c/coreboot/+/30079"/><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-Change-Id: I2c22fa36217766c2c4d6e8046f99989063066b16 </div>
<div style="display:none"> Gerrit-Change-Number: 30079 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Tristan Corrick <tristan@corrick.kiwi> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>