From Michael Lotz <mmlr@xxxxxxxx>:
Michael Lotz has uploaded this change for review. (
https://review.haiku-os.org/c/haiku/+/3022 ;)
Change subject: pci: Rework 64 bit base address register handling.
......................................................................
pci: Rework 64 bit base address register handling.
This moves the entire 64 bit handling into _GetBarInfo() and applies it
to the PCI address and size too. Previously only the RAM address was
handled for the 64 bit case.
Also fixes the 64 bit check to be done after the type determination.
Previously it would theoretically be possible for an IO BAR with the
lowest address bit set for that type (bit 3) to be mistaken to be 64 bit
and then skipping/mishandling the next BAR. Due to alignment this would
have needed to be a 4 byte IO BAR though.
This also corrects the limit for 64 bit BARs of type 1 devices. As there
are only two slots, only slot 0 can be 64 bit.
Also removes a copy&paste error that would lead to the high address of
64 bit BARs of type 1 devices to get taken from the h0 instead of h1
struct, corrupting its value.
Make the mandatory arguments to _GetBarInfo() references to make the
distinction more obvious and replace 0 with NULL in the default values.
---
M src/add-ons/kernel/bus_managers/pci/pci.cpp
M src/add-ons/kernel/bus_managers/pci/pci.h
2 files changed, 76 insertions(+), 59 deletions(-)
git pull ssh://git.haiku-os.org:22/haiku refs/changes/22/3022/1
diff --git a/src/add-ons/kernel/bus_managers/pci/pci.cpp
b/src/add-ons/kernel/bus_managers/pci/pci.cpp
index 61b0d0a..11a9935 100644
--- a/src/add-ons/kernel/bus_managers/pci/pci.cpp
+++ b/src/add-ons/kernel/bus_managers/pci/pci.cpp
@@ -1174,47 +1174,78 @@
}
-uint32
-PCI::_BarSize(uint32 bits, uint32 mask)
+uint64
+PCI::_BarSize(uint64 bits)
{
- bits &= mask;
if (!bits)
return 0;
- uint32 size = 1;
- while (!(bits & size))
+
+ uint64 size = 1;
+ while ((bits & size) == 0)
size <<= 1;
+
return size;
}
size_t
-PCI::_GetBarInfo(PCIDev *dev, uint8 offset, uint32 *_address, uint32 *_size,
- uint8 *_flags, uint32 *_highAddress)
+PCI::_GetBarInfo(PCIDev *dev, uint8 offset, uint32 &_ramAddress,
+ uint32 &_pciAddress, uint32 &_size, uint8 &flags, uint32
*_highRAMAddress,
+ uint32 *_highPCIAddress, uint32 *_highSize)
{
- uint32 oldValue = ReadConfig(dev->domain, dev->bus, dev->device,
dev->function,
- offset, 4);
+ uint64 pciAddress = ReadConfig(dev->domain, dev->bus, dev->device,
+ dev->function, offset, 4);
WriteConfig(dev->domain, dev->bus, dev->device, dev->function, offset,
4,
0xffffffff);
- uint32 newValue = ReadConfig(dev->domain, dev->bus, dev->device,
dev->function,
+ uint64 size = ReadConfig(dev->domain, dev->bus, dev->device,
dev->function,
offset, 4);
WriteConfig(dev->domain, dev->bus, dev->device, dev->function, offset,
4,
- oldValue);
+ pciAddress);
uint32 mask = PCI_address_memory_32_mask;
- bool is64bit = (oldValue & PCI_address_type_64) != 0;
- if ((oldValue & PCI_address_space) == PCI_address_space)
+ bool is64bit = false;
+ if ((pciAddress & PCI_address_space) != 0)
mask = PCI_address_io_mask;
- else if (is64bit && _highAddress != NULL) {
- *_highAddress = ReadConfig(dev->domain, dev->bus, dev->device,
- dev->function, offset + 4, 4);
+ else {
+ is64bit = (pciAddress & PCI_address_type) ==
PCI_address_type_64;
+
+ if (is64bit && _highRAMAddress != NULL) {
+ uint64 highPCIAddress = ReadConfig(dev->domain,
dev->bus,
+ dev->device, dev->function, offset + 4, 4);
+ WriteConfig(dev->domain, dev->bus, dev->device,
dev->function,
+ offset + 4, 4, 0xffffffff);
+ uint64 highSize = ReadConfig(dev->domain, dev->bus,
dev->device,
+ dev->function, offset + 4, 4);
+ WriteConfig(dev->domain, dev->bus, dev->device,
dev->function,
+ offset + 4, 4, highPCIAddress);
+
+ pciAddress |= highPCIAddress << 32;
+ size |= highSize << 32;
+ }
}
- *_address = oldValue & mask;
- if (_size != NULL)
- *_size = _BarSize(newValue, mask);
- if (_flags != NULL)
- *_flags = oldValue & ~mask;
- return is64bit ? 2 : 1;
+ flags = (uint32)pciAddress & ~mask;
+ pciAddress &= ((uint64)0xffffffff << 32) | mask;
+ size &= ((uint64)0xffffffff << 32) | mask;
+
+ size = _BarSize(size);
+ uint64 ramAddress = pci_ram_address(pciAddress);
+
+ _ramAddress = ramAddress;
+ _pciAddress = pciAddress;
+ _size = size;
+ if (!is64bit)
+ return 1;
+
+ if (_highRAMAddress == NULL || _highPCIAddress == NULL || _highSize ==
NULL)
+ panic("64 bit PCI BAR but no space to store high values\n");
+ else {
+ *_highRAMAddress = ramAddress >> 32;
+ *_highPCIAddress = pciAddress >> 32;
+ *_highSize = size >> 32;
+ }
+
+ return 2;
}
@@ -1233,7 +1264,7 @@
*_address = oldValue & PCI_rom_address_mask;
if (_size != NULL)
- *_size = _BarSize(newValue, PCI_rom_address_mask);
+ *_size = _BarSize(newValue & PCI_rom_address_mask);
if (_flags != NULL)
*_flags = newValue & 0xf;
}
@@ -1296,23 +1327,14 @@
_GetRomBarInfo(dev, PCI_rom_base,
&dev->info.u.h0.rom_base_pci,
&dev->info.u.h0.rom_size);
for (int i = 0; i < 6;) {
- size_t barSize = _GetBarInfo(dev,
PCI_base_registers + 4 * i,
- &dev->info.u.h0.base_registers_pci[i],
- &dev->info.u.h0.base_register_sizes[i],
- &dev->info.u.h0.base_register_flags[i],
- i < 5 ?
&dev->info.u.h0.base_registers_pci[i + 1] : NULL);
-
- phys_addr_t addr =
dev->info.u.h0.base_registers_pci[i];
- if (barSize == 2) {
- addr +=
((phys_addr_t)dev->info.u.h0.base_registers_pci[i + 1])
- << 32;
- }
- addr = pci_ram_address(addr);
- dev->info.u.h0.base_registers[i] = (uint32)addr;
- if (barSize == 2)
- dev->info.u.h0.base_registers[i + 1] =
(uint32)(addr >> 32);
-
- i += barSize;
+ i += _GetBarInfo(dev, PCI_base_registers + 4 *
i,
+ dev->info.u.h0.base_registers[i],
+ dev->info.u.h0.base_registers_pci[i],
+ dev->info.u.h0.base_register_sizes[i],
+ dev->info.u.h0.base_register_flags[i],
+ i < 5 ?
&dev->info.u.h0.base_registers[i + 1] : NULL,
+ i < 5 ?
&dev->info.u.h0.base_registers_pci[i + 1] : NULL,
+ i < 5 ?
&dev->info.u.h0.base_register_sizes[i + 1] : NULL);
}
// restore PCI device address decoding
@@ -1351,22 +1373,14 @@
_GetRomBarInfo(dev, PCI_bridge_rom_base,
&dev->info.u.h1.rom_base_pci);
for (int i = 0; i < 2;) {
- size_t barSize = _GetBarInfo(dev,
PCI_base_registers + 4 * i,
- &dev->info.u.h1.base_registers_pci[i],
- &dev->info.u.h1.base_register_sizes[i],
- &dev->info.u.h1.base_register_flags[i],
- i < 2 ?
&dev->info.u.h1.base_registers_pci[i + 1] : NULL);
-
- phys_addr_t addr =
dev->info.u.h1.base_registers_pci[i];
- if (barSize == 2) {
- addr +=
((phys_addr_t)dev->info.u.h0.base_registers_pci[i + 1])
- << 32;
- }
- addr = pci_ram_address(addr);
- dev->info.u.h1.base_registers[i] = (uint32)addr;
- if (barSize == 2)
- dev->info.u.h1.base_registers[i + 1] =
(uint32)(addr >> 32);
- i += barSize;
+ i += _GetBarInfo(dev, PCI_base_registers + 4 *
i,
+ dev->info.u.h1.base_registers[i],
+ dev->info.u.h1.base_registers_pci[i],
+ dev->info.u.h1.base_register_sizes[i],
+ dev->info.u.h1.base_register_flags[i],
+ i < 1 ?
&dev->info.u.h1.base_registers[i + 1] : NULL,
+ i < 1 ?
&dev->info.u.h1.base_registers_pci[i + 1] : NULL,
+ i < 1 ?
&dev->info.u.h1.base_register_sizes[i + 1] : NULL);
}
// restore PCI device address decoding
diff --git a/src/add-ons/kernel/bus_managers/pci/pci.h
b/src/add-ons/kernel/bus_managers/pci/pci.h
index 6b4d1a8..435a2a6 100644
--- a/src/add-ons/kernel/bus_managers/pci/pci.h
+++ b/src/add-ons/kernel/bus_managers/pci/pci.h
@@ -146,10 +146,13 @@
void _ConfigureBridges(PCIBus *bus);
void _RefreshDeviceInfo(PCIBus *bus);
- uint32 _BarSize(uint32 bits, uint32
mask);
+ uint64 _BarSize(uint64 bits);
size_t _GetBarInfo(PCIDev *dev, uint8
offset,
- uint32
*address, uint32 *size = 0,
- uint8 *flags =
0, uint32 *highAddress = 0);
+ uint32
&ramAddress, uint32 &pciAddress,
+ uint32 &size,
uint8 &flags,
+ uint32
*highRAMAddress = NULL,
+ uint32
*highPCIAddress = NULL,
+ uint32
*highSize = NULL);
void _GetRomBarInfo(PCIDev *dev,
uint8 offset,
uint32
*address, uint32 *size = 0,
uint8 *flags =
0);
--
To view, visit https://review.haiku-os.org/c/haiku/+/3022
To unsubscribe, or for help writing mail filters, visit
https://review.haiku-os.org/settings
Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: Iae59f2875c93f15411a4d9791e71e69ba7a42287
Gerrit-Change-Number: 3022
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Lotz <mmlr@xxxxxxxx>
Gerrit-MessageType: newchange