tools/pci: add Nuvoton bridge configuration
The Nuvoton PCI device requires the Memory Space bit to be set in its
command register in order to access the mailbox over MMIO.
Signed-off-by: Benjamin Fair <benjaminfair@google.com>
Change-Id: Ic262b907ae55c622999aa68891b618650ccad3f2
diff --git a/tools/pci.cpp b/tools/pci.cpp
index 6e9cfe4..0efb75d 100644
--- a/tools/pci.cpp
+++ b/tools/pci.cpp
@@ -114,6 +114,56 @@
std::memcpy(addr + dataOffset, data.data(), data.size());
}
+void NuvotonPciBridge::enableBridge()
+{
+ std::uint8_t value;
+ int ret;
+
+ ret = pci->pci_device_cfg_read_u8(dev, &value, bridge);
+ if (ret)
+ {
+ throw std::system_error(ret, std::generic_category(),
+ "Error reading bridge status");
+ }
+
+ if (value & bridgeEnabled)
+ {
+ std::fprintf(stderr, "Bridge already enabled\n");
+ return;
+ }
+
+ value |= bridgeEnabled;
+
+ ret = pci->pci_device_cfg_write_u8(dev, value, bridge);
+ if (ret)
+ {
+ throw std::system_error(ret, std::generic_category(),
+ "Error enabling bridge");
+ }
+}
+
+void NuvotonPciBridge::disableBridge()
+{
+ std::uint8_t value;
+ int ret;
+
+ ret = pci->pci_device_cfg_read_u8(dev, &value, bridge);
+ if (ret)
+ {
+ std::fprintf(stderr, "Error reading bridge status: %s\n",
+ std::strerror(ret));
+ return;
+ }
+ value &= ~bridgeEnabled;
+
+ ret = pci->pci_device_cfg_write_u8(dev, value, bridge);
+ if (ret)
+ {
+ std::fprintf(stderr, "Error disabling bridge: %s\n",
+ std::strerror(ret));
+ }
+}
+
void AspeedPciBridge::enableBridge()
{
/* We sent the open command before this, so the window should be open and
diff --git a/tools/pci.hpp b/tools/pci.hpp
index 7f0cfb7..3aeae7b 100644
--- a/tools/pci.hpp
+++ b/tools/pci.hpp
@@ -84,10 +84,14 @@
public:
explicit NuvotonPciBridge(const PciAccess* pci) :
PciAccessBridge(&match, bar, dataOffset, dataLength, pci)
- {}
+ {
+ enableBridge();
+ }
~NuvotonPciBridge()
- {}
+ {
+ disableBridge();
+ }
private:
static constexpr std::uint32_t vid = 0x1050;
@@ -103,6 +107,9 @@
static constexpr std::size_t dataOffset = 0x0;
static constexpr std::size_t dataLength = 0x4000;
+
+ void enableBridge();
+ void disableBridge();
};
class AspeedPciBridge : public PciAccessBridge
diff --git a/tools/pciaccess.cpp b/tools/pciaccess.cpp
index 8aced6f..0d2ae0b 100644
--- a/tools/pciaccess.cpp
+++ b/tools/pciaccess.cpp
@@ -45,6 +45,20 @@
return ::pci_device_probe(dev);
}
+int PciAccessImpl::pci_device_cfg_read_u8(struct pci_device* dev,
+ std::uint8_t* data,
+ pciaddr_t offset) const
+{
+ return ::pci_device_cfg_read_u8(dev, data, offset);
+}
+
+int PciAccessImpl::pci_device_cfg_write_u8(struct pci_device* dev,
+ std::uint8_t data,
+ pciaddr_t offset) const
+{
+ return ::pci_device_cfg_write_u8(dev, data, offset);
+}
+
int PciAccessImpl::pci_device_map_range(struct pci_device* dev, pciaddr_t base,
pciaddr_t size, unsigned map_flags,
void** addr) const
diff --git a/tools/pciaccess.hpp b/tools/pciaccess.hpp
index ea7d5b4..3b25594 100644
--- a/tools/pciaccess.hpp
+++ b/tools/pciaccess.hpp
@@ -21,6 +21,8 @@
#include <pciaccess.h>
} // extern "C"
+#include <cstdint>
+
namespace host_tool
{
@@ -38,6 +40,12 @@
virtual struct pci_device*
pci_device_next(struct pci_device_iterator* iter) const = 0;
virtual int pci_device_probe(struct pci_device* dev) const = 0;
+ virtual int pci_device_cfg_read_u8(struct pci_device* dev,
+ std::uint8_t* data,
+ pciaddr_t offset) const = 0;
+ virtual int pci_device_cfg_write_u8(struct pci_device* dev,
+ std::uint8_t data,
+ pciaddr_t offset) const = 0;
virtual int pci_device_map_range(struct pci_device* dev, pciaddr_t base,
pciaddr_t size, unsigned map_flags,
void** addr) const = 0;
@@ -61,6 +69,10 @@
struct pci_device*
pci_device_next(struct pci_device_iterator* iter) const override;
int pci_device_probe(struct pci_device* dev) const override;
+ int pci_device_cfg_read_u8(struct pci_device* dev, std::uint8_t* data,
+ pciaddr_t offset) const override;
+ int pci_device_cfg_write_u8(struct pci_device* dev, std::uint8_t data,
+ pciaddr_t offset) const override;
int pci_device_map_range(struct pci_device* dev, pciaddr_t base,
pciaddr_t size, unsigned map_flags,
void** addr) const override;
diff --git a/tools/test/pciaccess_mock.hpp b/tools/test/pciaccess_mock.hpp
index e717c74..a030a18 100644
--- a/tools/test/pciaccess_mock.hpp
+++ b/tools/test/pciaccess_mock.hpp
@@ -16,6 +16,12 @@
MOCK_CONST_METHOD1(pci_device_next,
struct pci_device*(struct pci_device_iterator*));
MOCK_CONST_METHOD1(pci_device_probe, int(struct pci_device*));
+ MOCK_CONST_METHOD3(pci_device_cfg_read_u8,
+ int(struct pci_device* dev, std::uint8_t* data,
+ pciaddr_t offset));
+ MOCK_CONST_METHOD3(pci_device_cfg_write_u8,
+ int(struct pci_device* dev, std::uint8_t data,
+ pciaddr_t offset));
MOCK_CONST_METHOD5(pci_device_map_range, int(struct pci_device*, pciaddr_t,
pciaddr_t, unsigned, void**));
MOCK_CONST_METHOD3(pci_device_unmap_range,
diff --git a/tools/test/tools_pci_unittest.cpp b/tools/test/tools_pci_unittest.cpp
index 00b3eb1..4c4f76a 100644
--- a/tools/test/tools_pci_unittest.cpp
+++ b/tools/test/tools_pci_unittest.cpp
@@ -68,6 +68,8 @@
public:
virtual const struct pci_id_match* getMatch() const = 0;
virtual struct pci_device getDevice() const = 0;
+ virtual void expectSetup(PciAccessMock& pciMock,
+ const struct pci_device& dev) const {};
virtual std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const = 0;
virtual std::string getName() const = 0;
};
@@ -94,6 +96,29 @@
return dev;
}
+ void expectSetup(PciAccessMock& pciMock,
+ const struct pci_device& dev) const override
+ {
+ static constexpr std::uint8_t defaultVal = 0x40;
+
+ InSequence in;
+
+ EXPECT_CALL(pciMock,
+ pci_device_cfg_read_u8(Eq(&dev), NotNull(), config))
+ .WillOnce(DoAll(SetArgPointee<1>(defaultVal), Return(0)));
+ EXPECT_CALL(pciMock, pci_device_cfg_write_u8(
+ Eq(&dev), defaultVal | bridgeEnabled, config))
+ .WillOnce(Return(0));
+
+ EXPECT_CALL(pciMock,
+ pci_device_cfg_read_u8(Eq(&dev), NotNull(), config))
+ .WillOnce(
+ DoAll(SetArgPointee<1>(defaultVal | bridgeEnabled), Return(0)));
+ EXPECT_CALL(pciMock,
+ pci_device_cfg_write_u8(Eq(&dev), defaultVal, config))
+ .WillOnce(Return(0));
+ }
+
std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const override
{
return std::make_unique<NuvotonPciBridge>(pci);
@@ -104,6 +129,11 @@
return "Nuvoton"s;
}
+ /* Offset to the config register */
+ static constexpr int config = 0x04;
+ /* Second bit determines whether bridge is enabled */
+ static constexpr std::uint8_t bridgeEnabled = 0x02;
+
private:
static constexpr struct pci_id_match match
{
@@ -253,6 +283,7 @@
mockRegionSize))
.WillOnce(Return(EFAULT));
+ GetParam()->expectSetup(pciMock, dev);
// This will print an error but not throw
GetParam()->getBridge(&pciMock);
}
@@ -260,7 +291,7 @@
/* 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)
+ std::uint8_t* region, bool deviceExpectations = true)
{
EXPECT_CALL(pciMock,
pci_id_match_iterator_create(PciIdMatch(param->getMatch())))
@@ -281,6 +312,9 @@
EXPECT_CALL(pciMock,
pci_device_unmap_range(Eq(&dev), Eq(region), mockRegionSize))
.WillOnce(Return(0));
+
+ if (deviceExpectations)
+ param->expectSetup(pciMock, dev);
}
/* Test finding device and mapping memory region */
@@ -361,6 +395,161 @@
EXPECT_EQ(bridge->getDataLength(), 0x4000);
}
+/* Make sure config register is left alone if the bridge is already enabled */
+TEST(NuvotonBridgeTest, AlreadyEnabledSuccess)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ constexpr std::uint8_t defaultVal = 0x40;
+
+ /* Only set standard expectations; not those from nuvotonDevice */
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data(), false);
+
+ {
+ InSequence in;
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(DoAll(
+ SetArgPointee<1>(defaultVal | NuvotonDevice::bridgeEnabled),
+ Return(0)));
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(DoAll(
+ SetArgPointee<1>(defaultVal | NuvotonDevice::bridgeEnabled),
+ Return(0)));
+ EXPECT_CALL(pciMock, pci_device_cfg_write_u8(Eq(&dev), defaultVal,
+ NuvotonDevice::config))
+ .WillOnce(Return(0));
+ }
+
+ nuvotonDevice.getBridge(&pciMock);
+}
+
+/* Read fails when attempting to setup the bridge */
+TEST(NuvotonBridgeTest, ReadSetupFail)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ /* Only set standard expectations; not those from nuvotonDevice */
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data(), false);
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(Return(EFAULT));
+
+ EXPECT_THROW(nuvotonDevice.getBridge(&pciMock), std::system_error);
+}
+
+/* Write fails when attempting to setup the bridge */
+TEST(NuvotonBridgeTest, WriteSetupFail)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ constexpr std::uint8_t defaultVal = 0x40;
+
+ /* Only set standard expectations; not those from nuvotonDevice */
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data(), false);
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(DoAll(SetArgPointee<1>(defaultVal), Return(0)));
+ EXPECT_CALL(pciMock,
+ pci_device_cfg_write_u8(
+ Eq(&dev), defaultVal | NuvotonDevice::bridgeEnabled,
+ NuvotonDevice::config))
+ .WillOnce(Return(EFAULT));
+
+ EXPECT_THROW(nuvotonDevice.getBridge(&pciMock), std::system_error);
+}
+
+/* Read fails when attempting to disable the bridge */
+TEST(NuvotonBridgeTest, ReadDisableFail)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ constexpr std::uint8_t defaultVal = 0x40;
+
+ /* Only set standard expectations; not those from nuvotonDevice */
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data(), false);
+
+ {
+ InSequence in;
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(DoAll(SetArgPointee<1>(defaultVal), Return(0)));
+ EXPECT_CALL(pciMock,
+ pci_device_cfg_write_u8(
+ Eq(&dev), defaultVal | NuvotonDevice::bridgeEnabled,
+ NuvotonDevice::config))
+ .WillOnce(Return(0));
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(Return(EFAULT));
+ }
+
+ nuvotonDevice.getBridge(&pciMock);
+}
+
+/* Write fails when attempting to disable the bridge */
+TEST(NuvotonBridgeTest, WriteDisableFail)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ constexpr std::uint8_t defaultVal = 0x40;
+
+ /* Only set standard expectations; not those from nuvotonDevice */
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data(), false);
+
+ {
+ InSequence in;
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(DoAll(SetArgPointee<1>(defaultVal), Return(0)));
+ EXPECT_CALL(pciMock,
+ pci_device_cfg_write_u8(
+ Eq(&dev), defaultVal | NuvotonDevice::bridgeEnabled,
+ NuvotonDevice::config))
+ .WillOnce(Return(0));
+
+ EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+ NuvotonDevice::config))
+ .WillOnce(DoAll(
+ SetArgPointee<1>(defaultVal | NuvotonDevice::bridgeEnabled),
+ Return(0)));
+ EXPECT_CALL(pciMock, pci_device_cfg_write_u8(Eq(&dev), defaultVal,
+ NuvotonDevice::config))
+ .WillOnce(Return(EFAULT));
+ }
+
+ nuvotonDevice.getBridge(&pciMock);
+}
+
+/* Make sure the bridge gets enabled when needed */
+TEST(NuvotonBridgeTest, NotEnabledSuccess)
+{
+ PciAccessMock pciMock;
+ struct pci_device dev;
+ std::vector<std::uint8_t> region(mockRegionSize);
+
+ expectSetup(pciMock, dev, &nuvotonDevice, region.data());
+ nuvotonDevice.getBridge(&pciMock);
+}
+
TEST(AspeedWriteTest, TooLarge)
{
PciAccessMock pciMock;