On 11/18/06, Vivek Goyal <[email protected]> wrote:
On Sat, Nov 18, 2006 at 10:14:31AM +0900, Magnus Damm wrote:
> On 11/18/06, Vivek Goyal <[email protected]> wrote:
> >I am about to add relocatable kernel support which has essentially
> >no cost so there is no point in retaining CONFIG_PHYSICAL_START
> >and retaining CONFIG_PHYSICAL_START makes implementation of and
> >testing of a relocatable kernel more difficult.
> >
> >Signed-off-by: Eric W. Biederman <[email protected]>
> >Signed-off-by: Vivek Goyal <[email protected]>
> >---
[snip]
>linux-2.6.19-rc6-reloc/arch/x86_64/mm/fault.c~x86_64-Remove-CONFIG_PHYSICAL_START
2006-11-17 00:12:50.000000000 -0500
> >+++ linux-2.6.19-rc6-reloc-root/arch/x86_64/mm/fault.c 2006-11-17
> >00:12:50.000000000 -0500
> >@@ -644,9 +644,9 @@ void vmalloc_sync_all(void)
> > start = address + PGDIR_SIZE;
> > }
> > /* Check that there is no need to do the same for the modules
> > area. */
> >- BUILD_BUG_ON(!(MODULES_VADDR > __START_KERNEL));
> >+ BUILD_BUG_ON(!(MODULES_VADDR > __START_KERNEL_map));
> > BUILD_BUG_ON(!(((MODULES_END - 1) & PGDIR_MASK) ==
> >- (__START_KERNEL & PGDIR_MASK)));
> >+ (__START_KERNEL_map & PGDIR_MASK)));
> > }
>
> This code looks either like a bugfix or a bug. If it's a fix then
> maybe it should be broken out and submitted separately for the
> rc-kernels?
>
Magnus, Eric got rid of __START_KERNEL because he was compiling kernel
for physical addr zero which made __START_KERNEL and __START_KERNEL_map
same, hence he got rid of __START_KERNEL. That's why above change.
But compiling for physical address zero has got drawback that one can
not directly load a vmlinux as it shall have to be loaded at physical
addr zero. Hence I changed the behavior back to compile the kernel for
physical addr 2MB. So now __START_KERNEL = __START_KERNEL_map + 2MB.
Now it makes sense to retain __START_KERNEL. I have done the changes.
I misunderstood and thought __START_KERNEL was a physical address and
__START_KERNEL_map was a virtual one. But now I understand. Thank you.
> >diff -puN include/asm-x86_64/page.h~x86_64-Remove-CONFIG_PHYSICAL_START
> >include/asm-x86_64/page.h
> >---
> >linux-2.6.19-rc6-reloc/include/asm-x86_64/page.h~x86_64-Remove-CONFIG_PHYSICAL_START 2006-11-17 00:12:50.000000000 -0500
> >+++ linux-2.6.19-rc6-reloc-root/include/asm-x86_64/page.h 2006-11-17
> >00:12:50.000000000 -0500
> >@@ -75,8 +75,6 @@ typedef struct { unsigned long pgprot; }
> >
> > #endif /* !__ASSEMBLY__ */
> >
> >-#define __PHYSICAL_START _AC(CONFIG_PHYSICAL_START,UL)
> >-#define __START_KERNEL (__START_KERNEL_map + __PHYSICAL_START)
> > #define __START_KERNEL_map _AC(0xffffffff80000000,UL)
> > #define __PAGE_OFFSET _AC(0xffff810000000000,UL)
>
> I understand that you want to remove the Kconfig option
> CONFIG_PHYSICAL_START and that is fine with me. I don't however like
> the idea of replacing __PHYSICAL_START and __START_KERNEL with
> hardcoded values. Is there any special reason behind this?
>
All the hardcodings for 2MB have disappeared in final version. See next
patch in the series which actually implements relocatable kernel. Actually
the whole logic itself has changed hence we did not require these
hardcodings. This patch retains these hardcodings so that even if somebody
removes the top patch, kernel can be compiled and booted.
So bottom line, all the hardcodings are not present once all the patches
have been applied.
My gut feeling said a big no when I saw that you replaced constants
with hardcoded values. But if the hardcoded values disappear when all
patches are applied then I'm happy!
> The code in page.h already has constants for __START_KERNEL_map and
> __PAGE_OFFSET (thank god) and none of them are adjustable via Kconfig.
> Why not change as little as possible and keep __PHYSICAL_START and
> __START_KERNEL in page.h and the places that use them but remove
> references to CONFIG_PHYSICAL_START in Kconfig, defconfig, and page.h?
Good suggestion. Now I have retained __START_KERNEL. But did not feel
the need to retain __PHYSICAL_START. It will be used only at one place
in page.h
Just to nitpick, isn't the 2M value used both in page.h and head.S? I
don't fully understand the reason why you are hardcoding, but it is
not that important. I think this version of the patch is much better.
Thank you!
/ magnus
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
