Re: [Xen-devel] [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

[Yijing Wang <wangyijing@xxxxxxxxxx>]

On 2014/9/28 19:21, Liviu Dudau wrote:
> On Sun, Sep 28, 2014 at 10:16:12AM +0800, Yijing Wang wrote:
>>>>>> What I would like to see is a way of creating the pci_host_bridge 
>>>>>> structure outside
>>>>>> the pci_create_root_bus(). That would then allow us to pass this sort of 
>>>>>> platform
>>>>>> details like associated msi_chip into the host bridge and the child 
>>>>>> busses will
>>>>>> have an easy way of finding the information needed by finding the root 
>>>>>> bus and then
>>>>>> the host bridge structure. Then the generic pci_scan_root_bus() can be 
>>>>>> used by (mostly)
>>>>>> everyone and the drivers can remove their kludges that try to work 
>>>>>> around the
>>>>>> current limitations.
>>>>
>>>> So I think maybe save msi chip in PCI arch sysdata is a good candidate.
>>>
>>> Except that arch sysdata at the moment is an opaque pointer. I am all in 
>>> favour in
>>> changing the type of sysdata from void* into pci_host_bridge* and arches 
>>> can wrap their old
>>> sysdata around the pci_host_bridge*.
>>
>> I inspected every arch and found there are almost no common stuff,
> 
> I will disagree here. Most (all?) of the structures that are passed as 
> sysdata argument to

Most.

> pci_create_root_bus() or pci_scan_root_bus() have a set of resources for 
> storing the MEM and
> IO ranges, which struct pci_host_bridge already has. So that can be factored 
> out of the
> arch code. Same for pci_domain_nr. Then there are some variables that are 
> used for communication
> with the platform code due to convoluted way(s) in which PCI code gets 
> instantiated.

Yes, currently some archs store MEM and IO resource in pci sysdata, and others 
not, move the MEM and IO
resource to pci_host_bride could make code become simple, we can clean up the 
resource list argument in
pci scan functions.

> 
> What I am arguing here is not that the arch equivalent of pci_host_bridge 
> structure is already
> common, but that by moving the members that are common out of arch sysdata 
> into pci_host_bridge
> we will have more commonality and it will be easier to re-factor the code.

Now, I got it, thanks!

> 
>> and generic data struct should
>> be created in generic PCI code.
> 
> Not necessarily. What I have in mind is something like this:

This is a good idea, what I'm worried is this series is already large, so I 
think we need to post
another series to do it.


> 
>  - drivers/pci/ exports pci_init_host_bridge() that does the initialisation 
> of bridge->windows
>    and anything else that is needed (like find_pci_host_bridge() function).
>  - arch code does:
> 
>       struct pci_controller {
>               struct pci_host_bridge bridge;
>               .....
>       };
> 
>       #define to_pci_controller(bridge)       container_of(bridge, struct 
> pci_controller, bridge)
> 
>       static inline struct pci_controller *get_host_controller(const struct 
> pci_bus *bus)
>       {
>               struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
>               if (bridge)
>                       return to_pci_controller(bridge);
> 
>               return NULL;
>       }
> 
>       int arch_pci_init(....)
>       {
>               struct pci_controller *hose;
>               ....
>               hose = kzalloc(sizeof(*hose), GFP_KERNEL);
>               pci_init_host_bridge(&hose->bridge);
>               ....
>               pci_scan_root_bus(...., &hose->bridge, &resources);
>               ....
>               return 0;
>       }
> 
> Then finding the right structure will be easy.
> 
>> Another, I don't like associate msi chip and every PCI device, further more,
>> almost all platforms except arm have only one MSI controller, and currently, 
>> PCI enumerating code doesn't need
>> to know the MSI chip details, like for legacy IRQ, PCI device doesn't need 
>> to know which IRQ controller they
>> should deliver IRQ to. I would think more about it, and hope other PCI guys 
>> can give some comments, especially from Bjorn.
>>
> 
> I wasn't suggesing to associate an msi chip with every PCI device, but with 
> the pci_host_bridge.
> I don't expect a host bridge to have more than one msi chip, so that should 
> be OK. Also, I'm
> thinking that getting the associated msi chip should be some sort of 
> pci_host_bridge ops function,
> and for arches that don't care about MSI it doesn't get implemented.

Currently, a property "msi-parent" was introduced in arm, and all msi chip 
integrated in irq chip controller will
be added to of_pci_msi_chip_list. PCI host driver find the match msi chip by 
its of_node.

Thanks!
Yijing.

> 
> Best regards,
> Liviu
> 
>  
>> Thanks!
>> Yijing.
>>
>>>
>>> Best regards,
>>> Liviu
>>>
>>>>
>>>>>
>>>>> I think both issues are orthogonal. Last time I checked a lot of work
>>>>> was still necessary to unify host bridges enough so that it could be
>>>>> shared across architectures. But perhaps some of that work has
>>>>> happened in the meantime.
>>>>>
>>>>> But like I said, when you create the root bus, you can easily attach the
>>>>> MSI chip to it.
>>>>>
>>>>> Thierry
>>>>>
>>>>
>>>>
>>>> -- 
>>>> Thanks!
>>>> Yijing
>>>>
>>>>
>>>
>>
>>
>> -- 
>> Thanks!
>> Yijing
>>
>>
> 


-- 
Thanks!
Yijing


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel