[PATCH 1/2] PPC: Fix PCI configuration space address access
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 -- 1.7.10.4
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, -- 1.7.10.4
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.
participants (2)
-
Hervé Poussineau -
Mark Cave-Ayland