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

[Alexander Graf]

On 04.10.2012, at 18:48, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:address@hidden
>> Sent: Thursday, October 04, 2012 9:37 PM
>> To: Bhushan Bharat-R65777
>> Cc: Avi Kivity; address@hidden; address@hidden
>> 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:address@hidden
>>>> Sent: Thursday, October 04, 2012 8:28 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: address@hidden; address@hidden; address@hidden
>>>> 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:address@hidden
>>>>>> Sent: Thursday, October 04, 2012 6:02 PM
>>>>>> To: Bhushan Bharat-R65777
>>>>>> Cc: address@hidden; address@hidden; address@hidden;
>>>>>> 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. 

In QEMU, we map everything 1:1 today.


Alex