To support 32/64 bits system, we have some definition conditionally. while
some definition is duplicated:
* __PHYSICAL_START has the same definition as LOAD_PHYSICAL_ADDR
* LOAD_OFFSET could be defined directly to __START_KERNEL_map
After these cleanup, we could reduce some complexity of vmlinux.lds.S.
Wei Yang (4):
vmlinux.lds.h: fix a typo in comment
x86/boot: replace __PHYSICAL_START with LOAD_PHYSICAL_ADDR
x86/vmlinux.lds.S: remove conditional definition of LOAD_OFFSET
x86/vmlinux.lds.S: take __START_KERNEL out conditional definition
arch/x86/include/asm/boot.h | 5 -----
arch/x86/include/asm/page_types.h | 8 +++++---
arch/x86/kernel/vmlinux.lds.S | 7 +------
include/asm-generic/vmlinux.lds.h | 2 +-
4 files changed, 7 insertions(+), 15 deletions(-)
--
2.34.1
One extra 'i' is used.
Signed-off-by: Wei Yang <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 5dd3a61d673d..514d3002ad8a 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -3,7 +3,7 @@
* linker scripts.
*
* A minimal linker scripts has following content:
- * [This is a sample, architectures may have special requiriements]
+ * [This is a sample, architectures may have special requirements]
*
* OUTPUT_FORMAT(...)
* OUTPUT_ARCH(...)
--
2.34.1
Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
which is only used to define __START_KERNEL.
Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
<asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
<asm/page_types.h>.
Signed-off-by: Wei Yang <[email protected]>
---
arch/x86/include/asm/boot.h | 5 -----
arch/x86/include/asm/page_types.h | 8 +++++---
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
index a38cc0afc90a..12cbc57d0128 100644
--- a/arch/x86/include/asm/boot.h
+++ b/arch/x86/include/asm/boot.h
@@ -6,11 +6,6 @@
#include <asm/pgtable_types.h>
#include <uapi/asm/boot.h>
-/* Physical address where kernel should be loaded. */
-#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
- + (CONFIG_PHYSICAL_ALIGN - 1)) \
- & ~(CONFIG_PHYSICAL_ALIGN - 1))
-
/* Minimum kernel alignment, as a power of two */
#ifdef CONFIG_X86_64
# define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
index 86bd4311daf8..acc1620fd121 100644
--- a/arch/x86/include/asm/page_types.h
+++ b/arch/x86/include/asm/page_types.h
@@ -31,10 +31,12 @@
#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
-#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
- CONFIG_PHYSICAL_ALIGN)
+/* Physical address where kernel should be loaded. */
+#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
+ + (CONFIG_PHYSICAL_ALIGN - 1)) \
+ & ~(CONFIG_PHYSICAL_ALIGN - 1))
-#define __START_KERNEL (__START_KERNEL_map + __PHYSICAL_START)
+#define __START_KERNEL (__START_KERNEL_map + LOAD_PHYSICAL_ADDR)
#ifdef CONFIG_X86_64
#include <asm/page_64_types.h>
--
2.34.1
In vmlinux.lds.S, we define LOAD_OFFSET conditionally to __PAGE_OFFSET
or __START_KERNEL_map. While __START_KERNEL_map is already defined to
the same value with the same condition.
So it is fine to define LOAD_OFFSET to __START_KERNEL_map directly.
Signed-off-by: Wei Yang <[email protected]>
---
arch/x86/kernel/vmlinux.lds.S | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 56451fd2099e..88dcf9366949 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -15,11 +15,7 @@
* put it inside the section definition.
*/
-#ifdef CONFIG_X86_32
-#define LOAD_OFFSET __PAGE_OFFSET
-#else
#define LOAD_OFFSET __START_KERNEL_map
-#endif
#define RUNTIME_DISCARD_EXIT
#define EMITS_PT_NOTE
--
2.34.1
If CONFIG_X86_32, the section start address is defined to be
"LOAD_OFFSET + LOAD_PHYSICAL_ADDR", which is the same as
__START_KERNEL_map.
Let's take it out to remove the complexity.
Signed-off-by: Wei Yang <[email protected]>
---
arch/x86/kernel/vmlinux.lds.S | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 88dcf9366949..a20409b0a3f2 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -110,11 +110,10 @@ PHDRS {
SECTIONS
{
+ . = __START_KERNEL;
#ifdef CONFIG_X86_32
- . = LOAD_OFFSET + LOAD_PHYSICAL_ADDR;
phys_startup_32 = ABSOLUTE(startup_32 - LOAD_OFFSET);
#else
- . = __START_KERNEL;
phys_startup_64 = ABSOLUTE(startup_64 - LOAD_OFFSET);
#endif
--
2.34.1
* Wei Yang <[email protected]> wrote:
> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
> which is only used to define __START_KERNEL.
>
> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
> <asm/page_types.h>.
>
> Signed-off-by: Wei Yang <[email protected]>
> ---
> arch/x86/include/asm/boot.h | 5 -----
> arch/x86/include/asm/page_types.h | 8 +++++---
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
> index a38cc0afc90a..12cbc57d0128 100644
> --- a/arch/x86/include/asm/boot.h
> +++ b/arch/x86/include/asm/boot.h
> @@ -6,11 +6,6 @@
> #include <asm/pgtable_types.h>
> #include <uapi/asm/boot.h>
>
> -/* Physical address where kernel should be loaded. */
> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
> -
> /* Minimum kernel alignment, as a power of two */
> #ifdef CONFIG_X86_64
> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
> index 86bd4311daf8..acc1620fd121 100644
> --- a/arch/x86/include/asm/page_types.h
> +++ b/arch/x86/include/asm/page_types.h
> @@ -31,10 +31,12 @@
>
> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
>
> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
> - CONFIG_PHYSICAL_ALIGN)
> +/* Physical address where kernel should be loaded. */
> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
I agree with this simplification, but the ALIGN() expression is far easier
to read, so please keep that one instead of the open-coded version.
Thanks,
Ingo
On 13.03.24 г. 9:58 ч., Wei Yang wrote:
> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
> which is only used to define __START_KERNEL.
>
> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
> <asm/page_types.h>.
>
> Signed-off-by: Wei Yang <[email protected]>
> ---
> arch/x86/include/asm/boot.h | 5 -----
> arch/x86/include/asm/page_types.h | 8 +++++---
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
> index a38cc0afc90a..12cbc57d0128 100644
> --- a/arch/x86/include/asm/boot.h
> +++ b/arch/x86/include/asm/boot.h
> @@ -6,11 +6,6 @@
> #include <asm/pgtable_types.h>
> #include <uapi/asm/boot.h>
>
> -/* Physical address where kernel should be loaded. */
> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
> -
> /* Minimum kernel alignment, as a power of two */
> #ifdef CONFIG_X86_64
> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
> index 86bd4311daf8..acc1620fd121 100644
> --- a/arch/x86/include/asm/page_types.h
> +++ b/arch/x86/include/asm/page_types.h
> @@ -31,10 +31,12 @@
>
> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
>
> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
> - CONFIG_PHYSICAL_ALIGN)
> +/* Physical address where kernel should be loaded. */
> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
Why don't you simply define LOAD_PHYSICAL_ADDR via
ALIGN(CONFIG_PHYSICAL_START, CONFIG_PHYSICAL_ALING) it's way more readable.
>
> -#define __START_KERNEL (__START_KERNEL_map + __PHYSICAL_START)
> +#define __START_KERNEL (__START_KERNEL_map + LOAD_PHYSICAL_ADDR)
>
> #ifdef CONFIG_X86_64
> #include <asm/page_64_types.h>
The following commit has been merged into the x86/build branch of tip:
Commit-ID: 9b67ce2c121fbf11c0c7b6216c08200fda23c9af
Gitweb: https://git.kernel.org/tip/9b67ce2c121fbf11c0c7b6216c08200fda23c9af
Author: Wei Yang <[email protected]>
AuthorDate: Wed, 13 Mar 2024 07:58:39
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 13 Mar 2024 11:29:11 +01:00
x86/vmlinux.lds.S: Take __START_KERNEL out conditional definition
If CONFIG_X86_32=y, the section start address is defined to be
"LOAD_OFFSET + LOAD_PHYSICAL_ADDR", which is the same as
__START_KERNEL_map.
Unify it with the 64-bit definition to simplify the code.
Signed-off-by: Wei Yang <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/vmlinux.lds.S | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 88dcf93..a20409b 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -110,11 +110,10 @@ PHDRS {
SECTIONS
{
+ . = __START_KERNEL;
#ifdef CONFIG_X86_32
- . = LOAD_OFFSET + LOAD_PHYSICAL_ADDR;
phys_startup_32 = ABSOLUTE(startup_32 - LOAD_OFFSET);
#else
- . = __START_KERNEL;
phys_startup_64 = ABSOLUTE(startup_64 - LOAD_OFFSET);
#endif
The following commit has been merged into the x86/build branch of tip:
Commit-ID: a5cffd056ef52280c07a7f6a3b3faacf6b318e8e
Gitweb: https://git.kernel.org/tip/a5cffd056ef52280c07a7f6a3b3faacf6b318e8e
Author: Wei Yang <[email protected]>
AuthorDate: Wed, 13 Mar 2024 07:58:38
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 13 Mar 2024 11:29:10 +01:00
x86/vmlinux.lds.S: Remove conditional definition of LOAD_OFFSET
In vmlinux.lds.S, we define LOAD_OFFSET conditionally to __PAGE_OFFSET
or __START_KERNEL_map. While __START_KERNEL_map is already defined to
the same value with the same condition.
So it is fine to define LOAD_OFFSET to __START_KERNEL_map directly.
Signed-off-by: Wei Yang <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/vmlinux.lds.S | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 56451fd..88dcf93 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -15,11 +15,7 @@
* put it inside the section definition.
*/
-#ifdef CONFIG_X86_32
-#define LOAD_OFFSET __PAGE_OFFSET
-#else
#define LOAD_OFFSET __START_KERNEL_map
-#endif
#define RUNTIME_DISCARD_EXIT
#define EMITS_PT_NOTE
The following commit has been merged into the x86/build branch of tip:
Commit-ID: 1793f69326266269a8117d3f5324ac78db18931c
Gitweb: https://git.kernel.org/tip/1793f69326266269a8117d3f5324ac78db18931c
Author: Wei Yang <[email protected]>
AuthorDate: Wed, 13 Mar 2024 07:58:36
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 13 Mar 2024 11:29:10 +01:00
vmlinux.lds.h: Fix a typo in comment
Signed-off-by: Wei Yang <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/asm-generic/vmlinux.lds.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 5dd3a61..514d300 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -3,7 +3,7 @@
* linker scripts.
*
* A minimal linker scripts has following content:
- * [This is a sample, architectures may have special requiriements]
+ * [This is a sample, architectures may have special requirements]
*
* OUTPUT_FORMAT(...)
* OUTPUT_ARCH(...)
On Wed, Mar 13, 2024 at 10:39:58AM -0000, tip-bot2 for Wei Yang wrote:
> -#ifdef CONFIG_X86_32
> -#define LOAD_OFFSET __PAGE_OFFSET
> -#else
> #define LOAD_OFFSET __START_KERNEL_map
> -#endif
And, as a next step, you can get rid of LOAD_OFFSET completely and use
__START_KERNEL_map everywhere.
Even less ifdeffery.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Wed, Mar 13, 2024 at 11:29:33AM +0100, Ingo Molnar wrote:
>
>* Wei Yang <[email protected]> wrote:
>
>> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
>> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
>> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
>> which is only used to define __START_KERNEL.
>>
>> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
>> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
>> <asm/page_types.h>.
>>
>> Signed-off-by: Wei Yang <[email protected]>
>> ---
>> arch/x86/include/asm/boot.h | 5 -----
>> arch/x86/include/asm/page_types.h | 8 +++++---
>> 2 files changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
>> index a38cc0afc90a..12cbc57d0128 100644
>> --- a/arch/x86/include/asm/boot.h
>> +++ b/arch/x86/include/asm/boot.h
>> @@ -6,11 +6,6 @@
>> #include <asm/pgtable_types.h>
>> #include <uapi/asm/boot.h>
>>
>> -/* Physical address where kernel should be loaded. */
>> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
>> -
>> /* Minimum kernel alignment, as a power of two */
>> #ifdef CONFIG_X86_64
>> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
>> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
>> index 86bd4311daf8..acc1620fd121 100644
>> --- a/arch/x86/include/asm/page_types.h
>> +++ b/arch/x86/include/asm/page_types.h
>> @@ -31,10 +31,12 @@
>>
>> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
>>
>> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
>> - CONFIG_PHYSICAL_ALIGN)
>> +/* Physical address where kernel should be loaded. */
>> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
>
>I agree with this simplification, but the ALIGN() expression is far easier
>to read, so please keep that one instead of the open-coded version.
Sure, will send v2.
>
>Thanks,
>
> Ingo
--
Wei Yang
Help you, Help me
On Wed, Mar 13, 2024 at 12:29:37PM +0200, Nikolay Borisov wrote:
>
>
>On 13.03.24 г. 9:58 ч., Wei Yang wrote:
>> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
>> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
>> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
>> which is only used to define __START_KERNEL.
>>
>> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
>> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
>> <asm/page_types.h>.
>>
>> Signed-off-by: Wei Yang <[email protected]>
>> ---
>> arch/x86/include/asm/boot.h | 5 -----
>> arch/x86/include/asm/page_types.h | 8 +++++---
>> 2 files changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
>> index a38cc0afc90a..12cbc57d0128 100644
>> --- a/arch/x86/include/asm/boot.h
>> +++ b/arch/x86/include/asm/boot.h
>> @@ -6,11 +6,6 @@
>> #include <asm/pgtable_types.h>
>> #include <uapi/asm/boot.h>
>> -/* Physical address where kernel should be loaded. */
>> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
>> -
>> /* Minimum kernel alignment, as a power of two */
>> #ifdef CONFIG_X86_64
>> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
>> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
>> index 86bd4311daf8..acc1620fd121 100644
>> --- a/arch/x86/include/asm/page_types.h
>> +++ b/arch/x86/include/asm/page_types.h
>> @@ -31,10 +31,12 @@
>> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
>> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
>> - CONFIG_PHYSICAL_ALIGN)
>> +/* Physical address where kernel should be loaded. */
>> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
>
>Why don't you simply define LOAD_PHYSICAL_ADDR via
>ALIGN(CONFIG_PHYSICAL_START, CONFIG_PHYSICAL_ALING) it's way more readable.
>
Sure, will do it.
>> -#define __START_KERNEL (__START_KERNEL_map + __PHYSICAL_START)
>> +#define __START_KERNEL (__START_KERNEL_map + LOAD_PHYSICAL_ADDR)
>> #ifdef CONFIG_X86_64
>> #include <asm/page_64_types.h>
--
Wei Yang
Help you, Help me
On Wed, Mar 13, 2024 at 02:18:33PM +0100, Borislav Petkov wrote:
>On Wed, Mar 13, 2024 at 10:39:58AM -0000, tip-bot2 for Wei Yang wrote:
>> -#ifdef CONFIG_X86_32
>> -#define LOAD_OFFSET __PAGE_OFFSET
>> -#else
>> #define LOAD_OFFSET __START_KERNEL_map
>> -#endif
>
>And, as a next step, you can get rid of LOAD_OFFSET completely and use
>__START_KERNEL_map everywhere.
>
>Even less ifdeffery.
You mean remove the definition of LOAD_OFFSET?
I have tried this, but I found this is used in vmlinux.lds.h. So I don't
figure out a way to get rid of it.
>
>--
>Regards/Gruss,
> Boris.
>
>https://people.kernel.org/tglx/notes-about-netiquette
--
Wei Yang
Help you, Help me
On Wed, Mar 13, 2024 at 11:29:33AM +0100, Ingo Molnar wrote:
>
>* Wei Yang <[email protected]> wrote:
>
>> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
>> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
>> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
>> which is only used to define __START_KERNEL.
>>
>> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
>> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
>> <asm/page_types.h>.
>>
>> Signed-off-by: Wei Yang <[email protected]>
>> ---
>> arch/x86/include/asm/boot.h | 5 -----
>> arch/x86/include/asm/page_types.h | 8 +++++---
>> 2 files changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
>> index a38cc0afc90a..12cbc57d0128 100644
>> --- a/arch/x86/include/asm/boot.h
>> +++ b/arch/x86/include/asm/boot.h
>> @@ -6,11 +6,6 @@
>> #include <asm/pgtable_types.h>
>> #include <uapi/asm/boot.h>
>>
>> -/* Physical address where kernel should be loaded. */
>> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
>> -
>> /* Minimum kernel alignment, as a power of two */
>> #ifdef CONFIG_X86_64
>> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
>> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
>> index 86bd4311daf8..acc1620fd121 100644
>> --- a/arch/x86/include/asm/page_types.h
>> +++ b/arch/x86/include/asm/page_types.h
>> @@ -31,10 +31,12 @@
>>
>> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
>>
>> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
>> - CONFIG_PHYSICAL_ALIGN)
>> +/* Physical address where kernel should be loaded. */
>> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
>
>I agree with this simplification, but the ALIGN() expression is far easier
>to read, so please keep that one instead of the open-coded version.
>
I just tried to define LOAD_PHYSICAL_ADDR by ALIGN, but face a compile error
on compressed/head_[32|64].o.
$ make arch/x86/boot/compressed/head_64.o
CALL scripts/checksyscalls.sh
DESCEND objtool
INSTALL libsubcmd_headers
AS arch/x86/boot/compressed/head_64.o
arch/x86/boot/compressed/head_64.S: Assembler messages:
arch/x86/boot/compressed/head_64.S:154: Error: junk (0x1000000,0x200000)' after expression
arch/x86/boot/compressed/head_64.S:154: Error: number of operands mismatch for 16' after expression
arch/x86/boot/compressed/head_64.S:157: Error: junk mov'
arch/x86/boot/compressed/head_64.S:330: Error: junk (0x1000000,0x200000)' after expression
arch/x86/boot/compressed/head_64.S:330: Error: number of operands mismatch for 16' after expression
arch/x86/boot/compressed/head_64.S:333: Error: junk movq'
If my understanding is correct, the reason is linkage.h defines ALIGN, which
is ".balign xxx". Maybe this is why original LOAD_PHYSICAL_ADDR doesn't use
ALIGN.
So is this ok to keep the open-coded definition?
>Thanks,
>
> Ingo
--
Wei Yang
Help you, Help me
* Wei Yang <[email protected]> wrote:
> On Wed, Mar 13, 2024 at 11:29:33AM +0100, Ingo Molnar wrote:
> >
> >* Wei Yang <[email protected]> wrote:
> >
> >> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
> >> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
> >> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
> >> which is only used to define __START_KERNEL.
> >>
> >> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
> >> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
> >> <asm/page_types.h>.
> >>
> >> Signed-off-by: Wei Yang <[email protected]>
> >> ---
> >> arch/x86/include/asm/boot.h | 5 -----
> >> arch/x86/include/asm/page_types.h | 8 +++++---
> >> 2 files changed, 5 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
> >> index a38cc0afc90a..12cbc57d0128 100644
> >> --- a/arch/x86/include/asm/boot.h
> >> +++ b/arch/x86/include/asm/boot.h
> >> @@ -6,11 +6,6 @@
> >> #include <asm/pgtable_types.h>
> >> #include <uapi/asm/boot.h>
> >>
> >> -/* Physical address where kernel should be loaded. */
> >> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
> >> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
> >> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
> >> -
> >> /* Minimum kernel alignment, as a power of two */
> >> #ifdef CONFIG_X86_64
> >> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
> >> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
> >> index 86bd4311daf8..acc1620fd121 100644
> >> --- a/arch/x86/include/asm/page_types.h
> >> +++ b/arch/x86/include/asm/page_types.h
> >> @@ -31,10 +31,12 @@
> >>
> >> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
> >>
> >> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
> >> - CONFIG_PHYSICAL_ALIGN)
> >> +/* Physical address where kernel should be loaded. */
> >> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
> >> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
> >> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
> >
> >I agree with this simplification, but the ALIGN() expression is far easier
> >to read, so please keep that one instead of the open-coded version.
> >
>
> I just tried to define LOAD_PHYSICAL_ADDR by ALIGN, but face a compile error
> on compressed/head_[32|64].o.
>
> $ make arch/x86/boot/compressed/head_64.o
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> INSTALL libsubcmd_headers
> AS arch/x86/boot/compressed/head_64.o
> arch/x86/boot/compressed/head_64.S: Assembler messages:
> arch/x86/boot/compressed/head_64.S:154: Error: junk (0x1000000,0x200000)' after expression
> arch/x86/boot/compressed/head_64.S:154: Error: number of operands mismatch for 16' after expression
> arch/x86/boot/compressed/head_64.S:157: Error: junk mov'
> arch/x86/boot/compressed/head_64.S:330: Error: junk (0x1000000,0x200000)' after expression
> arch/x86/boot/compressed/head_64.S:330: Error: number of operands mismatch for 16' after expression
> arch/x86/boot/compressed/head_64.S:333: Error: junk movq'
>
> If my understanding is correct, the reason is linkage.h defines ALIGN, which
> is ".balign xxx". Maybe this is why original LOAD_PHYSICAL_ADDR doesn't use
> ALIGN.
linkage.h defines __ALIGN, which is different from ALIGN().
Also, a number of .S files seem to be using some sort of ALIGN() method
within arch/x86/, according to:
git grep 'ALIGN(' -- 'arch/x86/*.S'
> So is this ok to keep the open-coded definition?
It would be nice to figure out why ALIGN() appears to be working in other
cases in .S files, while not in this case.
Thanks,
Ingo
On Thu, Mar 14, 2024 at 10:25:53AM +0100, Ingo Molnar wrote:
>
>* Wei Yang <[email protected]> wrote:
>
>> On Wed, Mar 13, 2024 at 11:29:33AM +0100, Ingo Molnar wrote:
>> >
>> >* Wei Yang <[email protected]> wrote:
>> >
>> >> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
>> >> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
>> >> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
>> >> which is only used to define __START_KERNEL.
>> >>
>> >> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
>> >> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
>> >> <asm/page_types.h>.
>> >>
>> >> Signed-off-by: Wei Yang <[email protected]>
>> >> ---
>> >> arch/x86/include/asm/boot.h | 5 -----
>> >> arch/x86/include/asm/page_types.h | 8 +++++---
>> >> 2 files changed, 5 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
>> >> index a38cc0afc90a..12cbc57d0128 100644
>> >> --- a/arch/x86/include/asm/boot.h
>> >> +++ b/arch/x86/include/asm/boot.h
>> >> @@ -6,11 +6,6 @@
>> >> #include <asm/pgtable_types.h>
>> >> #include <uapi/asm/boot.h>
>> >>
>> >> -/* Physical address where kernel should be loaded. */
>> >> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> >> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> >> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
>> >> -
>> >> /* Minimum kernel alignment, as a power of two */
>> >> #ifdef CONFIG_X86_64
>> >> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
>> >> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
>> >> index 86bd4311daf8..acc1620fd121 100644
>> >> --- a/arch/x86/include/asm/page_types.h
>> >> +++ b/arch/x86/include/asm/page_types.h
>> >> @@ -31,10 +31,12 @@
>> >>
>> >> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
>> >>
>> >> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
>> >> - CONFIG_PHYSICAL_ALIGN)
>> >> +/* Physical address where kernel should be loaded. */
>> >> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>> >> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
>> >> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
>> >
>> >I agree with this simplification, but the ALIGN() expression is far easier
>> >to read, so please keep that one instead of the open-coded version.
>> >
>>
>> I just tried to define LOAD_PHYSICAL_ADDR by ALIGN, but face a compile error
>> on compressed/head_[32|64].o.
>>
>> $ make arch/x86/boot/compressed/head_64.o
>> CALL scripts/checksyscalls.sh
>> DESCEND objtool
>> INSTALL libsubcmd_headers
>> AS arch/x86/boot/compressed/head_64.o
>> arch/x86/boot/compressed/head_64.S: Assembler messages:
>> arch/x86/boot/compressed/head_64.S:154: Error: junk (0x1000000,0x200000)' after expression
>> arch/x86/boot/compressed/head_64.S:154: Error: number of operands mismatch for 16' after expression
>> arch/x86/boot/compressed/head_64.S:157: Error: junk mov'
>> arch/x86/boot/compressed/head_64.S:330: Error: junk (0x1000000,0x200000)' after expression
>> arch/x86/boot/compressed/head_64.S:330: Error: number of operands mismatch for 16' after expression
>> arch/x86/boot/compressed/head_64.S:333: Error: junk movq'
>>
>> If my understanding is correct, the reason is linkage.h defines ALIGN, which
>> is ".balign xxx". Maybe this is why original LOAD_PHYSICAL_ADDR doesn't use
>> ALIGN.
>
>linkage.h defines __ALIGN, which is different from ALIGN().
>
linkage.h defines ALIGN to __ALIGN which is different from what we expect.
>Also, a number of .S files seem to be using some sort of ALIGN() method
>within arch/x86/, according to:
>
> git grep 'ALIGN(' -- 'arch/x86/*.S'
I tried below command by append '\<' to ALIGN
git grep '\<ALIGN(' -- 'arch/x86/*.S'
>
>> So is this ok to keep the open-coded definition?
>
>It would be nice to figure out why ALIGN() appears to be working in other
>cases in .S files, while not in this case.
>
Here is grep result
arch/x86/boot/compressed/vmlinux.lds.S: .data : ALIGN(0x1000) {
arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(. + 4, 0x200);
arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(L1_CACHE_BYTES);
arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(8); /* For convenience during zeroing */
arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); /* keep ZO size page aligned */
arch/x86/kernel/vmlinux.lds.S:#define X86_ALIGN_RODATA_BEGIN . = ALIGN(HPAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(HPAGE_SIZE); \
arch/x86/kernel/vmlinux.lds.S:#define ALIGN_ENTRY_TEXT_BEGIN . = ALIGN(PMD_SIZE);
arch/x86/kernel/vmlinux.lds.S:#define ALIGN_ENTRY_TEXT_END . = ALIGN(PMD_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PMD_SIZE); \
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); \
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PMD_SIZE); \
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); \
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); /* keep VO_INIT_SIZE page aligned */
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(HPAGE_SIZE);
arch/x86/kernel/vmlinux.lds.S: . = ALIGN(HPAGE_SIZE);
arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(4);
arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(16);
arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(PAGE_SIZE);
arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(128);
arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(4);
arch/x86/um/vdso/vdso-layout.lds.S: . = ALIGN(0x100);
It looks all happens in link script, not assembly source code.
And other grep result with "ALIGN" are:
SYM_CODE_START_NOALIGN
SYM_CODE_START_LOCAL_NOALIGN
SYM_FUNC_START_NOALIGN
SYM_FUNC_START_LOCAL_NOALIGN
SYM_INNER_LABEL_ALIGN
which are defined in linkage.h.
>Thanks,
>
> Ingo
--
Wei Yang
Help you, Help me
On Fri, Mar 15, 2024 at 12:57:37AM +0000, Wei Yang wrote:
>On Thu, Mar 14, 2024 at 10:25:53AM +0100, Ingo Molnar wrote:
>>
>>* Wei Yang <[email protected]> wrote:
>>
>>> On Wed, Mar 13, 2024 at 11:29:33AM +0100, Ingo Molnar wrote:
>>> >
>>> >* Wei Yang <[email protected]> wrote:
>>> >
>>> >> Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
>>> >> CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
>>> >> LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
>>> >> which is only used to define __START_KERNEL.
>>> >>
>>> >> Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
>>> >> <asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
>>> >> <asm/page_types.h>.
>>> >>
>>> >> Signed-off-by: Wei Yang <[email protected]>
>>> >> ---
>>> >> arch/x86/include/asm/boot.h | 5 -----
>>> >> arch/x86/include/asm/page_types.h | 8 +++++---
>>> >> 2 files changed, 5 insertions(+), 8 deletions(-)
>>> >>
>>> >> diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
>>> >> index a38cc0afc90a..12cbc57d0128 100644
>>> >> --- a/arch/x86/include/asm/boot.h
>>> >> +++ b/arch/x86/include/asm/boot.h
>>> >> @@ -6,11 +6,6 @@
>>> >> #include <asm/pgtable_types.h>
>>> >> #include <uapi/asm/boot.h>
>>> >>
>>> >> -/* Physical address where kernel should be loaded. */
>>> >> -#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>>> >> - + (CONFIG_PHYSICAL_ALIGN - 1)) \
>>> >> - & ~(CONFIG_PHYSICAL_ALIGN - 1))
>>> >> -
>>> >> /* Minimum kernel alignment, as a power of two */
>>> >> #ifdef CONFIG_X86_64
>>> >> # define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
>>> >> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
>>> >> index 86bd4311daf8..acc1620fd121 100644
>>> >> --- a/arch/x86/include/asm/page_types.h
>>> >> +++ b/arch/x86/include/asm/page_types.h
>>> >> @@ -31,10 +31,12 @@
>>> >>
>>> >> #define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
>>> >>
>>> >> -#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
>>> >> - CONFIG_PHYSICAL_ALIGN)
>>> >> +/* Physical address where kernel should be loaded. */
>>> >> +#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
>>> >> + + (CONFIG_PHYSICAL_ALIGN - 1)) \
>>> >> + & ~(CONFIG_PHYSICAL_ALIGN - 1))
>>> >
>>> >I agree with this simplification, but the ALIGN() expression is far easier
>>> >to read, so please keep that one instead of the open-coded version.
>>> >
>>>
>>> I just tried to define LOAD_PHYSICAL_ADDR by ALIGN, but face a compile error
>>> on compressed/head_[32|64].o.
>>>
>>> $ make arch/x86/boot/compressed/head_64.o
>>> CALL scripts/checksyscalls.sh
>>> DESCEND objtool
>>> INSTALL libsubcmd_headers
>>> AS arch/x86/boot/compressed/head_64.o
>>> arch/x86/boot/compressed/head_64.S: Assembler messages:
>>> arch/x86/boot/compressed/head_64.S:154: Error: junk (0x1000000,0x200000)' after expression
>>> arch/x86/boot/compressed/head_64.S:154: Error: number of operands mismatch for 16' after expression
>>> arch/x86/boot/compressed/head_64.S:157: Error: junk mov'
>>> arch/x86/boot/compressed/head_64.S:330: Error: junk (0x1000000,0x200000)' after expression
>>> arch/x86/boot/compressed/head_64.S:330: Error: number of operands mismatch for 16' after expression
>>> arch/x86/boot/compressed/head_64.S:333: Error: junk movq'
>>>
>>> If my understanding is correct, the reason is linkage.h defines ALIGN, which
>>> is ".balign xxx". Maybe this is why original LOAD_PHYSICAL_ADDR doesn't use
>>> ALIGN.
>>
>>linkage.h defines __ALIGN, which is different from ALIGN().
>>
>
>linkage.h defines ALIGN to __ALIGN which is different from what we expect.
>
>>Also, a number of .S files seem to be using some sort of ALIGN() method
>>within arch/x86/, according to:
>>
>> git grep 'ALIGN(' -- 'arch/x86/*.S'
>
>I tried below command by append '\<' to ALIGN
>
> git grep '\<ALIGN(' -- 'arch/x86/*.S'
>
>>
>>> So is this ok to keep the open-coded definition?
>>
>>It would be nice to figure out why ALIGN() appears to be working in other
>>cases in .S files, while not in this case.
>>
>
>Here is grep result
>
>arch/x86/boot/compressed/vmlinux.lds.S: .data : ALIGN(0x1000) {
>arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(. + 4, 0x200);
>arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(L1_CACHE_BYTES);
>arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(8); /* For convenience during zeroing */
>arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/boot/compressed/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); /* keep ZO size page aligned */
>arch/x86/kernel/vmlinux.lds.S:#define X86_ALIGN_RODATA_BEGIN . = ALIGN(HPAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(HPAGE_SIZE); \
>arch/x86/kernel/vmlinux.lds.S:#define ALIGN_ENTRY_TEXT_BEGIN . = ALIGN(PMD_SIZE);
>arch/x86/kernel/vmlinux.lds.S:#define ALIGN_ENTRY_TEXT_END . = ALIGN(PMD_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PMD_SIZE); \
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); \
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PMD_SIZE); \
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); \
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(8);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(PAGE_SIZE); /* keep VO_INIT_SIZE page aligned */
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(HPAGE_SIZE);
>arch/x86/kernel/vmlinux.lds.S: . = ALIGN(HPAGE_SIZE);
>arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(4);
>arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(16);
>arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(PAGE_SIZE);
>arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(128);
>arch/x86/realmode/rm/realmode.lds.S: . = ALIGN(4);
>arch/x86/um/vdso/vdso-layout.lds.S: . = ALIGN(0x100);
>
>It looks all happens in link script, not assembly source code.
>
>And other grep result with "ALIGN" are:
>
> SYM_CODE_START_NOALIGN
> SYM_CODE_START_LOCAL_NOALIGN
> SYM_FUNC_START_NOALIGN
> SYM_FUNC_START_LOCAL_NOALIGN
> SYM_INNER_LABEL_ALIGN
>
>which are defined in linkage.h.
>
Hi, Ingo
Take another look into the usage.
In linkage.h, we have following definition:
#ifdef __ASSEMBLY__
#ifndef LINKER_SCRIPT
#define ALIGN __ALIGN
#endif
#endif
We would include linkage.h from .S and .lds.S. We both define __ASSEMBLY__ in
command line when building these target, but we would define LINKER_SCRIPT
when building .lds from .lds.S. This introduces the different behavior of
ALIGN.
* For .S, ALING is replaced by __ALIGN and then to .balign xxx
* For .lds, ALIGN is is not expanded, which is a lds keyword
Also linkage.h may be included in .c, e.g. head64.c. But we don't define
__ASSEMBLY__ in command line, which leads the ALIGN undefined until kernel.h
is included.
>>Thanks,
>>
>> Ingo
>
>--
>Wei Yang
>Help you, Help me
--
Wei Yang
Help you, Help me
The following commit has been merged into the x86/build branch of tip:
Commit-ID: 29b0aab841da3cade64c7e41e99e9f4645e65fb1
Gitweb: https://git.kernel.org/tip/29b0aab841da3cade64c7e41e99e9f4645e65fb1
Author: Wei Yang <[email protected]>
AuthorDate: Wed, 13 Mar 2024 07:58:37
Committer: Ingo Molnar <[email protected]>
CommitterDate: Fri, 22 Mar 2024 11:36:45 +01:00
x86/boot: Replace __PHYSICAL_START with LOAD_PHYSICAL_ADDR
Both __PHYSICAL_START and LOAD_PHYSICAL_ADDR are defined to get aligned
CONFIG_PHYSICAL_START, so we can replace __PHYSICAL_START with
LOAD_PHYSICAL_ADDR. And then remove the definition of __PHYSICAL_START,
which is only used to define __START_KERNEL.
Since <asm/boot.h> includes <asm/pgtable_types.h>, which includes
<asm/page_types.h>, it is fine to move definition from <asm/boot.h> to
<asm/page_types.h>.
Signed-off-by: Wei Yang <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/boot.h | 5 -----
arch/x86/include/asm/page_types.h | 8 +++++---
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
index a38cc0a..12cbc57 100644
--- a/arch/x86/include/asm/boot.h
+++ b/arch/x86/include/asm/boot.h
@@ -6,11 +6,6 @@
#include <asm/pgtable_types.h>
#include <uapi/asm/boot.h>
-/* Physical address where kernel should be loaded. */
-#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
- + (CONFIG_PHYSICAL_ALIGN - 1)) \
- & ~(CONFIG_PHYSICAL_ALIGN - 1))
-
/* Minimum kernel alignment, as a power of two */
#ifdef CONFIG_X86_64
# define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT
diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
index 86bd431..acc1620 100644
--- a/arch/x86/include/asm/page_types.h
+++ b/arch/x86/include/asm/page_types.h
@@ -31,10 +31,12 @@
#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
-#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
- CONFIG_PHYSICAL_ALIGN)
+/* Physical address where kernel should be loaded. */
+#define LOAD_PHYSICAL_ADDR ((CONFIG_PHYSICAL_START \
+ + (CONFIG_PHYSICAL_ALIGN - 1)) \
+ & ~(CONFIG_PHYSICAL_ALIGN - 1))
-#define __START_KERNEL (__START_KERNEL_map + __PHYSICAL_START)
+#define __START_KERNEL (__START_KERNEL_map + LOAD_PHYSICAL_ADDR)
#ifdef CONFIG_X86_64
#include <asm/page_64_types.h>