xen/arm: register clocks used by the hypervisor
Subsystems (2)
PATCH 1/1 xen/arm: register clocks used by the hypervisor
Some clocks might be used by the Xen hypervisor and not by the Linux kernel. If these are not registered by the Linux kernel, they might be disabled by clk_disable_unused() as the kernel doesn't know that they are used. The clock of the serial console handled by Xen is one example for this. It might be disabled by clk_disable_unused() which stops the whole serial output, even from Xen, then.
Up to now, the workaround for this has been to use the Linux kernel command line parameter 'clk_ignore_unused'. See Xen bug
http://bugs.xenproject.org/xen/bug/45
too.
To fix this, we will add the "unused" clocks in Xen to the hypervisor node. The Linux kernel has to register the clocks from the hypervisor node, then.
Therefore, check if there is a "clocks" entry in the hypervisor node and if so register the given clocks to the Linux kernel clock framework and with this mark them as used. This prevents the clocks from being disabled.
--- arch/arm/xen/enlighten.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
arch/arm/xen/enlighten.c +35 -0
@@ -22,6 +22,7 @@#include <linux/of.h> #include <linux/of_irq.h> #include <linux/of_address.h> +#include <linux/clk-provider.h> #include <linux/cpuidle.h> #include <linux/cpufreq.h> #include <linux/cpu.h>
@@ -381,6 +382,40 @@ static int __init xen_pm_init(void) }late_initcall (xen_pm_init); +/* +* Check if we want to register some clocks, that they +* are not freed because unused by clk_disable_unused(). +* E.g. the serial console clock. +*/ +static int __init xen_arm_register_clks (void ) +{ +struct clk *clk; +unsigned int i, count; +int ret; + + count =of_clk_get_parent_count (xen_node); +if (!count) +return 0 ; + +for (i =0 ; i < count; i++) { + clk =of_clk_get (xen_node, i); +if (IS_ERR (clk)) { +pr_err ("Xen failed to register clock %i. Error: %li\n" , + i,PTR_ERR (clk)); +return PTR_ERR (clk); + } + + ret =clk_prepare_enable (clk); +if (ret <0 ) { +pr_err ("Xen failed to enable clock %i. Error: %i\n" , + i, ret); +return ret; + } + } + +return 0 ; +} +late_initcall (xen_arm_register_clks);/* empty stubs */ void xen_arch_pre_suspend (void ) { }
Signature · 1 line
2.8.0
PATCH 1/1 xen/arm: register clocks used by the hypervisor
Quoted 14 lines · Dirk Behme
Some clocks might be used by the Xen hypervisor and not by the Linux kernel. If these are not registered by the Linux kernel, they might be disabled by
clk_disable_unused()as the kernel doesn't know that they are used. The clock of the serial console handled by Xen is one example for this. It might be disabled byclk_disable_unused()which stops the whole serial output, even from Xen, then.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Quoted 8 lines
To fix this, we will add the "unused" clocks in Xen to the hypervisor node. The Linux kernel has to register the clocks from the hypervisor node, then.
Therefore, check if there is a "clocks" entry in the hypervisor node and if so register the given clocks to the Linux kernel clock framework and with this mark them as used. This prevents the clocks from being disabled.
Just for the logs, the Xen counterpart is
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02607.html
Dirk
Quoted 58 lines
--- arch/arm/xen/enlighten.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)arch/arm/xen/enlighten.c +35
-0@@ -22,6 +22,7 @@#include <linux/of.h> #include <linux/of_irq.h> #include <linux/of_address.h> +#include <linux/clk-provider.h> #include <linux/cpuidle.h> #include <linux/cpufreq.h> #include <linux/cpu.h> @@ -381,6 +382,40 @@ static int __init xen_pm_init(void) }late_initcall (xen_pm_init); +/* +* Check if we want to register some clocks, that they +* are not freed because unused by clk_disable_unused(). +* E.g. the serial console clock. +*/ +static int __init xen_arm_register_clks (void ) +{ +struct clk *clk; +unsigned int i, count; +int ret; + + count =of_clk_get_parent_count (xen_node); +if (!count) +return 0 ; + +for (i =0 ; i < count; i++) { + clk =of_clk_get (xen_node, i); +if (IS_ERR (clk)) { +pr_err ("Xen failed to register clock %i. Error: %li\n" , + i,PTR_ERR (clk)); +return PTR_ERR (clk); + } + + ret =clk_prepare_enable (clk); +if (ret <0 ) { +pr_err ("Xen failed to enable clock %i. Error: %i\n" , + i, ret); +return ret; + } + } + +return 0 ; +} +late_initcall (xen_arm_register_clks);/* empty stubs */ void xen_arch_pre_suspend (void ) { }
PATCH 1/1 xen/arm: register clocks used by the hypervisor
In reply to Dirk BehmeHello Dirk,
Quoted 14 lines · Dirk Behme
Some clocks might be used by the Xen hypervisor and not by the Linux kernel. If these are not registered by the Linux kernel, they might be disabled by
clk_disable_unused()as the kernel doesn't know that they are used. The clock of the serial console handled by Xen is one example for this. It might be disabled byclk_disable_unused()which stops the whole serial output, even from Xen, then.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Quoted 8 lines
To fix this, we will add the "unused" clocks in Xen to the hypervisor node. The Linux kernel has to register the clocks from the hypervisor node, then.
Therefore, check if there is a "clocks" entry in the hypervisor node and if so register the given clocks to the Linux kernel clock framework and with this mark them as used. This prevents the clocks from being disabled.
This new property would need to be documented in:
- linux/Documentation/devicetree/bindings/arm/xen.txt
- xen/docs/misc/arm/device-tree/guest.txt
Quoted 24 lines
Signed-off-by Dirk Behme <[email protected]>--- arch/arm/xen/enlighten.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)arch/arm/xen/enlighten.c +3
-0@@ -22,6 +22,7 @@#include <linux/of.h> #include <linux/of_irq.h> #include <linux/of_address.h> +#include <linux/clk-provider.h> #include <linux/cpuidle.h> #include <linux/cpufreq.h> #include <linux/cpu.h> @@ -381,6 +382,40 @@ static int __init xen_pm_init(void) }late_initcall (xen_pm_init); +/* +* Check if we want to register some clocks, that they
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 396
Quoted 10 lines
arch/arm/xen/enlighten.c +10
-0+* are not freed because unused by clk_disable_unused(). +* E.g. the serial console clock. +*/ +static int __init xen_arm_register_clks (void ) +{ +struct clk *clk; +unsigned int i, count; +int ret; + + count =of_clk_get_parent_count (xen_node);
The code in this file has changed quite a lot with the support of ACPI. For instance "xen_node" is not anymore a global variable. Can you rebase your rework on top of the branch for-linus-4.8?
You can find the branch in:
git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 408
Quoted 12 lines
arch/arm/xen/enlighten.c +12
-0+if (!count) +return 0 ; + +for (i =0 ; i < count; i++) { + clk =of_clk_get (xen_node, i); +if (IS_ERR (clk)) { +pr_err ("Xen failed to register clock %i. Error: %li\n" , + i,PTR_ERR (clk)); +return PTR_ERR (clk); + } + + ret =clk_prepare_enable (clk);
I am not sure why we would want Linux to enable the clock. Should not it be already done either by the firmware or Xen?
Would it be possible to use the flags CLK_IGNORE_UNUSED instead? So the clock will be ignored by clk_disable_unused_subtree.
Quoted 4 lines
arch/arm/xen/enlighten.c +3
-0+if (ret <0 ) { +pr_err ("Xen failed to enable clock %i. Error: %i\n" , + i, ret);
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 421
Quoted 10 lines
arch/arm/xen/enlighten.c +7
-0+return ret; + } + } + +return 0 ; +} +late_initcall (xen_arm_register_clks);/* empty stubs */ void xen_arch_pre_suspend (void ) { }
Regards,
Signature · 1 line
Julien Grall
PATCH 1/1 xen/arm: register clocks used by the hypervisor
Quoted 21 lines · Julien Grall
Hello Dirk,
Quoted 18 lines · Dirk Behme
Some clocks might be used by the Xen hypervisor and not by the Linux kernel. If these are not registered by the Linux kernel, they might be disabled by
clk_disable_unused()as the kernel doesn't know that they are used. The clock of the serial console handled by Xen is one example for this. It might be disabled byclk_disable_unused()which stops the whole serial output, even from Xen, then.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
To fix this, we will add the "unused" clocks in Xen to the hypervisor node. The Linux kernel has to register the clocks from the hypervisor node, then.
Quoted 8 lines
Quoted 4 lines
Therefore, check if there is a "clocks" entry in the hypervisor node and if so register the given clocks to the Linux kernel clock framework and with this mark them as used. This prevents the clocks from being disabled.
This new property would need to be documented in:
-linux/Documentation/devicetree/bindings/arm/xen.txt
-xen/docs/misc/arm/device-tree/guest.txt
This (series) should also be CC'd to devicetree at vger.kernel.org, and to the clock framework maintainers.
I have further questions, but I will wait for that posting.
Thanks,
Mark.
Quoted 21 lines · Julien Grall
Hello Dirk,
Quoted 18 lines · Dirk Behme
Some clocks might be used by the Xen hypervisor and not by the Linux kernel. If these are not registered by the Linux kernel, they might be disabled by
clk_disable_unused()as the kernel doesn't know that they are used. The clock of the serial console handled by Xen is one example for this. It might be disabled byclk_disable_unused()which stops the whole serial output, even from Xen, then.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
To fix this, we will add the "unused" clocks in Xen to the hypervisor node. The Linux kernel has to register the clocks from the hypervisor node, then.
Quoted 8 lines
Quoted 4 lines
Therefore, check if there is a "clocks" entry in the hypervisor node and if so register the given clocks to the Linux kernel clock framework and with this mark them as used. This prevents the clocks from being disabled.
This new property would need to be documented in:
-linux/Documentation/devicetree/bindings/arm/xen.txt
-xen/docs/misc/arm/device-tree/guest.txt
This (series) should also be CC'd to [email protected], and to the clock framework maintainers.
I have further questions, but I will wait for that posting.
Thanks,
Mark.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
PATCH 1/1 xen/arm: register clocks used by the hypervisor
In reply to Julien GrallHi Julien,
Quoted 21 lines · Julien Grall
Hello Dirk,
Quoted 18 lines · Dirk Behme
Some clocks might be used by the Xen hypervisor and not by the Linux kernel. If these are not registered by the Linux kernel, they might be disabled by
clk_disable_unused()as the kernel doesn't know that they are used. The clock of the serial console handled by Xen is one example for this. It might be disabled byclk_disable_unused()which stops the whole serial output, even from Xen, then.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
To fix this, we will add the "unused" clocks in Xen to the hypervisor node. The Linux kernel has to register the clocks from the hypervisor node, then.
Quoted 8 lines
Quoted 4 lines
Therefore, check if there is a "clocks" entry in the hypervisor node and if so register the given clocks to the Linux kernel clock framework and with this mark them as used. This prevents the clocks from being disabled.
This new property would need to be documented in:
-linux/Documentation/devicetree/bindings/arm/xen.txt
-xen/docs/misc/arm/device-tree/guest.txt
Yes, as already discussed, I'll have a look to that.
Quoted 38 lines
Quoted 23 lines
Signed-off-by Dirk Behme <[email protected]>--- arch/arm/xen/enlighten.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)arch/arm/xen/enlighten.c +3
-0@@ -22,6 +22,7 @@#include <linux/of.h> #include <linux/of_irq.h> #include <linux/of_address.h> +#include <linux/clk-provider.h> #include <linux/cpuidle.h> #include <linux/cpufreq.h> #include <linux/cpu.h> @@ -381,6 +382,40 @@ static int __init xen_pm_init(void) }late_initcall (xen_pm_init); +/* +* Check if we want to register some clocks, that they arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 396
Quoted 10 lines
arch/arm/xen/enlighten.c +10
-0+* are not freed because unused by clk_disable_unused(). +* E.g. the serial console clock. +*/ +static int __init xen_arm_register_clks (void ) +{ +struct clk *clk; +unsigned int i, count; +int ret; + + count =of_clk_get_parent_count (xen_node);The code in this file has changed quite a lot with the support of ACPI. For instance "
xen_node" is not anymore a global variable. Can you rebase your rework on top of the branch for-linus-4.8?
Quoted 3 lines
You can find the branch in:
git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git
I'll have a look to that, too. Thanks!
Hopefully it won't be too complicated to rebase ;)
Quoted 7 lines
Quoted 7 lines
arch/arm/xen/enlighten.c +7
-0+if (!count) +return 0 ; + +for (i =0 ; i < count; i++) { + clk =of_clk_get (xen_node, i); +if (IS_ERR (clk)) { + pr_err("Xen failed to register clock %i. Error: %li\n",
Quoted 8 lines
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 408
Quoted 5 lines
arch/arm/xen/enlighten.c +5
-0+ i, PTR_ERR(clk)); +return PTR_ERR (clk); + } + + ret =clk_prepare_enable (clk);I am not sure why we would want Linux to enable the clock. Should not it be already done either by the firmware or Xen?
After having a closer look to that, my understanding:
We have to call clk_prepare(clk) and clk_enable(clk) in this order (and clk_prepare_enable() does this in one call) to get the enable count incremented
http://lxr.free-electrons.com/source/drivers/clk/clk.c#L751
to prevent clk_disable_unused() to disable this clock.
That does mean that yes, you are right, the (hw-) clock itself is already enabled by the firmware or Xen. But we are not really interested in enabling the clock, here, but get the Linux kernel's enable_count incremented.
Quoted 2 lines
Would it be possible to use the flags
CLK_IGNORE_UNUSEDinstead? So the clock will be ignored byclk_disable_unused_subtree.
Yes, using CLK_IGNORE_UNUSED is an option. But ;)
But we can't set it directly in enlighten.c as we can't access the flags part of the clock structs from outside the core clock code.
That would mean that we have to to set the clock flags in the device tree. What would mean that for each clock we don't want to be disabled we have to manipulate the root clock entry of that clock in the device tree. While that would be possible, it doesn't sound that easy and I wonder if that wouldn't be too complicated to be done in Xen (?)
You will correct me if I'm wrong ;)
Many thanks!
Dirk
PATCH 1/1 xen/arm: register clocks used by the hypervisor
Hi Dirk,
Quoted 10 lines · Dirk Behme
Quoted 3 lines · Julien Grall
On 21/06/16 11:16, Dirk Behme wrote:
Would it be possible to use the flagsCLK_IGNORE_UNUSEDinstead? So the clock will be ignored byclk_disable_unused_subtree.Yes, using
CLK_IGNORE_UNUSEDis an option. But ;)But we can't set it directly in
enlighten.cas we can't access the flags part of the clock structs from outside the core clock code.
Maybe we can introduce a new function in the kernel to set the flags?
Anyway, I am not very familiar with the clock subsystem. So, as suggested by Mark, I would recommend you to start a discussion with with the clock maintainers to see what would be the best way to avoid DOM0 to disable any clock used by Xen or a guest.
Regards,
Signature · 1 line
Julien Grall
Hi Dirk,
Quoted 10 lines · Dirk Behme
Quoted 3 lines · Julien Grall
On 21/06/16 11:16, Dirk Behme wrote:
Would it be possible to use the flagsCLK_IGNORE_UNUSEDinstead? So the clock will be ignored byclk_disable_unused_subtree.Yes, using
CLK_IGNORE_UNUSEDis an option. But ;)But we can't set it directly in
enlighten.cas we can't access the flags part of the clock structs from outside the core clock code.
Maybe we can introduce a new function in the kernel to set the flags?
Anyway, I am not very familiar with the clock subsystem. So, as suggested by Mark, I would recommend you to start a discussion with with the clock maintainers to see what would be the best way to avoid DOM0 to disable any clock used by Xen or a guest.
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] http://lists.xen.org/xen-devel
Hello Dirk,
Quoted 14 lines · Dirk Behme
Some clocks might be used by the Xen hypervisor and not by the Linux kernel. If these are not registered by the Linux kernel, they might be disabled by
clk_disable_unused()as the kernel doesn't know that they are used. The clock of the serial console handled by Xen is one example for this. It might be disabled byclk_disable_unused()which stops the whole serial output, even from Xen, then.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Quoted 8 lines
To fix this, we will add the "unused" clocks in Xen to the hypervisor node. The Linux kernel has to register the clocks from the hypervisor node, then.
Therefore, check if there is a "clocks" entry in the hypervisor node and if so register the given clocks to the Linux kernel clock framework and with this mark them as used. This prevents the clocks from being disabled.
This new property would need to be documented in:
- linux/Documentation/devicetree/bindings/arm/xen.txt
- xen/docs/misc/arm/device-tree/guest.txt
Quoted 24 lines
Signed-off-by Dirk Behme <[email protected]>--- arch/arm/xen/enlighten.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)arch/arm/xen/enlighten.c +3
-0@@ -22,6 +22,7 @@#include <linux/of.h> #include <linux/of_irq.h> #include <linux/of_address.h> +#include <linux/clk-provider.h> #include <linux/cpuidle.h> #include <linux/cpufreq.h> #include <linux/cpu.h> @@ -381,6 +382,40 @@ static int __init xen_pm_init(void) }late_initcall (xen_pm_init); +/* +* Check if we want to register some clocks, that they
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 396
Quoted 10 lines
arch/arm/xen/enlighten.c +10
-0+* are not freed because unused by clk_disable_unused(). +* E.g. the serial console clock. +*/ +static int __init xen_arm_register_clks (void ) +{ +struct clk *clk; +unsigned int i, count; +int ret; + + count =of_clk_get_parent_count (xen_node);
The code in this file has changed quite a lot with the support of ACPI. For instance "xen_node" is not anymore a global variable. Can you rebase your rework on top of the branch for-linus-4.8?
You can find the branch in:
git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 408
Quoted 12 lines
arch/arm/xen/enlighten.c +12
-0+if (!count) +return 0 ; + +for (i =0 ; i < count; i++) { + clk =of_clk_get (xen_node, i); +if (IS_ERR (clk)) { +pr_err ("Xen failed to register clock %i. Error: %li\n" , + i,PTR_ERR (clk)); +return PTR_ERR (clk); + } + + ret =clk_prepare_enable (clk);
I am not sure why we would want Linux to enable the clock. Should not it be already done either by the firmware or Xen?
Would it be possible to use the flags CLK_IGNORE_UNUSED instead? So the clock will be ignored by clk_disable_unused_subtree.
Quoted 4 lines
arch/arm/xen/enlighten.c +3
-0+if (ret <0 ) { +pr_err ("Xen failed to enable clock %i. Error: %i\n" , + i, ret);
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 421
Quoted 10 lines
arch/arm/xen/enlighten.c +7
-0+return ret; + } + } + +return 0 ; +} +late_initcall (xen_arm_register_clks);/* empty stubs */ void xen_arch_pre_suspend (void ) { }
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] http://lists.xen.org/xen-devel