> -----Original Message-----
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, October 04, 2012 9:37 PM
> To: Bhushan Bharat-R65777
> Cc: Avi Kivity; qemu-devel@nongnu.org; qemu-...@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI controller
>
>
> On 04.10.2012, at 18:03, Bhushan Bharat-R65777 wrote:
>
> >
> >
> >> -----Original Message-----
> >> From: Avi Kivity [mailto:a...@redhat.com]
> >> Sent: Thursday, October 04, 2012 8:28 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; ag...@suse.de
> >> Subject: Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI
> >> controller
> >>
> >> On 10/04/2012 03:46 PM, Bhushan Bharat-R65777 wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Avi Kivity [mailto:a...@redhat.com]
> >>>> Sent: Thursday, October 04, 2012 6:02 PM
> >>>> To: Bhushan Bharat-R65777
> >>>> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; ag...@suse.de;
> >>>> Bhushan Bharat-
> >>>> R65777
> >>>> Subject: Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI
> >>>> controller
> >>>>
> >>>> On 10/03/2012 01:50 PM, Bharat Bhushan wrote:
> >>>>> 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;
> >>>>> };
> >>>>
> >>>> void *? Why?
> >>>
> >>> Passing parameter via qdev is either possible by value or by void *.
> >>> That's
> >> why I used void *.
> >>
> >> Then you shouldn't be using qdev for this. But if you make the
> >> changes below, it will likely not be necessary.
> >>
> >>>>
> >>>> However this is best done from the pci device's initialization
> >>>> function, not here (the MemoryRegion should be a member of the pci
> >>>> device as
> >> well).
> >>>
> >>> We want to set BAR0 of PCI controller so we did this here. But yes,
> >>> we also
> >> want that all devices under the PCI controller have this mapping, so
> >> when they access the MPIC register to send MSI then they can do that.
> >>
> >> Please elaborate. One way to describe what you want is to issue an 'info
> mtree'
> >> and show what changes you want.
> >
> > Setup is something like this:
> >
> > |-------------|
> > | PCI device |
> > | |
> > ---------------
> > |
> > |
> > |
> > |
> > |-------------------|
> > | |
> > | PCI controller |
> > | |
> > | -------------- |
> > | | BAR0 in | |
> > | | TYPE1 | |
> > | | Config HDR | |
> > | -------------- |
> > | |
> > -------------------
> >
> > BAR0: it is an inbound window, and on e500 devices this maps the pci bus
> address to CCSR address.
> >
> > My requirement are:
> > 1) when guest read pci controller BAR0, it is able to get proper size
> > ( Size of CCSR space)
> > 2) Guest can program this to any address in pci address space. When PCI
> > device
> access this address range then that address should be mapped to CCSR address
> space.
> >
> > Though this patch only met the requirement number (1).
>
> No, it also meets (2). The PCI address space is identical to the CPU memory
> space in our mapping right now. So if the guest maps BAR0 somewhere, it
> automatically maps CCSR into CPU address space, which exposes it to PCI
> address
> space.
Really? I think on powerpc the pci address space is defined as: it maps the
outbound window just below 0x1_0000_0000, then CCSR and then inbound window. So
inbound window is 1:1 map if guest physical starts from 0x0. But I do not think
CCSR is 1:1 map in pci address space and cpu address space.
>
> >
> >>
> >>>
> >>> Which device's initialization function you are talking about?
> >>
> >> static const TypeInfo e500_host_bridge_info = {
> >> .name = "e500-host-bridge",
> >> .parent = TYPE_PCI_DEVICE,
> >> .instance_size = sizeof(PCIDevice),
> >> .class_init = e500_host_bridge_class_init,
> >> };
> >>
> >> This needs to describe a derived class of PCIDevice, hold the BAR as
> >> a data member, and register the BAR in the init function (which
> >> doesn't exist yet because you haven't subclassed it). This way the
> >> BAR lives in the device which exposes it, like BARs everywhere.
> >
> > Do you mean that we add the "MemoryRegion bar0" in PCIDevice struct. Do the
> same thing that I was doing in e500_pcihost_initfn() in the k->init() (will
> add
> this) function of "e500-host-bridge"
>
> No, he means that you create a new struct like this:
>
> struct foo {
> PCIDevice p;
> MemoryRegion bar0;
> };
>
> Please check out any other random PCI device in QEMU. Almost all of them do
> this
> to store more information than their parent class can hold.
Ok,
Thanks
-Bharat
>
>
> Alex
>
> >
> > This way I should be able to met both of my requirements.
> >
> > Thanks
> > -Bharat
> >
> >>
> >> --
> >> error compiling committee.c: too many arguments to function
> >
> >
>
