[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

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?


Fixes: 4cfab4425d39 ("xen/arm: Add linux,pci-domain property for hwdom if not 
available.")

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
---
  xen/arch/arm/domain_build.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 124ade09123a..beeecf84a209 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -747,7 +747,8 @@ static int __init write_properties(struct domain *d, struct 
kernel_info *kinfo,
              return res;
      }
- if ( is_pci_passthrough_enabled() && dt_device_type_is_equal(node, "pci") )
+    if ( is_pci_passthrough_enabled() && dt_device_type_is_equal(node, "pci") 
&&
From my understanding, PCI device described in the DT would also have 'type="pci"'. So we would also return an error because we don't have a corresponding hostbridge.

I think this check should be replaced with device_get_class(node) == DEVICE_PCI (confusingly DEVICE_PCI indicates that this is an hostbridge...). Although, I would consider to pass the device class as a parameter of write_properties() to avoid calling device_class() multiple time (it is already used twice in handle_node()).

I am aware this is not a bug introduced by your patch, but I think this should be dealt in this patch as well.

+         dt_device_is_available(node) )

Shouldn't you also check that the hostbridge wasn't passthrough-ed?

      {
          if ( !dt_find_property(node, "linux,pci-domain", NULL) )
          {


Cheers,

[1] Documentation/devicetree/bindings/pci/pci.txt

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.