Re: [Xen-devel] [PATCH v2] xen/arm: Propagate clock-frequency to DOMU if present in the DT timer node
On Fri, 2015-06-19 at 13:41 +0100, Julien Grall wrote:
> When the property "clock-frequency" is present in the DT timer node, it
> means that the bootloader/firmware didn't correctly configure the
> CNTFRQ/CNTFRQ_EL0 on each processor.
>
> The best solution would be to fix the offending firmware/bootloader,
> although it may not always be possible to modify and re-flash it.
>
> As it's not possible to trap the register CNTFRQ/CNTFRQ_EL0, we have
> to extend xen_arch_domainconfig to provide the timer frequency to the
> toolstack when the property "clock-frequency" is present to the host DT
> timer node. Then, a property "clock-frequency" will be created in the
> guest
> DT timer node if the value is not 0.
>
> We could have set the property in the guest DT no matter if the property
> is present in the host DT. Although, we still want to let the guest
> using CNTFRQ in normal case. After all, the property "clock-frequency"
> is just a workaround for buggy firmware.
>
> Also add a stub for fdt_property_u32 which is not present in libfdt <
> 1.4.0 used by distribution such as Debian Wheezy.
>
> Signed-off-by: Julien Grall
> Tested-by: Chris Brand
I had this on my list to backport to 4.5 but it didn't apply at all cleanly
in a variety of ways.
I don't think this one is particularly worth the effort, so I've dropped it
from the list. If someone is keen to have it then please provide a
backport.
Ian.
>
> ---
> This patch requires to regenerate tools/configure.
>
> Changes in v2:
> - Typo in commit message
> - Make the patch compiling on wheezy where fdt_property_u32 is
> not present
> ---
> tools/configure.ac| 4
> tools/libxl/libxl_arm.c | 9 +++--
> tools/libxl/libxl_libfdt_compat.h | 9 +
> xen/arch/arm/domain.c | 2 +-
> xen/arch/arm/time.c | 5 +
> xen/arch/arm/vtimer.c | 4 +++-
> xen/arch/arm/vtimer.h | 3 ++-
> xen/include/asm-arm/time.h| 6 ++
> xen/include/public/arch-arm.h | 14 ++
> 9 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 1a06ddf..2886cb5 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -379,6 +379,10 @@ AS_IF([test "x$partial_dt" = "xy" ],
> # * The prototype exists but the functions are not exposed. Don't ask
> why...
> AC_CHECK_FUNCS([fdt_first_subnode fdt_next_subnode])
> AC_CHECK_DECLS([fdt_first_subnode, fdt_next_subnode],,,[#include
> ])
> +
> +# The helper fdt_property_u32 is only present in libfdt >= 1.4.0
> +# It's an inline function, so only check if the declaration is present
> +AC_CHECK_DECLS([fdt_property_u32],,,[#include ])
> esac
>
> # Checks for header files.
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 4fb5e26..f09c860 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -444,7 +444,9 @@ static int make_gicv3_node(libxl__gc *gc, void *fdt)
> return 0;
> }
>
> -static int make_timer_node(libxl__gc *gc, void *fdt, const struct
> arch_info *ainfo)
> +static int make_timer_node(libxl__gc *gc, void *fdt,
> + const struct arch_info *ainfo,
> + uint32_t frequency)
> {
> int res;
> gic_interrupt ints[3];
> @@ -462,6 +464,9 @@ static int make_timer_node(libxl__gc *gc, void *fdt,
> const struct arch_info *ain
> res = fdt_property_interrupts(gc, fdt, ints, 3);
> if (res) return res;
>
> +if ( frequency )
> +fdt_property_u32(fdt, "clock-frequency", frequency);
> +
> res = fdt_end_node(fdt);
> if (res) return res;
>
> @@ -805,7 +810,7 @@ next_resize:
> goto out;
> }
>
> -FDT( make_timer_node(gc, fdt, ainfo) );
> +FDT( make_timer_node(gc, fdt, ainfo, xc_config->clock_frequency)
> );
> FDT( make_hypervisor_node(gc, fdt, vers) );
>
> if (pfdt)
> diff --git a/tools/libxl/libxl_libfdt_compat.h
> b/tools/libxl/libxl_libfdt_compat.h
> index 53a5076..23230b5 100644
> --- a/tools/libxl/libxl_libfdt_compat.h
> +++ b/tools/libxl/libxl_libfdt_compat.h
> @@ -61,6 +61,7 @@
> #define LIBXL_LIBFDT_COMPAT_H
>
> #include "libxl_internal.h"
> +#include
>
> #if !HAVE_DECL_FDT_FIRST_SUBNODE
> _hidden int fdt_first_subnode(const void *fdt, int offset);
> @@ -70,6 +71,14 @@ _hidden int fdt_first_subnode(const void *fdt, int
> offset);
> _hidden int fdt_next_subnode(const void *fdt, int offset);
> #endif
>
> +#if !HAVE_DECL_FDT_PROPERTY_U32
> +static inline int fdt_property_u32(void *fdt, const char *name, uint32_t
> val)
> +{
> + uint32_t tmp = cpu_to_fdt32(val);
> + return fdt_property(fdt, name, &tmp, sizeof(tmp));
> +}
> +#endif
> +
> #endif
>
> /*
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 24b8938..8b1bf5a 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
>
Re: [Xen-devel] [PATCH v2] xen/arm: Propagate clock-frequency to DOMU if present in the DT timer node
On Fri, 2015-06-19 at 13:41 +0100, Julien Grall wrote: > When the property "clock-frequency" is present in the DT timer node, it > means that the bootloader/firmware didn't correctly configure the > CNTFRQ/CNTFRQ_EL0 on each processor. > > The best solution would be to fix the offending firmware/bootloader, > although it may not always be possible to modify and re-flash it. > > As it's not possible to trap the register CNTFRQ/CNTFRQ_EL0, we have > to extend xen_arch_domainconfig to provide the timer frequency to the > toolstack when the property "clock-frequency" is present to the host DT > timer node. Then, a property "clock-frequency" will be created in the guest > DT timer node if the value is not 0. > > We could have set the property in the guest DT no matter if the property > is present in the host DT. Although, we still want to let the guest > using CNTFRQ in normal case. After all, the property "clock-frequency" > is just a workaround for buggy firmware. > > Also add a stub for fdt_property_u32 which is not present in libfdt < > 1.4.0 used by distribution such as Debian Wheezy. > > Signed-off-by: Julien Grall > Tested-by: Chris Brand Acked + applied, thanks > This patch requires to regenerate tools/configure. Done. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xen/arm: Propagate clock-frequency to DOMU if present in the DT timer node
When the property "clock-frequency" is present in the DT timer node, it
means that the bootloader/firmware didn't correctly configure the
CNTFRQ/CNTFRQ_EL0 on each processor.
The best solution would be to fix the offending firmware/bootloader,
although it may not always be possible to modify and re-flash it.
As it's not possible to trap the register CNTFRQ/CNTFRQ_EL0, we have
to extend xen_arch_domainconfig to provide the timer frequency to the
toolstack when the property "clock-frequency" is present to the host DT
timer node. Then, a property "clock-frequency" will be created in the guest
DT timer node if the value is not 0.
We could have set the property in the guest DT no matter if the property
is present in the host DT. Although, we still want to let the guest
using CNTFRQ in normal case. After all, the property "clock-frequency"
is just a workaround for buggy firmware.
Also add a stub for fdt_property_u32 which is not present in libfdt <
1.4.0 used by distribution such as Debian Wheezy.
Signed-off-by: Julien Grall
Tested-by: Chris Brand
---
This patch requires to regenerate tools/configure.
Changes in v2:
- Typo in commit message
- Make the patch compiling on wheezy where fdt_property_u32 is
not present
---
tools/configure.ac| 4
tools/libxl/libxl_arm.c | 9 +++--
tools/libxl/libxl_libfdt_compat.h | 9 +
xen/arch/arm/domain.c | 2 +-
xen/arch/arm/time.c | 5 +
xen/arch/arm/vtimer.c | 4 +++-
xen/arch/arm/vtimer.h | 3 ++-
xen/include/asm-arm/time.h| 6 ++
xen/include/public/arch-arm.h | 14 ++
9 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/tools/configure.ac b/tools/configure.ac
index 1a06ddf..2886cb5 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -379,6 +379,10 @@ AS_IF([test "x$partial_dt" = "xy" ],
# * The prototype exists but the functions are not exposed. Don't ask why...
AC_CHECK_FUNCS([fdt_first_subnode fdt_next_subnode])
AC_CHECK_DECLS([fdt_first_subnode, fdt_next_subnode],,,[#include ])
+
+# The helper fdt_property_u32 is only present in libfdt >= 1.4.0
+# It's an inline function, so only check if the declaration is present
+AC_CHECK_DECLS([fdt_property_u32],,,[#include ])
esac
# Checks for header files.
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 4fb5e26..f09c860 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -444,7 +444,9 @@ static int make_gicv3_node(libxl__gc *gc, void *fdt)
return 0;
}
-static int make_timer_node(libxl__gc *gc, void *fdt, const struct arch_info
*ainfo)
+static int make_timer_node(libxl__gc *gc, void *fdt,
+ const struct arch_info *ainfo,
+ uint32_t frequency)
{
int res;
gic_interrupt ints[3];
@@ -462,6 +464,9 @@ static int make_timer_node(libxl__gc *gc, void *fdt, const
struct arch_info *ain
res = fdt_property_interrupts(gc, fdt, ints, 3);
if (res) return res;
+if ( frequency )
+fdt_property_u32(fdt, "clock-frequency", frequency);
+
res = fdt_end_node(fdt);
if (res) return res;
@@ -805,7 +810,7 @@ next_resize:
goto out;
}
-FDT( make_timer_node(gc, fdt, ainfo) );
+FDT( make_timer_node(gc, fdt, ainfo, xc_config->clock_frequency) );
FDT( make_hypervisor_node(gc, fdt, vers) );
if (pfdt)
diff --git a/tools/libxl/libxl_libfdt_compat.h
b/tools/libxl/libxl_libfdt_compat.h
index 53a5076..23230b5 100644
--- a/tools/libxl/libxl_libfdt_compat.h
+++ b/tools/libxl/libxl_libfdt_compat.h
@@ -61,6 +61,7 @@
#define LIBXL_LIBFDT_COMPAT_H
#include "libxl_internal.h"
+#include
#if !HAVE_DECL_FDT_FIRST_SUBNODE
_hidden int fdt_first_subnode(const void *fdt, int offset);
@@ -70,6 +71,14 @@ _hidden int fdt_first_subnode(const void *fdt, int offset);
_hidden int fdt_next_subnode(const void *fdt, int offset);
#endif
+#if !HAVE_DECL_FDT_PROPERTY_U32
+static inline int fdt_property_u32(void *fdt, const char *name, uint32_t val)
+{
+ uint32_t tmp = cpu_to_fdt32(val);
+ return fdt_property(fdt, name, &tmp, sizeof(tmp));
+}
+#endif
+
#endif
/*
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 24b8938..8b1bf5a 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -593,7 +593,7 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags,
if ( (rc = domain_vgic_init(d, config->nr_spis)) != 0 )
goto fail;
-if ( (rc = domain_vtimer_init(d)) != 0 )
+if ( (rc = domain_vtimer_init(d, config)) != 0 )
goto fail;
/*
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index ce6d3fd..5ded30c 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -42,6 +42,8 @@ uint64_t __read_mostly boot_count;
* register-mapped time source in the SoC. */
unsigned long __read_mostly cpu_khz;
