Patchwork [13/14] PPC: e500: Map PIO space into core memory region

login
register
mail settings
Submitter Alexander Graf
Date Oct. 8, 2012, 12:23 p.m.
Message ID <1349699033-6703-14-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/190029/
State New
Headers show

Comments

Alexander Graf - Oct. 8, 2012, 12:23 p.m.
On PPC, we don't have PIO. So usually PIO space behind a PCI bridge is
accessible via MMIO. Do this mapping explicitly by mapping the PIO space
of our PCI bus into a memory region that lives in memory space.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500.c    |    3 +--
 hw/ppce500_pci.c |    9 +++++++--
 2 files changed, 8 insertions(+), 4 deletions(-)
Scott Wood - Oct. 8, 2012, 8:20 p.m.
On 10/08/2012 07:23:52 AM, Alexander Graf wrote:
> On PPC, we don't have PIO. So usually PIO space behind a PCI bridge is
> accessible via MMIO. Do this mapping explicitly by mapping the PIO  
> space
> of our PCI bus into a memory region that lives in memory space.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/ppc/e500.c    |    3 +--
>  hw/ppce500_pci.c |    9 +++++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index d23f9b2..857d4dc 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -52,7 +52,6 @@
>  #define MPC8544_PCI_REGS_BASE      (MPC8544_CCSRBAR_BASE + 0x8000ULL)
>  #define MPC8544_PCI_REGS_SIZE      0x1000ULL
>  #define MPC8544_PCI_IO             0xE1000000ULL
> -#define MPC8544_PCI_IOLEN          0x10000ULL
>  #define MPC8544_UTIL_BASE          (MPC8544_CCSRBAR_BASE +  
> 0xe0000ULL)
>  #define MPC8544_SPIN_BASE          0xEF000000ULL
> 
> @@ -511,7 +510,7 @@ void ppce500_init(PPCE500Params *params)
>      if (!pci_bus)
>          printf("couldn't create PCI controller!\n");
> 
> -    isa_mmio_init(MPC8544_PCI_IO, MPC8544_PCI_IOLEN);
> +    sysbus_mmio_map(sysbus_from_qdev(dev), 1, MPC8544_PCI_IO);
> 
>      if (pci_bus) {
>          /* Register network interfaces. */
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 92b1dc0..27c6d7d 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -31,6 +31,8 @@
>  #define PCIE500_ALL_SIZE      0x1000
>  #define PCIE500_REG_SIZE      (PCIE500_ALL_SIZE - PCIE500_REG_BASE)
> 
> +#define PCIE500_PCI_IOLEN     0x10000ULL

I don't think this belongs in ppce500_pci.c -- it's board config (or  
rather, a board-related default of something that is supposed to be  
software configurable), just like the base address.

Any chance of similarly constraining PCI MMIO to its proper window?

-Scott
Alexander Graf - Oct. 8, 2012, 8:48 p.m.
On 08.10.2012, at 22:20, Scott Wood wrote:

> On 10/08/2012 07:23:52 AM, Alexander Graf wrote:
>> On PPC, we don't have PIO. So usually PIO space behind a PCI bridge is
>> accessible via MMIO. Do this mapping explicitly by mapping the PIO space
>> of our PCI bus into a memory region that lives in memory space.
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/ppc/e500.c    |    3 +--
>> hw/ppce500_pci.c |    9 +++++++--
>> 2 files changed, 8 insertions(+), 4 deletions(-)
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index d23f9b2..857d4dc 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -52,7 +52,6 @@
>> #define MPC8544_PCI_REGS_BASE      (MPC8544_CCSRBAR_BASE + 0x8000ULL)
>> #define MPC8544_PCI_REGS_SIZE      0x1000ULL
>> #define MPC8544_PCI_IO             0xE1000000ULL
>> -#define MPC8544_PCI_IOLEN          0x10000ULL
>> #define MPC8544_UTIL_BASE          (MPC8544_CCSRBAR_BASE + 0xe0000ULL)
>> #define MPC8544_SPIN_BASE          0xEF000000ULL
>> @@ -511,7 +510,7 @@ void ppce500_init(PPCE500Params *params)
>>     if (!pci_bus)
>>         printf("couldn't create PCI controller!\n");
>> -    isa_mmio_init(MPC8544_PCI_IO, MPC8544_PCI_IOLEN);
>> +    sysbus_mmio_map(sysbus_from_qdev(dev), 1, MPC8544_PCI_IO);
>>     if (pci_bus) {
>>         /* Register network interfaces. */
>> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
>> index 92b1dc0..27c6d7d 100644
>> --- a/hw/ppce500_pci.c
>> +++ b/hw/ppce500_pci.c
>> @@ -31,6 +31,8 @@
>> #define PCIE500_ALL_SIZE      0x1000
>> #define PCIE500_REG_SIZE      (PCIE500_ALL_SIZE - PCIE500_REG_BASE)
>> +#define PCIE500_PCI_IOLEN     0x10000ULL
> 
> I don't think this belongs in ppce500_pci.c -- it's board config (or rather, a board-related default of something that is supposed to be software configurable), just like the base address.

Actually this one is fixed in PCI. There are only 64k PIO ports.

> Any chance of similarly constraining PCI MMIO to its proper window?

We can't distinguish between inbound and outbound right now. If we only need to restrict CPU -> PCI access, then yes.


Alex

Patch

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index d23f9b2..857d4dc 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -52,7 +52,6 @@ 
 #define MPC8544_PCI_REGS_BASE      (MPC8544_CCSRBAR_BASE + 0x8000ULL)
 #define MPC8544_PCI_REGS_SIZE      0x1000ULL
 #define MPC8544_PCI_IO             0xE1000000ULL
-#define MPC8544_PCI_IOLEN          0x10000ULL
 #define MPC8544_UTIL_BASE          (MPC8544_CCSRBAR_BASE + 0xe0000ULL)
 #define MPC8544_SPIN_BASE          0xEF000000ULL
 
@@ -511,7 +510,7 @@  void ppce500_init(PPCE500Params *params)
     if (!pci_bus)
         printf("couldn't create PCI controller!\n");
 
-    isa_mmio_init(MPC8544_PCI_IO, MPC8544_PCI_IOLEN);
+    sysbus_mmio_map(sysbus_from_qdev(dev), 1, MPC8544_PCI_IO);
 
     if (pci_bus) {
         /* Register network interfaces. */
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 92b1dc0..27c6d7d 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -31,6 +31,8 @@ 
 #define PCIE500_ALL_SIZE      0x1000
 #define PCIE500_REG_SIZE      (PCIE500_ALL_SIZE - PCIE500_REG_BASE)
 
+#define PCIE500_PCI_IOLEN     0x10000ULL
+
 #define PPCE500_PCI_CONFIG_ADDR         0x0
 #define PPCE500_PCI_CONFIG_DATA         0x4
 #define PPCE500_PCI_INTACK              0x8
@@ -87,6 +89,7 @@  struct PPCE500PCIState {
     /* mmio maps */
     MemoryRegion container;
     MemoryRegion iomem;
+    MemoryRegion pio;
 };
 
 typedef struct PPCE500PCIState PPCE500PCIState;
@@ -314,7 +317,6 @@  static int e500_pcihost_initfn(SysBusDevice *dev)
     PCIBus *b;
     int i;
     MemoryRegion *address_space_mem = get_system_memory();
-    MemoryRegion *address_space_io = get_system_io();
 
     h = PCI_HOST_BRIDGE(dev);
     s = PPC_E500_PCI_HOST_BRIDGE(dev);
@@ -323,9 +325,11 @@  static int e500_pcihost_initfn(SysBusDevice *dev)
         sysbus_init_irq(dev, &s->irq[i]);
     }
 
+    memory_region_init(&s->pio, "pci-pio", PCIE500_PCI_IOLEN);
+
     b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
                          mpc85xx_pci_map_irq, s->irq, address_space_mem,
-                         address_space_io, PCI_DEVFN(0x11, 0), 4);
+                         &s->pio, PCI_DEVFN(0x11, 0), 4);
     h->bus = b;
 
     pci_create_simple(b, 0, "e500-host-bridge");
@@ -341,6 +345,7 @@  static int e500_pcihost_initfn(SysBusDevice *dev)
     memory_region_add_subregion(&s->container, PCIE500_CFGDATA, &h->data_mem);
     memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
     sysbus_init_mmio(dev, &s->container);
+    sysbus_init_mmio(dev, &s->pio);
 
     return 0;
 }