On 08.10.2012, at 10:23, Bhushan Bharat-R65777 wrote:
>>>>>>> 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.
>>>
>>> Just want to be sure I understood you correctly: Do you mean something
>>> like this : ( I know I have to switch to QOM mechanism to share
>>> parameters)
>>>
>>> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c index
>>> 92b1dc0..a948bc6 100644
>>> --- a/hw/ppce500_pci.c
>>> +++ b/hw/ppce500_pci.c
>>> @@ -89,6 +89,12 @@ struct PPCE500PCIState {
>>> MemoryRegion iomem;
>>> };
>>>
>>> +struct BHARAT {
>>> + PCIDevice p;
>>> + void *bar0;
>>
>> MemoryRegion *bar0
>
> If I uses " MemoryRegion *bar0" or " MemoryRegion bar0" then how the size
> and address of bar0 will be populated?
If I understand Avi's comments correctly, by creating a memory region alias.
Who would be responsible for creating that alias and how that alias would come
to be in this struct is still puzzling myself too though :).
Avi?
>
> As far as I can see, other PCI devices using this way to have extra
> information but they does not get MemoryRegion information from other object.
>
>>
>>> +};
>>> +
>>> +typedef struct BHARAT bharat;
>>> typedef struct PPCE500PCIState PPCE500PCIState;
>>>
>>> static uint64_t pci_reg_read4(void *opaque, target_phys_addr_t addr,
>>> @@ -307,6 +313,16 @@ static const VMStateDescription
>>> vmstate_ppce500_pci = {
>>>
>>> #include "exec-memory.h"
>>>
>>> +static int e500_pcihost_bridge_initfn(PCIDevice *d) {
>>> + bharat *b = DO_UPCAST(bharat, p, d);
>>> +
>>> + printf("Addr = %llx, size = %llx\n", ((MemoryRegion *)b->bar0)->addr,
>> (unsigned long long)int128_get64(((Me
>>> + pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
>>> + (MemoryRegion *)b->bar0);
>>
>> That one still has to call its parent initfn, no?
>
> I am really sorry, but I did not get. The object says its parent is
> TYPE_PCI_DEVICE, so which function are you talking about?
In object oriented programming, every time you overload a class, your
constructor should call the overloaded class's constructor. I don't see this
happening for other PCI device's init functions though.
Andreas, shouldn't parent class init be called for pci subclass devices?
>
>>
>>> + return 0;
>>> +}
>>> +
>>> +MemoryRegion ccsr;
>>> static int e500_pcihost_initfn(SysBusDevice *dev) {
>>> PCIHostState *h;
>>> @@ -315,6 +331,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>>> int i;
>>> MemoryRegion *address_space_mem = get_system_memory();
>>> MemoryRegion *address_space_io = get_system_io();
>>> + PCIDevice *d;
>>>
>>> h = PCI_HOST_BRIDGE(dev);
>>> s = PPC_E500_PCI_HOST_BRIDGE(dev); @@ -328,7 +345,12 @@ static int
>>> e500_pcihost_initfn(SysBusDevice *dev)
>>> address_space_io, PCI_DEVFN(0x11, 0), 4);
>>> h->bus = b;
>>>
>>> - pci_create_simple(b, 0, "e500-host-bridge");
>>> + d = pci_create(b, 0, "e500-host-bridge");
>>> + /* ccsr-> should be passed from hw/ppc/e500.c */
>>> + ccsr.addr = 0xE0000000;
>>> + ccsr.size = int128_make64(0x00100000);
>>
>> What is this?
>
> I am trying to pass the CCSR information to "e500-host-bridge". This is
> currently hardcoded, but finally this will come from hw/ppc/e500.c.
hw/ppc/e500.c owns the CCSR memory region and maps the CCSR memory region. In
Andreas' model, the whole CCSR region is a separate object. The PCI host device
should only ever take the memory region it gets from the CCSR device (or
hw/ppc/e500.c for now) and map that as BAR0. No need to know any details about
it.
Alex
