Re: [PATCH] xen/arm: do not try to add pci-domain for disabled devices

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 28 Oct 2021, Oleksandr Andrushchenko wrote:
> On 28.10.21 19:50, Stefano Stabellini wrote:
> > On Thu, 28 Oct 2021, Julien Grall wrote:
> >> Hi Stefano,
> >>
> >> On 28/10/2021 00:57, Stefano Stabellini wrote:
> >>> On Wed, 27 Oct 2021, Julien Grall wrote:
> >>>> Hi Oleksandr,
> >>>>
> >>>> On 27/10/2021 09:37, Oleksandr Andrushchenko wrote:
> >>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> >>>>>
> >>>>> If a PCI host bridge device is present in the device tree, but is
> >>>>> disabled, then its PCI host bridge driver was not instantiated.
> >>>>> This results in the following panic during Xen start:
> >>>>>
> >>>>> (XEN) Device tree generation failed (-22).
> >>>> It would good to clarify in the commit message where the error is coming
> >>>> from.
> >>>> I think this is from pci_get_host_bridge_segment().
> >>>>
> >>>>> (XEN)
> >>>>> (XEN) ****************************************
> >>>>> (XEN) Panic on CPU 0:
> >>>>> (XEN) Could not set up DOM0 guest OS
> >>>>> (XEN) ****************************************
> >>>>>
> >>>>> Fix this by not adding linux,pci-domain property for hwdom if it is
> >>>>> neither available nor device enabled.
> >>>>   From my reading of the binding [1], the property should be present in 
> >>>> all
> >>>> the
> >>>> hostbridges if one specify it. IOW, I think the property should also be
> >>>> added
> >>>> for hostbridges that are not available.
> >>> Just wanted to say that I think you are right:
> >>>
> >>> """
> >>> It is required to either not set this property at all or set it for all
> >>> host bridges in the system, otherwise potentially conflicting domain 
> >>> numbers
> >>> may be assigned to root buses behind different host bridges.  The domain
> >>> number for each host bridge in the system must be unique.
> >>> """
> >>>
> >>> and I am ready to believe device trees with disabled bridges might have
> >>> (incorrectly) ignored the rule.
> >> Looking at linux/arch/arm64/boot/dts/, there are a few Device-Tree that
> >> contain the property "linux,pci-domain". All of them seems to also add it 
> >> for
> >> disabled hostbridges.
> >>
> >> However, I am under the impression that it is more common to have a
> >> Devide-Tree without any property "linux,pci-domain". When PCI support is
> >> enabled, Xen will generate the domain ID for the hostbridge and write it to
> >> the DT.
> >>
> >> This doesn't work for disabled hostbridge and I think this is Oleksandr's
> >> problem. @Oleksandr can you confirm it?
> >>
> >>>
> >>>> AFAICT, Linux will ignore hostbridge that are not available. But it feels
> >>>> to
> >>>> me we are twisting the rule. So, could we consider to allocate an unused
> >>>> number?
> >>> I think that would be fine but it doesn't look easy from the current Xen
> >>> code point of view because the allocation depends on the Xen driver,
> >>> which we don't have. But I'll let others comment on it.
> >> So what matters is Xen doesn't make things worse. We have two cases to 
> >> care:
> >>    1) Xen only has drivers for a part of the hostbriges
> >>    2) Some hostbriges are disabled
> >>
> >> Case 1) will definitely generate a DT that will make Linux unhappy if we 
> >> have
> >> don't add the property consistently.
> > Good point!
> So, the bottom line: we add the property regardless of the status?
> And the segment we assign for the disabled ones is pci_get_new_domain_nr()?

Yeah I think so


> I guess nothing bad happens if we have say 3 bridges:
> okay - > seg 0
> disabled - > seg 1
> okay - > seg 2

It should be fine I think