Re: ARM Xen Bug #45: Is there a solution?

[Dirk Behme <dirk.behme@gmail.com>]

On 26.05.2016 11:00, Julien Grall wrote:
> On 25/05/2016 16:10, Dirk Behme wrote:
>> On 24.05.2016 22:05, Julien Grall wrote:
>>> On 24/05/2016 14:39, Dirk Behme wrote:
>>>> On 23.05.2016 22:15, Julien Grall wrote:
>>>>> Hello Dirk,
>>>>>
>>>>>> is there a solution for
>>>>>>
>>>>>> arm: domain 0 disables clocks which are in fact being used
>>>>>> http://bugs.xenproject.org/xen/bug/45
>>>>>>
>>>>>> ?
>>>>>>
>>>>>> On an ARM based board I have to use 'clk_ignore_unused' preventing
>>>>>> that
>>>>>> Dom0 disables the UART clock for the console UART configured with
>>>>>> console=hvc0.
>>>>>
>>>>> There is no better solution than passing "clk_ignore_unused" on the
>>>>> kernel command line so far.
>>>>
>>>>
>>>> What would be the solution for this issue? The
>>>>
>>>> "propagate any clock related properties from the UART
>>>> node into the Xen hypervisor node"
>>>>
>>>> mentioned in the ticket?
>>>
>>> That is correct. Xen would copy the property "clocks" of the UART into
>>> the hypervisor node.
>>>
>>> DOM0 would then parse the clocks associated to this node and mark them
>>> as used by Xen (I think CLK_IGNORE_UNUSED could do the job for us).
>>
>>
>> I've started to look into this:
>>
>> I'd think in arm_uart.c in dt_uart_init() after
>>
>> if ( !dev )
>>
>> we know the UART node we are looking for. From this we have to read the
>> clock configuration.
>>
>> To be clarified: How to read the clock configuration? I couldn't find
>> any convenient function dt_device_get_clock() or similar for that.
>
> Xen does not need to parse the content of the property "clocks" but
> copy the raw value to the DOM0 DT.
>
> You cand find the value of a property with dt_get_property.
>
>>
>> Now, we have the clocks we are looking for.
>>
>> These are needed in domain_build.c in make_hypervisor_node(), then.
>>
>> To be clarified: How to pass the clock configuration from arm_uart.c to
>> domain_build.c (and not break the non-dt / non-ARM platforms)?
>>
>> Any ideas or comments?
>
> All the devices (UART included) used by Xen will return DOMID_XEN when
> dt_device_used_by is called to the node.
>
> You could use it to collect the clocks of all those devices and gather
> the value in a single property to be created in the hypervisor node.


Anything like below (untested) [1]?

I'm unhappy about the global variables and the max clocks, though.

Best regards

Dirk

[1]

---
  xen/arch/arm/domain_build.c |   24 ++++++++++++++++++++++++
  1 file changed, 24 insertions(+)

Index: xen.git/xen/arch/arm/domain_build.c
===================================================================
--- xen.git.orig/xen/arch/arm/domain_build.c
+++ xen.git/xen/arch/arm/domain_build.c
@@ -42,6 +42,10 @@ static void __init parse_dom0_mem(const
  }
  custom_param("dom0_mem", parse_dom0_mem);

+#define MAX_DT_CLOCKS 256
+static unsigned char dt_clocks[MAX_DT_CLOCKS];
+static unsigned int clk_cnt;
+
  //#define DEBUG_DT

  #ifdef DEBUG_DT
@@ -657,6 +661,10 @@ static int make_hypervisor_node(const st
      if ( res )
          return res;

+    res = fdt_property(fdt, "clocks", dt_clocks, clk_cnt);
+    if ( res )
+        return res;
+
      res = fdt_end_node(fdt);

      return res;
@@ -1213,9 +1221,11 @@ static int handle_node(struct domain *d,
          { /* sentinel */ },
      };
      struct dt_device_node *child;
+    unsigned int len;
      int res;
      const char *name;
      const char *path;
+    const char *clocks;

      path = dt_node_full_name(node);

@@ -1246,6 +1256,20 @@ static int handle_node(struct domain *d,
      if ( dt_device_used_by(node) == DOMID_XEN )
      {
          DPRINT("  Skip it (used by Xen)\n");
+
+        /*
+         * Remember the clock used by the skipped node
+         * We add it later to the hypervisor node to make the
+         * Linux kernel aware of its usage
+         */
+        clocks = dt_get_property(node, "clocks", &len);
+        if ( clk_cnt + len >= MAX_DT_CLOCKS ) {
+            printk("Failed to remember the clock node of %s\n", path);
+            printk("Use the Linux kernel command 'clk_ignore_unused'\n");
+            return 0;
+        }
+        memcpy(&dt_clocks[clk_cnt], clocks, len);
+        clk_cnt += len;
          return 0;
      }


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