Mailing List Archive

[PATCH 13/14] PPC: e500: Map PIO space into core memory region
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
+
#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;
}
--
1.6.0.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 13/14] PPC: e500: Map PIO space into core memory region [ In reply to ]
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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 13/14] PPC: e500: Map PIO space into core memory region [ In reply to ]
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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 13/14] PPC: e500: Map PIO space into core memory region [ In reply to ]
On 10/08/2012 03:48:42 PM, Alexander Graf wrote:
>
> 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.

Are you sure about that? Certainly that's the limit on x86 due to the
I/O instructions, and some (buggy?) PCI devices might have trouble with
larger I/O addresses, but I didn't think it was actually illegal. Some
mpc85xx boards have default configs with larger I/O windows (though
probably not for any good reason).

> > 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.

Better than nothing. :-)

-Scott

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 13/14] PPC: e500: Map PIO space into core memory region [ In reply to ]
On 08.10.2012, at 23:05, Scott Wood wrote:

> On 10/08/2012 03:48:42 PM, Alexander Graf wrote:
>> 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.
>
> Are you sure about that? Certainly that's the limit on x86 due to the I/O instructions, and some (buggy?) PCI devices might have trouble with larger I/O addresses, but I didn't think it was actually illegal. Some mpc85xx boards have default configs with larger I/O windows (though probably not for any good reason).

What sense would it make to model an ioport range that exceeds what x86 can do? I'm fairly sure if you'd ever start using that, you'll see a lot of PCI devices fail at you anyway.

>
>> > 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.
>
> Better than nothing. :-)

If you can point me to the part of the spec that specifies how that window should look like, I can cook up a patch. Or you can do it of course ;).


Alex


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 13/14] PPC: e500: Map PIO space into core memory region [ In reply to ]
On 10/08/2012 04:08:31 PM, Alexander Graf wrote:
>
> On 08.10.2012, at 23:05, Scott Wood wrote:
>
> > On 10/08/2012 03:48:42 PM, Alexander Graf wrote:
> >> 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.
> >
> > Are you sure about that? Certainly that's the limit on x86 due to
> the I/O instructions, and some (buggy?) PCI devices might have
> trouble with larger I/O addresses, but I didn't think it was actually
> illegal. Some mpc85xx boards have default configs with larger I/O
> windows (though probably not for any good reason).
>
> What sense would it make to model an ioport range that exceeds what
> x86 can do?

Well, as I said there's probably not a good reason for actually doing
it, but if you want to more faithfully emulate the hardware...

That said, when I first complained I misread the constant and thought
you were hardcoding 1M rather than 64K.

> >> > 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.
> >
> > Better than nothing. :-)
>
> If you can point me to the part of the spec that specifies how that
> window should look like, I can cook up a patch. Or you can do it of
> course ;).

Outbound windows are configured by PO*AR/PEXO*AR, and inbound by
PI*AR/PEXI*AR (plus BAR0). It's not a high priority, just curious what
would be involved.

-Scott

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel