> -----Original Message-----
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Wednesday, October 03, 2012 5:41 PM
> To: Bhushan Bharat-R65777
> Cc: qemu-devel qemu-devel; qemu-...@nongnu.org List; Bhushan Bharat-R65777;
> Avi
> Kivity
> Subject: Re: [PATCH 2/2] Adding BAR0 for e500 PCI controller
>
>
> On 03.10.2012, at 13:50, Bharat Bhushan wrote:
>
> > PCI Root complex have TYPE-1 configuration header while PCI endpoint
> > have type-0 configuration header. The type-1 configuration header have
> > a BAR (BAR0). In Freescale PCI controller BAR0 is used for mapping pci
> > address space to CCSR address space. This can used for 2 purposes: 1)
> > for MSI interrupt generation 2) Allow CCSR registers access when
> > configured as PCI endpoint, which I am not sure is a use case with QEMU-KVM
> guest.
> >
> > What I observed is that when guest read the size of BAR0 of host
> > controller configuration header (TYPE1 header) then it always reads it
> > as 0. When looking into the QEMU hw/ppce500_pci.c, I do not find the
> > PCI controller device registering BAR0. I do not find any other
> > controller also doing so may they do not use BAR0.
> >
> > There are two issues when BAR0 is not there (which I can think of):
> > 1) There should be BAR0 emulated for PCI Root comaplex (TYPE1 header)
> > and when reading the size of BAR0, it should give size as per real h/w.
> >
> > This patch solves this problem.
> >
> > 2) Do we need this BAR0 inbound address translation?
> > When BAR0 is of non-zero size then it will be configured for
> > PCI address space to local address(CCSR) space translation on inbound
> > access.
> > The primary use case is for MSI interrupt generation. The device is
> > configured with a address offsets in PCI address space, which will be
> > translated to MSI interrupt generation MPIC registers. Currently I do
> > not understand the MSI interrupt generation mechanism in QEMU and also
> > IIRC we do not use QEMU MSI interrupt mechanism on e500 guest machines.
> > But this BAR0 will be used when using MSI on e500.
> >
> > I can see one more issue, There are ATMUs emulated in
> > hw/ppce500_pci.c, but i do not see these being used for address translation.
> > So far that works because pci address space and local address space
> > are 1:1 mapped. BAR0 inbound translation + ATMU translation will
> > complete the address translation of inbound traffic.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> > ---
> > hw/ppc/e500.c | 1 +
> > hw/ppce500_pci.c | 13 +++++++++++++
> > 2 files changed, 14 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 197411d..c7ae2b6
> > 100644
> > --- a/hw/ppc/e500.c
> > +++ b/hw/ppc/e500.c
> > @@ -518,6 +518,7 @@ void ppce500_init(PPCE500Params *params)
> >
> > /* PCI */
> > dev = qdev_create(NULL, "e500-pcihost");
> > + qdev_prop_set_ptr(dev, "bar0_region", ccsr);
> > qdev_init_nofail(dev);
> > s = sysbus_from_qdev(dev);
> > sysbus_connect_irq(s, 0, mpic[pci_irq_nrs[0]]); diff --git
> > a/hw/ppce500_pci.c b/hw/ppce500_pci.c index 92b1dc0..16e4af2 100644
> > --- a/hw/ppce500_pci.c
> > +++ b/hw/ppce500_pci.c
> > @@ -87,6 +87,7 @@ struct PPCE500PCIState {
> > /* mmio maps */
> > MemoryRegion container;
> > MemoryRegion iomem;
> > + void *bar0;
> > };
> >
> > typedef struct PPCE500PCIState PPCE500PCIState; @@ -315,6 +316,8 @@
> > static int e500_pcihost_initfn(SysBusDevice *dev)
> > int i;
> > MemoryRegion *address_space_mem = get_system_memory();
> > MemoryRegion *address_space_io = get_system_io();
> > + PCIDevice *pdev;
> > + MemoryRegion bar0;
> >
> > h = PCI_HOST_BRIDGE(dev);
> > s = PPC_E500_PCI_HOST_BRIDGE(dev); @@ -342,6 +345,10 @@ static int
> > e500_pcihost_initfn(SysBusDevice *dev)
> > memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
> > sysbus_init_mmio(dev, &s->container);
> >
> > + bar0 = *(MemoryRegion *)s->bar0;
> > + pdev = pci_find_device(b, 0, 0);
> > + pci_register_bar(pdev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &bar0);
>
> Any reason you can't just pass in s->bar0 here? Though I'm not sure whether we
> have to do something special to get a memory region alias.
Yes I think this should work:
pci_register_bar(pdev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, (MemoryRegion
*)s->bar0);
Thanks
-Bharat
>
> Avi, any ideas? We want the same memory region mapped twice. Once at a fixed
> address, once as BAR0 of the PCI host controller.
>
>
> Alex
>
> > +
> > return 0;
> > }
> >
> > @@ -363,6 +370,11 @@ static const TypeInfo e500_host_bridge_info = {
> > .class_init = e500_host_bridge_class_init,
> > };
> >
> > +static Property pci_host_dev_info[] = {
> > + DEFINE_PROP_PTR("bar0_region", PPCE500PCIState, bar0),
> > + DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > static void e500_pcihost_class_init(ObjectClass *klass, void *data) {
> > DeviceClass *dc = DEVICE_CLASS(klass); @@ -370,6 +382,7 @@ static
> > void e500_pcihost_class_init(ObjectClass *klass, void *data)
> >
> > k->init = e500_pcihost_initfn;
> > dc->vmsd = &vmstate_ppce500_pci;
> > + dc->props = pci_host_dev_info;
> > }
> >
> > static const TypeInfo e500_pcihost_info = {
> > --
> > 1.7.0.4
> >
> >
>
