All of lore.kernel.org
 help / color / mirror / Atom feed
* [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig
@ 2016-05-24 10:20 Julien Grall
  2016-05-24 10:20 ` [for-4.8 1/2] " Julien Grall
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Julien Grall @ 2016-05-24 10:20 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, sstabellini

Hello all,

This small patch series converts DEBUG_DT to Kconfig. This is easier
to enable than having to modify the code when the user wants to debug
the device tree parsing.

This series is based on the version 4 of "Kconfig debug options" [1].

Yours sincerely,

Julien Grall (2):
  xen/arm: Convert DEBUG_DT to Kconfig
  xen/arm: Provide device tree debugging helper in a single place

 xen/Kconfig.debug             |  7 +++++
 xen/arch/arm/domain_build.c   | 73 ++++++++++++++++++++-----------------------
 xen/common/device_tree.c      |  6 +---
 xen/include/xen/device_tree.h |  9 ++++++
 4 files changed, 51 insertions(+), 44 deletions(-)

-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [for-4.8 1/2] xen/arm: Convert DEBUG_DT to Kconfig
  2016-05-24 10:20 [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig Julien Grall
@ 2016-05-24 10:20 ` Julien Grall
  2016-05-24 13:38   ` Konrad Rzeszutek Wilk
  2016-05-24 14:16   ` Edgar E. Iglesias
  2016-05-24 10:20 ` [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place Julien Grall
  2016-05-24 10:21 ` [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig Julien Grall
  2 siblings, 2 replies; 9+ messages in thread
From: Julien Grall @ 2016-05-24 10:20 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Doug Goldstein

Convert device-tree debugging to 'Kconfig' as
CONFIG_DEBUG_TREE_DEBUG.

The option is not enabled by default because the output is very
verbose.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Doug Goldstein <cardoe@cardoe.com>
---
 xen/Kconfig.debug           | 7 +++++++
 xen/arch/arm/domain_build.c | 4 +---
 xen/common/device_tree.c    | 4 +---
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 303bf36..59be34d 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -55,6 +55,13 @@ config VERBOSE_DEBUG
 	  Guest output from HYPERVISOR_console_io and hypervisor parsing
 	  ELF images (dom0) is logged in the Xen ring buffer.
 
+config DEVICE_TREE_DEBUG
+	bool "Device tree debug messages"
+	depends on HAS_DEVICE_TREE
+	---help---
+	  Device tree parsing and DOM0 device tree building messages is
+	  logged in the Xen ring buffer
+
 endif # DEBUG || EXPERT
 
 endmenu
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 00dc07a..fb035ff 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -42,9 +42,7 @@ static void __init parse_dom0_mem(const char *s)
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
-//#define DEBUG_DT
-
-#ifdef DEBUG_DT
+#ifdef CONFIG_DEVICE_TREE_DEBUG
 # define DPRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
 #else
 # define DPRINT(fmt, args...) do {} while ( 0 )
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 06a2837..0df2e4b 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -54,9 +54,7 @@ struct dt_alias_prop {
 
 static LIST_HEAD(aliases_lookup);
 
-// #define DEBUG_DT
-
-#ifdef DEBUG_DT
+#ifdef CONFIG_DEVICE_TREE_DEBUG
 # define dt_dprintk(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
 static void dt_dump_addr(const char *s, const __be32 *addr, int na)
 {
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place
  2016-05-24 10:20 [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig Julien Grall
  2016-05-24 10:20 ` [for-4.8 1/2] " Julien Grall
@ 2016-05-24 10:20 ` Julien Grall
  2016-05-24 13:40   ` Konrad Rzeszutek Wilk
  2016-05-24 14:17   ` Edgar E. Iglesias
  2016-05-24 10:21 ` [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig Julien Grall
  2 siblings, 2 replies; 9+ messages in thread
From: Julien Grall @ 2016-05-24 10:20 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, sstabellini

Provide helper to debug the device tree in device_tree.h. This will
avoid to have to redeclare helper for each file requiring debug.

Also replace DPRINT by the new helper dt_dprintk in domain_build.c

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain_build.c   | 71 +++++++++++++++++++++----------------------
 xen/common/device_tree.c      |  2 --
 xen/include/xen/device_tree.h |  9 ++++++
 3 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index fb035ff..71ead8b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -42,12 +42,6 @@ static void __init parse_dom0_mem(const char *s)
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
-#ifdef CONFIG_DEVICE_TREE_DEBUG
-# define DPRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
-#else
-# define DPRINT(fmt, args...) do {} while ( 0 )
-#endif
-
 //#define DEBUG_11_ALLOCATION
 #ifdef DEBUG_11_ALLOCATION
 # define D11PRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
@@ -364,7 +358,7 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
     {
         int l;
 
-        DPRINT("memory node\n");
+        dt_dprintk("memory node\n");
 
         reg_size = dt_cells_to_size(dt_n_addr_cells(memory) + dt_n_size_cells(memory));
 
@@ -571,7 +565,8 @@ static int make_memory_node(const struct domain *d,
     __be32 reg[nr_cells];
     __be32 *cells;
 
-    DPRINT("Create memory node (reg size %d, nr cells %d)\n", reg_size, nr_cells);
+    dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
+               reg_size, nr_cells);
 
     /* ePAPR 3.4 */
     res = fdt_begin_node(fdt, "memory");
@@ -588,8 +583,8 @@ static int make_memory_node(const struct domain *d,
         u64 start = kinfo->mem.bank[i].start;
         u64 size = kinfo->mem.bank[i].size;
 
-        DPRINT("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
-                i, start, start + size);
+        dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
+                   i, start, start + size);
 
         dt_child_set_range(&cells, parent, start, size);
     }
@@ -618,7 +613,7 @@ static int make_hypervisor_node(const struct kernel_info *kinfo,
     int sizecells = dt_child_n_size_cells(parent);
     void *fdt = kinfo->fdt;
 
-    DPRINT("Create hypervisor node\n");
+    dt_dprintk("Create hypervisor node\n");
 
     /*
      * Sanity-check address sizes, since addresses and sizes which do
@@ -667,7 +662,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
         "arm,psci-0.2""\0"
         "arm,psci";
 
-    DPRINT("Create PSCI node\n");
+    dt_dprintk("Create PSCI node\n");
 
     /* See linux Documentation/devicetree/bindings/arm/psci.txt */
     res = fdt_begin_node(fdt, "psci");
@@ -710,7 +705,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
     bool_t clock_valid;
     uint64_t mpidr_aff;
 
-    DPRINT("Create cpus node\n");
+    dt_dprintk("Create cpus node\n");
 
     if ( !cpus )
     {
@@ -765,7 +760,8 @@ static int make_cpus_node(const struct domain *d, void *fdt,
          * is enough for the current max vcpu number.
          */
         mpidr_aff = vcpuid_to_vaffinity(cpu);
-        DPRINT("Create cpu@%"PRIx64" (logical CPUID: %d) node\n", mpidr_aff, cpu);
+        dt_dprintk("Create cpu@%"PRIx64" (logical CPUID: %d) node\n",
+                   mpidr_aff, cpu);
 
         snprintf(buf, sizeof(buf), "cpu@%"PRIx64, mpidr_aff);
         res = fdt_begin_node(fdt, buf);
@@ -821,11 +817,11 @@ static int make_gic_node(const struct domain *d, void *fdt,
      */
     if ( node != dt_interrupt_controller )
     {
-        DPRINT("  Skipping (secondary GIC)\n");
+        dt_dprintk("  Skipping (secondary GIC)\n");
         return 0;
     }
 
-    DPRINT("Create gic node\n");
+    dt_dprintk("Create gic node\n");
 
     res = fdt_begin_node(fdt, "interrupt-controller");
     if ( res )
@@ -837,7 +833,7 @@ static int make_gic_node(const struct domain *d, void *fdt,
      */
     if ( gic->phandle )
     {
-        DPRINT("  Set phandle = 0x%x\n", gic->phandle);
+        dt_dprintk("  Set phandle = 0x%x\n", gic->phandle);
         res = fdt_property_cell(fdt, "phandle", gic->phandle);
         if ( res )
             return res;
@@ -894,7 +890,7 @@ static int make_timer_node(const struct domain *d, void *fdt,
     u32 clock_frequency;
     bool_t clock_valid;
 
-    DPRINT("Create timer node\n");
+    dt_dprintk("Create timer node\n");
 
     dev = dt_find_matching_node(NULL, timer_ids);
     if ( !dev )
@@ -922,15 +918,15 @@ static int make_timer_node(const struct domain *d, void *fdt,
      * level-sensitive interrupt */
 
     irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);
-    DPRINT("  Secure interrupt %u\n", irq);
+    dt_dprintk("  Secure interrupt %u\n", irq);
     set_interrupt_ppi(intrs[0], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
 
     irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
-    DPRINT("  Non secure interrupt %u\n", irq);
+    dt_dprintk("  Non secure interrupt %u\n", irq);
     set_interrupt_ppi(intrs[1], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
 
     irq = timer_get_irq(TIMER_VIRT_PPI);
-    DPRINT("  Virt interrupt %u\n", irq);
+    dt_dprintk("  Virt interrupt %u\n", irq);
     set_interrupt_ppi(intrs[2], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
 
     res = fdt_property_interrupts(fdt, intrs, 3);
@@ -984,7 +980,7 @@ static int map_irq_to_domain(const struct dt_device_node *dev,
         }
     }
 
-    DPRINT("  - IRQ: %u\n", irq);
+    dt_dprintk("  - IRQ: %u\n", irq);
     return 0;
 }
 
@@ -1053,7 +1049,7 @@ static int map_range_to_domain(const struct dt_device_node *dev,
         }
     }
 
-    DPRINT("  - MMIO: %010"PRIx64" - %010"PRIx64"\n", addr, addr + len);
+    dt_dprintk("  - MMIO: %010"PRIx64" - %010"PRIx64"\n", addr, addr + len);
 
     return 0;
 }
@@ -1070,7 +1066,8 @@ static int map_device_children(struct domain *d,
 
     if ( dt_device_type_is_equal(dev, "pci") )
     {
-        DPRINT("Mapping children of %s to guest\n", dt_node_full_name(dev));
+        dt_dprintk("Mapping children of %s to guest\n",
+                   dt_node_full_name(dev));
 
         ret = dt_for_each_irq_map(dev, &map_dt_irq_to_domain, d);
         if ( ret < 0 )
@@ -1105,12 +1102,12 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
     nirq = dt_number_of_irq(dev);
     naddr = dt_number_of_address(dev);
 
-    DPRINT("%s passthrough = %d nirq = %d naddr = %u\n", dt_node_full_name(dev),
-           need_mapping, nirq, naddr);
+    dt_dprintk("%s passthrough = %d nirq = %d naddr = %u\n",
+               dt_node_full_name(dev), need_mapping, nirq, naddr);
 
     if ( dt_device_is_protected(dev) && need_mapping )
     {
-        DPRINT("%s setup iommu\n", dt_node_full_name(dev));
+        dt_dprintk("%s setup iommu\n", dt_node_full_name(dev));
         res = iommu_assign_dt_device(d, dev);
         if ( res )
         {
@@ -1137,8 +1134,8 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
          */
         if ( rirq.controller != dt_interrupt_controller )
         {
-            DPRINT("irq %u not connected to primary controller."
-                   "Connected to %s\n", i, dt_node_full_name(rirq.controller));
+            dt_dprintk("irq %u not connected to primary controller. Connected to %s\n",
+                      i, dt_node_full_name(rirq.controller));
             continue;
         }
 
@@ -1217,17 +1214,17 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
 
     path = dt_node_full_name(node);
 
-    DPRINT("handle %s\n", path);
+    dt_dprintk("handle %s\n", path);
 
     /* Skip theses nodes and the sub-nodes */
     if ( dt_match_node(skip_matches, node) )
     {
-        DPRINT("  Skip it (matched)\n");
+        dt_dprintk("  Skip it (matched)\n");
         return 0;
     }
     if ( platform_device_is_blacklisted(node) )
     {
-        DPRINT("  Skip it (blacklisted)\n");
+        dt_dprintk("  Skip it (blacklisted)\n");
         return 0;
     }
 
@@ -1243,7 +1240,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
     /* Skip nodes used by Xen */
     if ( dt_device_used_by(node) == DOMID_XEN )
     {
-        DPRINT("  Skip it (used by Xen)\n");
+        dt_dprintk("  Skip it (used by Xen)\n");
         return 0;
     }
 
@@ -1253,7 +1250,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
      */
     if ( device_get_class(node) == DEVICE_IOMMU )
     {
-        DPRINT(" IOMMU, skip it\n");
+        dt_dprintk(" IOMMU, skip it\n");
         return 0;
     }
 
@@ -1428,7 +1425,7 @@ static int acpi_make_chosen_node(const struct kernel_info *kinfo)
     const struct bootmodule *mod = kinfo->kernel_bootmodule;
     void *fdt = kinfo->fdt;
 
-    DPRINT("Create chosen node\n");
+    dt_dprintk("Create chosen node\n");
     res = fdt_begin_node(fdt, "chosen");
     if ( res )
         return res;
@@ -1472,7 +1469,7 @@ static int acpi_make_hypervisor_node(const struct kernel_info *kinfo,
     /* Convenience alias */
     void *fdt = kinfo->fdt;
 
-    DPRINT("Create hypervisor node\n");
+    dt_dprintk("Create hypervisor node\n");
 
     /* See linux Documentation/devicetree/bindings/arm/xen.txt */
     res = fdt_begin_node(fdt, "hypervisor");
@@ -1502,7 +1499,7 @@ static int create_acpi_dtb(struct kernel_info *kinfo, struct membank tbl_add[])
     int new_size;
     int ret;
 
-    DPRINT("Prepare a min DTB for DOM0\n");
+    dt_dprintk("Prepare a min DTB for DOM0\n");
 
     /* Allocate min size for DT */
     new_size = ACPI_DOM0_FDT_MIN_SIZE;
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 0df2e4b..b39c8ca 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -55,7 +55,6 @@ struct dt_alias_prop {
 static LIST_HEAD(aliases_lookup);
 
 #ifdef CONFIG_DEVICE_TREE_DEBUG
-# define dt_dprintk(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
 static void dt_dump_addr(const char *s, const __be32 *addr, int na)
 {
     dt_dprintk("%s", s);
@@ -64,7 +63,6 @@ static void dt_dump_addr(const char *s, const __be32 *addr, int na)
     dt_dprintk("\n");
 }
 #else
-# define dt_dprintk(fmt, args...) do {} while ( 0 )
 static void dt_dump_addr(const char *s, const __be32 *addr, int na) { }
 #endif
 
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index e3fe77c..d7d1b40 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -753,6 +753,15 @@ int dt_parse_phandle_with_args(const struct dt_device_node *np,
                                const char *cells_name, int index,
                                struct dt_phandle_args *out_args);
 
+#ifdef CONFIG_DEVICE_TREE_DEBUG
+#define dt_dprintk(fmt, args...)  \
+    printk(XENLOG_DEBUG fmt, ## args)
+#else
+static inline void
+__attribute__ ((__format__ (__printf__, 1, 2)))
+dt_dprintk(const char *fmt, ...) {}
+#endif
+
 #endif /* __XEN_DEVICE_TREE_H */
 
 /*
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig
  2016-05-24 10:20 [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig Julien Grall
  2016-05-24 10:20 ` [for-4.8 1/2] " Julien Grall
  2016-05-24 10:20 ` [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place Julien Grall
@ 2016-05-24 10:21 ` Julien Grall
  2 siblings, 0 replies; 9+ messages in thread
From: Julien Grall @ 2016-05-24 10:21 UTC (permalink / raw)
  To: xen-devel; +Cc: sstabellini



On 24/05/2016 11:20, Julien Grall wrote:
> Hello all,
>
> This small patch series converts DEBUG_DT to Kconfig. This is easier
> to enable than having to modify the code when the user wants to debug
> the device tree parsing.
>
> This series is based on the version 4 of "Kconfig debug options" [1].

I forgot to add a link to Doug's series:

http://lists.xen.org/archives/html/xen-devel/2016-05/msg02093.html

>
> Yours sincerely,
>
> Julien Grall (2):
>   xen/arm: Convert DEBUG_DT to Kconfig
>   xen/arm: Provide device tree debugging helper in a single place
>
>  xen/Kconfig.debug             |  7 +++++
>  xen/arch/arm/domain_build.c   | 73 ++++++++++++++++++++-----------------------
>  xen/common/device_tree.c      |  6 +---
>  xen/include/xen/device_tree.h |  9 ++++++
>  4 files changed, 51 insertions(+), 44 deletions(-)
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [for-4.8 1/2] xen/arm: Convert DEBUG_DT to Kconfig
  2016-05-24 10:20 ` [for-4.8 1/2] " Julien Grall
@ 2016-05-24 13:38   ` Konrad Rzeszutek Wilk
  2016-05-24 14:44     ` Julien Grall
  2016-05-24 14:16   ` Edgar E. Iglesias
  1 sibling, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-24 13:38 UTC (permalink / raw)
  To: Julien Grall
  Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Jan Beulich, Doug Goldstein

On Tue, May 24, 2016 at 11:20:40AM +0100, Julien Grall wrote:
> Convert device-tree debugging to 'Kconfig' as
> CONFIG_DEBUG_TREE_DEBUG.
> 
> The option is not enabled by default because the output is very
> verbose.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Doug Goldstein <cardoe@cardoe.com>
> ---
>  xen/Kconfig.debug           | 7 +++++++
>  xen/arch/arm/domain_build.c | 4 +---
>  xen/common/device_tree.c    | 4 +---
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 303bf36..59be34d 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -55,6 +55,13 @@ config VERBOSE_DEBUG
>  	  Guest output from HYPERVISOR_console_io and hypervisor parsing
>  	  ELF images (dom0) is logged in the Xen ring buffer.
>  
> +config DEVICE_TREE_DEBUG
> +	bool "Device tree debug messages"
> +	depends on HAS_DEVICE_TREE
> +	---help---
> +	  Device tree parsing and DOM0 device tree building messages is
> +	  logged in the Xen ring buffer

s/is logged/are logged/

Also missing stop at the end.

Perhaps also add:

"If unsure, say N here."

Or could this be part of the VERBOSE one (which spews out data about
ELF parsing and allows guests to do  the console_io_write hypercalls?).
> +
>  endif # DEBUG || EXPERT
>  
>  endmenu
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 00dc07a..fb035ff 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -42,9 +42,7 @@ static void __init parse_dom0_mem(const char *s)
>  }
>  custom_param("dom0_mem", parse_dom0_mem);
>  
> -//#define DEBUG_DT
> -
> -#ifdef DEBUG_DT
> +#ifdef CONFIG_DEVICE_TREE_DEBUG
>  # define DPRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
>  #else
>  # define DPRINT(fmt, args...) do {} while ( 0 )
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 06a2837..0df2e4b 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -54,9 +54,7 @@ struct dt_alias_prop {
>  
>  static LIST_HEAD(aliases_lookup);
>  
> -// #define DEBUG_DT
> -
> -#ifdef DEBUG_DT
> +#ifdef CONFIG_DEVICE_TREE_DEBUG
>  # define dt_dprintk(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
>  static void dt_dump_addr(const char *s, const __be32 *addr, int na)
>  {
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place
  2016-05-24 10:20 ` [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place Julien Grall
@ 2016-05-24 13:40   ` Konrad Rzeszutek Wilk
  2016-05-24 14:17   ` Edgar E. Iglesias
  1 sibling, 0 replies; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-24 13:40 UTC (permalink / raw)
  To: Julien Grall; +Cc: sstabellini, xen-devel

On Tue, May 24, 2016 at 11:20:41AM +0100, Julien Grall wrote:
> Provide helper to debug the device tree in device_tree.h. This will
> avoid to have to redeclare helper for each file requiring debug.
> 
> Also replace DPRINT by the new helper dt_dprintk in domain_build.c

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/domain_build.c   | 71 +++++++++++++++++++++----------------------
>  xen/common/device_tree.c      |  2 --
>  xen/include/xen/device_tree.h |  9 ++++++
>  3 files changed, 43 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index fb035ff..71ead8b 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -42,12 +42,6 @@ static void __init parse_dom0_mem(const char *s)
>  }
>  custom_param("dom0_mem", parse_dom0_mem);
>  
> -#ifdef CONFIG_DEVICE_TREE_DEBUG
> -# define DPRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
> -#else
> -# define DPRINT(fmt, args...) do {} while ( 0 )
> -#endif
> -
>  //#define DEBUG_11_ALLOCATION
>  #ifdef DEBUG_11_ALLOCATION
>  # define D11PRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
> @@ -364,7 +358,7 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
>      {
>          int l;
>  
> -        DPRINT("memory node\n");
> +        dt_dprintk("memory node\n");
>  
>          reg_size = dt_cells_to_size(dt_n_addr_cells(memory) + dt_n_size_cells(memory));
>  
> @@ -571,7 +565,8 @@ static int make_memory_node(const struct domain *d,
>      __be32 reg[nr_cells];
>      __be32 *cells;
>  
> -    DPRINT("Create memory node (reg size %d, nr cells %d)\n", reg_size, nr_cells);
> +    dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> +               reg_size, nr_cells);
>  
>      /* ePAPR 3.4 */
>      res = fdt_begin_node(fdt, "memory");
> @@ -588,8 +583,8 @@ static int make_memory_node(const struct domain *d,
>          u64 start = kinfo->mem.bank[i].start;
>          u64 size = kinfo->mem.bank[i].size;
>  
> -        DPRINT("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
> -                i, start, start + size);
> +        dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
> +                   i, start, start + size);
>  
>          dt_child_set_range(&cells, parent, start, size);
>      }
> @@ -618,7 +613,7 @@ static int make_hypervisor_node(const struct kernel_info *kinfo,
>      int sizecells = dt_child_n_size_cells(parent);
>      void *fdt = kinfo->fdt;
>  
> -    DPRINT("Create hypervisor node\n");
> +    dt_dprintk("Create hypervisor node\n");
>  
>      /*
>       * Sanity-check address sizes, since addresses and sizes which do
> @@ -667,7 +662,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
>          "arm,psci-0.2""\0"
>          "arm,psci";
>  
> -    DPRINT("Create PSCI node\n");
> +    dt_dprintk("Create PSCI node\n");
>  
>      /* See linux Documentation/devicetree/bindings/arm/psci.txt */
>      res = fdt_begin_node(fdt, "psci");
> @@ -710,7 +705,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
>      bool_t clock_valid;
>      uint64_t mpidr_aff;
>  
> -    DPRINT("Create cpus node\n");
> +    dt_dprintk("Create cpus node\n");
>  
>      if ( !cpus )
>      {
> @@ -765,7 +760,8 @@ static int make_cpus_node(const struct domain *d, void *fdt,
>           * is enough for the current max vcpu number.
>           */
>          mpidr_aff = vcpuid_to_vaffinity(cpu);
> -        DPRINT("Create cpu@%"PRIx64" (logical CPUID: %d) node\n", mpidr_aff, cpu);
> +        dt_dprintk("Create cpu@%"PRIx64" (logical CPUID: %d) node\n",
> +                   mpidr_aff, cpu);
>  
>          snprintf(buf, sizeof(buf), "cpu@%"PRIx64, mpidr_aff);
>          res = fdt_begin_node(fdt, buf);
> @@ -821,11 +817,11 @@ static int make_gic_node(const struct domain *d, void *fdt,
>       */
>      if ( node != dt_interrupt_controller )
>      {
> -        DPRINT("  Skipping (secondary GIC)\n");
> +        dt_dprintk("  Skipping (secondary GIC)\n");
>          return 0;
>      }
>  
> -    DPRINT("Create gic node\n");
> +    dt_dprintk("Create gic node\n");
>  
>      res = fdt_begin_node(fdt, "interrupt-controller");
>      if ( res )
> @@ -837,7 +833,7 @@ static int make_gic_node(const struct domain *d, void *fdt,
>       */
>      if ( gic->phandle )
>      {
> -        DPRINT("  Set phandle = 0x%x\n", gic->phandle);
> +        dt_dprintk("  Set phandle = 0x%x\n", gic->phandle);
>          res = fdt_property_cell(fdt, "phandle", gic->phandle);
>          if ( res )
>              return res;
> @@ -894,7 +890,7 @@ static int make_timer_node(const struct domain *d, void *fdt,
>      u32 clock_frequency;
>      bool_t clock_valid;
>  
> -    DPRINT("Create timer node\n");
> +    dt_dprintk("Create timer node\n");
>  
>      dev = dt_find_matching_node(NULL, timer_ids);
>      if ( !dev )
> @@ -922,15 +918,15 @@ static int make_timer_node(const struct domain *d, void *fdt,
>       * level-sensitive interrupt */
>  
>      irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);
> -    DPRINT("  Secure interrupt %u\n", irq);
> +    dt_dprintk("  Secure interrupt %u\n", irq);
>      set_interrupt_ppi(intrs[0], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
>  
>      irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
> -    DPRINT("  Non secure interrupt %u\n", irq);
> +    dt_dprintk("  Non secure interrupt %u\n", irq);
>      set_interrupt_ppi(intrs[1], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
>  
>      irq = timer_get_irq(TIMER_VIRT_PPI);
> -    DPRINT("  Virt interrupt %u\n", irq);
> +    dt_dprintk("  Virt interrupt %u\n", irq);
>      set_interrupt_ppi(intrs[2], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
>  
>      res = fdt_property_interrupts(fdt, intrs, 3);
> @@ -984,7 +980,7 @@ static int map_irq_to_domain(const struct dt_device_node *dev,
>          }
>      }
>  
> -    DPRINT("  - IRQ: %u\n", irq);
> +    dt_dprintk("  - IRQ: %u\n", irq);
>      return 0;
>  }
>  
> @@ -1053,7 +1049,7 @@ static int map_range_to_domain(const struct dt_device_node *dev,
>          }
>      }
>  
> -    DPRINT("  - MMIO: %010"PRIx64" - %010"PRIx64"\n", addr, addr + len);
> +    dt_dprintk("  - MMIO: %010"PRIx64" - %010"PRIx64"\n", addr, addr + len);
>  
>      return 0;
>  }
> @@ -1070,7 +1066,8 @@ static int map_device_children(struct domain *d,
>  
>      if ( dt_device_type_is_equal(dev, "pci") )
>      {
> -        DPRINT("Mapping children of %s to guest\n", dt_node_full_name(dev));
> +        dt_dprintk("Mapping children of %s to guest\n",
> +                   dt_node_full_name(dev));
>  
>          ret = dt_for_each_irq_map(dev, &map_dt_irq_to_domain, d);
>          if ( ret < 0 )
> @@ -1105,12 +1102,12 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
>      nirq = dt_number_of_irq(dev);
>      naddr = dt_number_of_address(dev);
>  
> -    DPRINT("%s passthrough = %d nirq = %d naddr = %u\n", dt_node_full_name(dev),
> -           need_mapping, nirq, naddr);
> +    dt_dprintk("%s passthrough = %d nirq = %d naddr = %u\n",
> +               dt_node_full_name(dev), need_mapping, nirq, naddr);
>  
>      if ( dt_device_is_protected(dev) && need_mapping )
>      {
> -        DPRINT("%s setup iommu\n", dt_node_full_name(dev));
> +        dt_dprintk("%s setup iommu\n", dt_node_full_name(dev));
>          res = iommu_assign_dt_device(d, dev);
>          if ( res )
>          {
> @@ -1137,8 +1134,8 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
>           */
>          if ( rirq.controller != dt_interrupt_controller )
>          {
> -            DPRINT("irq %u not connected to primary controller."
> -                   "Connected to %s\n", i, dt_node_full_name(rirq.controller));
> +            dt_dprintk("irq %u not connected to primary controller. Connected to %s\n",
> +                      i, dt_node_full_name(rirq.controller));
>              continue;
>          }
>  
> @@ -1217,17 +1214,17 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>  
>      path = dt_node_full_name(node);
>  
> -    DPRINT("handle %s\n", path);
> +    dt_dprintk("handle %s\n", path);
>  
>      /* Skip theses nodes and the sub-nodes */
>      if ( dt_match_node(skip_matches, node) )
>      {
> -        DPRINT("  Skip it (matched)\n");
> +        dt_dprintk("  Skip it (matched)\n");
>          return 0;
>      }
>      if ( platform_device_is_blacklisted(node) )
>      {
> -        DPRINT("  Skip it (blacklisted)\n");
> +        dt_dprintk("  Skip it (blacklisted)\n");
>          return 0;
>      }
>  
> @@ -1243,7 +1240,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>      /* Skip nodes used by Xen */
>      if ( dt_device_used_by(node) == DOMID_XEN )
>      {
> -        DPRINT("  Skip it (used by Xen)\n");
> +        dt_dprintk("  Skip it (used by Xen)\n");
>          return 0;
>      }
>  
> @@ -1253,7 +1250,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>       */
>      if ( device_get_class(node) == DEVICE_IOMMU )
>      {
> -        DPRINT(" IOMMU, skip it\n");
> +        dt_dprintk(" IOMMU, skip it\n");
>          return 0;
>      }
>  
> @@ -1428,7 +1425,7 @@ static int acpi_make_chosen_node(const struct kernel_info *kinfo)
>      const struct bootmodule *mod = kinfo->kernel_bootmodule;
>      void *fdt = kinfo->fdt;
>  
> -    DPRINT("Create chosen node\n");
> +    dt_dprintk("Create chosen node\n");
>      res = fdt_begin_node(fdt, "chosen");
>      if ( res )
>          return res;
> @@ -1472,7 +1469,7 @@ static int acpi_make_hypervisor_node(const struct kernel_info *kinfo,
>      /* Convenience alias */
>      void *fdt = kinfo->fdt;
>  
> -    DPRINT("Create hypervisor node\n");
> +    dt_dprintk("Create hypervisor node\n");
>  
>      /* See linux Documentation/devicetree/bindings/arm/xen.txt */
>      res = fdt_begin_node(fdt, "hypervisor");
> @@ -1502,7 +1499,7 @@ static int create_acpi_dtb(struct kernel_info *kinfo, struct membank tbl_add[])
>      int new_size;
>      int ret;
>  
> -    DPRINT("Prepare a min DTB for DOM0\n");
> +    dt_dprintk("Prepare a min DTB for DOM0\n");
>  
>      /* Allocate min size for DT */
>      new_size = ACPI_DOM0_FDT_MIN_SIZE;
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 0df2e4b..b39c8ca 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -55,7 +55,6 @@ struct dt_alias_prop {
>  static LIST_HEAD(aliases_lookup);
>  
>  #ifdef CONFIG_DEVICE_TREE_DEBUG
> -# define dt_dprintk(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
>  static void dt_dump_addr(const char *s, const __be32 *addr, int na)
>  {
>      dt_dprintk("%s", s);
> @@ -64,7 +63,6 @@ static void dt_dump_addr(const char *s, const __be32 *addr, int na)
>      dt_dprintk("\n");
>  }
>  #else
> -# define dt_dprintk(fmt, args...) do {} while ( 0 )
>  static void dt_dump_addr(const char *s, const __be32 *addr, int na) { }
>  #endif
>  
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index e3fe77c..d7d1b40 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -753,6 +753,15 @@ int dt_parse_phandle_with_args(const struct dt_device_node *np,
>                                 const char *cells_name, int index,
>                                 struct dt_phandle_args *out_args);
>  
> +#ifdef CONFIG_DEVICE_TREE_DEBUG
> +#define dt_dprintk(fmt, args...)  \
> +    printk(XENLOG_DEBUG fmt, ## args)
> +#else
> +static inline void
> +__attribute__ ((__format__ (__printf__, 1, 2)))
> +dt_dprintk(const char *fmt, ...) {}
> +#endif
> +
>  #endif /* __XEN_DEVICE_TREE_H */
>  
>  /*
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [for-4.8 1/2] xen/arm: Convert DEBUG_DT to Kconfig
  2016-05-24 10:20 ` [for-4.8 1/2] " Julien Grall
  2016-05-24 13:38   ` Konrad Rzeszutek Wilk
@ 2016-05-24 14:16   ` Edgar E. Iglesias
  1 sibling, 0 replies; 9+ messages in thread
From: Edgar E. Iglesias @ 2016-05-24 14:16 UTC (permalink / raw)
  To: Julien Grall
  Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Jan Beulich, Doug Goldstein

On Tue, May 24, 2016 at 11:20:40AM +0100, Julien Grall wrote:
> Convert device-tree debugging to 'Kconfig' as
> CONFIG_DEBUG_TREE_DEBUG.
         ^^^^^

Hi Julien,

You've got a typo in the commit message, other than that:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Cheers,
Edgar


> 
> The option is not enabled by default because the output is very
> verbose.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Doug Goldstein <cardoe@cardoe.com>
> ---
>  xen/Kconfig.debug           | 7 +++++++
>  xen/arch/arm/domain_build.c | 4 +---
>  xen/common/device_tree.c    | 4 +---
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 303bf36..59be34d 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -55,6 +55,13 @@ config VERBOSE_DEBUG
>  	  Guest output from HYPERVISOR_console_io and hypervisor parsing
>  	  ELF images (dom0) is logged in the Xen ring buffer.
>  
> +config DEVICE_TREE_DEBUG
> +	bool "Device tree debug messages"
> +	depends on HAS_DEVICE_TREE
> +	---help---
> +	  Device tree parsing and DOM0 device tree building messages is
> +	  logged in the Xen ring buffer
> +
>  endif # DEBUG || EXPERT
>  
>  endmenu
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 00dc07a..fb035ff 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -42,9 +42,7 @@ static void __init parse_dom0_mem(const char *s)
>  }
>  custom_param("dom0_mem", parse_dom0_mem);
>  
> -//#define DEBUG_DT
> -
> -#ifdef DEBUG_DT
> +#ifdef CONFIG_DEVICE_TREE_DEBUG
>  # define DPRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
>  #else
>  # define DPRINT(fmt, args...) do {} while ( 0 )
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 06a2837..0df2e4b 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -54,9 +54,7 @@ struct dt_alias_prop {
>  
>  static LIST_HEAD(aliases_lookup);
>  
> -// #define DEBUG_DT
> -
> -#ifdef DEBUG_DT
> +#ifdef CONFIG_DEVICE_TREE_DEBUG
>  # define dt_dprintk(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
>  static void dt_dump_addr(const char *s, const __be32 *addr, int na)
>  {
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place
  2016-05-24 10:20 ` [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place Julien Grall
  2016-05-24 13:40   ` Konrad Rzeszutek Wilk
@ 2016-05-24 14:17   ` Edgar E. Iglesias
  1 sibling, 0 replies; 9+ messages in thread
From: Edgar E. Iglesias @ 2016-05-24 14:17 UTC (permalink / raw)
  To: Julien Grall; +Cc: sstabellini, xen-devel

On Tue, May 24, 2016 at 11:20:41AM +0100, Julien Grall wrote:
> Provide helper to debug the device tree in device_tree.h. This will
> avoid to have to redeclare helper for each file requiring debug.
> 
> Also replace DPRINT by the new helper dt_dprintk in domain_build.c

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/domain_build.c   | 71 +++++++++++++++++++++----------------------
>  xen/common/device_tree.c      |  2 --
>  xen/include/xen/device_tree.h |  9 ++++++
>  3 files changed, 43 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index fb035ff..71ead8b 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -42,12 +42,6 @@ static void __init parse_dom0_mem(const char *s)
>  }
>  custom_param("dom0_mem", parse_dom0_mem);
>  
> -#ifdef CONFIG_DEVICE_TREE_DEBUG
> -# define DPRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
> -#else
> -# define DPRINT(fmt, args...) do {} while ( 0 )
> -#endif
> -
>  //#define DEBUG_11_ALLOCATION
>  #ifdef DEBUG_11_ALLOCATION
>  # define D11PRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
> @@ -364,7 +358,7 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
>      {
>          int l;
>  
> -        DPRINT("memory node\n");
> +        dt_dprintk("memory node\n");
>  
>          reg_size = dt_cells_to_size(dt_n_addr_cells(memory) + dt_n_size_cells(memory));
>  
> @@ -571,7 +565,8 @@ static int make_memory_node(const struct domain *d,
>      __be32 reg[nr_cells];
>      __be32 *cells;
>  
> -    DPRINT("Create memory node (reg size %d, nr cells %d)\n", reg_size, nr_cells);
> +    dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> +               reg_size, nr_cells);
>  
>      /* ePAPR 3.4 */
>      res = fdt_begin_node(fdt, "memory");
> @@ -588,8 +583,8 @@ static int make_memory_node(const struct domain *d,
>          u64 start = kinfo->mem.bank[i].start;
>          u64 size = kinfo->mem.bank[i].size;
>  
> -        DPRINT("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
> -                i, start, start + size);
> +        dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
> +                   i, start, start + size);
>  
>          dt_child_set_range(&cells, parent, start, size);
>      }
> @@ -618,7 +613,7 @@ static int make_hypervisor_node(const struct kernel_info *kinfo,
>      int sizecells = dt_child_n_size_cells(parent);
>      void *fdt = kinfo->fdt;
>  
> -    DPRINT("Create hypervisor node\n");
> +    dt_dprintk("Create hypervisor node\n");
>  
>      /*
>       * Sanity-check address sizes, since addresses and sizes which do
> @@ -667,7 +662,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
>          "arm,psci-0.2""\0"
>          "arm,psci";
>  
> -    DPRINT("Create PSCI node\n");
> +    dt_dprintk("Create PSCI node\n");
>  
>      /* See linux Documentation/devicetree/bindings/arm/psci.txt */
>      res = fdt_begin_node(fdt, "psci");
> @@ -710,7 +705,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
>      bool_t clock_valid;
>      uint64_t mpidr_aff;
>  
> -    DPRINT("Create cpus node\n");
> +    dt_dprintk("Create cpus node\n");
>  
>      if ( !cpus )
>      {
> @@ -765,7 +760,8 @@ static int make_cpus_node(const struct domain *d, void *fdt,
>           * is enough for the current max vcpu number.
>           */
>          mpidr_aff = vcpuid_to_vaffinity(cpu);
> -        DPRINT("Create cpu@%"PRIx64" (logical CPUID: %d) node\n", mpidr_aff, cpu);
> +        dt_dprintk("Create cpu@%"PRIx64" (logical CPUID: %d) node\n",
> +                   mpidr_aff, cpu);
>  
>          snprintf(buf, sizeof(buf), "cpu@%"PRIx64, mpidr_aff);
>          res = fdt_begin_node(fdt, buf);
> @@ -821,11 +817,11 @@ static int make_gic_node(const struct domain *d, void *fdt,
>       */
>      if ( node != dt_interrupt_controller )
>      {
> -        DPRINT("  Skipping (secondary GIC)\n");
> +        dt_dprintk("  Skipping (secondary GIC)\n");
>          return 0;
>      }
>  
> -    DPRINT("Create gic node\n");
> +    dt_dprintk("Create gic node\n");
>  
>      res = fdt_begin_node(fdt, "interrupt-controller");
>      if ( res )
> @@ -837,7 +833,7 @@ static int make_gic_node(const struct domain *d, void *fdt,
>       */
>      if ( gic->phandle )
>      {
> -        DPRINT("  Set phandle = 0x%x\n", gic->phandle);
> +        dt_dprintk("  Set phandle = 0x%x\n", gic->phandle);
>          res = fdt_property_cell(fdt, "phandle", gic->phandle);
>          if ( res )
>              return res;
> @@ -894,7 +890,7 @@ static int make_timer_node(const struct domain *d, void *fdt,
>      u32 clock_frequency;
>      bool_t clock_valid;
>  
> -    DPRINT("Create timer node\n");
> +    dt_dprintk("Create timer node\n");
>  
>      dev = dt_find_matching_node(NULL, timer_ids);
>      if ( !dev )
> @@ -922,15 +918,15 @@ static int make_timer_node(const struct domain *d, void *fdt,
>       * level-sensitive interrupt */
>  
>      irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);
> -    DPRINT("  Secure interrupt %u\n", irq);
> +    dt_dprintk("  Secure interrupt %u\n", irq);
>      set_interrupt_ppi(intrs[0], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
>  
>      irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
> -    DPRINT("  Non secure interrupt %u\n", irq);
> +    dt_dprintk("  Non secure interrupt %u\n", irq);
>      set_interrupt_ppi(intrs[1], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
>  
>      irq = timer_get_irq(TIMER_VIRT_PPI);
> -    DPRINT("  Virt interrupt %u\n", irq);
> +    dt_dprintk("  Virt interrupt %u\n", irq);
>      set_interrupt_ppi(intrs[2], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
>  
>      res = fdt_property_interrupts(fdt, intrs, 3);
> @@ -984,7 +980,7 @@ static int map_irq_to_domain(const struct dt_device_node *dev,
>          }
>      }
>  
> -    DPRINT("  - IRQ: %u\n", irq);
> +    dt_dprintk("  - IRQ: %u\n", irq);
>      return 0;
>  }
>  
> @@ -1053,7 +1049,7 @@ static int map_range_to_domain(const struct dt_device_node *dev,
>          }
>      }
>  
> -    DPRINT("  - MMIO: %010"PRIx64" - %010"PRIx64"\n", addr, addr + len);
> +    dt_dprintk("  - MMIO: %010"PRIx64" - %010"PRIx64"\n", addr, addr + len);
>  
>      return 0;
>  }
> @@ -1070,7 +1066,8 @@ static int map_device_children(struct domain *d,
>  
>      if ( dt_device_type_is_equal(dev, "pci") )
>      {
> -        DPRINT("Mapping children of %s to guest\n", dt_node_full_name(dev));
> +        dt_dprintk("Mapping children of %s to guest\n",
> +                   dt_node_full_name(dev));
>  
>          ret = dt_for_each_irq_map(dev, &map_dt_irq_to_domain, d);
>          if ( ret < 0 )
> @@ -1105,12 +1102,12 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
>      nirq = dt_number_of_irq(dev);
>      naddr = dt_number_of_address(dev);
>  
> -    DPRINT("%s passthrough = %d nirq = %d naddr = %u\n", dt_node_full_name(dev),
> -           need_mapping, nirq, naddr);
> +    dt_dprintk("%s passthrough = %d nirq = %d naddr = %u\n",
> +               dt_node_full_name(dev), need_mapping, nirq, naddr);
>  
>      if ( dt_device_is_protected(dev) && need_mapping )
>      {
> -        DPRINT("%s setup iommu\n", dt_node_full_name(dev));
> +        dt_dprintk("%s setup iommu\n", dt_node_full_name(dev));
>          res = iommu_assign_dt_device(d, dev);
>          if ( res )
>          {
> @@ -1137,8 +1134,8 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
>           */
>          if ( rirq.controller != dt_interrupt_controller )
>          {
> -            DPRINT("irq %u not connected to primary controller."
> -                   "Connected to %s\n", i, dt_node_full_name(rirq.controller));
> +            dt_dprintk("irq %u not connected to primary controller. Connected to %s\n",
> +                      i, dt_node_full_name(rirq.controller));
>              continue;
>          }
>  
> @@ -1217,17 +1214,17 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>  
>      path = dt_node_full_name(node);
>  
> -    DPRINT("handle %s\n", path);
> +    dt_dprintk("handle %s\n", path);
>  
>      /* Skip theses nodes and the sub-nodes */
>      if ( dt_match_node(skip_matches, node) )
>      {
> -        DPRINT("  Skip it (matched)\n");
> +        dt_dprintk("  Skip it (matched)\n");
>          return 0;
>      }
>      if ( platform_device_is_blacklisted(node) )
>      {
> -        DPRINT("  Skip it (blacklisted)\n");
> +        dt_dprintk("  Skip it (blacklisted)\n");
>          return 0;
>      }
>  
> @@ -1243,7 +1240,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>      /* Skip nodes used by Xen */
>      if ( dt_device_used_by(node) == DOMID_XEN )
>      {
> -        DPRINT("  Skip it (used by Xen)\n");
> +        dt_dprintk("  Skip it (used by Xen)\n");
>          return 0;
>      }
>  
> @@ -1253,7 +1250,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>       */
>      if ( device_get_class(node) == DEVICE_IOMMU )
>      {
> -        DPRINT(" IOMMU, skip it\n");
> +        dt_dprintk(" IOMMU, skip it\n");
>          return 0;
>      }
>  
> @@ -1428,7 +1425,7 @@ static int acpi_make_chosen_node(const struct kernel_info *kinfo)
>      const struct bootmodule *mod = kinfo->kernel_bootmodule;
>      void *fdt = kinfo->fdt;
>  
> -    DPRINT("Create chosen node\n");
> +    dt_dprintk("Create chosen node\n");
>      res = fdt_begin_node(fdt, "chosen");
>      if ( res )
>          return res;
> @@ -1472,7 +1469,7 @@ static int acpi_make_hypervisor_node(const struct kernel_info *kinfo,
>      /* Convenience alias */
>      void *fdt = kinfo->fdt;
>  
> -    DPRINT("Create hypervisor node\n");
> +    dt_dprintk("Create hypervisor node\n");
>  
>      /* See linux Documentation/devicetree/bindings/arm/xen.txt */
>      res = fdt_begin_node(fdt, "hypervisor");
> @@ -1502,7 +1499,7 @@ static int create_acpi_dtb(struct kernel_info *kinfo, struct membank tbl_add[])
>      int new_size;
>      int ret;
>  
> -    DPRINT("Prepare a min DTB for DOM0\n");
> +    dt_dprintk("Prepare a min DTB for DOM0\n");
>  
>      /* Allocate min size for DT */
>      new_size = ACPI_DOM0_FDT_MIN_SIZE;
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 0df2e4b..b39c8ca 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -55,7 +55,6 @@ struct dt_alias_prop {
>  static LIST_HEAD(aliases_lookup);
>  
>  #ifdef CONFIG_DEVICE_TREE_DEBUG
> -# define dt_dprintk(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
>  static void dt_dump_addr(const char *s, const __be32 *addr, int na)
>  {
>      dt_dprintk("%s", s);
> @@ -64,7 +63,6 @@ static void dt_dump_addr(const char *s, const __be32 *addr, int na)
>      dt_dprintk("\n");
>  }
>  #else
> -# define dt_dprintk(fmt, args...) do {} while ( 0 )
>  static void dt_dump_addr(const char *s, const __be32 *addr, int na) { }
>  #endif
>  
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index e3fe77c..d7d1b40 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -753,6 +753,15 @@ int dt_parse_phandle_with_args(const struct dt_device_node *np,
>                                 const char *cells_name, int index,
>                                 struct dt_phandle_args *out_args);
>  
> +#ifdef CONFIG_DEVICE_TREE_DEBUG
> +#define dt_dprintk(fmt, args...)  \
> +    printk(XENLOG_DEBUG fmt, ## args)
> +#else
> +static inline void
> +__attribute__ ((__format__ (__printf__, 1, 2)))
> +dt_dprintk(const char *fmt, ...) {}
> +#endif
> +
>  #endif /* __XEN_DEVICE_TREE_H */
>  
>  /*
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [for-4.8 1/2] xen/arm: Convert DEBUG_DT to Kconfig
  2016-05-24 13:38   ` Konrad Rzeszutek Wilk
@ 2016-05-24 14:44     ` Julien Grall
  0 siblings, 0 replies; 9+ messages in thread
From: Julien Grall @ 2016-05-24 14:44 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Jan Beulich, Doug Goldstein

Hi Konrad,

On 24/05/16 14:38, Konrad Rzeszutek Wilk wrote:
> On Tue, May 24, 2016 at 11:20:40AM +0100, Julien Grall wrote:
>> Convert device-tree debugging to 'Kconfig' as
>> CONFIG_DEBUG_TREE_DEBUG.
>>
>> The option is not enabled by default because the output is very
>> verbose.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Doug Goldstein <cardoe@cardoe.com>
>> ---
>>   xen/Kconfig.debug           | 7 +++++++
>>   xen/arch/arm/domain_build.c | 4 +---
>>   xen/common/device_tree.c    | 4 +---
>>   3 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
>> index 303bf36..59be34d 100644
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -55,6 +55,13 @@ config VERBOSE_DEBUG
>>   	  Guest output from HYPERVISOR_console_io and hypervisor parsing
>>   	  ELF images (dom0) is logged in the Xen ring buffer.
>>
>> +config DEVICE_TREE_DEBUG
>> +	bool "Device tree debug messages"
>> +	depends on HAS_DEVICE_TREE
>> +	---help---
>> +	  Device tree parsing and DOM0 device tree building messages is
>> +	  logged in the Xen ring buffer
>
> s/is logged/are logged/
>
> Also missing stop at the end.
>
> Perhaps also add:
>
> "If unsure, say N here."

I will do all the 3 changes in the next version.

>
> Or could this be part of the VERBOSE one (which spews out data about
> ELF parsing and allows guests to do  the console_io_write hypercalls?).

The debug messages from the device tree is really verbose (it will 
obscure useful boot messages). So it should only be enabled when Xen 
does not parse correctly the device tree.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-05-24 14:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-24 10:20 [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig Julien Grall
2016-05-24 10:20 ` [for-4.8 1/2] " Julien Grall
2016-05-24 13:38   ` Konrad Rzeszutek Wilk
2016-05-24 14:44     ` Julien Grall
2016-05-24 14:16   ` Edgar E. Iglesias
2016-05-24 10:20 ` [for-4.8 2/2] xen/arm: Provide device tree debugging helper in a single place Julien Grall
2016-05-24 13:40   ` Konrad Rzeszutek Wilk
2016-05-24 14:17   ` Edgar E. Iglesias
2016-05-24 10:21 ` [for-4.8 0/2] xen/arm: Convert DEBUG_DT to Kconfig Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.