From 14c7e57a8ae5779ddad79a24e1c20b7635547855 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Fri, 11 Nov 2022 06:36:20 -0500 Subject: [PATCH] fix parsing of IP parameters Signed-off-by: Steffen Vogel --- fpga/etc/vc707-xbar-pcie/vc707-xbar-pcie.json | 3 +- fpga/include/villas/fpga/core.hpp | 14 +++--- fpga/include/villas/fpga/ips/bram.hpp | 2 +- fpga/include/villas/fpga/ips/dma.hpp | 4 +- fpga/include/villas/fpga/ips/pcie.hpp | 2 +- fpga/include/villas/fpga/ips/switch.hpp | 2 +- fpga/include/villas/fpga/node.hpp | 3 +- fpga/lib/card.cpp | 9 ++-- fpga/lib/core.cpp | 32 ++++++------- fpga/lib/ips/bram.cpp | 18 +++---- fpga/lib/ips/dma.cpp | 47 ++++++++----------- fpga/lib/ips/pcie.cpp | 41 ++++++++-------- fpga/lib/ips/switch.cpp | 33 +++++++------ fpga/lib/node.cpp | 46 ++++++++---------- 14 files changed, 120 insertions(+), 136 deletions(-) diff --git a/fpga/etc/vc707-xbar-pcie/vc707-xbar-pcie.json b/fpga/etc/vc707-xbar-pcie/vc707-xbar-pcie.json index 4c56bd9d9..2fe40000b 100644 --- a/fpga/etc/vc707-xbar-pcie/vc707-xbar-pcie.json +++ b/fpga/etc/vc707-xbar-pcie/vc707-xbar-pcie.json @@ -1276,7 +1276,6 @@ "target": "axi_dma_0:S2MM", "name": "M04_AXIS" } - ], - "num_ports": 5 + ] } } diff --git a/fpga/include/villas/fpga/core.hpp b/fpga/include/villas/fpga/core.hpp index da86669dd..2659ebff8 100644 --- a/fpga/include/villas/fpga/core.hpp +++ b/fpga/include/villas/fpga/core.hpp @@ -312,17 +312,17 @@ protected: return villas::logging.get(getName()); } + // Configure IP instance from JSON config + virtual + void configure(Core &, json_t *) + { } + private: // Create a concrete IP instance virtual Core* create() = 0; - // Configure IP instance from JSON config - virtual bool configureJson(Core& /* ip */, json_t* /* json */) - { - return true; - } - - virtual void configurePollingMode(Core& /* ip */, PollingMode /* mode */) + virtual + void configurePollingMode(Core &, PollingMode) { } virtual Vlnv getCompatibleVlnv() const = 0; diff --git a/fpga/include/villas/fpga/ips/bram.hpp b/fpga/include/villas/fpga/ips/bram.hpp index 7dd55d9e3..14a4f652b 100644 --- a/fpga/include/villas/fpga/ips/bram.hpp +++ b/fpga/include/villas/fpga/ips/bram.hpp @@ -57,7 +57,7 @@ private: class BramFactory : public CoreFactory { public: - bool configureJson(Core &ip, json_t *json_ip); + void configure(Core &ip, json_t *cfg); Core* create() { diff --git a/fpga/include/villas/fpga/ips/dma.hpp b/fpga/include/villas/fpga/ips/dma.hpp index 83f25166f..1be671aee 100644 --- a/fpga/include/villas/fpga/ips/dma.hpp +++ b/fpga/include/villas/fpga/ips/dma.hpp @@ -176,8 +176,8 @@ public: return Vlnv("xilinx.com:ip:axi_dma:"); } - virtual bool - configureJson(Core& ip, json_t* json) override; + virtual + void configure(Core& ip, json_t* json) override; virtual void configurePollingMode(Core& ip, PollingMode mode) override diff --git a/fpga/include/villas/fpga/ips/pcie.hpp b/fpga/include/villas/fpga/ips/pcie.hpp index 09a2753e6..d7d825592 100644 --- a/fpga/include/villas/fpga/ips/pcie.hpp +++ b/fpga/include/villas/fpga/ips/pcie.hpp @@ -67,7 +67,7 @@ public: return "xilinx.com:ip:axi_pcie:"; } - bool configureJson(Core &ip, json_t *json_ip); + void configure(Core &ip, json_t *cfg); Core* create() { diff --git a/fpga/include/villas/fpga/ips/switch.hpp b/fpga/include/villas/fpga/ips/switch.hpp index de8b93357..f6bfba11f 100644 --- a/fpga/include/villas/fpga/ips/switch.hpp +++ b/fpga/include/villas/fpga/ips/switch.hpp @@ -78,7 +78,7 @@ public: return "xilinx.com:ip:axis_switch:"; } - bool configureJson(Core &ip, json_t *json_ip); + void configure(Core &ip, json_t *cfg); Core* create() { diff --git a/fpga/include/villas/fpga/node.hpp b/fpga/include/villas/fpga/node.hpp index b90ce1202..d0be942b9 100644 --- a/fpga/include/villas/fpga/node.hpp +++ b/fpga/include/villas/fpga/node.hpp @@ -168,7 +168,8 @@ class NodeFactory : public CoreFactory { public: using CoreFactory::CoreFactory; - virtual bool configureJson(Core &ip, json_t *json_ip); + virtual + void configure(Core &ip, json_t *cfg); }; } /* namespace ip */ diff --git a/fpga/lib/card.cpp b/fpga/lib/card.cpp index 39766629a..b8318b3a1 100644 --- a/fpga/lib/card.cpp +++ b/fpga/lib/card.cpp @@ -59,7 +59,8 @@ PCIeCardFactory::make(json_t *json, std::shared_ptr pci int affinity = 0; int polling = 0; - int ret = json_unpack(json_card, "{ s: o, s?: i, s?: b, s?: s, s?: s, s?: b }", + json_error_t err; + int ret = json_unpack_ex(json_card, &err, 0, "{ s: o, s?: i, s?: b, s?: s, s?: s, s?: b }", "ips", &json_ips, "affinity", &affinity, "do_reset", &do_reset, @@ -67,10 +68,8 @@ PCIeCardFactory::make(json_t *json, std::shared_ptr pci "id", &pci_id, "polling", &polling); - if (ret != 0) { - logger->warn("Cannot parse JSON config"); - continue; - } + if (ret != 0) + throw ConfigError(json_card, err, "", "Failed to parse card"); auto card = std::unique_ptr(create()); diff --git a/fpga/lib/core.cpp b/fpga/lib/core.cpp index cb08a16a8..b125cb230 100644 --- a/fpga/lib/core.cpp +++ b/fpga/lib/core.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -66,10 +67,11 @@ CoreFactory::make(PCIeCard* card, json_t *json_ips) json_t* json_ip; json_object_foreach(json_ips, ipName, json_ip) { const char* vlnv; - if (json_unpack(json_ip, "{ s: s }", "vlnv", &vlnv) != 0) { - loggerStatic->warn("IP {} has no VLNV", ipName); - continue; - } + + json_error_t err; + int ret = json_unpack_ex(json_ip, &err, 0, "{ s: s }", "vlnv", &vlnv); + if (ret != 0) + throw ConfigError(json_ip, err, "", "IP {} has no VLNV", ipName); allIps.push_back({vlnv, ipName}); } @@ -212,17 +214,16 @@ CoreFactory::make(PCIeCard* card, json_t *json_ips) json_object_foreach(json_instance, block_name, json_block) { json_int_t base, high, size; - int ret = json_unpack(json_block, "{ s: I, s: I, s: I }", - "baseaddr", &base, - "highaddr", &high, - "size", &size); - if (ret != 0) { - logger->error("Cannot parse address block {}/{}/{}/{}", + json_error_t err; + int ret = json_unpack_ex(json_block, &err, 0, "{ s: I, s: I, s: I }", + "baseaddr", &base, + "highaddr", &high, + "size", &size + ); + if (ret != 0) + throw ConfigError(json_block, err, "", "Cannot parse address block {}/{}/{}/{}", ip->getInstanceName(), bus_name, instance_name, block_name); - continue; - - } // Get or create the slave address space const std::string slaveAddrSpace = @@ -244,10 +245,7 @@ CoreFactory::make(PCIeCard* card, json_t *json_ips) } // IP-specific setup via JSON config - if (not CoreFactory->configureJson(*ip, json_ip)) { - logger->warn("Cannot configure IP from JSON"); - continue; - } + CoreFactory->configure(*ip, json_ip); // Set polling mode CoreFactory->configurePollingMode(*ip, (card->polling ? PollingMode::POLL : PollingMode::IRQ)); diff --git a/fpga/lib/ips/bram.cpp b/fpga/lib/ips/bram.cpp index 92ca57559..bff173b99 100644 --- a/fpga/lib/ips/bram.cpp +++ b/fpga/lib/ips/bram.cpp @@ -20,23 +20,25 @@ * along with this program. If not, see . *********************************************************************************/ +#include #include using namespace villas::fpga::ip; static BramFactory factory; -bool -BramFactory::configureJson(Core &ip, json_t* json_ip) +void BramFactory::configure(Core &ip, json_t* cfg) { + CoreFactory::configure(ip, cfg); + auto &bram = dynamic_cast(ip); - if (json_unpack(json_ip, "{ s: i }", "size", &bram.size) != 0) { - getLogger()->error("Cannot parse 'size'"); - return false; - } - - return true; + json_error_t err; + auto ret = json_unpack_ex(cfg, &err, 0, "{ s: i }", + "size", &bram.size + ); + if (ret != 0) + throw ConfigError(cfg, err, "", "Cannot parse BRAM config"); } bool Bram::init() diff --git a/fpga/lib/ips/dma.cpp b/fpga/lib/ips/dma.cpp index 62fe6c994..03a247b26 100644 --- a/fpga/lib/ips/dma.cpp +++ b/fpga/lib/ips/dma.cpp @@ -618,16 +618,11 @@ void Dma::dump() logger->info("MM2S_DMASR: {:x}", XAxiDma_ReadReg(xDma.RegBase, XAXIDMA_TX_OFFSET + XAXIDMA_SR_OFFSET)); } -bool -DmaFactory::configureJson(Core& ip, json_t* json) +void DmaFactory::configure(Core &ip, json_t *cfg) { - if (not NodeFactory::configureJson(ip, json)) - return false; + NodeFactory::configure(ip, cfg); auto &dma = dynamic_cast(ip); - json_t* json_params = json_object_get(json, "parameters"); - if (!json_is_object(json_params)) - return false; // Sensible default configuration dma.xConfig.HasStsCntrlStrm = 0; @@ -646,25 +641,23 @@ DmaFactory::configureJson(Core& ip, json_t* json) dma.xConfig.AddrWidth = 32; dma.xConfig.SgLengthWidth = 14; - int ret = json_unpack(json_params, "{ s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i }", - "c_sg_include_stscntrl_strm", &dma.xConfig.HasStsCntrlStrm, - "c_include_mm2s", &dma.xConfig.HasMm2S, - "c_include_mm2s_dre", &dma.xConfig.HasMm2SDRE, - "c_m_axi_mm2s_data_width", &dma.xConfig.Mm2SDataWidth, - "c_include_s2mm", &dma.xConfig.HasS2Mm, - "c_include_s2mm_dre", &dma.xConfig.HasS2MmDRE, - "c_m_axi_s2mm_data_width", &dma.xConfig.S2MmDataWidth, - "c_include_sg", &dma.xConfig.HasSg, - "c_num_mm2s_channels", &dma.xConfig.Mm2sNumChannels, - "c_num_s2mm_channels", &dma.xConfig.S2MmNumChannels, - "c_micro_dma", &dma.xConfig.MicroDmaMode, - "c_addr_width", &dma.xConfig.AddrWidth, - "c_sg_length_width", &dma.xConfig.SgLengthWidth + json_error_t err; + int ret = json_unpack_ex(cfg, &err, 0, "{ s: { s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i, s?: i } }", + "parameters", + "c_sg_include_stscntrl_strm", &dma.xConfig.HasStsCntrlStrm, + "c_include_mm2s", &dma.xConfig.HasMm2S, + "c_include_mm2s_dre", &dma.xConfig.HasMm2SDRE, + "c_m_axi_mm2s_data_width", &dma.xConfig.Mm2SDataWidth, + "c_include_s2mm", &dma.xConfig.HasS2Mm, + "c_include_s2mm_dre", &dma.xConfig.HasS2MmDRE, + "c_m_axi_s2mm_data_width", &dma.xConfig.S2MmDataWidth, + "c_include_sg", &dma.xConfig.HasSg, + "c_num_mm2s_channels", &dma.xConfig.Mm2sNumChannels, + "c_num_s2mm_channels", &dma.xConfig.S2MmNumChannels, + "c_micro_dma", &dma.xConfig.MicroDmaMode, + "c_addr_width", &dma.xConfig.AddrWidth, + "c_sg_length_width", &dma.xConfig.SgLengthWidth ); - if (ret != 0) { - logger->error("Failed to parse DMA configuration"); - return false; - } - dma.configDone = true; - return true; + if (ret != 0) + throw ConfigError(cfg, err, "", "Failed to parse DMA configuration"); } diff --git a/fpga/lib/ips/pcie.cpp b/fpga/lib/ips/pcie.cpp index 1b9e2ab4b..2edf530a0 100644 --- a/fpga/lib/ips/pcie.cpp +++ b/fpga/lib/ips/pcie.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -111,9 +112,11 @@ AxiPciExpressBridge::init() return true; } -bool -AxiPciExpressBridgeFactory::configureJson(Core &ip, json_t* json_ip) +void +AxiPciExpressBridgeFactory::configure(Core &ip, json_t *cfg) { + CoreFactory::configure(ip, cfg); + auto logger = getLogger(); auto &pcie = dynamic_cast(ip); @@ -121,31 +124,29 @@ AxiPciExpressBridgeFactory::configureJson(Core &ip, json_t* json_ip) "axi_bars", "pcie_bars" }) { - json_t* json_bars = json_object_get(json_ip, barType.c_str()); - if (not json_is_object(json_bars)) { - return false; - } + json_t *json_bars = json_object_get(cfg, barType.c_str()); + if (not json_is_object(json_bars)) + throw ConfigError(cfg, "", "Missing BAR config: {}", barType); json_t* json_bar; const char* bar_name; json_object_foreach(json_bars, bar_name, json_bar) { unsigned int translation; - int ret = json_unpack(json_bar, "{ s: i }", "translation", &translation); - if (ret != 0) { - logger->error("Cannot parse {}/{}", barType, bar_name); - return false; - } + + json_error_t err; + int ret = json_unpack_ex(json_bar, &err, 0, "{ s: i }", "translation", &translation); + if (ret != 0) + throw ConfigError(json_bar, err, "", "Cannot parse {}/{}", barType, bar_name); if (barType == "axi_bars") { json_int_t base, high, size; - int ret = json_unpack(json_bar, "{ s: I, s: I, s: I }", - "baseaddr", &base, - "highaddr", &high, - "size", &size); - if (ret != 0) { - logger->error("Cannot parse {}/{}", barType, bar_name); - return false; - } + int ret = json_unpack_ex(json_bar, &err, 0, "{ s: I, s: I, s: I }", + "baseaddr", &base, + "highaddr", &high, + "size", &size + ); + if (ret != 0) + throw ConfigError(json_bar, err, "", "Cannot parse {}/{}", barType, bar_name); pcie.axiToPcieTranslations[bar_name] = { .base = static_cast(base), @@ -158,6 +159,4 @@ AxiPciExpressBridgeFactory::configureJson(Core &ip, json_t* json_ip) }; } } - - return true; } diff --git a/fpga/lib/ips/switch.cpp b/fpga/lib/ips/switch.cpp index f598ee320..0f06650b7 100644 --- a/fpga/lib/ips/switch.cpp +++ b/fpga/lib/ips/switch.cpp @@ -26,6 +26,7 @@ #include #include +#include #include namespace villas { @@ -34,15 +35,9 @@ namespace ip { static AxiStreamSwitchFactory factory; -bool -AxiStreamSwitch::init() +bool AxiStreamSwitch::init() { - // Setup AXI-stream switch - XAxis_Switch_Config sw_cfg; - sw_cfg.MaxNumMI = num_ports; - sw_cfg.MaxNumSI = num_ports; - - if (XAxisScr_CfgInitialize(&xSwitch, &sw_cfg, getBaseAddr(registerMemory)) != XST_SUCCESS) { + if (XAxisScr_CfgInitialize(&xSwitch, &xConfig, getBaseAddr(registerMemory)) != XST_SUCCESS) { logger->error("Cannot initialize switch"); return false; } @@ -135,22 +130,26 @@ AxiStreamSwitch::portNameToNum(const std::string &portName) return std::stoi(number); } -bool -AxiStreamSwitchFactory::configureJson(Core &ip, json_t* json_ip) +void AxiStreamSwitchFactory::configure(Core &ip, json_t *cfg) { - if (not NodeFactory::configureJson(ip, json_ip)) - return false; + NodeFactory::configure(ip, cfg); auto logger = getLogger(); auto &axiSwitch = dynamic_cast(ip); - if (json_unpack(json_ip, "{ s: i }", "num_ports", &axiSwitch.num_ports) != 0) { - logger->error("Cannot parse 'num_ports'"); - return false; - } + int num_si, num_mi; + json_error_t err; + auto ret = json_unpack_ex(cfg, &err, 0, "{ s: { s: i, s: i } }", + "parameters", + "num_si", &num_si, + "num_mi", &num_mi + ); + if (ret != 0) + throw ConfigError(cfg, err, "", "Cannot parse switch config"); - return true; + axiSwitch.xConfig.MaxNumMI = num_mi; + axiSwitch.xConfig.MaxNumSI = num_si; } } /* namespace ip */ diff --git a/fpga/lib/node.cpp b/fpga/lib/node.cpp index f05d32d22..8824e41a6 100644 --- a/fpga/lib/node.cpp +++ b/fpga/lib/node.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -35,41 +36,36 @@ using namespace villas::fpga::ip; StreamGraph Node::streamGraph; -bool -NodeFactory::configureJson(Core &ip, json_t* json_ip) +void NodeFactory::configure(Core &ip, json_t *cfg) { + CoreFactory::configure(ip, cfg); + auto &Node = dynamic_cast(ip); auto logger = getLogger(); - json_t* json_ports = json_object_get(json_ip, "ports"); - if (not json_is_array(json_ports)) { - logger->debug("IP has no ports"); - return true; - } + json_t* json_ports = json_object_get(cfg, "ports"); + if (not json_is_array(json_ports)) + throw ConfigError(json_ports, "", "IP port list must be an array"); size_t index; - json_t* json_port; + json_t *json_port; json_array_foreach(json_ports, index, json_port) { - if (not json_is_object(json_port)) { - logger->error("Port {} is not an object", index); - return false; - } + if (not json_is_object(json_port)) + throw ConfigError(json_port, "", "Port {} is not an object", index); const char *role_raw, *target_raw, *name_raw; - int ret = json_unpack(json_port, "{ s: s, s: s, s: s }", - "role", &role_raw, - "target", &target_raw, - "name", &name_raw); - if (ret != 0) { - logger->error("Cannot parse port {}", index); - return false; - } + + json_error_t err; + int ret = json_unpack_ex(json_port, &err, 0, "{ s: s, s: s, s: s }", + "role", &role_raw, + "target", &target_raw, + "name", &name_raw); + if (ret != 0) + throw ConfigError(json_port, err, "", "Cannot parse port {}", index); const auto tokens = utils::tokenize(target_raw, ":"); - if (tokens.size() != 2) { - logger->error("Cannot parse 'target' of port {}", index); - return false; - } + if (tokens.size() != 2) + throw ConfigError(json_port, err, "", "Cannot parse 'target' of port {}", index); const std::string role(role_raw); const bool isMaster = (role == "master" or role == "initiator"); @@ -92,8 +88,6 @@ NodeFactory::configureJson(Core &ip, json_t* json_ip) else // Slave Node.portsSlave[name_raw] = thisVertex; } - - return true; } std::pair