ARM Xen Bug #45: Is there a solution?

ARM Xen Bug #45: Is there a solution?

[Dirk Behme]

Hello,

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.

Best regards

Dirk

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

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

[Julien Grall]

On 23/05/2016 16:47, Dirk Behme wrote:
> Hello,

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.

Regards,

-- 
Julien Grall

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

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

[Dirk Behme]

Hi Julien,

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?

Best regards

Dirk


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

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

[Julien Grall]

On 24/05/2016 14:39, Dirk Behme wrote:
> Hi Julien,

Hello Dirk,

> 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).

Regards,

-- 
Julien Grall

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

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

[Dirk Behme]

Hi Julien,

On 24.05.2016 22:05, Julien Grall wrote:
>
>
> On 24/05/2016 14:39, Dirk Behme wrote:
>> Hi Julien,
>
> Hello Dirk,
>
>> 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.

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?


Best regards

Dirk


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

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

[Julien Grall]

On 25/05/2016 16:10, Dirk Behme wrote:
> Hi Julien,

Hello Dirk,

> On 24.05.2016 22:05, Julien Grall wrote:
>>
>>
>> On 24/05/2016 14:39, Dirk Behme wrote:
>>> Hi Julien,
>>
>> Hello Dirk,
>>
>>> 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.

Regards,

-- 
Julien Grall

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

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

[Dirk Behme]

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

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

[Julien Grall]

Hello Dirk,

On 27/05/16 13:34, Dirk Behme wrote:
> 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:
>> 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]?

Yes.

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

How about moving those variables in the structure kernel_info?

>
> 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;
>       }
>

-- 
Julien Grall

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

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

[Dirk Behme]

> Subject: Re: ARM Xen Bug #45: Is there a solution?
> Date: Tue, 31 May 2016 11:44:23 +0100
> From: Julien Grall <julien.grall@arm.com>
> To: Dirk Behme <dirk.behme@gmail.com>, xen-devel@lists.xen.org
> <xen-devel@lists.xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>, Ian Jackson
> <Ian.Jackson@eu.citrix.com>
>
> Hello Dirk,
>
> On 27/05/16 13:34, Dirk Behme wrote:
>> 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:
>>> 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]?
>
> Yes.
>
>> I'm unhappy about the global variables and the max clocks, though.
>
> How about moving those variables in the structure kernel_info?


Just for the logs to finish this thread:

https://lists.xen.org/archives/html/xen-devel/2016-06/msg02607.html

http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438067.html


Best regards

Dirk


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