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

Re: [PATCH v1 05/14] xen/arm: PCI host bridge discovery within XEN on ARM

On 10/09/2021 12:22, Rahul Singh wrote:
Hi Julien

Hi Rahul,
On 7 Sep 2021, at 10:05 am, Julien Grall <julien@xxxxxxx> wrote:
On 19/08/2021 13:02, Rahul Singh wrote:
+#include <xen/rwlock.h>
+#include <xen/sched.h>
+#include <xen/vmap.h>
+ * List for all the pci host bridges.
+ */
+static LIST_HEAD(pci_host_bridges);
+static atomic_t domain_nr = ATOMIC_INIT(-1);
+bool dt_pci_parse_bus_range(struct dt_device_node *dev,
+                            struct pci_config_window *cfg)

Aside, "pci_config_window", the function is not Arm specific. Would it be possible to 
consider to introduce "struct resource" in Xen so this function can be moved in 

I can introduce the "struct resource” but I am not sure whether "struct 
resource” will be
useful later point in time. What I prefer as of now, we can have this function 
and we can
move this to common/device_tree.c once we have the requirement for "struct 

TBH, I wasn't asking about using "struct resource". This was simply a suggestion how to make the code common.

What I am more interested is moving this function in common code so it can be re-used by RISC-v in the future. I don't particularly mind whether this is using "struct resource" or a different structure.

diff --git a/xen/arch/arm/pci/pci-host-generic.c 
new file mode 100644
index 0000000000..13d0f7f999
--- /dev/null
+++ b/xen/arch/arm/pci/pci-host-generic.c
@@ -0,0 +1,55 @@
+ * Copyright (C) 2021 Arm Ltd.
+ *
+ * Based on Linux drivers/pci/controller/pci-host-common.c
+ * Based on Linux drivers/pci/controller/pci-host-generic.c
+ * Copyright (C) 2014 ARM Limited Will Deacon <will.deacon@xxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <asm/device.h>
+#include <xen/pci.h>
+#include <asm/pci.h>
+static const struct dt_device_match gen_pci_dt_match[] = {
+    { .compatible = "pci-host-ecam-generic" },
+    { },
+static int gen_pci_dt_init(struct dt_device_node *dev, const void *data)
+    const struct dt_device_match *of_id;
+    of_id = dt_match_node(gen_pci_dt_match, dev->dev.of_node);

This seems to be a bit pointless to me as you already know the compatible 
(there is only one possible...).
  As of now we are only implementing the "pci-host-ecam-generic” compatible 
PCI, but in future we might
need to implement the other compatible like Linux see as below.


Right... You know the node matched and you don't need any data. So this is a bit pointless to have to walk again the structure for just printing the compatible that matched.


Julien Grall



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