Re: [PATCH] PCI: drop pci_segments_init()

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 2/27/25 01:58, Jan Beulich wrote:
> On 26.02.2025 20:57, Stewart Hildebrand wrote:
>> On 2/26/25 06:38, Jan Beulich wrote:
>>> Have callers invoke pci_add_segment() directly instead: With radix tree
>>> initialization moved out of the function, its name isn't quite
>>> describing anymore what it actually does.
>>>
>>> On x86 move the logic into __start_xen() itself, to reduce the risk of
>>> re-introducing ordering issues like the one which was addressed by
>>> 26fe09e34566 ("radix-tree: introduce RADIX_TREE{,_INIT}()").
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> ---
>>> This is entirely optional and up for discussion. There certainly also is
>>> an argument towards keeping the function. Otoh on Arm there is the still
>>> open question whether segment 0 really is kind of special there (as it
>>> is on x86, largely for historical reasons), or whether the code can be
>>> dropped there altogether.
>>
>> Segment 0 is not special on Arm as far as I'm aware. You can have a
>> perfectly functioning system with only, say, segment 1, for example:
>>
>> (XEN) ==== PCI devices ====
>> (XEN) ==== segment 0001 ====
>> (XEN) 0001:00:01.0 - d0 - node -1
>> (XEN) 0001:00:00.0 - d0 - node -1
>>
>> Segment numbers can be arbitrarily chosen by specifying the
>> linux,pci-domain device tree property.
> 
> Right, that was the vague understanding I had.
> 
>>> --- a/xen/arch/arm/pci/pci.c
>>> +++ b/xen/arch/arm/pci/pci.c
>>> @@ -88,7 +88,8 @@ static int __init pci_init(void)
>>>      if ( !pci_passthrough_enabled )
>>>          return 0;
>>>  
>>> -    pci_segments_init();
>>> +    if ( pci_add_segment(0) )
>>> +        panic("Could not initialize PCI segment 0\n");
>>
>> IMO it's okay to remove the call here since there is already a call to
>> pci_add_segment() in
>> xen/arch/arm/pci/pci-host-common.c:pci_host_common_probe()
> 
> Is there? I can't see one, so maybe you're working from a tree with extra
> patches applied?

Ah, you're right, sorry, I was looking at a downstream tree. The
associated segment would be added in Xen upon the first time Dom0 calls
PHYSDEVOP_pci_device_add.