[PATCH -mm -v4 1/3] i386/x86_64 boot: setup data - Kernel

This is a discussion on [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data - Kernel ; This patch add a field of 64-bit physical pointer to NULL terminated single linked list of struct setup_data to real-mode kernel header. This is used as a more extensible boot parameters passing mechanism. Signed-off-by: Huang Ying --- arch/i386/Kconfig | 3 ...

+ Reply to Thread
Results 1 to 10 of 10

Thread: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

  1. [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    This patch add a field of 64-bit physical pointer to NULL terminated
    single linked list of struct setup_data to real-mode kernel
    header. This is used as a more extensible boot parameters passing
    mechanism.

    Signed-off-by: Huang Ying

    ---

    arch/i386/Kconfig | 3 -
    arch/i386/boot/header.S | 8 +++
    arch/i386/kernel/setup.c | 92 +++++++++++++++++++++++++++++++++++++++++++
    arch/x86_64/kernel/setup.c | 37 +++++++++++++++++
    include/asm-i386/bootparam.h | 15 +++++++
    include/asm-i386/io.h | 7 +++
    include/linux/mm.h | 2
    mm/memory.c | 24 +++++++++++
    8 files changed, 184 insertions(+), 4 deletions(-)

    Index: linux-2.6.23-rc8/include/asm-i386/bootparam.h
    ================================================== =================
    --- linux-2.6.23-rc8.orig/include/asm-i386/bootparam.h 2007-10-09 11:26:06.000000000 +0800
    +++ linux-2.6.23-rc8/include/asm-i386/bootparam.h 2007-10-09 14:15:14.000000000 +0800
    @@ -9,6 +9,17 @@
    #include
    #include

  2. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    On Tuesday 09 October 2007 16:40, Huang, Ying wrote:

    > +unsigned long copy_from_phys(void *to, unsigned long from_phys,
    > + unsigned long n)
    > +{
    > + struct page *page;
    > + void *from;
    > + unsigned long remain = n, offset, trunck;
    > +
    > + while (remain) {
    > + page = pfn_to_page(from_phys >> PAGE_SHIFT);
    > + from = kmap_atomic(page, KM_USER0);
    > + offset = from_phys & ~PAGE_MASK;
    > + if (remain > PAGE_SIZE - offset)
    > + trunck = PAGE_SIZE - offset;
    > + else
    > + trunck = remain;
    > + memcpy(to, from + offset, trunck);
    > + kunmap_atomic(from, KM_USER0);
    > + to += trunck;
    > + from_phys += trunck;
    > + remain -= trunck;
    > + }
    > + return n;
    > +}



    I suppose that's not unreasonable to put in mm/memory.c, although
    it's not really considered a problem to do this kind of stuff in
    a low level arch file...

    You have no kernel virtual mapping for the source data?

    Should it be __init?

    Care to add a line of documentation if you keep it in mm/memory.c?

    Thanks,
    Nick
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    On Tue, 2007-10-09 at 01:25 +1000, Nick Piggin wrote:
    > On Tuesday 09 October 2007 16:40, Huang, Ying wrote:
    >
    > > +unsigned long copy_from_phys(void *to, unsigned long from_phys,
    > > + unsigned long n)
    > > +{
    > > + struct page *page;
    > > + void *from;
    > > + unsigned long remain = n, offset, trunck;
    > > +
    > > + while (remain) {
    > > + page = pfn_to_page(from_phys >> PAGE_SHIFT);
    > > + from = kmap_atomic(page, KM_USER0);
    > > + offset = from_phys & ~PAGE_MASK;
    > > + if (remain > PAGE_SIZE - offset)
    > > + trunck = PAGE_SIZE - offset;
    > > + else
    > > + trunck = remain;
    > > + memcpy(to, from + offset, trunck);
    > > + kunmap_atomic(from, KM_USER0);
    > > + to += trunck;
    > > + from_phys += trunck;
    > > + remain -= trunck;
    > > + }
    > > + return n;
    > > +}

    >
    >
    > I suppose that's not unreasonable to put in mm/memory.c, although
    > it's not really considered a problem to do this kind of stuff in
    > a low level arch file...
    >
    > You have no kernel virtual mapping for the source data?
    >


    On 32-bit platform such as i386. Some memory zones have no kernel
    virtual mapping (highmem region etc). So I think this may be useful as a
    universal way to access physical memory. But it can be more efficient to
    implement it in arch file for some arch. Should this implementation be
    used as a fall back implementation with attribute "weak"?

    > Should it be __init?
    >
    > Care to add a line of documentation if you keep it in mm/memory.c?
    >


    OK, I will add the document in the next version.

    Best Regards,
    Huang Ying
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    On Tuesday 09 October 2007 18:22, Huang, Ying wrote:
    > On Tue, 2007-10-09 at 01:25 +1000, Nick Piggin wrote:
    > > On Tuesday 09 October 2007 16:40, Huang, Ying wrote:
    > > > +unsigned long copy_from_phys(void *to, unsigned long from_phys,
    > > > + unsigned long n)


    > > I suppose that's not unreasonable to put in mm/memory.c, although
    > > it's not really considered a problem to do this kind of stuff in
    > > a low level arch file...
    > >
    > > You have no kernel virtual mapping for the source data?

    >
    > On 32-bit platform such as i386. Some memory zones have no kernel
    > virtual mapping (highmem region etc).


    I'm just wondering whether you really need to access highmem in
    boot code...


    > So I think this may be useful as a
    > universal way to access physical memory. But it can be more efficient to
    > implement it in arch file for some arch. Should this implementation be
    > used as a fall back implementation with attribute "weak"?


    Definitely on most architectures it would just amount to
    memcpy(dst, __va(phys), n);, right? However I don't know if
    it's worth the trouble of overriding it unless there is some
    non-__init user of it.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    On Tue, 2007-10-09 at 02:06 +1000, Nick Piggin wrote:
    > On Tuesday 09 October 2007 18:22, Huang, Ying wrote:
    > > On Tue, 2007-10-09 at 01:25 +1000, Nick Piggin wrote:
    > > > On Tuesday 09 October 2007 16:40, Huang, Ying wrote:
    > > > > +unsigned long copy_from_phys(void *to, unsigned long from_phys,
    > > > > + unsigned long n)

    >
    > > > I suppose that's not unreasonable to put in mm/memory.c, although
    > > > it's not really considered a problem to do this kind of stuff in
    > > > a low level arch file...
    > > >
    > > > You have no kernel virtual mapping for the source data?

    > >
    > > On 32-bit platform such as i386. Some memory zones have no kernel
    > > virtual mapping (highmem region etc).

    >
    > I'm just wondering whether you really need to access highmem in
    > boot code...


    Because the zero page (boot_parameters) of i386 boot protocol has 4k
    limitation, a linked list style boot parameter passing mechanism (struct
    setup_data) is proposed by Peter Anvin. The linked list is provided by
    bootloader, so it is possible to be in highmem region.

    >
    > > So I think this may be useful as a
    > > universal way to access physical memory. But it can be more efficient to
    > > implement it in arch file for some arch. Should this implementation be
    > > used as a fall back implementation with attribute "weak"?

    >
    > Definitely on most architectures it would just amount to
    > memcpy(dst, __va(phys), n);, right? However I don't know if


    Yes.

    > it's worth the trouble of overriding it unless there is some
    > non-__init user of it.


    To support debugging and kexec, the boot parameters include the linked
    list above are exported into sysfs. This function is used there too. The
    patch is the No. 2 of the series.

    Best Regards,
    Huang Ying
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    On Tuesday 09 October 2007 18:55, Huang, Ying wrote:
    > On Tue, 2007-10-09 at 02:06 +1000, Nick Piggin wrote:


    > > I'm just wondering whether you really need to access highmem in
    > > boot code...

    >
    > Because the zero page (boot_parameters) of i386 boot protocol has 4k
    > limitation, a linked list style boot parameter passing mechanism (struct
    > setup_data) is proposed by Peter Anvin. The linked list is provided by
    > bootloader, so it is possible to be in highmem region.


    OK. I don't really know the code, but I trust you


    > > Definitely on most architectures it would just amount to
    > > memcpy(dst, __va(phys), n);, right? However I don't know if

    >
    > Yes.
    >
    > > it's worth the trouble of overriding it unless there is some
    > > non-__init user of it.

    >
    > To support debugging and kexec, the boot parameters include the linked
    > list above are exported into sysfs. This function is used there too. The
    > patch is the No. 2 of the series.


    Ah, I see. I missed that.

    OK, well rather than make it weak, and have everyone except
    i386 override it, can you just ifdef CONFIG_HIGHMEM?

    After that, I guess most other architectures wouldn't even
    use the function. Maybe it can go into lib/ instead so that
    it can be discarded by the linker if it isn't used?
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    On Tue, 2007-10-09 at 02:28 +1000, Nick Piggin wrote:
    > On Tuesday 09 October 2007 18:55, Huang, Ying wrote:
    > > On Tue, 2007-10-09 at 02:06 +1000, Nick Piggin wrote:

    >
    > > > I'm just wondering whether you really need to access highmem in
    > > > boot code...

    > >
    > > Because the zero page (boot_parameters) of i386 boot protocol has 4k
    > > limitation, a linked list style boot parameter passing mechanism (struct
    > > setup_data) is proposed by Peter Anvin. The linked list is provided by
    > > bootloader, so it is possible to be in highmem region.

    >
    > OK. I don't really know the code, but I trust you


    Thank you

    >
    > > > Definitely on most architectures it would just amount to
    > > > memcpy(dst, __va(phys), n);, right? However I don't know if

    > >
    > > Yes.
    > >
    > > > it's worth the trouble of overriding it unless there is some
    > > > non-__init user of it.

    > >
    > > To support debugging and kexec, the boot parameters include the linked
    > > list above are exported into sysfs. This function is used there too. The
    > > patch is the No. 2 of the series.

    >
    > Ah, I see. I missed that.
    >
    > OK, well rather than make it weak, and have everyone except
    > i386 override it, can you just ifdef CONFIG_HIGHMEM?


    Yes. This is better. I will do it. Maybe it can be defined as a macro
    for these architectures, as follow:

    /* in linux/mm.h */
    #ifdef CONFIG_HIGHMEM
    void *copy_from_phys(void *to, unsigned long from_phys, size_t n);
    #else
    #define copy_from_phys(dst, phys, n) memcpy(dst, __va(phys), n)
    #endif

    > After that, I guess most other architectures wouldn't even
    > use the function. Maybe it can go into lib/ instead so that
    > it can be discarded by the linker if it isn't used?


    Yes. I will do it.

    Best Regards,
    Huang Ying
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data


    > Care to add a line of documentation if you keep it in mm/memory.c?


    It would be better to just use early_ioremap() (or ioremap())

    That is how ACPI who has similar issues accessing its tables solves this.

    -Andi
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    * Tue, 09 Oct 2007 16:55:23 +0800
    >
    > On Tue, 2007-10-09 at 02:06 +1000, Nick Piggin wrote:
    >> On Tuesday 09 October 2007 18:22, Huang, Ying wrote:

    []
    >> I'm just wondering whether you really need to access highmem in
    >> boot code...

    >
    > Because the zero page (boot_parameters) of i386 boot protocol has 4k
    > limitation, a linked list style boot parameter passing mechanism (struct
    > setup_data) is proposed by Peter Anvin. The linked list is provided by
    > bootloader, so it is possible to be in highmem region.


    Can it be explained, why boot protocol and boot line must be expanded?
    This amount of code for what?

    arch/i386/Kconfig | 3 -
    arch/i386/boot/header.S | 8 +++
    arch/i386/kernel/setup.c | 92 +++++++++++++++++++++++++++++++++++++++++++
    arch/x86_64/kernel/setup.c | 37 +++++++++++++++++
    include/asm-i386/bootparam.h | 15 +++++++
    include/asm-i386/io.h | 7 +++
    include/linux/mm.h | 2
    mm/memory.c | 24 +++++++++++


    If it is proposed for passing ACPI makeup language bugfixes by boot
    line for ACPI parser in the kernel, or "telling to kernel what to do
    via EFI" then it's kind of very nasty red flag.

    I'd suggest to have initramfs image ready with all possible
    data/options/actions based on very small amount of possible boot line
    information.

    Any _right_ use-cases explained for dummies are appreciated.

    Thanks.
    --
    -o--=O`C
    #oo'L O
    <___=E M
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

    On Tuesday 09 October 2007 16:00:57 huang ying wrote:
    > On 10/9/07, Andi Kleen wrote:
    > >
    > > > Care to add a line of documentation if you keep it in mm/memory.c?

    > >
    > > It would be better to just use early_ioremap() (or ioremap())
    > >
    > > That is how ACPI who has similar issues accessing its tables solves this.

    >
    > Yes. That is another solution. But there is some problem about
    > early_ioremap (boot_ioremap, bt_ioremap for i386) or ioremap.
    >
    > - ioremap can not be used before mem_init.
    > - For i386, boot_ioremap can map at most 4 pages, bt_ioremap can map
    > at most 16 pages. This will be an unnecessary constrains for size of
    > setup_data.


    That could be easily extended if needed. But I don't see why we would
    need that much setup data anyways. Limiting it to let's say 16KB
    seems entirely reasonable. And if some kernel ever needs more it can
    be still extended.

    The biggest item is probably the command line and i don't see why
    that should be more than a one or two KB.

    > - For i386, the virtual memory space of ioremap is limited too.


    That will be all freed and again the data shouldn't be that big.

    -Andi


    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread