[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 02/28] ARM: GICv3 ITS: parse and store ITS subnodes from hardware DT
Hi Andre,
On 30/01/17 18:31, Andre Przywara wrote:
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
new file mode 100644
index 0000000..ff0f571
--- /dev/null
+++ b/xen/arch/arm/gic-v3-its.c
@@ -0,0 +1,71 @@
+/*
+ * xen/arch/arm/gic-v3-its.c
+ *
+ * ARM GICv3 Interrupt Translation Service (ITS) support
+ *
+ * Copyright (C) 2016,2017 - ARM Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/config.h>
No need to include xen/config.h it will be done by default.
+#include <xen/lib.h>
+#include <xen/device_tree.h>
+#include <xen/libfdt/libfdt.h>
+#include <asm/gic.h>
+#include <asm/gic_v3_defs.h>
The 3 headers above does not look necessary for now. Please try to
include them when needed.
+#include <asm/gic_v3_its.h>
+
+/* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
+void gicv3_its_dt_init(const struct dt_device_node *node)
+{
+ const struct dt_device_node *its = NULL;
+ struct host_its *its_data;
+
+ /*
+ * Check for ITS MSI subnodes. If any, add the ITS register
+ * frames to the ITS list.
+ */
+ dt_for_each_child_node(node, its)
+ {
+ paddr_t addr, size;
NIT: dt_device_get_address is taking uint64_t variables in parameter. So
I would prefer to use uint64_t here for consistency.
+
+ if ( !dt_device_is_compatible(its, "arm,gic-v3-its") )
+ continue;
+
+ if ( !dt_device_is_available(its) )
+ continue;
Can an ITS really be disabled? Or is it just for debugging?
+
+ if ( dt_device_get_address(its, 0, &addr, &size) )
+ panic("GICv3: Cannot find a valid ITS frame address");
+
+ its_data = xzalloc(struct host_its);
+ if ( !its_data )
+ panic("GICv3: Cannot allocate memory for ITS frame");
+
+ its_data->addr = addr;
+ its_data->size = size;
+ its_data->dt_node = its;
+
+ printk("GICv3: Found ITS @0x%lx\n", addr);
+
+ list_add_tail(&its_data->entry, &host_its_list);
+ }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index b8be395..838dd11 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -43,9 +43,12 @@
#include <asm/device.h>
#include <asm/gic.h>
#include <asm/gic_v3_defs.h>
+#include <asm/gic_v3_its.h>
#include <asm/cpufeature.h>
#include <asm/acpi.h>
+LIST_HEAD(host_its_list);
I would rather limit the number of variable exported. I've looked at how
host_its_list is used accross this series and I don't think it is
necessary to export it.
The 2 users (not including gic-v3-its.c) are in gic-v3.c and vgic-v3.c.
I will explain how to replace the one in vgic-v3.c on the corresponding
patch.
For gic-v3.c, you use host_its_list to check if ITS is available and
going through the list. For the former, you could have gicv3_its_dt_init
returning the number ITS available. For the latter, the loop is calling
a function living in gic-v3-its.c where host_its_list is already available.
I will mention again when review the associated patches.
+
/* Global state */
static struct {
void __iomem *map_dbase; /* Mapped address of distributor registers */
@@ -1224,11 +1227,12 @@ static void __init gicv3_dt_init(void)
*/
res = dt_device_get_address(node, 1 + gicv3.rdist_count,
&cbase, &csize);
- if ( res )
- return;
+ if ( !res )
+ dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
+ &vbase, &vsize);
- dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
- &vbase, &vsize);
+ /* Check for ITS child nodes and build the host ITS list accordingly. */
+ gicv3_its_dt_init(node);
}
static int gicv3_iomem_deny_access(const struct domain *d)
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
new file mode 100644
index 0000000..2f5c51c
--- /dev/null
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -0,0 +1,57 @@
+/*
+ * ARM GICv3 ITS support
+ *
+ * Andre Przywara <andre.przywara@xxxxxxx>
+ * Copyright (c) 2016 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __ASM_ARM_ITS_H__
+#define __ASM_ARM_ITS_H__
+
+#ifndef __ASSEMBLY__
Do you expect the ITS header to be included in the assembly code? If
not, then I would drop the #ifndef __ASSEMBLY.
+#include <xen/device_tree.h>
+
+/* data structure for each hardware ITS */
+struct host_its {
+ struct list_head entry;
+ const struct dt_device_node *dt_node;
+ paddr_t addr;
+ paddr_t size;
+};
+
+extern struct list_head host_its_list;
+
+#ifdef CONFIG_HAS_ITS
+
+/* Parse the host DT and pick up all host ITSes. */
+void gicv3_its_dt_init(const struct dt_device_node *node);
+
+#else
+
+static inline void gicv3_its_dt_init(const struct dt_device_node *node)
+{
+}
+
+#endif /* CONFIG_HAS_ITS */
+
+#endif /* __ASSEMBLY__ */
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
