Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI controller

[Avi Kivity <avi@redhat.com>]

On 10/04/2012 03:46 PM, Bhushan Bharat-R65777 wrote:
> 
> 
>> -----Original Message-----
>> From: Avi Kivity [mailto:avi@redhat.com]
>> Sent: Thursday, October 04, 2012 6:02 PM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org; agraf@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.

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

-- 
error compiling committee.c: too many arguments to function