Prev: vfs: Add name to file handle conversion support
Next: Research on Open Source Software Development
From: H. Peter Anvin on 12 Jul 2010 14:20
On 07/12/2010 10:44 AM, Cyrill Gorcunov wrote:
>
> Peter, while reviewing this patch I found another nit in
> context of early_param usage, so the patch is below. It's
> completely trivial. Actually I thought I've already fixed
> all early_param cases long ago but this one somehow sneaked ;)
>
> Anyway, Yinghai, Peter,
>
> I'm not sure but can't we use some boot_param "pad" field for
> "being copied" flag instead of new variable? There is a case
> when boot_param is used as __initdata and I'm not sure we clear
> this section explicitly.
>

Actually, even better would be to simply use boot_params.hdr.version,
which will never be zero.

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jeremy Fitzhardinge on 12 Jul 2010 19:00
On 07/12/2010 11:11 AM, Yinghai Lu wrote:
> On 07/12/2010 11:09 AM, H. Peter Anvin wrote:
>
>> On 07/12/2010 10:44 AM, Cyrill Gorcunov wrote:
>>
>>> Peter, while reviewing this patch I found another nit in
>>> context of early_param usage, so the patch is below. It's
>>> completely trivial. Actually I thought I've already fixed
>>> all early_param cases long ago but this one somehow sneaked ;)
>>>
>>> Anyway, Yinghai, Peter,
>>>
>>> I'm not sure but can't we use some boot_param "pad" field for
>>> "being copied" flag instead of new variable? There is a case
>>> when boot_param is used as __initdata and I'm not sure we clear
>>> this section explicitly.
>>>
>>>
>> Actually, even better would be to simply use boot_params.hdr.version,
>> which will never be zero.
>>
> Jeremy,
>
> any reason that xen cat not use x86_64_start_kernel directly?
>

As I remember it, I split x86_64_start_kernel into two pieces, one
containing the bits that were awkward with Xen. I don't remember which
were the problematic parts, but it all looks pretty tricky. Specifically:

* Xen will pre-clear the bss, so that's not necessary
* we don't go via head, so cleanup_highmap isn't either
* PV domains don't have an IDT available to them, or any of their
associated structures

So the whole thing looks at best reundant, and at worst has the
potential for causing subtle damage.

Why do you ask? Does it relate to the early console stuff, or are you
just asking because you're looking at it?

J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on 12 Jul 2010 19:50
On 07/12/2010 03:57 PM, Jeremy Fitzhardinge wrote:
> On 07/12/2010 11:11 AM, Yinghai Lu wrote:
>> On 07/12/2010 11:09 AM, H. Peter Anvin wrote:
>>
>>> On 07/12/2010 10:44 AM, Cyrill Gorcunov wrote:
>>>
>>>> Peter, while reviewing this patch I found another nit in
>>>> context of early_param usage, so the patch is below. It's
>>>> completely trivial. Actually I thought I've already fixed
>>>> all early_param cases long ago but this one somehow sneaked ;)
>>>>
>>>> Anyway, Yinghai, Peter,
>>>>
>>>> I'm not sure but can't we use some boot_param "pad" field for
>>>> "being copied" flag instead of new variable? There is a case
>>>> when boot_param is used as __initdata and I'm not sure we clear
>>>> this section explicitly.
>>>>
>>>>
>>> Actually, even better would be to simply use boot_params.hdr.version,
>>> which will never be zero.
>>>
>> Jeremy,
>>
>> any reason that xen cat not use x86_64_start_kernel directly?
>>
>
> As I remember it, I split x86_64_start_kernel into two pieces, one
> containing the bits that were awkward with Xen. I don't remember which
> were the problematic parts, but it all looks pretty tricky. Specifically:
>
> * Xen will pre-clear the bss, so that's not necessary
> * we don't go via head, so cleanup_highmap isn't either
> * PV domains don't have an IDT available to them, or any of their
> associated structures
>
> So the whole thing looks at best reundant, and at worst has the
> potential for causing subtle damage.
>
> Why do you ask? Does it relate to the early console stuff, or are you
> just asking because you're looking at it?

yes, make setup_early_console()/copy_bootdata two times....

and that looks strange.


+static int __initdata bootdata_copied;
static void __init copy_bootdata(char *real_mode_data)
{
char * command_line;

+ if (bootdata_copied)
+ return;
+
memcpy(&boot_params, real_mode_data, sizeof boot_params);
if (boot_params.hdr.cmd_line_ptr) {
command_line = __va(boot_params.hdr.cmd_line_ptr);
memcpy(boot_command_line, command_line, COMMAND_LINE_SIZE);
}
+
+ bootdata_copied = 1;
}

void __init x86_64_start_kernel(char * real_mode_data)
@@ -73,6 +80,10 @@ void __init x86_64_start_kernel(char * real_mode_data)
/* clear bss before set_intr_gate with early_idt_handler */
clear_bss();

+ /* boot_params is in bss */
+ copy_bootdata(__va(real_mode_data));
+ setup_early_console();
+
/* Make NULL pointers segfault */
zap_identity_mappings();

@@ -97,8 +108,11 @@ void __init x86_64_start_kernel(char * real_mode_data)
void __init x86_64_start_reservations(char *real_mode_data)
{
copy_bootdata(__va(real_mode_data));
+ setup_early_console();
+

Thanks

Yinghai Lu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Pekka Enberg on 13 Jul 2010 13:50
Hi Yingahai,

Yinghai Lu wrote:
> On 07/12/2010 08:47 AM, H. Peter Anvin wrote:
>> On 07/12/2010 01:58 AM, Pekka Enberg wrote:
>>> Hi Yinghai,
>>>
>>> Yinghai Lu wrote:
>>>> Analyze "console=uart8250,io,0x3f8,115200n8" in
>>>> i386_start_kernel/x86_64_start_kernel,
>>>> and call setup_early_serial8250_console() to init early serial console.
>>>>
>>>> only can handle io port kind of 8250. because mmio need ioremap.
>>>>
>>>> Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>
>>> What's the purpose of this patch? Does it make my early boot I/O patch
>>> obsolete?
>>>
>>> Pekka
>> No, they're complementary. Your patch serial-port enables the RM
>> kernel, whereas Yinghai pushes the initialization earlier in the PM kernel.
>
> yes. cover more range.
>
> Can you consider to ask Pekka to anaylze "console=uart8250,io, 0x3f8,115200n8" instead?
>
> it looks like we can remove "earlyprintk=ttyS0,115200", or "earlyprintk=serial" etc.
>
> earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
> because it is shared between different archs already.

So just to clarify: I wasn't ignoring your comment here. I simply
followed hpa's recommendation on which I also happen to agree with
completely. ;-)

Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on 13 Jul 2010 16:50
On 07/13/2010 10:43 AM, Pekka Enberg wrote:
> Hi Yingahai,
>
> Yinghai Lu wrote:
>> On 07/12/2010 08:47 AM, H. Peter Anvin wrote:
>>> On 07/12/2010 01:58 AM, Pekka Enberg wrote:
>>>> Hi Yinghai,
>>>>
>>>> Yinghai Lu wrote:
>>>>> Analyze "console=uart8250,io,0x3f8,115200n8" in
>>>>> i386_start_kernel/x86_64_start_kernel,
>>>>> and call setup_early_serial8250_console() to init early serial
>>>>> console.
>>>>>
>>>>> only can handle io port kind of 8250. because mmio need ioremap.
>>>>>
>>>>> Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>
>>>> What's the purpose of this patch? Does it make my early boot I/O patch
>>>> obsolete?
>>>>
>>>> Pekka
>>> No, they're complementary. Your patch serial-port enables the RM
>>> kernel, whereas Yinghai pushes the initialization earlier in the PM
>>> kernel.
>>
>> yes. cover more range.
>>
>> Can you consider to ask Pekka to anaylze "console=uart8250,io,
>> 0x3f8,115200n8" instead?
>>
>> it looks like we can remove "earlyprintk=ttyS0,115200", or
>> "earlyprintk=serial" etc.
>>
>> earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
>> because it is shared between different archs already.
>
> So just to clarify: I wasn't ignoring your comment here. I simply
> followed hpa's recommendation on which I also happen to agree with
> completely. ;-)

never mind.

following patch add that checking.

also you missed simple_guess_base(), your patch may have problem with baud rate reading.

baud = simple_strtoull(arg + pos, &e, 0);
if (baud == 0 || arg + pos == e)
baud = DEFAULT_BAUD;

and your copied simple_strtoull does not calling simple_guess_base(), so base will 0.
so you are always using DEFAULT_BAUD.

Thanks

Yinghai Lu


[PATCH] x86: make boot code to analyze console=uart8250 too

So we use console=uart8250,io,0x2f8,115200n all the way

Also add back simple_guess_base(), otherwise those simple_strtoull(,,0) are not
going to work.

Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>

---
arch/x86/boot/string.c | 22 ++++++++++++++++++
arch/x86/boot/tty.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 80 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/boot/tty.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/tty.c
+++ linux-2.6/arch/x86/boot/tty.c
@@ -170,7 +170,7 @@ static void early_serial_init(int baud)
outb(c & ~DLAB, early_serial_base + LCR);
}

-void console_init(void)
+static int parse_earlyprintk(void)
{
int baud = DEFAULT_BAUD;
char arg[32];
@@ -208,6 +208,63 @@ void console_init(void)
baud = DEFAULT_BAUD;
}

+ return baud;
+}
+
+#define BASE_BAUD (1843200/16)
+static unsigned int probe_baud(int port)
+{
+ unsigned char lcr, dll, dlh;
+ unsigned int quot;
+
+ lcr = inb(port + LCR);
+ outb(lcr | DLAB, port + LCR);
+ dll = inb(port + DLL);
+ dlh = inb(port + DLH);
+ outb(lcr, port + LCR);
+ quot = (dlh << 8) | dll;
+
+ return BASE_BAUD / quot;
+}
+
+static int parse_console_uart8250(void)
+{
+ char optstr[64], *options;
+ int baud = DEFAULT_BAUD;
+
+ /*
+ * console=uart8250,io,0x3f8,115200n8
+ * need to make sure it is last one console !
+ */
+ if (cmdline_find_option("console", optstr, sizeof optstr) <= 0)
+ return baud;
+
+ options = optstr;
+
+ if (!strncmp(options, "uart8250,io,", 12))
+ early_serial_base = simple_strtoull(options + 12, &options, 0);
+ else if (!strncmp(options, "uart,io,", 8))
+ early_serial_base = simple_strtoull(options + 8, &options, 0);
+ else
+ return baud;
+
+ if (options && (options[0] == ','))
+ baud = simple_strtoull(options + 1, &options, 0);
+ else
+ baud = probe_baud(early_serial_base);
+
+ return baud;
+}
+
+void console_init(void)
+{
+ int baud;
+
+ baud = parse_earlyprintk();
+
+ if (!early_serial_base)
+ baud = parse_console_uart8250();
+
if (early_serial_base != 0)
early_serial_init(baud);
}
Index: linux-2.6/arch/x86/boot/string.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/string.c
+++ linux-2.6/arch/x86/boot/string.c
@@ -68,10 +68,32 @@ unsigned int atou(const char *s)
/* Works only for digits and letters, but small and fast */
#define TOLOWER(x) ((x) | 0x20)

+static unsigned int simple_guess_base(const char *cp)
+{
+ if (cp[0] == '0') {
+ if (TOLOWER(cp[1]) == 'x' && isxdigit(cp[2]))
+ return 16;
+ else
+ return 8;
+ } else {
+ return 10;
+ }
+}
+
+/**
+ * simple_strtoull - convert a string to an unsigned long long
+ * @cp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ */
+
unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
{
unsigned long long result = 0;

+ if (!base)
+ base = simple_guess_base(cp);
+
if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
cp += 2;




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: vfs: Add name to file handle conversion support
Next: Research on Open Source Software Development