pci: deny access to registers used by pci driver

Fixes #1532

repos/os/src/drivers/pci/pci_config_access.h

@@ -67,6 +67,15 @@ namespace Pci {
 				         (function <<  8) |
 				         (addr      & ~3) ); }
 
+			Genode::Bit_array<256> _used;
+
+			void _use_register(unsigned char addr, unsigned short width)
+			{
+				for (unsigned i = 0; i < width; i++)
+					if (!_used.get(addr + i, 1))
+						_used.set(addr + i, 1);
+			}
+
 		public:
 
 			/**
@@ -82,18 +91,32 @@ namespace Pci {
 			 *
 			 * There is no range check for the input values.
 			 */
-			unsigned read(int bus, int device, int function, int addr,
+			unsigned read(int bus, int device, int function,
-			              Device::Access_size size = Device::ACCESS_32BIT)
+			              unsigned char addr, Device::Access_size size,
+			              bool track = true)
 			{
 				/* write target address */
 				_io_port<REG_ADDR>()->outl(REG_ADDR, _cfg_addr(bus, device, function, addr));
 
 				/* return read value */
 				switch (size) {
-				case Device::ACCESS_8BIT:  return _io_port<REG_DATA>()->inb(REG_DATA + (addr & 3));
+				case Device::ACCESS_8BIT:
-				case Device::ACCESS_16BIT: return _io_port<REG_DATA>()->inw(REG_DATA + (addr & 2));
+					if (track)
-				case Device::ACCESS_32BIT: return _io_port<REG_DATA>()->inl(REG_DATA);
+						_use_register(addr, 1);
-				default:                   return ~0;
+
+					return _io_port<REG_DATA>()->inb(REG_DATA + (addr & 3));
+				case Device::ACCESS_16BIT:
+					if (track)
+						_use_register(addr, 2);
+
+					return _io_port<REG_DATA>()->inw(REG_DATA + (addr & 2));
+				case Device::ACCESS_32BIT:
+					if (track)
+						_use_register(addr, 4);
+
+					return _io_port<REG_DATA>()->inl(REG_DATA);
+				default:
+					return ~0U;
 				}
 			}
 
@@ -109,25 +132,50 @@ namespace Pci {
 			 *
 			 * There is no range check for the input values.
 			 */
-			void write(int bus, int device, int function, int addr, unsigned value,
+			void write(int bus, int device, int function, unsigned char addr,
-			           Device::Access_size size = Device::ACCESS_32BIT)
+			           unsigned value, Device::Access_size size,
+			           bool track = true)
 			{
 				/* write target address */
-				_io_port<REG_ADDR>()->outl(REG_ADDR, _cfg_addr(bus, device, function, addr));
+				_io_port<REG_ADDR>()->outl(REG_ADDR, _cfg_addr(bus, device,
+				                                               function, addr));
 
 				/* write value to targeted address */
 				switch (size) {
 				case Device::ACCESS_8BIT:
+					if (track)
+						_use_register(addr, 1);
+
 					_io_port<REG_DATA>()->outb(REG_DATA + (addr & 3), value);
 					break;
 				case Device::ACCESS_16BIT:
+					if (track)
+						_use_register(addr, 2);
+
 					_io_port<REG_DATA>()->outw(REG_DATA + (addr & 2), value);
 					break;
 				case Device::ACCESS_32BIT:
+					if (track)
+						_use_register(addr, 4);
+
 					_io_port<REG_DATA>()->outl(REG_DATA, value);
 					break;
 				}
 			}
+
+			bool reg_in_use(unsigned char addr, Device::Access_size size)
+			{
+				switch (size) {
+				case Device::ACCESS_8BIT:
+					return _used.get(addr, 1);
+				case Device::ACCESS_16BIT:
+					return _used.get(addr, 2);
+				case Device::ACCESS_32BIT:
+					return _used.get(addr, 4);
+				default:
+					return true;
+				}
+			}
 	};
 }
 

repos/os/src/drivers/pci/pci_device.cc

@@ -66,8 +66,16 @@ void Pci::Device_component::config_write(unsigned char address, unsigned value,
 {
 	/* white list of ports which we permit to write */
 	switch (address) {
-		case 0x40 ... 0xff: /* allow access to device-specific registers */
+		case 0x40 ... 0xff:
-			break;
+			/* allow access to device-specific registers if not used by us */
+			if (!_device_config.reg_in_use(&_config_access, address, size))
+				break;
+
+			PERR("%x:%x:%x write access to address=%x value=0x%x"
+			     " size=0x%x denied - it is used by the platform driver.",
+			     _device_config.bus_number(), _device_config.device_number(),
+			     _device_config.function_number(), address, value, size);
+			return;
 		case PCI_CMD_REG: /* COMMAND register - first byte */
 			if (size == Access_size::ACCESS_16BIT)
 				break;
@@ -93,6 +101,7 @@ void Pci::Device_component::config_write(unsigned char address, unsigned value,
 	if (address == PCI_CMD_REG && value & PCI_CMD_DMA && _session)
 		_session->assign_device(this);
 
-	_device_config.write(&_config_access, address, value, size);
+	_device_config.write(&_config_access, address, value, size,
+	                     _device_config.DONT_TRACK_ACCESS);
 }
 

repos/os/src/drivers/pci/pci_device_component.h

@@ -318,7 +318,8 @@ class Pci::Device_component : public Genode::Rpc_object<Pci::Device>,
 
 		unsigned config_read(unsigned char address, Access_size size) override
 		{
-			return _device_config.read(&_config_access, address, size);
+			return _device_config.read(&_config_access, address, size,
+			                           _device_config.DONT_TRACK_ACCESS);
 		}
 
 		void config_write(unsigned char address, unsigned value,

repos/os/src/drivers/pci/pci_device_config.h

@@ -78,7 +78,7 @@ namespace Pci {
 					return;
 
 				_device_id    = pci_config->read(bus, device, function, 2, Device::ACCESS_16BIT);
-				_class_code   = pci_config->read(bus, device, function, 8) >> 8;
+				_class_code   = pci_config->read(bus, device, function, 8, Device::ACCESS_32BIT) >> 8;
 				_class_code  &= 0xffffff;
 				_header_type  = pci_config->read(bus, device, function, 0xe, Device::ACCESS_8BIT);
 				_header_type &= 0x7f;
@@ -101,7 +101,7 @@ namespace Pci {
 					unsigned bar_idx = 0x10 + 4 * i;
 
 					/* read original base-address register value */
-					unsigned orig_bar = pci_config->read(bus, device, function, bar_idx);
+					unsigned orig_bar = pci_config->read(bus, device, function, bar_idx, Device::ACCESS_32BIT);
 
 					/* check for invalid resource */
 					if (orig_bar == (unsigned)~0) {
@@ -115,9 +115,9 @@ namespace Pci {
 					 * of lowest-significant bits corresponding to the resource size.
 					 * Finally, we write back the original value as assigned by the BIOS.
 					 */
-					pci_config->write(bus, device, function, bar_idx, ~0);
+					pci_config->write(bus, device, function, bar_idx, ~0, Device::ACCESS_32BIT);
-					unsigned bar = pci_config->read(bus, device, function, bar_idx);
+					unsigned bar = pci_config->read(bus, device, function, bar_idx, Device::ACCESS_32BIT);
-					pci_config->write(bus, device, function, bar_idx, orig_bar);
+					pci_config->write(bus, device, function, bar_idx, orig_bar, Device::ACCESS_32BIT);
 
 					/*
 					 * Scan base-address-register value for the lowest set bit but
@@ -172,20 +172,28 @@ namespace Pci {
 			/**
 			 * Read configuration space
 			 */
+			enum { DONT_TRACK_ACCESS = false };
 			unsigned read(Config_access *pci_config, unsigned char address,
-			              Device::Access_size size)
+			              Device::Access_size size, bool track = true)
 			{
-				return pci_config->read(_bus, _device, _function, address, size);
+				return pci_config->read(_bus, _device, _function, address,
+				                        size, track);
 			}
 
 			/**
 			 * Write configuration space
 			 */
 			void write(Config_access *pci_config, unsigned char address,
-			           unsigned long value, Device::Access_size size)
+			           unsigned long value, Device::Access_size size,
+			           bool track = true)
 			{
-				pci_config->write(_bus, _device, _function, address, value, size);
+				pci_config->write(_bus, _device, _function, address, value,
+				                  size, track);
 			}
+
+			bool reg_in_use(Config_access *pci_config, unsigned char address,
+			                Device::Access_size size) {
+				return pci_config->reg_in_use(address, size); }
 	};
 
 	class Config_space : public Genode::List<Config_space>::Element