Re: [Qemu-devel] [PATCH 13/14] PPC: e500: Map PIO space into core memory region

[Scott Wood <scottwood@freescale.com>]

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