| Message ID | 1465315583-1278-11-git-send-email-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Stefano, On 22/06/16 10:59, Stefano Stabellini wrote: > On Tue, 7 Jun 2016, Julien Grall wrote: >> The CPU capabilities will be set depending on the value found in the CPU >> registers. This patch provides a generic to go through a set of capabilities >> and find which one should be enabled. >> >> The parameter "info" is used to display the kind of capability updated (e.g >> workaround, feature...). >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> >> --- >> Changes in v3: >> - Patch added. The code was previously part of "Detect >> silicon...". >> --- >> xen/arch/arm/cpufeature.c | 16 ++++++++++++++++ >> xen/include/asm-arm/cpufeature.h | 9 +++++++++ >> 2 files changed, 25 insertions(+) >> >> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c >> index 7a1b56b..088625b 100644 >> --- a/xen/arch/arm/cpufeature.c >> +++ b/xen/arch/arm/cpufeature.c >> @@ -24,6 +24,22 @@ >> >> DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS); >> >> +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, >> + const char *info) >> +{ >> + int i; >> + >> + for ( i = 0; caps[i].matches; i++ ) >> + { >> + if ( !caps[i].matches(&caps[i]) ) >> + continue; >> + >> + if ( !cpus_have_cap(caps[i].capability) && caps[i].desc ) >> + printk(XENLOG_INFO "%s: %s\n", info, caps[i].desc); > > The patch is OK. I still have the same comment as before: if an info > parameter is passed to this function with a string, then I think the > string should contain some useful information. Not a generic message. It is useful. As mentioned in the commit message the parameter "info" is used to display the kind of capability updated (e.g workaround, feature, ...). > I would just do: > > printk(XENLOG_INFO "enable workaround for: %s\n", caps[i].desc); > > If one day we need to change the message depending on the caller, then > that day we'll add a parameter to update_cpu_capabilities. We both know that this function will be used for other purpose very soon. So I would rather avoid to modify again in the future. That is why I described the purpose in the commit message. Cheers,
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c index 7a1b56b..088625b 100644 --- a/xen/arch/arm/cpufeature.c +++ b/xen/arch/arm/cpufeature.c @@ -24,6 +24,22 @@ DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS); +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, + const char *info) +{ + int i; + + for ( i = 0; caps[i].matches; i++ ) + { + if ( !caps[i].matches(&caps[i]) ) + continue; + + if ( !cpus_have_cap(caps[i].capability) && caps[i].desc ) + printk(XENLOG_INFO "%s: %s\n", info, caps[i].desc); + cpus_set_cap(caps[i].capability); + } +} + /* * Local variables: * mode: C diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h index 2bebad1..be2414c 100644 --- a/xen/include/asm-arm/cpufeature.h +++ b/xen/include/asm-arm/cpufeature.h @@ -62,6 +62,15 @@ static inline void cpus_set_cap(unsigned int num) __set_bit(num, cpu_hwcaps); } +struct arm_cpu_capabilities { + const char *desc; + u16 capability; + bool_t (*matches)(const struct arm_cpu_capabilities *); +}; + +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, + const char *info); + #endif /* __ASSEMBLY__ */ #endif
The CPU capabilities will be set depending on the value found in the CPU registers. This patch provides a generic to go through a set of capabilities and find which one should be enabled. The parameter "info" is used to display the kind of capability updated (e.g workaround, feature...). Signed-off-by: Julien Grall <julien.grall@arm.com> --- Changes in v3: - Patch added. The code was previously part of "Detect silicon...". --- xen/arch/arm/cpufeature.c | 16 ++++++++++++++++ xen/include/asm-arm/cpufeature.h | 9 +++++++++ 2 files changed, 25 insertions(+)