| Message ID | 20121207193143.2d2029daf2041683bc7b2474@mail.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 07, 2012 at 07:31:43PM +0400, Alexander Shiyan wrote: > Hello. > > Today I was tested multiple boards (not multiplatform) in the kernel > and found problems with booting. Err, I test this almost constantly and it works, and it's been working 100% correctly for about the last 12 years. There is no problem here. > Exacly, if multiple boards defined > in. config, we can proceed to boot only the last (determined by mach number). The mach-types file doesn't have anything to do with which board gets chosen. That file just provides the IDs, and a bunch of _optimized_ macros to allow the compiler to perform optimizations. Let's look at how this works today - let me pull out two entries: #ifdef CONFIG_ARCH_EBSA285 # ifdef machine_arch_type # undef machine_arch_type # define machine_arch_type __machine_arch_type # else # define machine_arch_type MACH_TYPE_EBSA285 # endif # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) #else # define machine_is_ebsa285() (0) #endif #ifdef CONFIG_ARCH_NETWINDER # ifdef machine_arch_type # undef machine_arch_type # define machine_arch_type __machine_arch_type # else # define machine_arch_type MACH_TYPE_NETWINDER # endif # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) #else # define machine_is_netwinder() (0) #endif #ifndef machine_arch_type #define machine_arch_type __machine_arch_type #endif Now, if CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=n, then follow through what the preprocessor does. In the first case, machine_arch_type is _not_ defined as a pre-processor symbol, so the "ifdef" is false. That means machine_arch_type gets defined as MACH_TYPE_EBSA285. machine_is_ebsa285() gets defined as (machine_arch_type == MACH_TYPE_EBSA285) and when all the macro subsitutions occur (which happens where it's used) this ends up becoming (MACH_TYPE_EBSA285 == MACH_TYPE_EBSA285). This is always true, so any code inside an if (machine_is_ebsa285()) {} block will be compiled into the kernel and will be executed unconditionally - which is exactly what we want. Now, machine_is_netwinder() gets defined as constant (0). This is always false, so any code inside an if (machine_is_netwinder()) {} block will be optimized away - exactly what we want. This isn't affected by the machine_arch_type definition. Next, you can do the same thing for the CONFIG_ARCH_EBSA285=n and CONFIG_ARCH_NETWINDER=y case, and you'll end up with similar results. Finally for the case where CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=y. In this case, it starts off just like the CONFIG_ARCH_EBSA285=y case above. When we hit the CONFIG_ARCH_NETWINDER block a very important change happens. This time machine_arch_type is already defined. So, what happens is machine_arch_type first gets undefined to avoid any compiler warnings about multiple definitions. We then define machine_arch_type to be the C variable __machine_arch_type. This makes any references to (machine_arch_type == MACH_TYPE_WHATEVER) become a runtime interpreted condition, which occurs for any machine type that has its config symbol enabled. So, machine_is_ebsa285() becomes (__machine_arch_type == MACH_TYPE_EBSA285) and machine_is_netwinder() becomes (__machine_arch_type == MACH_TYPE_NETWINDER), while other platforms machine_is_xxx() macros remain defined to constant 0. So, this all works exactly as we want. There is no bug here.
On Fri, 7 Dec 2012 16:17:54 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > Today I was tested multiple boards (not multiplatform) in the kernel > > and found problems with booting. > > Err, I test this almost constantly and it works, and it's been working > 100% correctly for about the last 12 years. There is no problem here. > > > Exacly, if multiple boards defined > > in. config, we can proceed to boot only the last (determined by mach number). > > The mach-types file doesn't have anything to do with which board gets chosen. > That file just provides the IDs, and a bunch of _optimized_ macros to > allow the compiler to perform optimizations. > > Let's look at how this works today - let me pull out two entries: > > #ifdef CONFIG_ARCH_EBSA285 > # ifdef machine_arch_type > # undef machine_arch_type > # define machine_arch_type __machine_arch_type > # else > # define machine_arch_type MACH_TYPE_EBSA285 > # endif > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > #else > # define machine_is_ebsa285() (0) > #endif > > #ifdef CONFIG_ARCH_NETWINDER > # ifdef machine_arch_type > # undef machine_arch_type > # define machine_arch_type __machine_arch_type > # else > # define machine_arch_type MACH_TYPE_NETWINDER > # endif > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > #else > # define machine_is_netwinder() (0) > #endif > > #ifndef machine_arch_type > #define machine_arch_type __machine_arch_type > #endif > > Now, if CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=n, then follow > through what the preprocessor does. In the first case, machine_arch_type > is _not_ defined as a pre-processor symbol, so the "ifdef" is false. > That means machine_arch_type gets defined as MACH_TYPE_EBSA285. > > machine_is_ebsa285() gets defined as (machine_arch_type == MACH_TYPE_EBSA285) > and when all the macro subsitutions occur (which happens where it's used) > this ends up becoming (MACH_TYPE_EBSA285 == MACH_TYPE_EBSA285). This is > always true, so any code inside an if (machine_is_ebsa285()) {} block will > be compiled into the kernel and will be executed unconditionally - which > is exactly what we want. > > Now, machine_is_netwinder() gets defined as constant (0). This is always > false, so any code inside an if (machine_is_netwinder()) {} block will > be optimized away - exactly what we want. This isn't affected by the > machine_arch_type definition. > > Next, you can do the same thing for the CONFIG_ARCH_EBSA285=n and > CONFIG_ARCH_NETWINDER=y case, and you'll end up with similar results. > > Finally for the case where CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=y. > In this case, it starts off just like the CONFIG_ARCH_EBSA285=y case above. > When we hit the CONFIG_ARCH_NETWINDER block a very important change happens. > This time machine_arch_type is already defined. > > So, what happens is machine_arch_type first gets undefined to avoid any > compiler warnings about multiple definitions. We then define > machine_arch_type to be the C variable __machine_arch_type. > > This makes any references to (machine_arch_type == MACH_TYPE_WHATEVER) > become a runtime interpreted condition, which occurs for any machine type > that has its config symbol enabled. So, machine_is_ebsa285() becomes > (__machine_arch_type == MACH_TYPE_EBSA285) and machine_is_netwinder() > becomes (__machine_arch_type == MACH_TYPE_NETWINDER), while other > platforms machine_is_xxx() macros remain defined to constant 0. > > So, this all works exactly as we want. There is no bug here. Apparently the problem is that I have a custom symbol for their own board, but I specify an existing ID in the MACHINE_START. The initial boot is fine, because support for the ID is in the kernel, but the mach_desc is taken from a different machine. Below is a test program to demonstrate. In this test I boot kernel with ID=4 and I have a valid machines with ID=4 and 5, but symbol associated with ID=4 is missing. This case will a result boot a machine ID=5. How to solve the problem, I do not know yet... <<< test.c #include <stdio.h> #include <string.h> #include <stdlib.h> //#define CONFIG_ARCH_EBSA285 1 #define CONFIG_ARCH_NETWINDER 1 #define MACH_TYPE_EBSA285 4 #define MACH_TYPE_NETWINDER 5 #ifdef CONFIG_ARCH_EBSA285 # ifdef machine_arch_type # undef machine_arch_type # define machine_arch_type __machine_arch_type # else # define machine_arch_type MACH_TYPE_EBSA285 # endif # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) #else # define machine_is_ebsa285() (0) #endif #ifdef CONFIG_ARCH_NETWINDER # ifdef machine_arch_type # undef machine_arch_type # define machine_arch_type __machine_arch_type # else # define machine_arch_type MACH_TYPE_NETWINDER # endif # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) #else # define machine_is_netwinder() (0) #endif #ifndef machine_arch_type #define machine_arch_type __machine_arch_type #endif unsigned int __machine_arch_type = MACH_TYPE_EBSA285; //From bootloader int main(int argc, char** argv) { if (machine_is_ebsa285()) printf("machine_is_ebsa285()\n"); if (machine_is_netwinder()) printf("machine_is_netwinder()\n"); printf(" machine_arch_type=%u\n", machine_arch_type); printf("__machine_arch_type=%u\n", __machine_arch_type); return 0; } >>> shc@shc /home/git/test $ ./test machine_is_netwinder() machine_arch_type=5 __machine_arch_type=4
On Sat, Dec 08, 2012 at 12:28:11PM +0400, Alexander Shiyan wrote: > On Fri, 7 Dec 2012 16:17:54 +0000 > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > > Today I was tested multiple boards (not multiplatform) in the kernel > > > and found problems with booting. > > > > Err, I test this almost constantly and it works, and it's been working > > 100% correctly for about the last 12 years. There is no problem here. > > > > > Exacly, if multiple boards defined > > > in. config, we can proceed to boot only the last (determined by mach number). > > > > The mach-types file doesn't have anything to do with which board gets chosen. > > That file just provides the IDs, and a bunch of _optimized_ macros to > > allow the compiler to perform optimizations. > > > > Let's look at how this works today - let me pull out two entries: > > > > #ifdef CONFIG_ARCH_EBSA285 > > # ifdef machine_arch_type > > # undef machine_arch_type > > # define machine_arch_type __machine_arch_type > > # else > > # define machine_arch_type MACH_TYPE_EBSA285 > > # endif > > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > > #else > > # define machine_is_ebsa285() (0) > > #endif > > > > #ifdef CONFIG_ARCH_NETWINDER > > # ifdef machine_arch_type > > # undef machine_arch_type > > # define machine_arch_type __machine_arch_type > > # else > > # define machine_arch_type MACH_TYPE_NETWINDER > > # endif > > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > > #else > > # define machine_is_netwinder() (0) > > #endif > > > > #ifndef machine_arch_type > > #define machine_arch_type __machine_arch_type > > #endif > > > > Now, if CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=n, then follow > > through what the preprocessor does. In the first case, machine_arch_type > > is _not_ defined as a pre-processor symbol, so the "ifdef" is false. > > That means machine_arch_type gets defined as MACH_TYPE_EBSA285. > > > > machine_is_ebsa285() gets defined as (machine_arch_type == MACH_TYPE_EBSA285) > > and when all the macro subsitutions occur (which happens where it's used) > > this ends up becoming (MACH_TYPE_EBSA285 == MACH_TYPE_EBSA285). This is > > always true, so any code inside an if (machine_is_ebsa285()) {} block will > > be compiled into the kernel and will be executed unconditionally - which > > is exactly what we want. > > > > Now, machine_is_netwinder() gets defined as constant (0). This is always > > false, so any code inside an if (machine_is_netwinder()) {} block will > > be optimized away - exactly what we want. This isn't affected by the > > machine_arch_type definition. > > > > Next, you can do the same thing for the CONFIG_ARCH_EBSA285=n and > > CONFIG_ARCH_NETWINDER=y case, and you'll end up with similar results. > > > > Finally for the case where CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=y. > > In this case, it starts off just like the CONFIG_ARCH_EBSA285=y case above. > > When we hit the CONFIG_ARCH_NETWINDER block a very important change happens. > > This time machine_arch_type is already defined. > > > > So, what happens is machine_arch_type first gets undefined to avoid any > > compiler warnings about multiple definitions. We then define > > machine_arch_type to be the C variable __machine_arch_type. > > > > This makes any references to (machine_arch_type == MACH_TYPE_WHATEVER) > > become a runtime interpreted condition, which occurs for any machine type > > that has its config symbol enabled. So, machine_is_ebsa285() becomes > > (__machine_arch_type == MACH_TYPE_EBSA285) and machine_is_netwinder() > > becomes (__machine_arch_type == MACH_TYPE_NETWINDER), while other > > platforms machine_is_xxx() macros remain defined to constant 0. > > > > So, this all works exactly as we want. There is no bug here. > > Apparently the problem is that I have a custom symbol for their own board, > but I specify an existing ID in the MACHINE_START. The initial boot is fine, > because support for the ID is in the kernel, but the mach_desc is taken from > a different machine. Below is a test program to demonstrate. In this test I > boot kernel with ID=4 and I have a valid machines with ID=4 and 5, but symbol > associated with ID=4 is missing. This case will a result boot a machine ID=5. > How to solve the problem, I do not know yet... > > <<< test.c > #include <stdio.h> > #include <string.h> > #include <stdlib.h> > > //#define CONFIG_ARCH_EBSA285 1 > #define CONFIG_ARCH_NETWINDER 1 > > #define MACH_TYPE_EBSA285 4 > #define MACH_TYPE_NETWINDER 5 > > #ifdef CONFIG_ARCH_EBSA285 > # ifdef machine_arch_type > # undef machine_arch_type > # define machine_arch_type __machine_arch_type > # else > # define machine_arch_type MACH_TYPE_EBSA285 > # endif > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > #else > # define machine_is_ebsa285() (0) > #endif > > #ifdef CONFIG_ARCH_NETWINDER > # ifdef machine_arch_type > # undef machine_arch_type > # define machine_arch_type __machine_arch_type > # else > # define machine_arch_type MACH_TYPE_NETWINDER > # endif > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > #else > # define machine_is_netwinder() (0) > #endif > > #ifndef machine_arch_type > #define machine_arch_type __machine_arch_type > #endif > > unsigned int __machine_arch_type = MACH_TYPE_EBSA285; //From bootloader > > int main(int argc, char** argv) > { > if (machine_is_ebsa285()) > printf("machine_is_ebsa285()\n"); > if (machine_is_netwinder()) > printf("machine_is_netwinder()\n"); > printf(" machine_arch_type=%u\n", machine_arch_type); > printf("__machine_arch_type=%u\n", __machine_arch_type); > > return 0; > } > >>> > > shc@shc /home/git/test $ ./test > machine_is_netwinder() > machine_arch_type=5 > __machine_arch_type=4 That is working as designed, nothing wrong there. You have EBSA285 support disabled (you commented out CONFIG_ARCH_EBSA285) and you have Netwinder support enabled (CONFIG_ARCH_NETWINDER). So, machine_is_ebsa285() is constant false and machine_is_netwinder() is constant true. However, such a kernel won't boot when passed ID 5 because it won't find the platform record in the kernel, and we explicitly error out in that case.
On Sat, 8 Dec 2012 09:26:18 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Sat, Dec 08, 2012 at 12:28:11PM +0400, Alexander Shiyan wrote: > > On Fri, 7 Dec 2012 16:17:54 +0000 > > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > > > > Today I was tested multiple boards (not multiplatform) in the kernel > > > > and found problems with booting. > > > > > > Err, I test this almost constantly and it works, and it's been working > > > 100% correctly for about the last 12 years. There is no problem here. > > > > > > > Exacly, if multiple boards defined > > > > in. config, we can proceed to boot only the last (determined by mach number). > > > > > > The mach-types file doesn't have anything to do with which board gets chosen. > > > That file just provides the IDs, and a bunch of _optimized_ macros to > > > allow the compiler to perform optimizations. > > > > > > Let's look at how this works today - let me pull out two entries: > > > > > > #ifdef CONFIG_ARCH_EBSA285 > > > # ifdef machine_arch_type > > > # undef machine_arch_type > > > # define machine_arch_type __machine_arch_type > > > # else > > > # define machine_arch_type MACH_TYPE_EBSA285 > > > # endif > > > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > > > #else > > > # define machine_is_ebsa285() (0) > > > #endif > > > > > > #ifdef CONFIG_ARCH_NETWINDER > > > # ifdef machine_arch_type > > > # undef machine_arch_type > > > # define machine_arch_type __machine_arch_type > > > # else > > > # define machine_arch_type MACH_TYPE_NETWINDER > > > # endif > > > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > > > #else > > > # define machine_is_netwinder() (0) > > > #endif > > > > > > #ifndef machine_arch_type > > > #define machine_arch_type __machine_arch_type > > > #endif > > > > > > Now, if CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=n, then follow > > > through what the preprocessor does. In the first case, machine_arch_type > > > is _not_ defined as a pre-processor symbol, so the "ifdef" is false. > > > That means machine_arch_type gets defined as MACH_TYPE_EBSA285. > > > > > > machine_is_ebsa285() gets defined as (machine_arch_type == MACH_TYPE_EBSA285) > > > and when all the macro subsitutions occur (which happens where it's used) > > > this ends up becoming (MACH_TYPE_EBSA285 == MACH_TYPE_EBSA285). This is > > > always true, so any code inside an if (machine_is_ebsa285()) {} block will > > > be compiled into the kernel and will be executed unconditionally - which > > > is exactly what we want. > > > > > > Now, machine_is_netwinder() gets defined as constant (0). This is always > > > false, so any code inside an if (machine_is_netwinder()) {} block will > > > be optimized away - exactly what we want. This isn't affected by the > > > machine_arch_type definition. > > > > > > Next, you can do the same thing for the CONFIG_ARCH_EBSA285=n and > > > CONFIG_ARCH_NETWINDER=y case, and you'll end up with similar results. > > > > > > Finally for the case where CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=y. > > > In this case, it starts off just like the CONFIG_ARCH_EBSA285=y case above. > > > When we hit the CONFIG_ARCH_NETWINDER block a very important change happens. > > > This time machine_arch_type is already defined. > > > > > > So, what happens is machine_arch_type first gets undefined to avoid any > > > compiler warnings about multiple definitions. We then define > > > machine_arch_type to be the C variable __machine_arch_type. > > > > > > This makes any references to (machine_arch_type == MACH_TYPE_WHATEVER) > > > become a runtime interpreted condition, which occurs for any machine type > > > that has its config symbol enabled. So, machine_is_ebsa285() becomes > > > (__machine_arch_type == MACH_TYPE_EBSA285) and machine_is_netwinder() > > > becomes (__machine_arch_type == MACH_TYPE_NETWINDER), while other > > > platforms machine_is_xxx() macros remain defined to constant 0. > > > > > > So, this all works exactly as we want. There is no bug here. > > > > Apparently the problem is that I have a custom symbol for their own board, > > but I specify an existing ID in the MACHINE_START. The initial boot is fine, > > because support for the ID is in the kernel, but the mach_desc is taken from > > a different machine. Below is a test program to demonstrate. In this test I > > boot kernel with ID=4 and I have a valid machines with ID=4 and 5, but symbol ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > associated with ID=4 is missing. This case will a result boot a machine ID=5. > > How to solve the problem, I do not know yet... > > > > <<< test.c > > #include <stdio.h> > > #include <string.h> > > #include <stdlib.h> > > > > //#define CONFIG_ARCH_EBSA285 1 > > #define CONFIG_ARCH_NETWINDER 1 > > > > #define MACH_TYPE_EBSA285 4 > > #define MACH_TYPE_NETWINDER 5 > > > > #ifdef CONFIG_ARCH_EBSA285 > > # ifdef machine_arch_type > > # undef machine_arch_type > > # define machine_arch_type __machine_arch_type > > # else > > # define machine_arch_type MACH_TYPE_EBSA285 > > # endif > > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > > #else > > # define machine_is_ebsa285() (0) > > #endif > > > > #ifdef CONFIG_ARCH_NETWINDER > > # ifdef machine_arch_type > > # undef machine_arch_type > > # define machine_arch_type __machine_arch_type > > # else > > # define machine_arch_type MACH_TYPE_NETWINDER > > # endif > > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > > #else > > # define machine_is_netwinder() (0) > > #endif > > > > #ifndef machine_arch_type > > #define machine_arch_type __machine_arch_type > > #endif > > > > unsigned int __machine_arch_type = MACH_TYPE_EBSA285; //From bootloader > > > > int main(int argc, char** argv) > > { > > if (machine_is_ebsa285()) > > printf("machine_is_ebsa285()\n"); > > if (machine_is_netwinder()) > > printf("machine_is_netwinder()\n"); > > printf(" machine_arch_type=%u\n", machine_arch_type); > > printf("__machine_arch_type=%u\n", __machine_arch_type); > > > > return 0; > > } > > >>> > > > > shc@shc /home/git/test $ ./test > > machine_is_netwinder() > > machine_arch_type=5 > > __machine_arch_type=4 > > That is working as designed, nothing wrong there. You have EBSA285 > support disabled (you commented out CONFIG_ARCH_EBSA285) and you have > Netwinder support enabled (CONFIG_ARCH_NETWINDER). So, > machine_is_ebsa285() is constant false and machine_is_netwinder() is > constant true. > > However, such a kernel won't boot when passed ID 5 because it won't > find the platform record in the kernel, and we explicitly error out > in that case. Why not? As I say before that I have a two valid machines with ID 4 and 5. I can boot machine ID 5 by specifying this ID in the bootloader. In any case, it's my fault that I do not specify the correct symbol, associated with the ID. But even such issues can be avoided if we make a comparison with the ID passed from the bootloader and not redefine it, ie: #ifdef CONFIG_ARCH_NETWINDER # define machine_is_netwinder() (__machine_arch_type == MACH_TYPE_NETWINDER) #else # define machine_is_netwinder() (0) #endif In this case, if we pass the ID 4 to bootloader, we will never have a macro machine_is_xx() (that's right), but boot the correct machine. Look at my solution for "gen_mach_types" in the first message of this thread. Fixme, but on my opinion it will not affect the optimization.
On Sat, Dec 08, 2012 at 01:58:45PM +0400, Alexander Shiyan wrote: > On Sat, 8 Dec 2012 09:26:18 +0000 > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > On Sat, Dec 08, 2012 at 12:28:11PM +0400, Alexander Shiyan wrote: > > > On Fri, 7 Dec 2012 16:17:54 +0000 > > > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > > > > > > Today I was tested multiple boards (not multiplatform) in the kernel > > > > > and found problems with booting. > > > > > > > > Err, I test this almost constantly and it works, and it's been working > > > > 100% correctly for about the last 12 years. There is no problem here. > > > > > > > > > Exacly, if multiple boards defined > > > > > in. config, we can proceed to boot only the last (determined by mach number). > > > > > > > > The mach-types file doesn't have anything to do with which board gets chosen. > > > > That file just provides the IDs, and a bunch of _optimized_ macros to > > > > allow the compiler to perform optimizations. > > > > > > > > Let's look at how this works today - let me pull out two entries: > > > > > > > > #ifdef CONFIG_ARCH_EBSA285 > > > > # ifdef machine_arch_type > > > > # undef machine_arch_type > > > > # define machine_arch_type __machine_arch_type > > > > # else > > > > # define machine_arch_type MACH_TYPE_EBSA285 > > > > # endif > > > > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > > > > #else > > > > # define machine_is_ebsa285() (0) > > > > #endif > > > > > > > > #ifdef CONFIG_ARCH_NETWINDER > > > > # ifdef machine_arch_type > > > > # undef machine_arch_type > > > > # define machine_arch_type __machine_arch_type > > > > # else > > > > # define machine_arch_type MACH_TYPE_NETWINDER > > > > # endif > > > > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > > > > #else > > > > # define machine_is_netwinder() (0) > > > > #endif > > > > > > > > #ifndef machine_arch_type > > > > #define machine_arch_type __machine_arch_type > > > > #endif > > > > > > > > Now, if CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=n, then follow > > > > through what the preprocessor does. In the first case, machine_arch_type > > > > is _not_ defined as a pre-processor symbol, so the "ifdef" is false. > > > > That means machine_arch_type gets defined as MACH_TYPE_EBSA285. > > > > > > > > machine_is_ebsa285() gets defined as (machine_arch_type == MACH_TYPE_EBSA285) > > > > and when all the macro subsitutions occur (which happens where it's used) > > > > this ends up becoming (MACH_TYPE_EBSA285 == MACH_TYPE_EBSA285). This is > > > > always true, so any code inside an if (machine_is_ebsa285()) {} block will > > > > be compiled into the kernel and will be executed unconditionally - which > > > > is exactly what we want. > > > > > > > > Now, machine_is_netwinder() gets defined as constant (0). This is always > > > > false, so any code inside an if (machine_is_netwinder()) {} block will > > > > be optimized away - exactly what we want. This isn't affected by the > > > > machine_arch_type definition. > > > > > > > > Next, you can do the same thing for the CONFIG_ARCH_EBSA285=n and > > > > CONFIG_ARCH_NETWINDER=y case, and you'll end up with similar results. > > > > > > > > Finally for the case where CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=y. > > > > In this case, it starts off just like the CONFIG_ARCH_EBSA285=y case above. > > > > When we hit the CONFIG_ARCH_NETWINDER block a very important change happens. > > > > This time machine_arch_type is already defined. > > > > > > > > So, what happens is machine_arch_type first gets undefined to avoid any > > > > compiler warnings about multiple definitions. We then define > > > > machine_arch_type to be the C variable __machine_arch_type. > > > > > > > > This makes any references to (machine_arch_type == MACH_TYPE_WHATEVER) > > > > become a runtime interpreted condition, which occurs for any machine type > > > > that has its config symbol enabled. So, machine_is_ebsa285() becomes > > > > (__machine_arch_type == MACH_TYPE_EBSA285) and machine_is_netwinder() > > > > becomes (__machine_arch_type == MACH_TYPE_NETWINDER), while other > > > > platforms machine_is_xxx() macros remain defined to constant 0. > > > > > > > > So, this all works exactly as we want. There is no bug here. > > > > > > Apparently the problem is that I have a custom symbol for their own board, > > > but I specify an existing ID in the MACHINE_START. The initial boot is fine, > > > because support for the ID is in the kernel, but the mach_desc is taken from > > > a different machine. Below is a test program to demonstrate. In this test I > > > boot kernel with ID=4 and I have a valid machines with ID=4 and 5, but symbol > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > associated with ID=4 is missing. This case will a result boot a machine ID=5. > > > How to solve the problem, I do not know yet... > > > > > > <<< test.c > > > #include <stdio.h> > > > #include <string.h> > > > #include <stdlib.h> > > > > > > //#define CONFIG_ARCH_EBSA285 1 > > > #define CONFIG_ARCH_NETWINDER 1 > > > > > > #define MACH_TYPE_EBSA285 4 > > > #define MACH_TYPE_NETWINDER 5 > > > > > > #ifdef CONFIG_ARCH_EBSA285 > > > # ifdef machine_arch_type > > > # undef machine_arch_type > > > # define machine_arch_type __machine_arch_type > > > # else > > > # define machine_arch_type MACH_TYPE_EBSA285 > > > # endif > > > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > > > #else > > > # define machine_is_ebsa285() (0) > > > #endif > > > > > > #ifdef CONFIG_ARCH_NETWINDER > > > # ifdef machine_arch_type > > > # undef machine_arch_type > > > # define machine_arch_type __machine_arch_type > > > # else > > > # define machine_arch_type MACH_TYPE_NETWINDER > > > # endif > > > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > > > #else > > > # define machine_is_netwinder() (0) > > > #endif > > > > > > #ifndef machine_arch_type > > > #define machine_arch_type __machine_arch_type > > > #endif > > > > > > unsigned int __machine_arch_type = MACH_TYPE_EBSA285; //From bootloader > > > > > > int main(int argc, char** argv) > > > { > > > if (machine_is_ebsa285()) > > > printf("machine_is_ebsa285()\n"); > > > if (machine_is_netwinder()) > > > printf("machine_is_netwinder()\n"); > > > printf(" machine_arch_type=%u\n", machine_arch_type); > > > printf("__machine_arch_type=%u\n", __machine_arch_type); > > > > > > return 0; > > > } > > > >>> > > > > > > shc@shc /home/git/test $ ./test > > > machine_is_netwinder() > > > machine_arch_type=5 > > > __machine_arch_type=4 > > > > That is working as designed, nothing wrong there. You have EBSA285 > > support disabled (you commented out CONFIG_ARCH_EBSA285) and you have > > Netwinder support enabled (CONFIG_ARCH_NETWINDER). So, > > machine_is_ebsa285() is constant false and machine_is_netwinder() is > > constant true. > > > > However, such a kernel won't boot when passed ID 5 because it won't > > find the platform record in the kernel, and we explicitly error out > > in that case. > > Why not? As I say before that I have a two valid machines with ID 4 and 5. > I can boot machine ID 5 by specifying this ID in the bootloader. If you have two machines with ID 4 and ID 5, and you configure out of the kernel support for ID 4, then pass ID 4 to the kernel, the kernel should error out. > In any case, it's my fault that I do not specify the correct symbol, > associated with the ID. That is why we have the machine database, to keep things straight. > But even such issues can be avoided if we make > a comparison with the ID passed from the bootloader and not redefine it, ie: > #ifdef CONFIG_ARCH_NETWINDER > # define machine_is_netwinder() (__machine_arch_type == MACH_TYPE_NETWINDER) > #else > # define machine_is_netwinder() (0) > #endif No, you haven't listened to what I've said. Go back and read my explanation about how this works, particularly the part where these macros end up being constant false and constant true. > In this case, if we pass the ID 4 to bootloader, we will never have a macro > machine_is_xx() (that's right), but boot the correct machine. Again, you're not listening to what I've said. > Look at my solution for "gen_mach_types" in the first message of this thread. > Fixme, but on my opinion it will not affect the optimization. You're wrong. You don't understand what's going on here and why it is the way that it is. There is nothing wrong here, at all. It is all working as designed. Your changes make it work sub-optimally by getting rid of the constant-true optimization.
On Sat, 8 Dec 2012 10:38:12 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > > > > Today I was tested multiple boards (not multiplatform) in the kernel > > > > > > and found problems with booting. > > > > > > > > > > Err, I test this almost constantly and it works, and it's been working > > > > > 100% correctly for about the last 12 years. There is no problem here. > > > > > > > > > > > Exacly, if multiple boards defined > > > > > > in. config, we can proceed to boot only the last (determined by mach number). > > > > > > > > > > The mach-types file doesn't have anything to do with which board gets chosen. > > > > > That file just provides the IDs, and a bunch of _optimized_ macros to > > > > > allow the compiler to perform optimizations. > > > > > > > > > > Let's look at how this works today - let me pull out two entries: > > > > > > > > > > #ifdef CONFIG_ARCH_EBSA285 > > > > > # ifdef machine_arch_type > > > > > # undef machine_arch_type > > > > > # define machine_arch_type __machine_arch_type > > > > > # else > > > > > # define machine_arch_type MACH_TYPE_EBSA285 > > > > > # endif > > > > > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > > > > > #else > > > > > # define machine_is_ebsa285() (0) > > > > > #endif > > > > > > > > > > #ifdef CONFIG_ARCH_NETWINDER > > > > > # ifdef machine_arch_type > > > > > # undef machine_arch_type > > > > > # define machine_arch_type __machine_arch_type > > > > > # else > > > > > # define machine_arch_type MACH_TYPE_NETWINDER > > > > > # endif > > > > > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > > > > > #else > > > > > # define machine_is_netwinder() (0) > > > > > #endif > > > > > > > > > > #ifndef machine_arch_type > > > > > #define machine_arch_type __machine_arch_type > > > > > #endif > > > > > > > > > > Now, if CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=n, then follow > > > > > through what the preprocessor does. In the first case, machine_arch_type > > > > > is _not_ defined as a pre-processor symbol, so the "ifdef" is false. > > > > > That means machine_arch_type gets defined as MACH_TYPE_EBSA285. > > > > > > > > > > machine_is_ebsa285() gets defined as (machine_arch_type == MACH_TYPE_EBSA285) > > > > > and when all the macro subsitutions occur (which happens where it's used) > > > > > this ends up becoming (MACH_TYPE_EBSA285 == MACH_TYPE_EBSA285). This is > > > > > always true, so any code inside an if (machine_is_ebsa285()) {} block will > > > > > be compiled into the kernel and will be executed unconditionally - which > > > > > is exactly what we want. > > > > > > > > > > Now, machine_is_netwinder() gets defined as constant (0). This is always > > > > > false, so any code inside an if (machine_is_netwinder()) {} block will > > > > > be optimized away - exactly what we want. This isn't affected by the > > > > > machine_arch_type definition. > > > > > > > > > > Next, you can do the same thing for the CONFIG_ARCH_EBSA285=n and > > > > > CONFIG_ARCH_NETWINDER=y case, and you'll end up with similar results. > > > > > > > > > > Finally for the case where CONFIG_ARCH_EBSA285=y and CONFIG_ARCH_NETWINDER=y. > > > > > In this case, it starts off just like the CONFIG_ARCH_EBSA285=y case above. > > > > > When we hit the CONFIG_ARCH_NETWINDER block a very important change happens. > > > > > This time machine_arch_type is already defined. > > > > > > > > > > So, what happens is machine_arch_type first gets undefined to avoid any > > > > > compiler warnings about multiple definitions. We then define > > > > > machine_arch_type to be the C variable __machine_arch_type. > > > > > > > > > > This makes any references to (machine_arch_type == MACH_TYPE_WHATEVER) > > > > > become a runtime interpreted condition, which occurs for any machine type > > > > > that has its config symbol enabled. So, machine_is_ebsa285() becomes > > > > > (__machine_arch_type == MACH_TYPE_EBSA285) and machine_is_netwinder() > > > > > becomes (__machine_arch_type == MACH_TYPE_NETWINDER), while other > > > > > platforms machine_is_xxx() macros remain defined to constant 0. > > > > > > > > > > So, this all works exactly as we want. There is no bug here. > > > > > > > > Apparently the problem is that I have a custom symbol for their own board, > > > > but I specify an existing ID in the MACHINE_START. The initial boot is fine, > > > > because support for the ID is in the kernel, but the mach_desc is taken from > > > > a different machine. Below is a test program to demonstrate. In this test I > > > > boot kernel with ID=4 and I have a valid machines with ID=4 and 5, but symbol > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > > > associated with ID=4 is missing. This case will a result boot a machine ID=5. > > > > How to solve the problem, I do not know yet... > > > > > > > > <<< test.c > > > > #include <stdio.h> > > > > #include <string.h> > > > > #include <stdlib.h> > > > > > > > > //#define CONFIG_ARCH_EBSA285 1 > > > > #define CONFIG_ARCH_NETWINDER 1 > > > > > > > > #define MACH_TYPE_EBSA285 4 > > > > #define MACH_TYPE_NETWINDER 5 > > > > > > > > #ifdef CONFIG_ARCH_EBSA285 > > > > # ifdef machine_arch_type > > > > # undef machine_arch_type > > > > # define machine_arch_type __machine_arch_type > > > > # else > > > > # define machine_arch_type MACH_TYPE_EBSA285 > > > > # endif > > > > # define machine_is_ebsa285() (machine_arch_type == MACH_TYPE_EBSA285) > > > > #else > > > > # define machine_is_ebsa285() (0) > > > > #endif > > > > > > > > #ifdef CONFIG_ARCH_NETWINDER > > > > # ifdef machine_arch_type > > > > # undef machine_arch_type > > > > # define machine_arch_type __machine_arch_type > > > > # else > > > > # define machine_arch_type MACH_TYPE_NETWINDER > > > > # endif > > > > # define machine_is_netwinder() (machine_arch_type == MACH_TYPE_NETWINDER) > > > > #else > > > > # define machine_is_netwinder() (0) > > > > #endif > > > > > > > > #ifndef machine_arch_type > > > > #define machine_arch_type __machine_arch_type > > > > #endif > > > > > > > > unsigned int __machine_arch_type = MACH_TYPE_EBSA285; //From bootloader > > > > > > > > int main(int argc, char** argv) > > > > { > > > > if (machine_is_ebsa285()) > > > > printf("machine_is_ebsa285()\n"); > > > > if (machine_is_netwinder()) > > > > printf("machine_is_netwinder()\n"); > > > > printf(" machine_arch_type=%u\n", machine_arch_type); > > > > printf("__machine_arch_type=%u\n", __machine_arch_type); > > > > > > > > return 0; > > > > } > > > > >>> > > > > > > > > shc@shc /home/git/test $ ./test > > > > machine_is_netwinder() > > > > machine_arch_type=5 > > > > __machine_arch_type=4 > > > > > > That is working as designed, nothing wrong there. You have EBSA285 > > > support disabled (you commented out CONFIG_ARCH_EBSA285) and you have > > > Netwinder support enabled (CONFIG_ARCH_NETWINDER). So, > > > machine_is_ebsa285() is constant false and machine_is_netwinder() is > > > constant true. > > > > > > However, such a kernel won't boot when passed ID 5 because it won't > > > find the platform record in the kernel, and we explicitly error out > > > in that case. > > > > Why not? As I say before that I have a two valid machines with ID 4 and 5. > > I can boot machine ID 5 by specifying this ID in the bootloader. > > If you have two machines with ID 4 and ID 5, and you configure out of the > kernel support for ID 4, then pass ID 4 to the kernel, the kernel should > error out. > > > In any case, it's my fault that I do not specify the correct symbol, > > associated with the ID. > > That is why we have the machine database, to keep things straight. > > > But even such issues can be avoided if we make > > a comparison with the ID passed from the bootloader and not redefine it, ie: > > #ifdef CONFIG_ARCH_NETWINDER > > # define machine_is_netwinder() (__machine_arch_type == MACH_TYPE_NETWINDER) > > #else > > # define machine_is_netwinder() (0) > > #endif > > No, you haven't listened to what I've said. Go back and read my > explanation about how this works, particularly the part where these > macros end up being constant false and constant true. > > > In this case, if we pass the ID 4 to bootloader, we will never have a macro > > machine_is_xx() (that's right), but boot the correct machine. > > Again, you're not listening to what I've said. > > > Look at my solution for "gen_mach_types" in the first message of this thread. > > Fixme, but on my opinion it will not affect the optimization. > > You're wrong. You don't understand what's going on here and why it is > the way that it is. There is nothing wrong here, at all. It is all > working as designed. Your changes make it work sub-optimally by getting > rid of the constant-true optimization. I understand that optimization occurs only if one machine is defined. With two or more machines, the constant is replaced by the variable and processing is done at runtime. For my case, when only one character is specified, the processing at runtime does not work... Well, I understand your position on optimization. Can we at least replace definition machine_arch_type by variable __machine_arch_type, which is passed to the function setup_machine_tags() in setup_arch() procedure? Or have I misunderstood something again?
On Sat, Dec 08, 2012 at 03:18:59PM +0400, Alexander Shiyan wrote: > Well, I understand your position on optimization. Can we at least replace > definition machine_arch_type by variable __machine_arch_type, which is passed > to the function setup_machine_tags() in setup_arch() procedure? Or have I > misunderstood something again? Err, no I don't think you have. Looks like the initial erroring out got broken by the addition of DT support and all the changes that happened after that. Given that the initial assembly code which errored out on non-supported platforms seems to have been deleted, that does need to be changed. Please send a patch just for that.
On Sat, 8 Dec 2012 11:23:53 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Sat, Dec 08, 2012 at 03:18:59PM +0400, Alexander Shiyan wrote: > > Well, I understand your position on optimization. Can we at least replace > > definition machine_arch_type by variable __machine_arch_type, which is passed > > to the function setup_machine_tags() in setup_arch() procedure? Or have I > > misunderstood something again? > > Err, no I don't think you have. Looks like the initial erroring out got > broken by the addition of DT support and all the changes that happened > after that. > > Given that the initial assembly code which errored out on non-supported > platforms seems to have been deleted, that does need to be changed. > Please send a patch just for that. Do it now, please check the following patch.
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c index df89983..86a2b0d 100644 --- a/arch/arm/boot/compressed/misc.c +++ b/arch/arm/boot/compressed/misc.c @@ -16,8 +16,6 @@ * This allows for a much quicker boot time. */ -unsigned int __machine_arch_type; - #include <linux/compiler.h> /* for inline */ #include <linux/types.h> #include <linux/linkage.h> @@ -112,6 +110,7 @@ unsigned char *output_data; unsigned long free_mem_ptr; unsigned long free_mem_end_ptr; +unsigned int machine_arch_type; #ifndef arch_error #define arch_error(x) @@ -146,7 +145,7 @@ decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p, output_data = (unsigned char *)output_start; free_mem_ptr = free_mem_ptr_p; free_mem_end_ptr = free_mem_ptr_end_p; - __machine_arch_type = arch_id; + machine_arch_type = arch_id; arch_decomp_setup(); diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c index 70f1bde..bbb9a02 100644 --- a/arch/arm/kernel/devtree.c +++ b/arch/arm/kernel/devtree.c @@ -232,7 +232,7 @@ struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys) of_scan_flat_dt(early_init_dt_scan_memory, NULL); /* Change machine number to match the mdesc we're using */ - __machine_arch_type = mdesc_best->nr; + machine_arch_type = mdesc_best->nr; return mdesc_best; } diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S index 2f560c5..6e02251 100644 --- a/arch/arm/kernel/head-common.S +++ b/arch/arm/kernel/head-common.S @@ -112,7 +112,7 @@ __mmap_switched_data: .long __bss_start @ r6 .long _end @ r7 .long processor_id @ r4 - .long __machine_arch_type @ r5 + .long machine_arch_type @ r5 .long __atags_pointer @ r6 #ifdef CONFIG_CPU_CP15 .long cr_alignment @ r7 diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 86d1429..dc3c4b0 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -78,8 +78,8 @@ extern void setup_dma_zone(struct machine_desc *desc); unsigned int processor_id; EXPORT_SYMBOL(processor_id); -unsigned int __machine_arch_type __read_mostly; -EXPORT_SYMBOL(__machine_arch_type); +unsigned int machine_arch_type __read_mostly; +EXPORT_SYMBOL(machine_arch_type); unsigned int cacheid __read_mostly; EXPORT_SYMBOL(cacheid); diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c index 1b64114..939079a 100644 --- a/arch/arm/mach-pxa/em-x270.c +++ b/arch/arm/mach-pxa/em-x270.c @@ -1272,8 +1272,6 @@ static void __init em_x270_init(void) em_x270_module_init(); else if (machine_is_exeda()) em_x270_exeda_init(); - else - panic("Unsupported machine: %d\n", machine_arch_type); em_x270_init_da9030(); em_x270_init_dm9000(); diff --git a/arch/arm/mach-pxa/trizeps4.c b/arch/arm/mach-pxa/trizeps4.c index fbbcbed..7988e8b 100644 --- a/arch/arm/mach-pxa/trizeps4.c +++ b/arch/arm/mach-pxa/trizeps4.c @@ -544,11 +544,11 @@ static void __init trizeps4_map_io(void) if ((__raw_readl(MSC0) & 0x8) && (__raw_readl(BOOT_DEF) & 0x1)) { /* if flash is 16 bit wide its a Trizeps4 WL */ - __machine_arch_type = MACH_TYPE_TRIZEPS4WL; + machine_arch_type = MACH_TYPE_TRIZEPS4WL; trizeps4_flash_data[0].width = 2; } else { /* if flash is 32 bit wide its a Trizeps4 */ - __machine_arch_type = MACH_TYPE_TRIZEPS4; + machine_arch_type = MACH_TYPE_TRIZEPS4; trizeps4_flash_data[0].width = 4; } } diff --git a/arch/arm/tools/gen-mach-types b/arch/arm/tools/gen-mach-types index 04fef71..d36642d 100644 --- a/arch/arm/tools/gen-mach-types +++ b/arch/arm/tools/gen-mach-types @@ -30,7 +30,7 @@ END { printf("#define __ASM_ARM_MACH_TYPE_H\n\n"); printf("#ifndef __ASSEMBLY__\n"); printf("/* The type of machine we're running on */\n"); - printf("extern unsigned int __machine_arch_type;\n"); + printf("extern unsigned int machine_arch_type;\n"); printf("#endif\n\n"); printf("/* see arch/arm/kernel/arch.c for a description of these */\n"); @@ -43,12 +43,6 @@ END { for (i = 0; i < nr; i++) if (num[i] ~ /..*/) { printf("#ifdef %s\n", config[i]); - printf("# ifdef machine_arch_type\n"); - printf("# undef machine_arch_type\n"); - printf("# define machine_arch_type\t__machine_arch_type\n"); - printf("# else\n"); - printf("# define machine_arch_type\t%s\n", mach_type[i]); - printf("# endif\n"); printf("# define %s()\t(machine_arch_type == %s)\n", machine_is[i], mach_type[i]); printf("#else\n"); printf("# define %s()\t(0)\n", machine_is[i]); @@ -65,8 +59,5 @@ END { printf("#define %s()\t(0)\n", machine_is[i]); } - printf("\n#ifndef machine_arch_type\n"); - printf("#define machine_arch_type\t__machine_arch_type\n"); - printf("#endif\n\n"); printf("#endif\n"); }