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

[Julien Grall <julien@xxxxxxx>]

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.
I believe that in case 2), current Linux will not check for the 
consistency. But that something, we probably should avoid to rely on.
I think in the two cases we can generate the domain ID by calling 
pci_get_new_domain_nr().
Now if we have to support inconsistent device-tree. Then we could 
collect the "linux,pci-domain" and find the maximum one. We would 
allocate a number above for any hostbridges with no property.
> Otherwise
> skipping the disabled host bridge node for Dom0 sounds OK.
At the moment, I haven't found any example of Device-Tree where 
"linux,pci-domain" will be only on part of the hostbridges (see above).
So I think we should avoid breaking the rule here at least until we have 
a "real" DT that break it.
Cheers,

--
Julien Grall