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

[Alexander Graf]

On 07.10.2012, at 11:48, Avi Kivity <address@hidden> wrote:

> On 10/05/2012 01:59 PM, Alexander Graf wrote:
>>>>> 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
> 
> MemoryRegion bar0;

Why? We want the same region that is mapped outside of the pci device be 
available as BAR0 too.

Alex

> 
>> 
>>> +};
>>> +
>>> +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
> 
> memory_region_init_io(&d->bar0, ...);
> 
>>> +    pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, (MemoryRegion 
>>> *)b->bar0);
>> 
>> That one still has to call its parent initfn, no?
>> 
> 
> yes
> 
> -- 
> error compiling committee.c: too many arguments to function