tools/pci: refactor PCI bridge
Use polymorphism to handle the differences between Aspeed and Nuvoton
PCI devices.
Add unit tests (now at 100% line coverage for tools/pci.cpp).
Signed-off-by: Benjamin Fair <benjaminfair@google.com>
Change-Id: I43e63ec5eb9fce5fb0fc74e0e69667dd13b7433f
diff --git a/README.md b/README.md
index 7774718..db8b873 100644
--- a/README.md
+++ b/README.md
@@ -48,6 +48,19 @@
make install
```
+#### Building fmtlib
+
+Check out the [fmtlib source](https://github.com/fmtlib/fmt).
+
+Then run these commands in the source directory.
+
+```
+mkdir build && cd build
+cmake ..
+make
+make install
+```
+
#### Building stdplus
Check out the [stdplus source](https://github.com/openbmc/stdplus).
diff --git a/configure.ac b/configure.ac
index 94ed8b7..0ed6517 100644
--- a/configure.ac
+++ b/configure.ac
@@ -128,6 +128,18 @@
[],
[AC_MSG_ERROR([Could not find libpciaccess... pciaccess package required])]
)
+ PKG_CHECK_MODULES(
+ [STDPLUS],
+ [stdplus],
+ [],
+ [AC_MSG_ERROR([Could not find stdplus... openbmc/stdplus package required])]
+ )
+ PKG_CHECK_MODULES(
+ [FMT],
+ [fmt],
+ [],
+ [AC_MSG_ERROR([Could not find fmt... fmtlib/fmt package required])]
+ )
])
# These packages are required for both the BMC and the host-tool.
diff --git a/tools/Makefile.am b/tools/Makefile.am
index f63a112..9c41e1b 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -7,12 +7,20 @@
$(CODE_COVERAGE_CXXFLAGS)
noinst_LTLIBRARIES = libupdater.la
-libupdater_la_LDFLAGS = -static $(CODE_COVERAGE_LIBS) $(IPMIBLOB_LIBS) $(PCIACCESS_LIBS)
+libupdater_la_LDFLAGS = \
+ -static \
+ $(CODE_COVERAGE_LIBS) \
+ $(FMT_LIBS) \
+ $(IPMIBLOB_LIBS) \
+ $(PCIACCESS_LIBS) \
+ $(STDPLUS_LIBS)
libupdater_la_CXXFLAGS = \
-I$(top_srcdir) \
$(CODE_COVERAGE_CXXFLAGS) \
+ $(FMT_CFLAGS) \
$(IPMIBLOB_CFLAGS) \
- $(PCIACCESS_CFLAGS)
+ $(PCIACCESS_CFLAGS) \
+ $(STDPLUS_CFLAGS)
libupdater_la_SOURCES = \
updater.cpp \
handler.cpp \
diff --git a/tools/main.cpp b/tools/main.cpp
index dd43c11..3876844 100644
--- a/tools/main.cpp
+++ b/tools/main.cpp
@@ -20,6 +20,7 @@
#include "net.hpp"
#include "p2a.hpp"
#include "pci.hpp"
+#include "pciaccess.hpp"
#include "progress.hpp"
#include "tool_errors.hpp"
#include "updater.hpp"
@@ -249,9 +250,9 @@
}
else if (interface == IPMIPCI)
{
- auto& pci = host_tool::PciUtilImpl::getInstance();
- handler = std::make_unique<host_tool::P2aDataHandler>(
- &blob, &devmem, &pci, &progress);
+ auto& pci = host_tool::PciAccessImpl::getInstance();
+ handler = std::make_unique<host_tool::P2aDataHandler>(&blob, &pci,
+ &progress);
}
if (!handler)
diff --git a/tools/p2a.cpp b/tools/p2a.cpp
index 9de60f0..c18df99 100644
--- a/tools/p2a.cpp
+++ b/tools/p2a.cpp
@@ -19,8 +19,12 @@
#include "data.hpp"
#include "flags.hpp"
#include "pci.hpp"
+#include "tool_errors.hpp"
+
+#include <fmt/format.h>
#include <ipmiblob/blob_errors.hpp>
+#include <stdplus/handle/managed.hpp>
#include <cstdint>
#include <cstring>
@@ -32,221 +36,99 @@
namespace
{
-void disablePciBridge(HostIoInterface* io, std::size_t address)
-{
- /* Read current value, and just blindly unset the bit. */
- std::uint32_t value;
- if (!io->read(address + aspeedP2aConfig, sizeof(value), &value))
- {
- return;
- }
- value &= ~p2ABridgeEnabled;
- io->write(address + aspeedP2aConfig, sizeof(value), &value);
+/** @brief RAII wrapper and its destructor for opening a file descriptor */
+static void closeFd(int&& fd, const internal::Sys* const& sys)
+{
+ sys->close(fd);
}
+using Fd = stdplus::Managed<int, const internal::Sys* const>::Handle<closeFd>;
} // namespace
bool P2aDataHandler::sendContents(const std::string& input,
std::uint16_t session)
{
- PciDevice result;
- PciFilter filter;
- bool found = false;
- pciaddr_t bar;
- bool returnValue = false;
- int inputFd = -1;
+ std::unique_ptr<PciBridgeIntf> bridge;
ipmi_flash::PciConfigResponse pciResp;
std::int64_t fileSize;
- std::unique_ptr<std::uint8_t[]> readBuffer;
- std::uint16_t pciDeviceVID;
- std::uint16_t pciDeviceDID;
- std::uint32_t p2aOffset;
- std::uint32_t p2aLength;
-
- for (auto device : PCIDeviceList)
+ try
{
- filter.vid = device.VID;
- filter.did = device.DID;
-
- /* Find the PCI device entry we want. */
- auto output = pci->getPciDevices(filter);
- for (const auto& d : output)
- {
- std::fprintf(stderr, "[0x%x 0x%x] \n", d.vid, d.did);
-
- /* Verify it's a memory-based bar. */
- bar = d.bars[device.bar];
-
- if ((bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO)
- {
- /* We want it to not be IO-based access. */
- continue;
- }
-
- /* For now capture the entire device even if we're only using BAR0
- */
- result = d;
- found = true;
- break;
- }
-
- if (found)
- {
- std::fprintf(stderr, "Find [0x%x 0x%x] \n", device.VID, device.DID);
- std::fprintf(stderr, "bar%u[0x%x] \n", device.bar,
- (unsigned int)bar);
- pciDeviceVID = device.VID;
- pciDeviceDID = device.DID;
- p2aOffset = device.Offset;
- p2aLength = device.Length;
- break;
- }
+ bridge = std::make_unique<NuvotonPciBridge>(pci);
}
+ catch (NotFoundException& e)
+ {}
- if (!found)
+ try
{
- return false;
+ bridge = std::make_unique<AspeedPciBridge>(pci);
}
+ catch (NotFoundException& e)
+ {}
- std::fprintf(stderr, "\n");
-
- /* We sent the open command before this, so the window should be open and
- * the bridge enabled on the BMC.
- */
- if (pciDeviceVID == aspeedPciDeviceInfo.VID &&
- pciDeviceDID == aspeedPciDeviceInfo.DID)
+ if (!bridge)
{
- std::uint32_t value;
- if (!io->read(bar + aspeedP2aConfig, sizeof(value), &value))
- {
- std::fprintf(stderr, "PCI config read failed\n");
- return false;
- }
-
- if (0 == (value & p2ABridgeEnabled))
- {
- std::fprintf(stderr, "Bridge not enabled - Enabling from host\n");
-
- value |= p2ABridgeEnabled;
- if (!io->write(bar + aspeedP2aConfig, sizeof(value), &value))
- {
- std::fprintf(stderr, "PCI config write failed\n");
- return false;
- }
- }
-
- /* From this point down we need to disable the bridge. */
- std::fprintf(stderr, "The bridge is enabled!\n");
+ throw NotFoundException("supported PCI device");
}
/* Read the configuration via blobs metadata (stat). */
ipmiblob::StatResponse stat = blob->getStat(session);
if (stat.metadata.size() != sizeof(ipmi_flash::PciConfigResponse))
{
- std::fprintf(stderr, "Didn't receive expected size of metadata for "
- "PCI Configuration response\n");
- goto exit;
+ throw ToolException("Didn't receive expected size of metadata for "
+ "PCI Configuration response");
}
std::memcpy(&pciResp, stat.metadata.data(), sizeof(pciResp));
- std::fprintf(stderr, "Received address: 0x%x\n", pciResp.address);
-
- if (pciDeviceVID == aspeedPciDeviceInfo.VID &&
- pciDeviceDID == aspeedPciDeviceInfo.DID)
- {
- /* Configure the mmio to point there. */
- if (!io->write(bar + aspeedP2aBridge, sizeof(pciResp.address),
- &pciResp.address))
- {
- // Failed to set it up, so fall back.
- std::fprintf(stderr, "Failed to update the bridge address\n");
- goto exit;
- }
- }
+ bridge->configure(pciResp);
/* For data blocks in 64kb, stage data, and send blob write command. */
- inputFd = sys->open(input.c_str(), 0);
- if (inputFd < 0)
+ Fd inputFd(sys->open(input.c_str(), 0), sys);
+ if (*inputFd < 0)
{
- std::fprintf(stderr, "Unable to open file: '%s'\n", input.c_str());
- goto exit;
+ (void)inputFd.release();
+ throw internal::errnoException(
+ fmt::format("Error opening file '{}'", input));
}
fileSize = sys->getSize(input.c_str());
if (fileSize == 0)
{
- std::fprintf(stderr, "Zero-length file, or other file access error\n");
- goto exit;
+ throw ToolException("Zero-length file, or other file access error");
}
progress->start(fileSize);
- readBuffer = std::make_unique<std::uint8_t[]>(p2aLength);
- if (nullptr == readBuffer)
- {
- std::fprintf(stderr, "Unable to allocate memory for read buffer.\n");
- goto exit;
- }
+ std::vector<std::uint8_t> readBuffer(bridge->getDataLength());
- try
- {
- int bytesRead = 0;
- std::uint32_t offset = 0;
+ int bytesRead = 0;
+ std::uint32_t offset = 0;
- do
+ do
+ {
+ bytesRead = sys->read(*inputFd, readBuffer.data(), readBuffer.size());
+ if (bytesRead > 0)
{
- bytesRead = sys->read(inputFd, readBuffer.get(), p2aLength);
- if (bytesRead > 0)
- {
- /* TODO: Will likely need to store an rv somewhere to know when
- * we're exiting from failure.
- */
- if (!io->write(bar + p2aOffset, bytesRead, readBuffer.get()))
- {
- std::fprintf(stderr,
- "Failed to write to region in memory!\n");
- goto exit;
- }
+ bridge->write(stdplus::span<const std::uint8_t>(readBuffer.data(),
+ bytesRead));
- /* Ok, so the data is staged, now send the blob write with the
- * details.
- */
- struct ipmi_flash::ExtChunkHdr chunk;
- chunk.length = bytesRead;
- std::vector<std::uint8_t> chunkBytes(sizeof(chunk));
- std::memcpy(chunkBytes.data(), &chunk, sizeof(chunk));
+ /* Ok, so the data is staged, now send the blob write with the
+ * details.
+ */
+ struct ipmi_flash::ExtChunkHdr chunk;
+ chunk.length = bytesRead;
+ std::vector<std::uint8_t> chunkBytes(sizeof(chunk));
+ std::memcpy(chunkBytes.data(), &chunk, sizeof(chunk));
- /* This doesn't return anything on success. */
- blob->writeBytes(session, offset, chunkBytes);
- offset += bytesRead;
- progress->updateProgress(bytesRead);
- }
- } while (bytesRead > 0);
- }
- catch (const ipmiblob::BlobException& b)
- {
- goto exit;
- }
+ /* This doesn't return anything on success. */
+ blob->writeBytes(session, offset, chunkBytes);
+ offset += bytesRead;
+ progress->updateProgress(bytesRead);
+ }
+ } while (bytesRead > 0);
- /* defaults to failure. */
- returnValue = true;
-
-exit:
- /* disable ASPEED PCI bridge. */
- if (pciDeviceVID == aspeedPciDeviceInfo.VID &&
- pciDeviceDID == aspeedPciDeviceInfo.DID)
- {
- disablePciBridge(io, bar);
- }
-
- /* close input file. */
- if (inputFd != -1)
- {
- sys->close(inputFd);
- }
- return returnValue;
+ return true;
}
} // namespace host_tool
diff --git a/tools/p2a.hpp b/tools/p2a.hpp
index 1d1cb2d..08153d2 100644
--- a/tools/p2a.hpp
+++ b/tools/p2a.hpp
@@ -2,7 +2,6 @@
#include "interface.hpp"
#include "internal/sys.hpp"
-#include "io.hpp"
#include "pci.hpp"
#include "progress.hpp"
@@ -11,30 +10,17 @@
#include <cstdint>
#include <vector>
-constexpr std::size_t aspeedP2aConfig = 0x0f000;
-constexpr std::size_t aspeedP2aBridge = 0x0f004;
-constexpr std::uint32_t p2ABridgeEnabled = 0x1;
-
-struct PciDeviceInfo
-{
- std::uint16_t VID;
- std::uint16_t DID;
- std::size_t Offset;
- std::size_t Length;
- std::uint16_t bar;
-};
-
namespace host_tool
{
class P2aDataHandler : public DataInterface
{
public:
- P2aDataHandler(ipmiblob::BlobInterface* blob, HostIoInterface* io,
- PciUtilInterface* pci, ProgressInterface* progress,
+ P2aDataHandler(ipmiblob::BlobInterface* blob, const PciAccess* pci,
+ ProgressInterface* progress,
const internal::Sys* sys = &internal::sys_impl) :
blob(blob),
- io(io), pci(pci), progress(progress), sys(sys)
+ pci(pci), progress(progress), sys(sys)
{}
bool sendContents(const std::string& input, std::uint16_t session) override;
@@ -45,16 +31,9 @@
private:
ipmiblob::BlobInterface* blob;
- HostIoInterface* io;
- PciUtilInterface* pci;
+ const PciAccess* pci;
ProgressInterface* progress;
const internal::Sys* sys;
-
- constexpr struct PciDeviceInfo static aspeedPciDeviceInfo = {
- 0x1a03, 0x2000, 0x10000, 0x10000, 1};
- constexpr struct PciDeviceInfo static nuvotonPciDeviceInfo = {
- 0x1050, 0x0750, 0x0, 0x4000, 0};
- const std::vector<PciDeviceInfo> PCIDeviceList = {aspeedPciDeviceInfo,
- nuvotonPciDeviceInfo};
};
+
} // namespace host_tool
diff --git a/tools/pci.cpp b/tools/pci.cpp
index 7fa4d59..6e9cfe4 100644
--- a/tools/pci.cpp
+++ b/tools/pci.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright 2019 Google Inc.
+ * Copyright 2020 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,58 +16,141 @@
#include "pci.hpp"
+#include "tool_errors.hpp"
+
extern "C"
{
#include <pciaccess.h>
} // extern "C"
-#include <linux/pci_regs.h>
+#include <fmt/format.h>
+
+#include <stdplus/handle/managed.hpp>
#include <cstring>
-#include <optional>
-#include <vector>
+#include <system_error>
namespace host_tool
{
-std::vector<PciDevice>
- PciUtilImpl::getPciDevices(std::optional<PciFilter> filter)
+namespace
{
- struct pci_id_match match = {PCI_MATCH_ANY, PCI_MATCH_ANY, PCI_MATCH_ANY,
- PCI_MATCH_ANY};
- std::vector<PciDevice> results;
- if (filter.has_value())
+/** @brief RAII wrapper and its destructor for creating a pci_device_iterator */
+static void closeIt(struct pci_device_iterator*&& it,
+ const PciAccess* const& pci)
+{
+ pci->pci_iterator_destroy(it);
+}
+using It = stdplus::Managed<struct pci_device_iterator*,
+ const PciAccess* const>::Handle<closeIt>;
+
+} // namespace
+
+PciAccessBridge::PciAccessBridge(const struct pci_id_match* match, int bar,
+ std::size_t dataOffset, std::size_t dataLength,
+ const PciAccess* pci) :
+ dataOffset(dataOffset),
+ dataLength(dataLength), pci(pci)
+{
+ It it(pci->pci_id_match_iterator_create(match), pci);
+
+ while ((dev = pci->pci_device_next(*it)))
{
- match.vendor_id = filter.value().vid;
- match.device_id = filter.value().did;
- }
-
- auto it = pci_id_match_iterator_create(&match);
- struct pci_device* dev;
-
- while ((dev = pci_device_next(it)))
- {
- PciDevice item;
-
- pci_device_probe(dev);
-
- item.bus = dev->bus;
- item.dev = dev->dev;
- item.func = dev->func;
- item.vid = dev->vendor_id;
- item.did = dev->device_id;
-
- for (int i = 0; i < PCI_STD_NUM_BARS; i++)
+ int ret = pci->pci_device_probe(dev);
+ if (ret)
{
- item.bars[i] = dev->regions[i].base_addr;
+ throw std::system_error(ret, std::generic_category(),
+ "Error probing PCI device");
}
- results.push_back(item);
+ /* Verify it's a memory-based bar. */
+ if (!dev->regions[bar].is_IO)
+ break;
}
- pci_iterator_destroy(it);
- return results;
+ if (!dev)
+ {
+ throw NotFoundException(fmt::format(
+ "PCI device {:#04x}:{:#04x}", match->vendor_id, match->device_id));
+ }
+
+ std::fprintf(stderr, "Find [0x%x 0x%x] \n", match->vendor_id,
+ match->device_id);
+ std::fprintf(stderr, "bar%d[0x%x] \n", bar,
+ static_cast<unsigned int>(dev->regions[bar].base_addr));
+
+ size = dev->regions[bar].size;
+ int ret = pci->pci_device_map_range(
+ dev, dev->regions[bar].base_addr, dev->regions[bar].size,
+ PCI_DEV_MAP_FLAG_WRITABLE, reinterpret_cast<void**>(&addr));
+ if (ret)
+ {
+ throw std::system_error(ret, std::generic_category(),
+ "Error mapping PCI device memory");
+ }
+}
+
+PciAccessBridge::~PciAccessBridge()
+{
+ int ret = pci->pci_device_unmap_range(dev, addr, size);
+
+ if (ret)
+ {
+ std::fprintf(stderr, "Error while unmapping PCI device memory: %s\n",
+ std::strerror(ret));
+ }
+}
+
+void PciAccessBridge::write(const stdplus::span<const std::uint8_t> data)
+{
+ if (data.size() > dataLength)
+ {
+ throw ToolException(
+ fmt::format("Write of {} bytes exceeds maximum of {}", data.size(),
+ dataLength));
+ }
+
+ std::memcpy(addr + dataOffset, data.data(), data.size());
+}
+
+void AspeedPciBridge::enableBridge()
+{
+ /* We sent the open command before this, so the window should be open and
+ * the bridge enabled on the BMC.
+ */
+ std::uint32_t value;
+ std::memcpy(&value, addr + config, sizeof(value));
+
+ if (0 == (value & bridgeEnabled))
+ {
+ std::fprintf(stderr, "Bridge not enabled - Enabling from host\n");
+
+ value |= bridgeEnabled;
+ std::memcpy(addr + config, &value, sizeof(value));
+ }
+
+ std::fprintf(stderr, "The bridge is enabled!\n");
+}
+
+void AspeedPciBridge::disableBridge()
+{
+ /* addr is valid if the constructor completed */
+
+ /* Read current value, and just blindly unset the bit. */
+ std::uint32_t value;
+ std::memcpy(&value, addr + config, sizeof(value));
+
+ value &= ~bridgeEnabled;
+ std::memcpy(addr + config, &value, sizeof(value));
+}
+
+void AspeedPciBridge::configure(const ipmi_flash::PciConfigResponse& configResp)
+{
+ std::fprintf(stderr, "Received address: 0x%x\n", configResp.address);
+
+ /* Configure the mmio to point there. */
+ std::memcpy(addr + bridge, &configResp.address, sizeof(configResp.address));
}
} // namespace host_tool
diff --git a/tools/pci.hpp b/tools/pci.hpp
index f421892..7f0cfb7 100644
--- a/tools/pci.hpp
+++ b/tools/pci.hpp
@@ -1,19 +1,30 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
#pragma once
+#include "data.hpp"
#include "internal/sys.hpp"
-
-extern "C"
-{
-#include <pciaccess.h>
-} // extern "C"
+#include "pciaccess.hpp"
#include <linux/pci_regs.h>
-#include <cstdint>
-#include <memory>
-#include <optional>
-#include <vector>
+#include <stdplus/types.hpp>
+// Some versions of the linux/pci_regs.h header don't define this
#ifndef PCI_STD_NUM_BARS
#define PCI_STD_NUM_BARS 6
#endif // !PCI_STD_NUM_BARS
@@ -21,76 +32,113 @@
namespace host_tool
{
-/* The ASPEED AST2400 & AST2500 have the same VIDDID. */
-
-/**
- * The PciDevice structure is a copy of the information to uniquely identify a
- * PCI device.
- */
-struct PciDevice
-{
- std::uint16_t vid;
- std::uint16_t did;
- std::uint8_t bus;
- std::uint8_t dev;
- std::uint8_t func;
- pciaddr_t bars[PCI_STD_NUM_BARS];
-};
-
-/**
- * The PciFilter structure is a simple mechanism for filtering devices by their
- * vendor and/or device ids.
- */
-struct PciFilter
-{
- std::uint16_t vid;
- std::uint16_t did;
-};
-
-class PciUtilInterface
+class PciBridgeIntf
{
public:
- virtual ~PciUtilInterface() = default;
+ virtual ~PciBridgeIntf() = default;
- /**
- * Get a list of PCI devices from a system.
- *
- * @param[in] filter - optional filter for the list.
- * @return the list of devices.
- */
- virtual std::vector<PciDevice>
- getPciDevices(std::optional<PciFilter> filter = std::nullopt) = 0;
+ virtual void write(const stdplus::span<const std::uint8_t> data) = 0;
+ virtual void configure(const ipmi_flash::PciConfigResponse& config) = 0;
+
+ virtual std::size_t getDataLength() = 0;
};
-class PciUtilImpl : public PciUtilInterface
+class PciAccessBridge : public PciBridgeIntf
{
public:
- static PciUtilImpl& getInstance()
+ virtual ~PciAccessBridge();
+
+ virtual void write(const stdplus::span<const std::uint8_t> data) override;
+ virtual void
+ configure(const ipmi_flash::PciConfigResponse& configResp) override{};
+
+ std::size_t getDataLength() override
{
- static PciUtilImpl instance;
- return instance;
+ return dataLength;
}
- std::vector<PciDevice>
- getPciDevices(std::optional<PciFilter> filter = std::nullopt) override;
+ protected:
+ /**
+ * Finds the PCI device matching @a match and saves a reference to it in @a
+ * dev. Also maps the memory region described in BAR number @a bar to
+ * address @a addr,
+ */
+ PciAccessBridge(const struct pci_id_match* match, int bar,
+ std::size_t dataOffset, std::size_t dataLength,
+ const PciAccess* pci);
- PciUtilImpl(const PciUtilImpl&) = delete;
- PciUtilImpl& operator=(const PciUtilImpl&) = delete;
+ struct pci_device* dev = nullptr;
+ std::uint8_t* addr = nullptr;
+ std::size_t size = 0;
private:
- PciUtilImpl()
+ std::size_t dataOffset;
+ std::size_t dataLength;
+
+ protected:
+ const PciAccess* pci;
+};
+
+class NuvotonPciBridge : public PciAccessBridge
+{
+ public:
+ explicit NuvotonPciBridge(const PciAccess* pci) :
+ PciAccessBridge(&match, bar, dataOffset, dataLength, pci)
+ {}
+
+ ~NuvotonPciBridge()
+ {}
+
+ private:
+ static constexpr std::uint32_t vid = 0x1050;
+ static constexpr std::uint32_t did = 0x0750;
+ static constexpr int bar = 0;
+ static constexpr struct pci_id_match match
{
- int ret = pci_system_init();
- if (ret)
- {
- throw internal::errnoException("pci_system_init");
- }
+ vid, did, PCI_MATCH_ANY, PCI_MATCH_ANY
+ };
+
+ static constexpr pciaddr_t bridge = 0x04;
+ static constexpr std::uint8_t bridgeEnabled = 0x02;
+
+ static constexpr std::size_t dataOffset = 0x0;
+ static constexpr std::size_t dataLength = 0x4000;
+};
+
+class AspeedPciBridge : public PciAccessBridge
+{
+ public:
+ explicit AspeedPciBridge(const PciAccess* pci) :
+ PciAccessBridge(&match, bar, dataOffset, dataLength, pci)
+ {
+ enableBridge();
}
- ~PciUtilImpl()
+ ~AspeedPciBridge()
{
- pci_system_cleanup();
+ disableBridge();
}
+
+ void configure(const ipmi_flash::PciConfigResponse& configResp) override;
+
+ private:
+ static constexpr std::uint32_t vid = 0x1a03;
+ static constexpr std::uint32_t did = 0x2000;
+ static constexpr int bar = 1;
+ static constexpr struct pci_id_match match
+ {
+ vid, did, PCI_MATCH_ANY, PCI_MATCH_ANY
+ };
+
+ static constexpr std::size_t config = 0x0f000;
+ static constexpr std::size_t bridge = 0x0f004;
+ static constexpr std::uint32_t bridgeEnabled = 0x1;
+
+ static constexpr std::size_t dataOffset = 0x10000;
+ static constexpr std::size_t dataLength = 0x10000;
+
+ void enableBridge();
+ void disableBridge();
};
} // namespace host_tool
diff --git a/tools/test/tools_pci_unittest.cpp b/tools/test/tools_pci_unittest.cpp
index cc67926..00b3eb1 100644
--- a/tools/test/tools_pci_unittest.cpp
+++ b/tools/test/tools_pci_unittest.cpp
@@ -1,5 +1,30 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
#include "internal_sys_mock.hpp"
+#include "pci.hpp"
#include "pciaccess_mock.hpp"
+#include "tool_errors.hpp"
+
+#include <stdplus/raw.hpp>
+
+#include <algorithm>
+#include <cstdlib>
+#include <string>
+#include <vector>
#include <gtest/gtest.h>
@@ -8,11 +33,456 @@
namespace
{
-TEST(PciHandleTest, empty)
+using namespace std::string_literals;
+
+using ::testing::Assign;
+using ::testing::ContainerEq;
+using ::testing::DoAll;
+using ::testing::Each;
+using ::testing::Eq;
+using ::testing::InSequence;
+using ::testing::NotNull;
+using ::testing::Return;
+using ::testing::SetArgPointee;
+
+// TODO: switch to ConainerEq for C++20
+MATCHER_P(SpanEq, s, "")
+{
+ return arg.size() == s.size() && !memcmp(arg.data(), s.data(), s.size());
+}
+
+MATCHER_P(PciIdMatch, m, "")
+{
+ return (arg->vendor_id == m->vendor_id && arg->device_id == m->device_id &&
+ arg->subvendor_id == m->subvendor_id &&
+ arg->subdevice_id == m->subdevice_id);
+}
+
+pci_device_iterator* mockIter = reinterpret_cast<pci_device_iterator*>(0x42);
+
+constexpr pciaddr_t mockBaseAddr = 0xdeadbeef;
+constexpr pciaddr_t mockRegionSize = 0x20000;
+
+class Device
+{
+ public:
+ virtual const struct pci_id_match* getMatch() const = 0;
+ virtual struct pci_device getDevice() const = 0;
+ virtual std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const = 0;
+ virtual std::string getName() const = 0;
+};
+
+class NuvotonDevice : public Device
+{
+ public:
+ const struct pci_id_match* getMatch() const override
+ {
+ return &match;
+ }
+
+ struct pci_device getDevice() const override
+ {
+ struct pci_device dev;
+
+ dev.vendor_id = match.vendor_id;
+ dev.device_id = match.device_id;
+
+ dev.regions[0].is_IO = false;
+ dev.regions[0].base_addr = mockBaseAddr;
+ dev.regions[0].size = mockRegionSize;
+
+ return dev;
+ }
+
+ std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const override
+ {
+ return std::make_unique<NuvotonPciBridge>(pci);
+ }
+
+ std::string getName() const override
+ {
+ return "Nuvoton"s;
+ }
+
+ private:
+ static constexpr struct pci_id_match match
+ {
+ 0x1050, 0x0750, PCI_MATCH_ANY, PCI_MATCH_ANY
+ };
+};
+
+class AspeedDevice : public Device
+{
+ public:
+ const struct pci_id_match* getMatch() const override
+ {
+ return &match;
+ }
+
+ struct pci_device getDevice() const override
+ {
+ struct pci_device dev;
+
+ dev.vendor_id = match.vendor_id;
+ dev.device_id = match.device_id;
+
+ dev.regions[1].is_IO = false;
+ dev.regions[1].base_addr = mockBaseAddr;
+ dev.regions[1].size = mockRegionSize;
+
+ return dev;
+ }
+
+ std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const override
+ {
+ return std::make_unique<AspeedPciBridge>(pci);
+ }
+
+ std::string getName() const override
+ {
+ return "Aspeed"s;
+ }
+
+ /* Offset to the config region */
+ static constexpr int config = 0x0f000;
+ /* Lower bit determines whether bridge is enabled */
+ static constexpr std::uint8_t bridgeEnabled = 0x01;
+ /* Offset to the MMIO address configuration */
+ static constexpr int bridge = 0x0f004;
+
+ private:
+ static constexpr struct pci_id_match match
+ {
+ 0x1a03, 0x2000, PCI_MATCH_ANY, PCI_MATCH_ANY
+ };
+};
+
+NuvotonDevice nuvotonDevice;
+AspeedDevice aspeedDevice;
+
+class PciSetupTest : public testing::TestWithParam<Device*>
+{};
+
+/* Handle device not found */
+TEST_P(PciSetupTest, NotFound)
{
PciAccessMock pciMock;
- (void)pciMock;
+ InSequence in;
+
+ EXPECT_CALL(pciMock, pci_id_match_iterator_create(
+ PciIdMatch(GetParam()->getMatch())))
+ .WillOnce(Return(mockIter));
+ EXPECT_CALL(pciMock, pci_device_next(Eq(mockIter)))
+ .WillOnce(Return(nullptr));
+ EXPECT_CALL(pciMock, pci_iterator_destroy(Eq(mockIter))).Times(1);
+
+ EXPECT_THROW(GetParam()->getBridge(&pciMock), NotFoundException);
+}
+
+/* Test finding device but probe fails */
+TEST_P(PciSetupTest, ProbeFail)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+
+ EXPECT_CALL(pciMock, pci_id_match_iterator_create(
+ PciIdMatch(GetParam()->getMatch())))
+ .WillOnce(Return(mockIter));
+ EXPECT_CALL(pciMock, pci_device_next(Eq(mockIter)))
+ .WillOnce(Return(&dev))
+ .WillRepeatedly(Return(nullptr));
+
+ EXPECT_CALL(pciMock, pci_device_probe(Eq(&dev))).WillOnce(Return(EFAULT));
+
+ EXPECT_CALL(pciMock, pci_iterator_destroy(Eq(mockIter))).Times(1);
+
+ EXPECT_THROW(GetParam()->getBridge(&pciMock), std::system_error);
+}
+
+/* Test finding device but mapping fails */
+TEST_P(PciSetupTest, MapFail)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+
+ EXPECT_CALL(pciMock, pci_id_match_iterator_create(
+ PciIdMatch(GetParam()->getMatch())))
+ .WillOnce(Return(mockIter));
+ EXPECT_CALL(pciMock, pci_device_next(Eq(mockIter)))
+ .WillOnce(Return(&dev))
+ .WillRepeatedly(Return(nullptr));
+
+ EXPECT_CALL(pciMock, pci_device_probe(Eq(&dev)))
+ .WillOnce(DoAll(Assign(&dev, GetParam()->getDevice()), Return(0)));
+
+ EXPECT_CALL(pciMock,
+ pci_device_map_range(Eq(&dev), mockBaseAddr, mockRegionSize,
+ PCI_DEV_MAP_FLAG_WRITABLE, NotNull()))
+ .WillOnce(Return(EFAULT));
+
+ EXPECT_CALL(pciMock, pci_iterator_destroy(Eq(mockIter))).Times(1);
+
+ EXPECT_THROW(GetParam()->getBridge(&pciMock), std::system_error);
+}
+
+/* Test finding device but unmapping fails */
+TEST_P(PciSetupTest, UnmapFail)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ EXPECT_CALL(pciMock, pci_id_match_iterator_create(
+ PciIdMatch(GetParam()->getMatch())))
+ .WillOnce(Return(mockIter));
+ EXPECT_CALL(pciMock, pci_device_next(Eq(mockIter)))
+ .WillOnce(Return(&dev))
+ .WillRepeatedly(Return(nullptr));
+
+ EXPECT_CALL(pciMock, pci_device_probe(Eq(&dev)))
+ .WillOnce(DoAll(Assign(&dev, GetParam()->getDevice()), Return(0)));
+
+ EXPECT_CALL(pciMock,
+ pci_device_map_range(Eq(&dev), mockBaseAddr, mockRegionSize,
+ PCI_DEV_MAP_FLAG_WRITABLE, NotNull()))
+ .WillOnce(DoAll(SetArgPointee<4>(region.data()), Return(0)));
+
+ EXPECT_CALL(pciMock, pci_iterator_destroy(Eq(mockIter))).Times(1);
+ EXPECT_CALL(pciMock, pci_device_unmap_range(Eq(&dev), Eq(region.data()),
+ mockRegionSize))
+ .WillOnce(Return(EFAULT));
+
+ // This will print an error but not throw
+ GetParam()->getBridge(&pciMock);
+}
+
+/* Create expectations on pciMock for finding device and mapping memory region
+ */
+void expectSetup(PciAccessMock& pciMock, struct pci_device& dev, Device* param,
+ std::uint8_t* region)
+{
+ EXPECT_CALL(pciMock,
+ pci_id_match_iterator_create(PciIdMatch(param->getMatch())))
+ .WillOnce(Return(mockIter));
+ EXPECT_CALL(pciMock, pci_device_next(Eq(mockIter)))
+ .WillOnce(Return(&dev))
+ .WillRepeatedly(Return(nullptr));
+
+ EXPECT_CALL(pciMock, pci_device_probe(Eq(&dev)))
+ .WillOnce(DoAll(Assign(&dev, param->getDevice()), Return(0)));
+
+ EXPECT_CALL(pciMock,
+ pci_device_map_range(Eq(&dev), mockBaseAddr, mockRegionSize,
+ PCI_DEV_MAP_FLAG_WRITABLE, NotNull()))
+ .WillOnce(DoAll(SetArgPointee<4>(region), Return(0)));
+
+ EXPECT_CALL(pciMock, pci_iterator_destroy(Eq(mockIter))).Times(1);
+ EXPECT_CALL(pciMock,
+ pci_device_unmap_range(Eq(&dev), Eq(region), mockRegionSize))
+ .WillOnce(Return(0));
+}
+
+/* Test finding device and mapping memory region */
+TEST_P(PciSetupTest, Success)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ expectSetup(pciMock, dev, GetParam(), region.data());
+
+ GetParam()->getBridge(&pciMock);
+}
+
+INSTANTIATE_TEST_SUITE_P(Default, PciSetupTest,
+ ::testing::Values(&nuvotonDevice, &aspeedDevice),
+ [](const testing::TestParamInfo<Device*>& info) {
+ return info.param->getName();
+ });
+
+TEST(NuvotonWriteTest, TooLarge)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+ std::vector<std::uint8_t> data(0x4001);
+
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = nuvotonDevice.getBridge(&pciMock);
+ EXPECT_THROW(bridge->write(stdplus::span<std::uint8_t>(data)),
+ ToolException);
+}
+
+TEST(NuvotonWriteTest, Success)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+ std::vector<std::uint8_t> data(0x4000);
+
+ std::generate(data.begin(), data.end(), std::rand);
+
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = nuvotonDevice.getBridge(&pciMock);
+ bridge->write(stdplus::span<std::uint8_t>(data));
+
+ EXPECT_THAT(stdplus::span<uint8_t>(®ion[0], data.size()),
+ SpanEq(stdplus::span<uint8_t>(data)));
+}
+
+TEST(NuvotonConfigureTest, Success)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+ ipmi_flash::PciConfigResponse config{0x123bea51};
+
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = nuvotonDevice.getBridge(&pciMock);
+ bridge->configure(config);
+
+ /* No effect from calling configure(), so the whole region should be 0 */
+ EXPECT_THAT(region, Each(0));
+}
+
+TEST(NuvotonDataLengthTest, Success)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = nuvotonDevice.getBridge(&pciMock);
+ EXPECT_EQ(bridge->getDataLength(), 0x4000);
+}
+
+TEST(AspeedWriteTest, TooLarge)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+ std::vector<std::uint8_t> data(0x10001);
+
+ expectSetup(pciMock, dev, &aspeedDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = aspeedDevice.getBridge(&pciMock);
+ EXPECT_THROW(bridge->write(stdplus::span<std::uint8_t>(data)),
+ ToolException);
+}
+
+TEST(AspeedWriteTest, Success)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+ std::vector<std::uint8_t> data(0x10000);
+
+ std::generate(data.begin(), data.end(), std::rand);
+
+ expectSetup(pciMock, dev, &aspeedDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = aspeedDevice.getBridge(&pciMock);
+ bridge->write(stdplus::span<std::uint8_t>(data));
+
+ EXPECT_THAT(stdplus::span<uint8_t>(®ion[0x10000], data.size()),
+ SpanEq(stdplus::span<uint8_t>(data)));
+}
+
+TEST(AspeedConfigureTest, Success)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+ ipmi_flash::PciConfigResponse config{0x123bea51};
+
+ expectSetup(pciMock, dev, &aspeedDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = aspeedDevice.getBridge(&pciMock);
+ bridge->configure(config);
+
+ auto configSpan = stdplus::raw::asSpan<uint8_t>(config);
+ EXPECT_THAT(
+ stdplus::span<uint8_t>(®ion[AspeedDevice::bridge], sizeof(config)),
+ SpanEq(configSpan));
+}
+
+TEST(AspeedDataLengthTest, Success)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ expectSetup(pciMock, dev, &aspeedDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = aspeedDevice.getBridge(&pciMock);
+ EXPECT_EQ(bridge->getDataLength(), 0x10000);
+}
+
+/* Make sure config region is left alone if the bridge is already enabled */
+TEST(AspeedBridgeTest, AlreadyEnabledSuccess)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ constexpr std::uint8_t defaultVal = 0x42;
+
+ region[AspeedDevice::config] = defaultVal | AspeedDevice::bridgeEnabled;
+
+ expectSetup(pciMock, dev, &aspeedDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = aspeedDevice.getBridge(&pciMock);
+
+ {
+ std::vector<std::uint8_t> enabledRegion(mockRegionSize);
+ enabledRegion[AspeedDevice::config] =
+ defaultVal | AspeedDevice::bridgeEnabled;
+ EXPECT_THAT(region, ContainerEq(enabledRegion));
+ }
+
+ bridge.reset();
+
+ {
+ std::vector<std::uint8_t> disabledRegion(mockRegionSize);
+ disabledRegion[AspeedDevice::config] = defaultVal;
+ EXPECT_THAT(region, ContainerEq(disabledRegion));
+ }
+}
+
+/* Make sure the bridge gets enabled when needed */
+TEST(AspeedBridgeTest, NotEnabledSuccess)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ constexpr std::uint8_t defaultVal = 0x42;
+
+ region[AspeedDevice::config] = defaultVal;
+
+ expectSetup(pciMock, dev, &aspeedDevice, region.data());
+
+ std::unique_ptr<PciBridgeIntf> bridge = aspeedDevice.getBridge(&pciMock);
+
+ {
+ std::vector<std::uint8_t> enabledRegion(mockRegionSize);
+ enabledRegion[AspeedDevice::config] =
+ defaultVal | AspeedDevice::bridgeEnabled;
+ EXPECT_THAT(region, ContainerEq(enabledRegion));
+ }
+
+ bridge.reset();
+
+ {
+ std::vector<std::uint8_t> disabledRegion(mockRegionSize);
+ disabledRegion[AspeedDevice::config] = defaultVal;
+ EXPECT_THAT(region, ContainerEq(disabledRegion));
+ }
}
} // namespace
diff --git a/tools/tool_errors.hpp b/tools/tool_errors.hpp
index cf898e6..96dfa10 100644
--- a/tools/tool_errors.hpp
+++ b/tools/tool_errors.hpp
@@ -20,4 +20,11 @@
std::string message;
};
+class NotFoundException : public ToolException
+{
+ public:
+ explicit NotFoundException(const std::string& device) :
+ ToolException(std::string("Couldn't find " + device)){};
+};
+
} // namespace host_tool