This patch corrects two long-standing bugs with PPC PCI configuration space access. Firstly fix the calculation of PCI configuration space addresses by the PCI_ADDR macro; this was incorrectly using arch->cfg_base which is the mapped address and has nothing to do with the PCI configuration space address. Instead just set bit 31 to initiate a configuration cycle as per the PCI specification.
Secondly, fix pci_config_read32() and pci_config_write16() which were incorrectly adding the register offset to the PCI IO dataport address causing them to write into unknown address space for registers > 0. It appears that this only worked purely by coincidence with QEMU due to the way in which the configuration address was calculated for an oversized PCI configuration IO dataport MemoryRegion.
Reported-by: Hervé Poussineau hpoussin@reactos.org CC: Hervé Poussineau hpoussin@reactos.org CC: Andreas Färber afaerber@suse.de Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/include/arch/ppc/pci.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/include/arch/ppc/pci.h b/openbios-devel/include/arch/ppc/pci.h index cccefb1..d96bd7e 100644 --- a/openbios-devel/include/arch/ppc/pci.h +++ b/openbios-devel/include/arch/ppc/pci.h @@ -12,7 +12,7 @@ /* PCI Configuration Mechanism #1 */
#define PCI_ADDR(bus, dev, fn) \ - ((pci_addr) (arch->cfg_base \ + ((pci_addr) (0x80000000u \ | (uint32_t) (bus) << 16 \ | (uint32_t) (dev) << 11 \ | (uint32_t) (fn) << 8)) @@ -41,7 +41,7 @@ static inline uint32_t pci_config_read32(pci_addr dev, uint8_t reg) { uint32_t res; out_le32((unsigned *)arch->cfg_addr, dev | reg); - res = in_le32((unsigned *)(arch->cfg_data + reg)); + res = in_le32((unsigned *)(arch->cfg_data)); return res; }
@@ -60,7 +60,7 @@ static inline void pci_config_write16(pci_addr dev, uint8_t reg, uint16_t val) static inline void pci_config_write32(pci_addr dev, uint8_t reg, uint32_t val) { out_le32((unsigned *)arch->cfg_addr, dev | reg); - out_le32((unsigned *)(arch->cfg_data + reg), val); + out_le32((unsigned *)(arch->cfg_data), val); } #else /* !PCI_CONFIG_1 */ #error PCI Configuration Mechanism is not specified or implemented
This patch corrects the PCI configuration and data IO port addresses, along with detection of the Raven PCI bridge. With this in place, we can now enumerate a PReP PCI bus.
Reported-by: Hervé Poussineau hpoussin@reactos.org CC: Hervé Poussineau hpoussin@reactos.org CC: Andreas Färber afaerber@suse.de Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/ppc/qemu/init.c | 4 ++-- openbios-devel/drivers/pci_database.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/openbios-devel/arch/ppc/qemu/init.c b/openbios-devel/arch/ppc/qemu/init.c index 54ac6f2..e21317c 100644 --- a/openbios-devel/arch/ppc/qemu/init.c +++ b/openbios-devel/arch/ppc/qemu/init.c @@ -95,8 +95,8 @@ static const pci_arch_t known_arch[] = { .name = "PREP", .vendor_id = PCI_VENDOR_ID_MOTOROLA, .device_id = PCI_DEVICE_ID_MOTOROLA_RAVEN, - .cfg_addr = 0x80800000, - .cfg_data = 0x800c0000, + .cfg_addr = 0x80000cf8, + .cfg_data = 0x80000cfc, .cfg_base = 0x80000000, .cfg_len = 0x00100000, .host_mem_base = 0xf0000000, diff --git a/openbios-devel/drivers/pci_database.c b/openbios-devel/drivers/pci_database.c index 27a28cb..8d1765b 100644 --- a/openbios-devel/drivers/pci_database.c +++ b/openbios-devel/drivers/pci_database.c @@ -336,9 +336,9 @@ static const pci_dev_t hbrg_devices[] = { }, { PCI_VENDOR_ID_MOTOROLA, PCI_DEVICE_ID_MOTOROLA_RAVEN, NULL, - "pci-bridge", "PREP Host PCI Bridge - Motorola Raven", NULL, + "pci", "PREP Host PCI Bridge - Motorola Raven", NULL, 3, 2, 1, - NULL, NULL, + host_config_cb, NULL, }, { PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SABRE, NULL,
Mark Cave-Ayland a écrit :
This patch corrects the PCI configuration and data IO port addresses, along with detection of the Raven PCI bridge. With this in place, we can now enumerate a PReP PCI bus.
Reported-by: Hervé Poussineau hpoussin@reactos.org CC: Hervé Poussineau hpoussin@reactos.org CC: Andreas Färber afaerber@suse.de Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
openbios-devel/arch/ppc/qemu/init.c | 4 ++-- openbios-devel/drivers/pci_database.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Tested-by: Hervé Poussineau hpoussin@reactos.org
Hervé
Mark Cave-Ayland a écrit :
This patch corrects two long-standing bugs with PPC PCI configuration space access. Firstly fix the calculation of PCI configuration space addresses by the PCI_ADDR macro; this was incorrectly using arch->cfg_base which is the mapped address and has nothing to do with the PCI configuration space address. Instead just set bit 31 to initiate a configuration cycle as per the PCI specification.
Secondly, fix pci_config_read32() and pci_config_write16() which were incorrectly adding the register offset to the PCI IO dataport address causing them to write into unknown address space for registers > 0. It appears that this only worked purely by coincidence with QEMU due to the way in which the configuration address was calculated for an oversized PCI configuration IO dataport MemoryRegion.
Reported-by: Hervé Poussineau hpoussin@reactos.org CC: Hervé Poussineau hpoussin@reactos.org CC: Andreas Färber afaerber@suse.de Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
Tested-by: Hervé Poussineau hpoussin@reactos.org
However, note the typo in the commit message, where the wrong function is pci_config_write32, not pci_config_write16
Hervé
On 31/08/13 22:18, Hervé Poussineau wrote:
Mark Cave-Ayland a écrit :
This patch corrects two long-standing bugs with PPC PCI configuration space access. Firstly fix the calculation of PCI configuration space addresses by the PCI_ADDR macro; this was incorrectly using arch->cfg_base which is the mapped address and has nothing to do with the PCI configuration space address. Instead just set bit 31 to initiate a configuration cycle as per the PCI specification.
Secondly, fix pci_config_read32() and pci_config_write16() which were incorrectly adding the register offset to the PCI IO dataport address causing them to write into unknown address space for registers > 0. It appears that this only worked purely by coincidence with QEMU due to the way in which the configuration address was calculated for an oversized PCI configuration IO dataport MemoryRegion.
Reported-by: Hervé Poussineau hpoussin@reactos.org CC: Hervé Poussineau hpoussin@reactos.org CC: Andreas Färber afaerber@suse.de Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
Tested-by: Hervé Poussineau hpoussin@reactos.org
However, note the typo in the commit message, where the wrong function is pci_config_write32, not pci_config_write16
Thanks for testing - committed with a corrected commit message.
ATB,
Mark.
-
Hervé Poussineau -
Mark Cave-Ayland