Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Sünnavend 05 Februar 2005 02:34, Anton Blanchard wrote: > Interesting :) However we already get bug reports with the current > CONFIG_POWER4_ONLY option. I worry about adding more options that users > could get wrong unless there is a noticeable improvement in performance. > The patch that I posted doesn't add any new user selectable options, it only limits the supported CPUs to the ones that are available on the supported platforms. If you select powermac or maple, the only supported CPU will be PowerPC970, so the C compiler can optimize away all runtime checks for CPU features. I don't expect much noticeable performance advantage from the patch, but it allows to make some of the source code nicer. E.g. you can replace every instance of '#ifdef CONFIG_ALTIVEC' with 'if (CPU_FTR_POSSIBLE & CPU_FTR_ALTIVEC)' or an inline function wrapping that. Arnd <>< pgp1i3ZIJpHvc.pgp Description: signature
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 11:20 -0600, Olof Johansson wrote: > On Fri, Feb 04, 2005 at 10:17:48AM +0200, Pekka Enberg wrote: > > Please drop the CPU_FTR_##x macro magic as it makes grepping more > > complicated. If the enum names are too long, just do s/CPU_FTR_/CPU_/g > > or something similar. Also, could you please make this a static inline > > function? I tend to agree with Pekka... > I considered that for a while, but decided against it because: > > * cpu-has-feature(cpu-feature-foo) v cpu-has-feature(foo): I picked the > latter for readability. I don't think it really matters compared to the usefullness of grep, and is still more readable than the old way... > * Renaming CPU_FTR_ -> CPU_ makes it less obvious that > it's actually a cpu feature it's describing (i.e. CPU_ALTIVEC vs > CPU_FTR_ALTIVEC). Agreed. > * Renaming would clobber the namespace, CPU_* definitions are used in > other places in the tree. > * Can't make it an inline and still use the preprocessor concatenation. I'd like to keep the constants as-is and have the stuff inline with no macro trick as Pekka suggest since I did use grep on those things quite often. > That being said, you do have a point about grepability. However, > personally I'd be more likely to look for CPU_HAS_FEATURE than the > feature itself when reading the code, and would find that easily. The > other way around (finding all uses of a feature) is harder, but the > concatenation macro is right below the bit definitions and easy to spot. No, when I grep, i'm looking for the feature itself... Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 11:20 -0600, Olof Johansson wrote: On Fri, Feb 04, 2005 at 10:17:48AM +0200, Pekka Enberg wrote: Please drop the CPU_FTR_##x macro magic as it makes grepping more complicated. If the enum names are too long, just do s/CPU_FTR_/CPU_/g or something similar. Also, could you please make this a static inline function? I tend to agree with Pekka... I considered that for a while, but decided against it because: * cpu-has-feature(cpu-feature-foo) v cpu-has-feature(foo): I picked the latter for readability. I don't think it really matters compared to the usefullness of grep, and is still more readable than the old way... * Renaming CPU_FTR_x - CPU_x makes it less obvious that it's actually a cpu feature it's describing (i.e. CPU_ALTIVEC vs CPU_FTR_ALTIVEC). Agreed. * Renaming would clobber the namespace, CPU_* definitions are used in other places in the tree. * Can't make it an inline and still use the preprocessor concatenation. I'd like to keep the constants as-is and have the stuff inline with no macro trick as Pekka suggest since I did use grep on those things quite often. That being said, you do have a point about grepability. However, personally I'd be more likely to look for CPU_HAS_FEATURE than the feature itself when reading the code, and would find that easily. The other way around (finding all uses of a feature) is harder, but the concatenation macro is right below the bit definitions and easy to spot. No, when I grep, i'm looking for the feature itself... Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Sünnavend 05 Februar 2005 02:34, Anton Blanchard wrote: Interesting :) However we already get bug reports with the current CONFIG_POWER4_ONLY option. I worry about adding more options that users could get wrong unless there is a noticeable improvement in performance. The patch that I posted doesn't add any new user selectable options, it only limits the supported CPUs to the ones that are available on the supported platforms. If you select powermac or maple, the only supported CPU will be PowerPC970, so the C compiler can optimize away all runtime checks for CPU features. I don't expect much noticeable performance advantage from the patch, but it allows to make some of the source code nicer. E.g. you can replace every instance of '#ifdef CONFIG_ALTIVEC' with 'if (CPU_FTR_POSSIBLE CPU_FTR_ALTIVEC)' or an inline function wrapping that. Arnd pgp1i3ZIJpHvc.pgp Description: signature
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 11:20 -0600, Olof Johansson wrote:
> * cpu-has-feature(cpu-feature-foo) v cpu-has-feature(foo): I picked the
> latter for readability.
> * Renaming CPU_FTR_ -> CPU_ makes it less obvious that
> it's actually a cpu feature it's describing (i.e. CPU_ALTIVEC vs
> CPU_FTR_ALTIVEC).
> * Renaming would clobber the namespace, CPU_* definitions are used in
> other places in the tree.
> * Can't make it an inline and still use the preprocessor concatenation.
Seriously, if readability is your argument, macro magic is not the
answer. Ok, we can't clobber the CPU_ definitions, so pick another
prefix.
If you want readability, please consider using named enums:
enum cpu_feature {
CF_ALTIVEC = /* ... */
};
static inline int cpu_has_feature(enum cpu_feature cf) { }
Pekka
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
Hi, > This is the patch to evaluate CPU_HAS_FEATURE() at compile time whenever > possible. Testing showed that vmlinux shrinks around 4000 bytes with > g5_defconfig. I also checked that pSeries code is completely unaltered > semantically when support for all CPU types is enabled, although a few > instructions are emitted in a different order by gcc. > > I have made cpu_has_feature() an inline function that expects the full > name of a feature bit while the CPU_HAS_FEATURE() macro still behaves > the same way as in Olofs original patch for now. Interesting :) However we already get bug reports with the current CONFIG_POWER4_ONLY option. I worry about adding more options that users could get wrong unless there is a noticeable improvement in performance. Anton - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
> This is the patch to evaluate CPU_HAS_FEATURE() at compile time whenever > possible. Testing showed that vmlinux shrinks around 4000 bytes with > g5_defconfig. I also checked that pSeries code is completely unaltered > semantically when support for all CPU types is enabled, although a few > instructions are emitted in a different order by gcc. > > I have made cpu_has_feature() an inline function that expects the full > name of a feature bit while the CPU_HAS_FEATURE() macro still behaves > the same way as in Olofs original patch for now. Note that this doesn't the asm part of it, where feature "sections" are nop'ed out... it may be interesting to get rid of the nops too here, oh well, that's too complicated for now. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Sünnavend 05 Februar 2005 00:49, Benjamin Herrenschmidt wrote:
> On Fri, 2005-02-04 at 13:36 +0100, Arnd Bergmann wrote:
> > I have a somewhat similar patch that does the same to the
> > systemcfg->platform checks. I'm not sure if we should use the same inline
> > function for both checks, but I do think that they should be used in a
> > similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x).
>
> Note that I would prefer cpu_has_feature(), it doesn't strictly have to
> be a macro and has function semantics anyway.
> > [ ... ]
> > which will always result in the shortest code for any combination of
> > CONFIG_PPC_ISERIES, CONFIG_PPC_PSERIES and the other platforms.
>
> That's a good idea !
This is the patch to evaluate CPU_HAS_FEATURE() at compile time whenever
possible. Testing showed that vmlinux shrinks around 4000 bytes with
g5_defconfig. I also checked that pSeries code is completely unaltered
semantically when support for all CPU types is enabled, although a few
instructions are emitted in a different order by gcc.
I have made cpu_has_feature() an inline function that expects the full
name of a feature bit while the CPU_HAS_FEATURE() macro still behaves
the same way as in Olofs original patch for now.
I'm not sure if I got the Kconfig dependencies right, maybe you can
check them.
Signed-off-by: Arnd Bergmann <[EMAIL PROTECTED]>
---
Index: linux-2.6-64/include/asm-ppc64/cputable.h
===
--- linux-2.6-64.orig/include/asm-ppc64/cputable.h 2005-02-05
01:24:58.975674192 +0100
+++ linux-2.6-64/include/asm-ppc64/cputable.h 2005-02-05 01:26:17.328762712
+0100
@@ -66,9 +66,6 @@
extern struct cpu_spec cpu_specs[];
extern struct cpu_spec *cur_cpu_spec;
-#define CPU_HAS_FEATURE(x) (cur_cpu_spec->cpu_features & CPU_FTR_##x)
-
-
/* firmware feature bitmask values */
#define FIRMWARE_MAX_FEATURES 63
@@ -154,6 +151,80 @@
#define CPU_FTR_PPCAS_ARCH_V2 (CPU_FTR_PPCAS_ARCH_V2_BASE | CPU_FTR_16M_PAGE)
#endif
+/* We only set the altivec features if the kernel was compiled with altivec
+ * support
+ */
+#ifdef CONFIG_ALTIVEC
+#define CPU_FTR_ALTIVEC_COMP CPU_FTR_ALTIVEC
+#define PPC_FEATURE_HAS_ALTIVEC_COMP PPC_FEATURE_HAS_ALTIVEC
+#else
+#define CPU_FTR_ALTIVEC_COMP 0
+#define PPC_FEATURE_HAS_ALTIVEC_COMP0
+#endif
+
+enum {
+ CPU_FTR_POWER3 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_IABR | CPU_FTR_PMC8,
+ CPU_FTR_RS64 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_IABR | CPU_FTR_PMC8 |
+CPU_FTR_MMCRA,
+ CPU_FTR_POWER4 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 |
+CPU_FTR_PMC8 | CPU_FTR_MMCRA,
+ CPU_FTR_PPC970 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 |
+CPU_FTR_ALTIVEC_COMP | CPU_FTR_CAN_NAP |
+CPU_FTR_PMC8 | CPU_FTR_MMCRA,
+ CPU_FTR_POWER5 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 |
+CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_COHERENT_ICACHE |
+CPU_FTR_LOCKLESS_TLBIE | CPU_FTR_MMCRA_SIHV,
+ CPU_FTR_COMPATIBLE = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2,
+ CPU_FTR_POSSIBLE =
+#ifdef CONFIG_CPU_POWER3
+CPU_FTR_POWER3 |
+#endif
+#ifdef CONFIG_CPU_RS64
+CPU_FTR_RS64 |
+#endif
+#ifdef CONFIG_CPU_POWER4
+CPU_FTR_POWER4 |
+#endif
+#ifdef CONFIG_CPU_PPC970
+CPU_FTR_PPC970 |
+#endif
+#ifdef CONFIG_CPU_POWER5
+CPU_FTR_POWER5 |
+#endif
+0,
+ CPU_FTR_ALWAYS =
+#ifdef CONFIG_CPU_POWER3
+CPU_FTR_POWER3 &
+#endif
+#ifdef CONFIG_CPU_RS64
+CPU_FTR_RS64 &
+#endif
+#ifdef CONFIG_CPU_POWER4
+CPU_FTR_POWER4 &
+#endif
+#ifdef CONFIG_CPU_PPC970
+CPU_FTR_PPC970 &
+#endif
+#ifdef CONFIG_CPU_POWER5
+CPU_FTR_POWER5 &
+#endif
+CPU_FTR_POSSIBLE,
+};
+
+static inline int cpu_has_feature(unsigned long feature)
+{
+ return (CPU_FTR_ALWAYS & feature) ||
+ (CPU_FTR_POSSIBLE & feature & cur_cpu_spec->cpu_features);
+}
+
+#define CPU_HAS_FEATURE(x) cpu_has_feature(CPU_FTR_##x)
+
#define COMMON_PPC64_FW(0)
#endif
Index: linux-2.6-64/arch/ppc64/Kconfig
===
--- linux-2.6-64.orig/arch/ppc64/Kconfig2005-02-05 01:24:31.098912104
+0100
+++ linux-2.6-64/arch/ppc64/Kconfig 2005-02-05
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 12:35 -0600, Olof Johansson wrote: > On Fri, Feb 04, 2005 at 01:36:55PM +0100, Arnd Bergmann wrote: > > I have a somewhat similar patch that does the same to the > > systemcfg->platform checks. I'm not sure if we should use the same inline > > function for both checks, but I do think that they should be used in a > > similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x). > > Yep. Firmware features are also on the list. I figured I'd do CPU features > first though since they are the ones that started bugging me. > > > The same stuff is obviously possible for cur_cpu_spec->cpu_features as well. > > Do you think that it will help there? > > Nice. It won't be quite as easy to do compile-time for cpu features. > pSeries will need all cpus enabled since we have them all on various > machines, etc. I guess Powermac/Maple could benefit from it. In the > end it depends on how hairy the implementation would get vs performance > improvement. One other thing we did on ppc32 was to have separate ELF sections for pmac, chrp and prep specific code & get rid of them after boot... It may be worth bringing this back in... Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 13:36 +0100, Arnd Bergmann wrote:
> On Freedag 04 Februar 2005 08:22, Olof Johansson wrote:
> > It's getting pretty old to have see and type cur_cpu_spec->cpu_features
> > & CPU_FTR_, when a shorter and less TLA-ridden macro is more
> > readable.
> >
> > This also takes care of the differences between PPC and PPC64 cpu
> > features for the common code; most places in PPC could be replaced with
> > the macro as well.
>
> I have a somewhat similar patch that does the same to the
> systemcfg->platform checks. I'm not sure if we should use the same inline
> function for both checks, but I do think that they should be used in a
> similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x).
Note that I would prefer cpu_has_feature(), it doesn't strictly have to
be a macro and has function semantics anyway.
> My implementation of the platform checks tries to be extra clever by turning
> runtime checks into compile time checks if possible. This reduces code size
> and in some cases execution speed. It can also be used to replace compile
> time checks, i.e. it allows us to write
>
> static inline unsigned int readl(const volatile void __iomem *addr)
> {
> if (platform_is(PLATFORM_PPC_ISERIES))
> return iSeries_readl(addr);
> if (platform_possible(PLATFORM_PPC_PSERIES))
> return eeh_readl(addr);
> return in_le32();
> }
>
> which will always result in the shortest code for any combination of
> CONFIG_PPC_ISERIES, CONFIG_PPC_PSERIES and the other platforms.
That's a good idea !
> The required code for this is roughly
>
> enum {
> PPC64_PLATFORM_POSSIBLE =
> #ifdef CONFIG_PPC_ISERIES
> PLATFORM_ISERIES |
> #endif
> #ifdef CONFIG_PPC_PSERIES
> PLATFORM_PSERIES |
> #endif
> #ifdef CONFIG_PPC_PSERIES
> PLATFORM_PSERIES_LPAR |
> #endif
> #ifdef CONFIG_PPC_POWERMAC
> PLATFORM_POWERMAC |
> #endif
> #ifdef CONFIG_PPC_MAPLE
> PLATFORM_MAPLE |
> #endif
> 0,
> PPC64_PLATFORM_ONLY =
> #ifdef CONFIG_PPC_ISERIES
> PLATFORM_ISERIES &
> #endif
> #ifdef CONFIG_PPC_PSERIES
> PLATFORM_PSERIES &
> #endif
> #ifdef CONFIG_PPC_POWERMAC
> PLATFORM_POWERMAC &
> #endif
> #ifdef CONFIG_PPC_MAPLE
> PLATFORM_MAPLE &
> #endif
> -1ul,
> };
>
> static inline platform_is(unsigned long platform)
> {
> return ((PPC64_PLATFORM_ONLY & platform)
>|| (PPC64_PLATFORM_POSSIBLE & platform & systemcfg->platform));
> }
>
> static inline platform_possible(unsigned long platform)
> {
> reutrn !!(PPC64_PLATFORM_POSSIBLE & platform);
> }
>
> The same stuff is obviously possible for cur_cpu_spec->cpu_features as well.
> Do you think that it will help there?
>
> Arnd <><
> ___
> Linuxppc64-dev mailing list
> [EMAIL PROTECTED]
> https://ozlabs.org/cgi-bin/mailman/listinfo/linuxppc64-dev
--
Benjamin Herrenschmidt <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Freedag 04 Februar 2005 19:35, Olof Johansson wrote: > pSeries will need all cpus enabled since we have them all on various > machines, etc. I guess Powermac/Maple could benefit from it. Even on pSeries, we already have CONFIG_POWER4_ONLY, which could be used to optimize away some of the checks at compile time. I think it makes sense to extend this a bit to look more like the CPU selection on i386 or s390 where can set the oldest CPU you want to support. This also fits nicely with the gcc -mcpu= options. > In the end it depends on how hairy the implementation would get vs > performance improvement. Fortunately, that optimization should be easy to do on top of your patch, so we don't have to decide now. Arnd <>< pgplbQQ74sn0R.pgp Description: signature
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, Feb 04, 2005 at 01:36:55PM +0100, Arnd Bergmann wrote: > I have a somewhat similar patch that does the same to the > systemcfg->platform checks. I'm not sure if we should use the same inline > function for both checks, but I do think that they should be used in a > similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x). Yep. Firmware features are also on the list. I figured I'd do CPU features first though since they are the ones that started bugging me. > The same stuff is obviously possible for cur_cpu_spec->cpu_features as well. > Do you think that it will help there? Nice. It won't be quite as easy to do compile-time for cpu features. pSeries will need all cpus enabled since we have them all on various machines, etc. I guess Powermac/Maple could benefit from it. In the end it depends on how hairy the implementation would get vs performance improvement. -Olof - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, Feb 04, 2005 at 10:17:48AM +0200, Pekka Enberg wrote: > Please drop the CPU_FTR_##x macro magic as it makes grepping more > complicated. If the enum names are too long, just do s/CPU_FTR_/CPU_/g > or something similar. Also, could you please make this a static inline > function? I considered that for a while, but decided against it because: * cpu-has-feature(cpu-feature-foo) v cpu-has-feature(foo): I picked the latter for readability. * Renaming CPU_FTR_ -> CPU_ makes it less obvious that it's actually a cpu feature it's describing (i.e. CPU_ALTIVEC vs CPU_FTR_ALTIVEC). * Renaming would clobber the namespace, CPU_* definitions are used in other places in the tree. * Can't make it an inline and still use the preprocessor concatenation. That being said, you do have a point about grepability. However, personally I'd be more likely to look for CPU_HAS_FEATURE than the feature itself when reading the code, and would find that easily. The other way around (finding all uses of a feature) is harder, but the concatenation macro is right below the bit definitions and easy to spot. -Olof - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, Feb 04, 2005 at 01:22:54AM -0600, Olof Johansson wrote: > Hi, > > It's getting pretty old to have see and type cur_cpu_spec->cpu_features > & CPU_FTR_, when a shorter and less TLA-ridden macro is more > readable. > > This also takes care of the differences between PPC and PPC64 cpu > features for the common code; most places in PPC could be replaced with > the macro as well. It'd be nice if someone went and changed ppc32's cpu feature from an array and matched ppc64, while we're in here... -- Tom Rini http://gate.crashing.org/~trini/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Freedag 04 Februar 2005 08:22, Olof Johansson wrote:
> It's getting pretty old to have see and type cur_cpu_spec->cpu_features
> & CPU_FTR_, when a shorter and less TLA-ridden macro is more
> readable.
>
> This also takes care of the differences between PPC and PPC64 cpu
> features for the common code; most places in PPC could be replaced with
> the macro as well.
I have a somewhat similar patch that does the same to the
systemcfg->platform checks. I'm not sure if we should use the same inline
function for both checks, but I do think that they should be used in a
similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x).
My implementation of the platform checks tries to be extra clever by turning
runtime checks into compile time checks if possible. This reduces code size
and in some cases execution speed. It can also be used to replace compile
time checks, i.e. it allows us to write
static inline unsigned int readl(const volatile void __iomem *addr)
{
if (platform_is(PLATFORM_PPC_ISERIES))
return iSeries_readl(addr);
if (platform_possible(PLATFORM_PPC_PSERIES))
return eeh_readl(addr);
return in_le32();
}
which will always result in the shortest code for any combination of
CONFIG_PPC_ISERIES, CONFIG_PPC_PSERIES and the other platforms.
The required code for this is roughly
enum {
PPC64_PLATFORM_POSSIBLE =
#ifdef CONFIG_PPC_ISERIES
PLATFORM_ISERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES_LPAR |
#endif
#ifdef CONFIG_PPC_POWERMAC
PLATFORM_POWERMAC |
#endif
#ifdef CONFIG_PPC_MAPLE
PLATFORM_MAPLE |
#endif
0,
PPC64_PLATFORM_ONLY =
#ifdef CONFIG_PPC_ISERIES
PLATFORM_ISERIES &
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES &
#endif
#ifdef CONFIG_PPC_POWERMAC
PLATFORM_POWERMAC &
#endif
#ifdef CONFIG_PPC_MAPLE
PLATFORM_MAPLE &
#endif
-1ul,
};
static inline platform_is(unsigned long platform)
{
return ((PPC64_PLATFORM_ONLY & platform)
|| (PPC64_PLATFORM_POSSIBLE & platform & systemcfg->platform));
}
static inline platform_possible(unsigned long platform)
{
reutrn !!(PPC64_PLATFORM_POSSIBLE & platform);
}
The same stuff is obviously possible for cur_cpu_spec->cpu_features as well.
Do you think that it will help there?
Arnd <><
pgp24Z65cMhVN.pgp
Description: signature
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
Hi, On Fri, 4 Feb 2005 01:22:54 -0600, Olof Johansson <[EMAIL PROTECTED]> wrote: > +#define CPU_HAS_FEATURE(x) (cur_cpu_spec->cpu_features & CPU_FTR_##x) > + Please drop the CPU_FTR_##x macro magic as it makes grepping more complicated. If the enum names are too long, just do s/CPU_FTR_/CPU_/g or something similar. Also, could you please make this a static inline function? Pekka - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Sünnavend 05 Februar 2005 00:49, Benjamin Herrenschmidt wrote:
On Fri, 2005-02-04 at 13:36 +0100, Arnd Bergmann wrote:
I have a somewhat similar patch that does the same to the
systemcfg-platform checks. I'm not sure if we should use the same inline
function for both checks, but I do think that they should be used in a
similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x).
Note that I would prefer cpu_has_feature(), it doesn't strictly have to
be a macro and has function semantics anyway.
[ ... ]
which will always result in the shortest code for any combination of
CONFIG_PPC_ISERIES, CONFIG_PPC_PSERIES and the other platforms.
That's a good idea !
This is the patch to evaluate CPU_HAS_FEATURE() at compile time whenever
possible. Testing showed that vmlinux shrinks around 4000 bytes with
g5_defconfig. I also checked that pSeries code is completely unaltered
semantically when support for all CPU types is enabled, although a few
instructions are emitted in a different order by gcc.
I have made cpu_has_feature() an inline function that expects the full
name of a feature bit while the CPU_HAS_FEATURE() macro still behaves
the same way as in Olofs original patch for now.
I'm not sure if I got the Kconfig dependencies right, maybe you can
check them.
Signed-off-by: Arnd Bergmann [EMAIL PROTECTED]
---
Index: linux-2.6-64/include/asm-ppc64/cputable.h
===
--- linux-2.6-64.orig/include/asm-ppc64/cputable.h 2005-02-05
01:24:58.975674192 +0100
+++ linux-2.6-64/include/asm-ppc64/cputable.h 2005-02-05 01:26:17.328762712
+0100
@@ -66,9 +66,6 @@
extern struct cpu_spec cpu_specs[];
extern struct cpu_spec *cur_cpu_spec;
-#define CPU_HAS_FEATURE(x) (cur_cpu_spec-cpu_features CPU_FTR_##x)
-
-
/* firmware feature bitmask values */
#define FIRMWARE_MAX_FEATURES 63
@@ -154,6 +151,80 @@
#define CPU_FTR_PPCAS_ARCH_V2 (CPU_FTR_PPCAS_ARCH_V2_BASE | CPU_FTR_16M_PAGE)
#endif
+/* We only set the altivec features if the kernel was compiled with altivec
+ * support
+ */
+#ifdef CONFIG_ALTIVEC
+#define CPU_FTR_ALTIVEC_COMP CPU_FTR_ALTIVEC
+#define PPC_FEATURE_HAS_ALTIVEC_COMP PPC_FEATURE_HAS_ALTIVEC
+#else
+#define CPU_FTR_ALTIVEC_COMP 0
+#define PPC_FEATURE_HAS_ALTIVEC_COMP0
+#endif
+
+enum {
+ CPU_FTR_POWER3 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_IABR | CPU_FTR_PMC8,
+ CPU_FTR_RS64 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_IABR | CPU_FTR_PMC8 |
+CPU_FTR_MMCRA,
+ CPU_FTR_POWER4 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 |
+CPU_FTR_PMC8 | CPU_FTR_MMCRA,
+ CPU_FTR_PPC970 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 |
+CPU_FTR_ALTIVEC_COMP | CPU_FTR_CAN_NAP |
+CPU_FTR_PMC8 | CPU_FTR_MMCRA,
+ CPU_FTR_POWER5 = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 |
+CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_COHERENT_ICACHE |
+CPU_FTR_LOCKLESS_TLBIE | CPU_FTR_MMCRA_SIHV,
+ CPU_FTR_COMPATIBLE = CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB |
+CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2,
+ CPU_FTR_POSSIBLE =
+#ifdef CONFIG_CPU_POWER3
+CPU_FTR_POWER3 |
+#endif
+#ifdef CONFIG_CPU_RS64
+CPU_FTR_RS64 |
+#endif
+#ifdef CONFIG_CPU_POWER4
+CPU_FTR_POWER4 |
+#endif
+#ifdef CONFIG_CPU_PPC970
+CPU_FTR_PPC970 |
+#endif
+#ifdef CONFIG_CPU_POWER5
+CPU_FTR_POWER5 |
+#endif
+0,
+ CPU_FTR_ALWAYS =
+#ifdef CONFIG_CPU_POWER3
+CPU_FTR_POWER3
+#endif
+#ifdef CONFIG_CPU_RS64
+CPU_FTR_RS64
+#endif
+#ifdef CONFIG_CPU_POWER4
+CPU_FTR_POWER4
+#endif
+#ifdef CONFIG_CPU_PPC970
+CPU_FTR_PPC970
+#endif
+#ifdef CONFIG_CPU_POWER5
+CPU_FTR_POWER5
+#endif
+CPU_FTR_POSSIBLE,
+};
+
+static inline int cpu_has_feature(unsigned long feature)
+{
+ return (CPU_FTR_ALWAYS feature) ||
+ (CPU_FTR_POSSIBLE feature cur_cpu_spec-cpu_features);
+}
+
+#define CPU_HAS_FEATURE(x) cpu_has_feature(CPU_FTR_##x)
+
#define COMMON_PPC64_FW(0)
#endif
Index: linux-2.6-64/arch/ppc64/Kconfig
===
--- linux-2.6-64.orig/arch/ppc64/Kconfig2005-02-05 01:24:31.098912104
+0100
+++ linux-2.6-64/arch/ppc64/Kconfig 2005-02-05 01:25:01.430301032 +0100
@@
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
This is the patch to evaluate CPU_HAS_FEATURE() at compile time whenever possible. Testing showed that vmlinux shrinks around 4000 bytes with g5_defconfig. I also checked that pSeries code is completely unaltered semantically when support for all CPU types is enabled, although a few instructions are emitted in a different order by gcc. I have made cpu_has_feature() an inline function that expects the full name of a feature bit while the CPU_HAS_FEATURE() macro still behaves the same way as in Olofs original patch for now. Note that this doesn't the asm part of it, where feature sections are nop'ed out... it may be interesting to get rid of the nops too here, oh well, that's too complicated for now. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
Hi, This is the patch to evaluate CPU_HAS_FEATURE() at compile time whenever possible. Testing showed that vmlinux shrinks around 4000 bytes with g5_defconfig. I also checked that pSeries code is completely unaltered semantically when support for all CPU types is enabled, although a few instructions are emitted in a different order by gcc. I have made cpu_has_feature() an inline function that expects the full name of a feature bit while the CPU_HAS_FEATURE() macro still behaves the same way as in Olofs original patch for now. Interesting :) However we already get bug reports with the current CONFIG_POWER4_ONLY option. I worry about adding more options that users could get wrong unless there is a noticeable improvement in performance. Anton - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 11:20 -0600, Olof Johansson wrote:
* cpu-has-feature(cpu-feature-foo) v cpu-has-feature(foo): I picked the
latter for readability.
* Renaming CPU_FTR_x - CPU_x makes it less obvious that
it's actually a cpu feature it's describing (i.e. CPU_ALTIVEC vs
CPU_FTR_ALTIVEC).
* Renaming would clobber the namespace, CPU_* definitions are used in
other places in the tree.
* Can't make it an inline and still use the preprocessor concatenation.
Seriously, if readability is your argument, macro magic is not the
answer. Ok, we can't clobber the CPU_ definitions, so pick another
prefix.
If you want readability, please consider using named enums:
enum cpu_feature {
CF_ALTIVEC = /* ... */
};
static inline int cpu_has_feature(enum cpu_feature cf) { }
Pekka
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
Hi, On Fri, 4 Feb 2005 01:22:54 -0600, Olof Johansson [EMAIL PROTECTED] wrote: +#define CPU_HAS_FEATURE(x) (cur_cpu_spec-cpu_features CPU_FTR_##x) + Please drop the CPU_FTR_##x macro magic as it makes grepping more complicated. If the enum names are too long, just do s/CPU_FTR_/CPU_/g or something similar. Also, could you please make this a static inline function? Pekka - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Freedag 04 Februar 2005 08:22, Olof Johansson wrote:
It's getting pretty old to have see and type cur_cpu_spec-cpu_features
CPU_FTR_feature, when a shorter and less TLA-ridden macro is more
readable.
This also takes care of the differences between PPC and PPC64 cpu
features for the common code; most places in PPC could be replaced with
the macro as well.
I have a somewhat similar patch that does the same to the
systemcfg-platform checks. I'm not sure if we should use the same inline
function for both checks, but I do think that they should be used in a
similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x).
My implementation of the platform checks tries to be extra clever by turning
runtime checks into compile time checks if possible. This reduces code size
and in some cases execution speed. It can also be used to replace compile
time checks, i.e. it allows us to write
static inline unsigned int readl(const volatile void __iomem *addr)
{
if (platform_is(PLATFORM_PPC_ISERIES))
return iSeries_readl(addr);
if (platform_possible(PLATFORM_PPC_PSERIES))
return eeh_readl(addr);
return in_le32();
}
which will always result in the shortest code for any combination of
CONFIG_PPC_ISERIES, CONFIG_PPC_PSERIES and the other platforms.
The required code for this is roughly
enum {
PPC64_PLATFORM_POSSIBLE =
#ifdef CONFIG_PPC_ISERIES
PLATFORM_ISERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES_LPAR |
#endif
#ifdef CONFIG_PPC_POWERMAC
PLATFORM_POWERMAC |
#endif
#ifdef CONFIG_PPC_MAPLE
PLATFORM_MAPLE |
#endif
0,
PPC64_PLATFORM_ONLY =
#ifdef CONFIG_PPC_ISERIES
PLATFORM_ISERIES
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES
#endif
#ifdef CONFIG_PPC_POWERMAC
PLATFORM_POWERMAC
#endif
#ifdef CONFIG_PPC_MAPLE
PLATFORM_MAPLE
#endif
-1ul,
};
static inline platform_is(unsigned long platform)
{
return ((PPC64_PLATFORM_ONLY platform)
|| (PPC64_PLATFORM_POSSIBLE platform systemcfg-platform));
}
static inline platform_possible(unsigned long platform)
{
reutrn !!(PPC64_PLATFORM_POSSIBLE platform);
}
The same stuff is obviously possible for cur_cpu_spec-cpu_features as well.
Do you think that it will help there?
Arnd
pgp24Z65cMhVN.pgp
Description: signature
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, Feb 04, 2005 at 01:22:54AM -0600, Olof Johansson wrote: Hi, It's getting pretty old to have see and type cur_cpu_spec-cpu_features CPU_FTR_feature, when a shorter and less TLA-ridden macro is more readable. This also takes care of the differences between PPC and PPC64 cpu features for the common code; most places in PPC could be replaced with the macro as well. It'd be nice if someone went and changed ppc32's cpu feature from an array and matched ppc64, while we're in here... -- Tom Rini http://gate.crashing.org/~trini/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, Feb 04, 2005 at 10:17:48AM +0200, Pekka Enberg wrote: Please drop the CPU_FTR_##x macro magic as it makes grepping more complicated. If the enum names are too long, just do s/CPU_FTR_/CPU_/g or something similar. Also, could you please make this a static inline function? I considered that for a while, but decided against it because: * cpu-has-feature(cpu-feature-foo) v cpu-has-feature(foo): I picked the latter for readability. * Renaming CPU_FTR_x - CPU_x makes it less obvious that it's actually a cpu feature it's describing (i.e. CPU_ALTIVEC vs CPU_FTR_ALTIVEC). * Renaming would clobber the namespace, CPU_* definitions are used in other places in the tree. * Can't make it an inline and still use the preprocessor concatenation. That being said, you do have a point about grepability. However, personally I'd be more likely to look for CPU_HAS_FEATURE than the feature itself when reading the code, and would find that easily. The other way around (finding all uses of a feature) is harder, but the concatenation macro is right below the bit definitions and easy to spot. -Olof - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, Feb 04, 2005 at 01:36:55PM +0100, Arnd Bergmann wrote: I have a somewhat similar patch that does the same to the systemcfg-platform checks. I'm not sure if we should use the same inline function for both checks, but I do think that they should be used in a similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x). Yep. Firmware features are also on the list. I figured I'd do CPU features first though since they are the ones that started bugging me. The same stuff is obviously possible for cur_cpu_spec-cpu_features as well. Do you think that it will help there? Nice. It won't be quite as easy to do compile-time for cpu features. pSeries will need all cpus enabled since we have them all on various machines, etc. I guess Powermac/Maple could benefit from it. In the end it depends on how hairy the implementation would get vs performance improvement. -Olof - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Freedag 04 Februar 2005 19:35, Olof Johansson wrote: pSeries will need all cpus enabled since we have them all on various machines, etc. I guess Powermac/Maple could benefit from it. Even on pSeries, we already have CONFIG_POWER4_ONLY, which could be used to optimize away some of the checks at compile time. I think it makes sense to extend this a bit to look more like the CPU selection on i386 or s390 where can set the oldest CPU you want to support. This also fits nicely with the gcc -mcpu= options. In the end it depends on how hairy the implementation would get vs performance improvement. Fortunately, that optimization should be easy to do on top of your patch, so we don't have to decide now. Arnd pgplbQQ74sn0R.pgp Description: signature
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 13:36 +0100, Arnd Bergmann wrote:
On Freedag 04 Februar 2005 08:22, Olof Johansson wrote:
It's getting pretty old to have see and type cur_cpu_spec-cpu_features
CPU_FTR_feature, when a shorter and less TLA-ridden macro is more
readable.
This also takes care of the differences between PPC and PPC64 cpu
features for the common code; most places in PPC could be replaced with
the macro as well.
I have a somewhat similar patch that does the same to the
systemcfg-platform checks. I'm not sure if we should use the same inline
function for both checks, but I do think that they should be used in a
similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x).
Note that I would prefer cpu_has_feature(), it doesn't strictly have to
be a macro and has function semantics anyway.
My implementation of the platform checks tries to be extra clever by turning
runtime checks into compile time checks if possible. This reduces code size
and in some cases execution speed. It can also be used to replace compile
time checks, i.e. it allows us to write
static inline unsigned int readl(const volatile void __iomem *addr)
{
if (platform_is(PLATFORM_PPC_ISERIES))
return iSeries_readl(addr);
if (platform_possible(PLATFORM_PPC_PSERIES))
return eeh_readl(addr);
return in_le32();
}
which will always result in the shortest code for any combination of
CONFIG_PPC_ISERIES, CONFIG_PPC_PSERIES and the other platforms.
That's a good idea !
The required code for this is roughly
enum {
PPC64_PLATFORM_POSSIBLE =
#ifdef CONFIG_PPC_ISERIES
PLATFORM_ISERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES_LPAR |
#endif
#ifdef CONFIG_PPC_POWERMAC
PLATFORM_POWERMAC |
#endif
#ifdef CONFIG_PPC_MAPLE
PLATFORM_MAPLE |
#endif
0,
PPC64_PLATFORM_ONLY =
#ifdef CONFIG_PPC_ISERIES
PLATFORM_ISERIES
#endif
#ifdef CONFIG_PPC_PSERIES
PLATFORM_PSERIES
#endif
#ifdef CONFIG_PPC_POWERMAC
PLATFORM_POWERMAC
#endif
#ifdef CONFIG_PPC_MAPLE
PLATFORM_MAPLE
#endif
-1ul,
};
static inline platform_is(unsigned long platform)
{
return ((PPC64_PLATFORM_ONLY platform)
|| (PPC64_PLATFORM_POSSIBLE platform systemcfg-platform));
}
static inline platform_possible(unsigned long platform)
{
reutrn !!(PPC64_PLATFORM_POSSIBLE platform);
}
The same stuff is obviously possible for cur_cpu_spec-cpu_features as well.
Do you think that it will help there?
Arnd
___
Linuxppc64-dev mailing list
[EMAIL PROTECTED]
https://ozlabs.org/cgi-bin/mailman/listinfo/linuxppc64-dev
--
Benjamin Herrenschmidt [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
On Fri, 2005-02-04 at 12:35 -0600, Olof Johansson wrote: On Fri, Feb 04, 2005 at 01:36:55PM +0100, Arnd Bergmann wrote: I have a somewhat similar patch that does the same to the systemcfg-platform checks. I'm not sure if we should use the same inline function for both checks, but I do think that they should be used in a similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x). Yep. Firmware features are also on the list. I figured I'd do CPU features first though since they are the ones that started bugging me. The same stuff is obviously possible for cur_cpu_spec-cpu_features as well. Do you think that it will help there? Nice. It won't be quite as easy to do compile-time for cpu features. pSeries will need all cpus enabled since we have them all on various machines, etc. I guess Powermac/Maple could benefit from it. In the end it depends on how hairy the implementation would get vs performance improvement. One other thing we did on ppc32 was to have separate ELF sections for pmac, chrp and prep specific code get rid of them after boot... It may be worth bringing this back in... Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
Hi,
It's getting pretty old to have see and type cur_cpu_spec->cpu_features
& CPU_FTR_, when a shorter and less TLA-ridden macro is more
readable.
This also takes care of the differences between PPC and PPC64 cpu
features for the common code; most places in PPC could be replaced with
the macro as well.
Signed-off-by: Olof Johansson <[EMAIL PROTECTED]>
---
linux-2.5-olof/arch/ppc/kernel/ppc_htab.c|8 +++---
linux-2.5-olof/arch/ppc/kernel/setup.c |4 +--
linux-2.5-olof/arch/ppc/kernel/temp.c|2 -
linux-2.5-olof/arch/ppc/mm/mmu_decl.h|2 -
linux-2.5-olof/arch/ppc/mm/ppc_mmu.c |4 +--
linux-2.5-olof/arch/ppc/platforms/pmac_cpufreq.c |2 -
linux-2.5-olof/arch/ppc/platforms/pmac_setup.c |2 -
linux-2.5-olof/arch/ppc/platforms/pmac_smp.c |4 +--
linux-2.5-olof/arch/ppc/platforms/sandpoint.c|6 ++---
linux-2.5-olof/arch/ppc64/kernel/align.c |2 -
linux-2.5-olof/arch/ppc64/kernel/iSeries_setup.c |2 -
linux-2.5-olof/arch/ppc64/kernel/pSeries_lpar.c |2 -
linux-2.5-olof/arch/ppc64/kernel/process.c |4 +--
linux-2.5-olof/arch/ppc64/kernel/setup.c |6 ++---
linux-2.5-olof/arch/ppc64/kernel/smp.c |2 -
linux-2.5-olof/arch/ppc64/kernel/sysfs.c | 22 +--
linux-2.5-olof/arch/ppc64/mm/hash_native.c | 14 ++--
linux-2.5-olof/arch/ppc64/mm/hash_utils.c|2 -
linux-2.5-olof/arch/ppc64/mm/hugetlbpage.c |2 -
linux-2.5-olof/arch/ppc64/mm/init.c | 10
linux-2.5-olof/arch/ppc64/mm/slb.c |4 +--
linux-2.5-olof/arch/ppc64/mm/stab.c |2 -
linux-2.5-olof/arch/ppc64/oprofile/op_model_power4.c |2 -
linux-2.5-olof/arch/ppc64/oprofile/op_model_rs64.c |2 -
linux-2.5-olof/arch/ppc64/xmon/xmon.c|8 +++---
linux-2.5-olof/drivers/macintosh/via-pmu.c |2 -
linux-2.5-olof/drivers/md/raid6altivec.uc|2 -
linux-2.5-olof/include/asm-ppc/cputable.h|2 +
linux-2.5-olof/include/asm-ppc64/cacheflush.h|2 -
linux-2.5-olof/include/asm-ppc64/cputable.h |2 +
linux-2.5-olof/include/asm-ppc64/mmu_context.h |4 +--
linux-2.5-olof/include/asm-ppc64/page.h |2 -
32 files changed, 70 insertions(+), 66 deletions(-)
diff -puN include/asm-ppc64/cputable.h~cpu-has-feature
include/asm-ppc64/cputable.h
--- linux-2.5/include/asm-ppc64/cputable.h~cpu-has-feature 2005-02-04
00:33:25.0 -0600
+++ linux-2.5-olof/include/asm-ppc64/cputable.h 2005-02-04 00:33:26.0
-0600
@@ -66,6 +66,8 @@ struct cpu_spec {
extern struct cpu_spec cpu_specs[];
extern struct cpu_spec *cur_cpu_spec;
+#define CPU_HAS_FEATURE(x) (cur_cpu_spec->cpu_features & CPU_FTR_##x)
+
/* firmware feature bitmask values */
#define FIRMWARE_MAX_FEATURES 63
diff -puN arch/ppc64/kernel/align.c~cpu-has-feature arch/ppc64/kernel/align.c
--- linux-2.5/arch/ppc64/kernel/align.c~cpu-has-feature 2005-02-04
00:33:25.0 -0600
+++ linux-2.5-olof/arch/ppc64/kernel/align.c2005-02-04 00:33:26.0
-0600
@@ -238,7 +238,7 @@ fix_alignment(struct pt_regs *regs)
dsisr = regs->dsisr;
- if (cur_cpu_spec->cpu_features & CPU_FTR_NODSISRALIGN) {
+ if (CPU_HAS_FEATURE(NODSISRALIGN)) {
unsigned int real_instr;
if (__get_user(real_instr, (unsigned int __user *)regs->nip))
return 0;
diff -puN arch/ppc64/kernel/iSeries_setup.c~cpu-has-feature
arch/ppc64/kernel/iSeries_setup.c
--- linux-2.5/arch/ppc64/kernel/iSeries_setup.c~cpu-has-feature 2005-02-04
00:33:25.0 -0600
+++ linux-2.5-olof/arch/ppc64/kernel/iSeries_setup.c2005-02-04
00:33:26.0 -0600
@@ -267,7 +267,7 @@ unsigned long iSeries_process_mainstore_
unsigned long i;
unsigned long mem_blocks = 0;
- if (cur_cpu_spec->cpu_features & CPU_FTR_SLB)
+ if (CPU_HAS_FEATURE(SLB))
mem_blocks = iSeries_process_Regatta_mainstore_vpd(mb_array,
max_entries);
else
diff -puN arch/ppc64/kernel/idle.c~cpu-has-feature arch/ppc64/kernel/idle.c
diff -puN arch/ppc64/kernel/process.c~cpu-has-feature
arch/ppc64/kernel/process.c
--- linux-2.5/arch/ppc64/kernel/process.c~cpu-has-feature 2005-02-04
00:33:26.0 -0600
+++ linux-2.5-olof/arch/ppc64/kernel/process.c 2005-02-04 00:33:26.0
-0600
@@ -388,12 +388,12 @@ copy_thread(int nr, unsigned long clone_
kregs = (struct pt_regs *) sp;
sp -= STACK_FRAME_OVERHEAD;
p->thread.ksp = sp;
- if (cur_cpu_spec->cpu_features & CPU_FTR_SLB) {
+ if (CPU_HAS_FEATURE(SLB)) {
unsigned long sp_vsid = get_kernel_vsid(sp);
sp_vsid <<= SLB_VSID_SHIFT;
[PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro
Hi,
It's getting pretty old to have see and type cur_cpu_spec-cpu_features
CPU_FTR_feature, when a shorter and less TLA-ridden macro is more
readable.
This also takes care of the differences between PPC and PPC64 cpu
features for the common code; most places in PPC could be replaced with
the macro as well.
Signed-off-by: Olof Johansson [EMAIL PROTECTED]
---
linux-2.5-olof/arch/ppc/kernel/ppc_htab.c|8 +++---
linux-2.5-olof/arch/ppc/kernel/setup.c |4 +--
linux-2.5-olof/arch/ppc/kernel/temp.c|2 -
linux-2.5-olof/arch/ppc/mm/mmu_decl.h|2 -
linux-2.5-olof/arch/ppc/mm/ppc_mmu.c |4 +--
linux-2.5-olof/arch/ppc/platforms/pmac_cpufreq.c |2 -
linux-2.5-olof/arch/ppc/platforms/pmac_setup.c |2 -
linux-2.5-olof/arch/ppc/platforms/pmac_smp.c |4 +--
linux-2.5-olof/arch/ppc/platforms/sandpoint.c|6 ++---
linux-2.5-olof/arch/ppc64/kernel/align.c |2 -
linux-2.5-olof/arch/ppc64/kernel/iSeries_setup.c |2 -
linux-2.5-olof/arch/ppc64/kernel/pSeries_lpar.c |2 -
linux-2.5-olof/arch/ppc64/kernel/process.c |4 +--
linux-2.5-olof/arch/ppc64/kernel/setup.c |6 ++---
linux-2.5-olof/arch/ppc64/kernel/smp.c |2 -
linux-2.5-olof/arch/ppc64/kernel/sysfs.c | 22 +--
linux-2.5-olof/arch/ppc64/mm/hash_native.c | 14 ++--
linux-2.5-olof/arch/ppc64/mm/hash_utils.c|2 -
linux-2.5-olof/arch/ppc64/mm/hugetlbpage.c |2 -
linux-2.5-olof/arch/ppc64/mm/init.c | 10
linux-2.5-olof/arch/ppc64/mm/slb.c |4 +--
linux-2.5-olof/arch/ppc64/mm/stab.c |2 -
linux-2.5-olof/arch/ppc64/oprofile/op_model_power4.c |2 -
linux-2.5-olof/arch/ppc64/oprofile/op_model_rs64.c |2 -
linux-2.5-olof/arch/ppc64/xmon/xmon.c|8 +++---
linux-2.5-olof/drivers/macintosh/via-pmu.c |2 -
linux-2.5-olof/drivers/md/raid6altivec.uc|2 -
linux-2.5-olof/include/asm-ppc/cputable.h|2 +
linux-2.5-olof/include/asm-ppc64/cacheflush.h|2 -
linux-2.5-olof/include/asm-ppc64/cputable.h |2 +
linux-2.5-olof/include/asm-ppc64/mmu_context.h |4 +--
linux-2.5-olof/include/asm-ppc64/page.h |2 -
32 files changed, 70 insertions(+), 66 deletions(-)
diff -puN include/asm-ppc64/cputable.h~cpu-has-feature
include/asm-ppc64/cputable.h
--- linux-2.5/include/asm-ppc64/cputable.h~cpu-has-feature 2005-02-04
00:33:25.0 -0600
+++ linux-2.5-olof/include/asm-ppc64/cputable.h 2005-02-04 00:33:26.0
-0600
@@ -66,6 +66,8 @@ struct cpu_spec {
extern struct cpu_spec cpu_specs[];
extern struct cpu_spec *cur_cpu_spec;
+#define CPU_HAS_FEATURE(x) (cur_cpu_spec-cpu_features CPU_FTR_##x)
+
/* firmware feature bitmask values */
#define FIRMWARE_MAX_FEATURES 63
diff -puN arch/ppc64/kernel/align.c~cpu-has-feature arch/ppc64/kernel/align.c
--- linux-2.5/arch/ppc64/kernel/align.c~cpu-has-feature 2005-02-04
00:33:25.0 -0600
+++ linux-2.5-olof/arch/ppc64/kernel/align.c2005-02-04 00:33:26.0
-0600
@@ -238,7 +238,7 @@ fix_alignment(struct pt_regs *regs)
dsisr = regs-dsisr;
- if (cur_cpu_spec-cpu_features CPU_FTR_NODSISRALIGN) {
+ if (CPU_HAS_FEATURE(NODSISRALIGN)) {
unsigned int real_instr;
if (__get_user(real_instr, (unsigned int __user *)regs-nip))
return 0;
diff -puN arch/ppc64/kernel/iSeries_setup.c~cpu-has-feature
arch/ppc64/kernel/iSeries_setup.c
--- linux-2.5/arch/ppc64/kernel/iSeries_setup.c~cpu-has-feature 2005-02-04
00:33:25.0 -0600
+++ linux-2.5-olof/arch/ppc64/kernel/iSeries_setup.c2005-02-04
00:33:26.0 -0600
@@ -267,7 +267,7 @@ unsigned long iSeries_process_mainstore_
unsigned long i;
unsigned long mem_blocks = 0;
- if (cur_cpu_spec-cpu_features CPU_FTR_SLB)
+ if (CPU_HAS_FEATURE(SLB))
mem_blocks = iSeries_process_Regatta_mainstore_vpd(mb_array,
max_entries);
else
diff -puN arch/ppc64/kernel/idle.c~cpu-has-feature arch/ppc64/kernel/idle.c
diff -puN arch/ppc64/kernel/process.c~cpu-has-feature
arch/ppc64/kernel/process.c
--- linux-2.5/arch/ppc64/kernel/process.c~cpu-has-feature 2005-02-04
00:33:26.0 -0600
+++ linux-2.5-olof/arch/ppc64/kernel/process.c 2005-02-04 00:33:26.0
-0600
@@ -388,12 +388,12 @@ copy_thread(int nr, unsigned long clone_
kregs = (struct pt_regs *) sp;
sp -= STACK_FRAME_OVERHEAD;
p-thread.ksp = sp;
- if (cur_cpu_spec-cpu_features CPU_FTR_SLB) {
+ if (CPU_HAS_FEATURE(SLB)) {
unsigned long sp_vsid = get_kernel_vsid(sp);
sp_vsid = SLB_VSID_SHIFT;
