legacy_platform_drv: configurable PCI BAR remapping

If PCI devices happen to miss complete configuration after boot, the
platform driver supports <pci-fixup> nodes for concrete devices
(specified by bus-device-functions tuples). The
<bar> node instructs the platform driver to remap BAR id 0 to address
0x4017002000, which amends the BIOS configuration and is stringently
required for BARs with address 0.

! <pci-fixup bus="0" device="0x15" function="3">
!   <bar id="0" address="0x4017002000"/>
! </pci-fixup>

The issue was discovered with Intel LPSS devices in Fujitsu notebooks.

Fixes #4501

repos/os/src/drivers/platform/legacy/x86/README

@@ -151,6 +151,20 @@ USB4       0c  03  40
 VGA        03  00  00
 WIFI       02  80   -
 
+Fixups for insufficient PCI BAR configuration
+---------------------------------------------
+
+If PCI devices happen to miss complete configuration after boot, the
+platform driver supports <pci-fixup> nodes for concrete devices
+(specified by bus-device-functions tuples). As depicted below, the
+<bar> node instructs the platform driver to remap BAR id 0 to address
+0x4017002000, which amends the BIOS configuration and is stringently
+required for BARs with address 0.
+
+!<pci-fixup bus="0" device="0x15" function="3">
+!  <bar id="0" address="0x4017002000"/>
+!</pci-fixup>
+
 
 Supported non PCI devices
 -------------------------

repos/os/src/drivers/platform/legacy/x86/pci_config_access.h

@@ -34,6 +34,14 @@ struct Platform::Pci::Bdf
 		             .function =  bdf       & 0x07u };
 	}
 
+	static Bdf from_xml(Xml_node node)
+	{
+		return Bdf { .bus      = node.attribute_value("bus",      0U),
+		             .device   = node.attribute_value("device",   0U),
+		             .function = node.attribute_value("function", 0U) };
+	}
+
+
 	uint16_t value() const {
 		return ((bus & 0xff) << 8) | ((device & 0x1f) << 3) | (function & 7); }
 
@@ -43,8 +51,8 @@ struct Platform::Pci::Bdf
 	void print(Output &out) const
 	{
 		using Genode::print;
-		print(out, Hex(bus, Hex::Prefix::OMIT_PREFIX, Hex::Pad::PAD),
+		print(out, Hex((uint8_t)bus, Hex::Prefix::OMIT_PREFIX, Hex::Pad::PAD),
-		      ":", Hex(device, Hex::Prefix::OMIT_PREFIX, Hex::Pad::PAD),
+		      ":", Hex((uint8_t)device, Hex::Prefix::OMIT_PREFIX, Hex::Pad::PAD),
 		      ".", Hex(function, Hex::Prefix::OMIT_PREFIX));
 	}
 };

repos/os/src/drivers/platform/legacy/x86/pci_device_config.h

@@ -69,6 +69,7 @@ class Platform::Pci::Resource
 
 	bool valid()    const { return !!_bar[0]; } /* no base address -> invalid */
 	bool mem()      const { return Bar::mem(_bar[0]); }
+	bool mem64()    const { return mem() && Bar::mem64(_bar[0]); }
 	uint64_t base() const { return mem() ? Bar::mem_address(_bar[0], _bar[1])
 	                                     : Bar::port_address(_bar[0]); }
 	uint64_t size() const { return _size; }
@@ -85,6 +86,9 @@ class Platform::Pci::Resource
 	void print(Output &out) const
 	{
 		Genode::print(out, Hex_range(base(), size()));
+		Genode::print(out, " (");
+		Genode::print(out, valid() ? mem() ? Bar::mem64(_bar[0]) ? "MEM64" : "MEM" : "IO" : "invalid");
+		Genode::print(out, ")");
 	}
 };
 
@@ -115,7 +119,7 @@ namespace Platform {
 
 			Platform::Pci::Resource _resource[Device::NUM_RESOURCES];
 
-			bool _resource_id_is_valid(int resource_id)
+			bool _resource_id_is_valid(int resource_id) const
 			{
 				/*
 				 * The maximum number of PCI resources depends on the
@@ -153,11 +157,14 @@ namespace Platform {
 				};
 			};
 
-			struct Device_bars {
+			struct Device_bars
+			{
 				Pci::Bdf bdf;
+
 				uint32_t bar_addr[Device::NUM_RESOURCES] { };
 
-				bool all_invalid() const {
+				bool all_invalid() const
+				{
 					for (unsigned i = 0; i < Device::NUM_RESOURCES; i++) {
 						if (bar_addr[i] != 0 && bar_addr[i] != ~0U)
 							return false;
@@ -219,26 +226,30 @@ namespace Platform {
 					/* index of base-address register in configuration space */
 					unsigned const bar_idx = 0x10 + 4 * i;
 
-					/* read base-address register value */
+					/* First, save initial base-address register value. */
-					unsigned const bar_value =
+					unsigned const bar_value = pci_config->read(bdf, bar_idx, Device::ACCESS_32BIT);
-						pci_config->read(bdf, bar_idx, Device::ACCESS_32BIT);
+
+					/*
+					 * Second, determine resource size (and validity) by writing
+					 * a magic value (all bits set) to the base-address
+					 * register. In response, the device clears a number of
+					 * lowest-significant bits corresponding to the resource
+					 * size.
+					 */
+					pci_config->write(bdf, bar_idx, ~0, Device::ACCESS_32BIT);
+					unsigned const bar_size = pci_config->read(bdf, bar_idx, Device::ACCESS_32BIT);
 
 					/* skip invalid resource BARs */
-					if (bar_value == ~0U || bar_value == 0U) {
+					if (bar_value == ~0U || bar_size == 0U) {
 						_resource[i] = Resource();
 						++i;
 						continue;
 					}
 
 					/*
-					 * Determine resource size by writing a magic value (all
+					 * Finally, we write back the bar-address value as assigned
-					 * bits set) to the base-address register. In response, the
+					 * by the BIOS.
-					 * device clears a number of lowest-significant bits
-					 * corresponding to the resource size. Finally, we write
-					 * back the bar-address value as assigned by the BIOS.
 					 */
-					pci_config->write(bdf, bar_idx, ~0, Device::ACCESS_32BIT);
-					unsigned const bar_size = pci_config->read(bdf, bar_idx, Device::ACCESS_32BIT);
 					pci_config->write(bdf, bar_idx, bar_value, Device::ACCESS_32BIT);
 
 					if (!Resource::Bar::mem64(bar_value)) {
@@ -300,6 +311,58 @@ namespace Platform {
 				return _resource[resource_id];
 			}
 
+			void remap_resource(Config_access &config, int const id,
+			                    uint64_t const base_address)
+			{
+				if (!_resource_id_is_valid(id))
+					return;
+
+				using Pci::Resource;
+
+				Resource &res = _resource[id];
+
+				log(*this, " remap BAR", id, " ", res, " to ", Hex(base_address));
+
+				struct Resource_params { uint32_t bar; uint32_t size; };
+
+				auto update_bar = [&] (int const id, uint32_t const address) {
+					unsigned const off = 0x10 + 4 * id;
+
+					config.write(_bdf, off, ~0U, Device::ACCESS_32BIT);
+
+					uint32_t const size = config.read(_bdf, off, Device::ACCESS_32BIT);
+
+					config.write(_bdf, off, address, Device::ACCESS_32BIT);
+
+					return Resource_params {
+						.bar  = config.read(_bdf, off, Device::ACCESS_32BIT),
+						.size = size
+					};
+				};
+
+				Resource_params const bar0 = update_bar(id, base_address & 0xffffffff);
+
+				if (!res.mem64()) {
+					res = Resource(bar0.bar, bar0.size);
+					return;
+				}
+
+				Resource_params const bar1 = update_bar(id + 1, (base_address >> 32) & 0xffffffff);
+
+				res = Resource(bar0.bar, bar0.size, bar1.bar, bar1.size);
+			}
+
+			template <typename FN> void for_each_resource(FN const &fn) const
+			{
+				for (unsigned r = 0; r < Device::NUM_RESOURCES; r++) {
+					if (!_resource_id_is_valid(r))
+						break;
+
+					if (_resource[r].valid())
+						fn(r, _resource[r]);
+				}
+			}
+
 			/**
 			 * Read configuration space
 			 */

repos/os/src/drivers/platform/legacy/x86/pci_session_component.h

@@ -71,8 +71,8 @@ class Platform::Rmrr : public List<Platform::Rmrr>::Element
 {
 	public:
 
-		class Bdf : public List<Bdf>::Element {
+		class Bdf : public List<Bdf>::Element
-
+		{
 			private:
 
 				uint8_t _bus, _dev, _func;
@@ -137,8 +137,8 @@ class Platform::Pci_buses
 
 		Bit_array<Device_config::MAX_BUSES> _valid { };
 
-		void scan_bus(Config_access &, Allocator &, Device_bars_pool &,
+		void _scan_bus(Config_access &, Allocator &, Device_bars_pool &,
-		              unsigned char bus = 0);
+		               unsigned char bus, Xml_node const &config);
 
 		bool _bus_valid(int bus)
 		{
@@ -152,10 +152,11 @@ class Platform::Pci_buses
 
 		Pci_buses(Allocator &heap,
 		          Attached_io_mem_dataspace &pciconf,
-		          Device_bars_pool &devices_bars)
+		          Device_bars_pool &devices_bars,
+		          Xml_node const &config_node)
 		{
 			Config_access c(pciconf);
-			scan_bus(c, heap, devices_bars);
+			_scan_bus(c, heap, devices_bars, 0 /* root bus */, config_node);
 		}
 
 		/**
@@ -365,13 +366,6 @@ class Platform::Session_component : public Rpc_object<Session>
 			    && node.has_attribute("function");
 		}
 
-		static Pci::Bdf _bdf_from_xml(Xml_node node)
-		{
-			return Pci::Bdf { .bus      = node.attribute_value("bus",      0U),
-			                  .device   = node.attribute_value("device",   0U),
-			                  .function = node.attribute_value("function", 0U) };
-		}
-
 		static bool _bdf_attributes_in_valid_range(Xml_node const &node)
 		{
 			return _bdf_exactly_specified(node)
@@ -382,7 +376,7 @@ class Platform::Session_component : public Rpc_object<Session>
 
 		static bool _bdf_matches(Xml_node const &node, Pci::Bdf const &bdf)
 		{
-			return _bdf_from_xml(node) == bdf;
+			return Pci::Bdf::from_xml(node) == bdf;
 		}
 
 		/**
@@ -586,7 +580,7 @@ class Platform::Session_component : public Rpc_object<Session>
 						throw Service_denied();
 					}
 
-					Pci::Bdf const bdf = _bdf_from_xml(node);
+					Pci::Bdf const bdf = Pci::Bdf::from_xml(node);
 
 					enum { DOUBLET = false };
 					if (find_dev_in_policy(bdf, DOUBLET)) {
@@ -1067,7 +1061,7 @@ class Platform::Root : public Root_component<Session_component>
 			/* try surviving wrong ACPI ECAM/MMCONF table information */
 			while (true) {
 				try {
-					_buses.construct(_heap, *_pci_confspace, _devices_bars);
+					_buses.construct(_heap, *_pci_confspace, _devices_bars, _config.xml());
 					/* construction and scan succeeded */
 					break;
 				} catch (Platform::Config_access::Invalid_mmio_access) {

repos/os/src/drivers/platform/legacy/x86/session.cc

@@ -38,10 +38,11 @@ unsigned short Platform::bridge_bdf(unsigned char bus)
 	return Platform::Bridge::root_bridge_bdf;
 }
 
-void Platform::Pci_buses::scan_bus(Config_access &config_access,
+void Platform::Pci_buses::_scan_bus(Config_access &config_access,
-                                   Allocator &heap,
+                                    Allocator &heap,
-                                   Device_bars_pool &devices_bars,
+                                    Device_bars_pool &devices_bars,
-                                   unsigned char bus)
+                                    unsigned char bus,
+                                    Xml_node const &config_node)
 {
 	for (unsigned dev = 0; dev < Device_config::MAX_DEVICES; ++dev) {
 		for (unsigned fun = 0; fun < Device_config::MAX_FUNCTIONS; ++fun) {
@@ -51,12 +52,40 @@ void Platform::Pci_buses::scan_bus(Config_access &config_access,
 			/* read config space */
 			Device_config config(bdf, &config_access);
 
+			if (!config.valid())
+				continue;
+
+			/* apply fixups to BAR memory resources */
+			config.for_each_resource([&] (int const id, Platform::Pci::Resource const res)
+			{
+				uint64_t remap_address = 0;
+				config_node.for_each_sub_node("pci-fixup", [&] (Xml_node node) {
+					if (!node.has_attribute("bus")
+					 || !node.has_attribute("device")
+					 || !node.has_attribute("function")
+					 || !(bdf == Pci::Bdf::from_xml(node)))
+						return;
+
+					node.for_each_sub_node("bar", [&] (Xml_node node) {
+						if (node.attribute_value("id", (long)-1) == id)
+							remap_address = node.attribute_value("address", (uint64_t)0);
+					});
+				});
+
+				if (remap_address) {
+					config.remap_resource(config_access, id, 0x4017002000);
+					return;
+				}
+
+				if (!res.base() && res.mem())
+					warning(bdf, " BAR", id, " ", res,
+					        " has invalid base address - consider <pci-fixup>");
+			});
+
 			/* remember Device BARs required after power off and/or reset */
-			if (config.valid()) {
+			Device_config::Device_bars bars = config.save_bars();
-				Device_config::Device_bars bars = config.save_bars();
+			if (!bars.all_invalid())
-				if (!bars.all_invalid())
+				new (heap) Registered<Device_config::Device_bars>(devices_bars, bars);
-					new (heap) Registered<Device_config::Device_bars>(devices_bars, bars);
-			}
 
 			/*
 			 * Switch off PCI bus master DMA for some classes of devices,
@@ -85,9 +114,6 @@ void Platform::Pci_buses::scan_bus(Config_access &config_access,
 				}
 			}
 
-			if (!config.valid())
-				continue;
-
 			/*
 			 * There is at least one device on the current bus, so
 			 * we mark it as valid.
@@ -123,7 +149,7 @@ void Platform::Pci_buses::scan_bus(Config_access &config_access,
 				    Hex(sec_bus, Hex::Prefix::OMIT_PREFIX, Hex::Pad::PAD),
 				    ":00.0", !enabled ? " enabled" : "");
 
-				scan_bus(config_access, heap, devices_bars, sec_bus);
+				_scan_bus(config_access, heap, devices_bars, sec_bus, config_node);
 			}
 		}
 	}