* [PATCH v1 2/6] x86/boot: Introduce helpers for serial I/O
2018-01-14 14:32 [PATCH v1 1/6] x86/boot: Convert early_serial_base to unsigned long Andy Shevchenko
@ 2018-01-14 14:32 ` Andy Shevchenko
2018-01-14 14:32 ` [PATCH v1 3/6] x86/boot: Add MMIO byte accessors Andy Shevchenko
` (3 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-14 14:32 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
Cc: Andy Shevchenko
As preparatory to enable earlyprintk on non-standard ports on x86,
introduce serial_in() and serial_out() helpers to perform serial I/O.
No functional change intended.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/x86/boot/boot.h | 2 +
arch/x86/boot/compressed/early_serial_console.c | 3 ++
arch/x86/boot/compressed/misc.c | 4 +-
arch/x86/boot/compressed/misc.h | 4 ++
arch/x86/boot/early_serial_console.c | 60 +++++++++++++++++--------
arch/x86/boot/tty.c | 7 ++-
6 files changed, 57 insertions(+), 23 deletions(-)
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index d9f8279774f6..7e6ac187275c 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -299,6 +299,8 @@ int check_knl_erratum(void);
int validate_cpu(void);
/* early_serial_console.c */
+extern unsigned int (*serial_in)(unsigned long addr, int offset);
+extern void (*serial_out)(unsigned long addr, int offset, int value);
extern unsigned long early_serial_base;
void console_init(void);
diff --git a/arch/x86/boot/compressed/early_serial_console.c b/arch/x86/boot/compressed/early_serial_console.c
index 5e3a66478754..4b6269624b59 100644
--- a/arch/x86/boot/compressed/early_serial_console.c
+++ b/arch/x86/boot/compressed/early_serial_console.c
@@ -1,5 +1,8 @@
#include "misc.h"
+unsigned int (*serial_in)(unsigned long addr, int offset);
+void (*serial_out)(unsigned long addr, int offset, int value);
+
unsigned long early_serial_base;
#include "../early_serial_console.c"
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 98761a1576ce..0b16675c2523 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -97,10 +97,10 @@ static void serial_putchar(int ch)
{
unsigned timeout = 0xffff;
- while ((inb(early_serial_base + LSR) & XMTRDY) == 0 && --timeout)
+ while ((serial_in(early_serial_base, LSR) & XMTRDY) == 0 && --timeout)
cpu_relax();
- outb(ch, early_serial_base + TXR);
+ serial_out(early_serial_base, TXR, ch);
}
void __putstr(const char *s)
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index d36d4398a6b0..266e6b93788c 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -101,9 +101,13 @@ static inline void finalize_identity_maps(void)
#ifdef CONFIG_EARLY_PRINTK
/* early_serial_console.c */
+extern unsigned int (*serial_in)(unsigned long addr, int offset);
+extern void (*serial_out)(unsigned long addr, int offset, int value);
extern unsigned long early_serial_base;
void console_init(void);
#else
+static unsigned int (*serial_in)(unsigned long addr, int offset);
+static void (*serial_out)(unsigned long addr, int offset, int value);
static const unsigned long early_serial_base;
static inline void console_init(void)
{ }
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index a5aec142c91a..8b091919b3b9 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -23,22 +23,46 @@
#define DEFAULT_BAUD 9600
-static void early_serial_init(unsigned long port, int baud)
+static unsigned int io_serial_in(unsigned long addr, int offset)
+{
+ return inb(addr + offset);
+}
+
+static void io_serial_out(unsigned long addr, int offset, int value)
+{
+ outb(value, addr + offset);
+}
+
+static void early_serial_configure(unsigned long port, int baud)
{
unsigned char c;
unsigned divisor;
- outb(0x3, port + LCR); /* 8n1 */
- outb(0, port + IER); /* no interrupt */
- outb(0, port + FCR); /* no fifo */
- outb(0x3, port + MCR); /* DTR + RTS */
+ serial_out(port, LCR, 0x3); /* 8n1 */
+ serial_out(port, IER, 0); /* no interrupt */
+ serial_out(port, FCR, 0); /* no fifo */
+ serial_out(port, MCR, 0x3); /* DTR + RTS */
divisor = 115200 / baud;
- c = inb(port + LCR);
- outb(c | DLAB, port + LCR);
- outb(divisor & 0xff, port + DLL);
- outb((divisor >> 8) & 0xff, port + DLH);
- outb(c & ~DLAB, port + LCR);
+ c = serial_in(port, LCR);
+ serial_out(port, LCR, c | DLAB);
+ serial_out(port, DLL, divisor & 0xff);
+ serial_out(port, DLH, (divisor >> 8) & 0xff);
+ serial_out(port, LCR, c & ~DLAB);
+}
+
+static void early_serial_init(unsigned long port, int baud)
+{
+ /* Assign serial I/O accessors */
+ if (port) {
+ /* These will always be IO based ports */
+ serial_in = io_serial_in;
+ serial_out = io_serial_out;
+ } else {
+ return;
+ }
+
+ early_serial_configure(port, baud);
early_serial_base = port;
}
@@ -94,8 +118,7 @@ static void parse_earlyprintk(void)
baud = DEFAULT_BAUD;
}
- if (port)
- early_serial_init(port, baud);
+ early_serial_init(port, baud);
}
#define BASE_BAUD (1843200/16)
@@ -104,11 +127,11 @@ 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);
+ lcr = serial_in(port, LCR);
+ serial_out(port, LCR, lcr | DLAB);
+ dll = serial_in(port, DLL);
+ dlh = serial_in(port, DLH);
+ serial_out(port, LCR, lcr);
quot = (dlh << 8) | dll;
return BASE_BAUD / quot;
@@ -141,8 +164,7 @@ static void parse_console_uart8250(void)
else
baud = probe_baud(port);
- if (port)
- early_serial_init(port, baud);
+ early_serial_init(port, baud);
}
void console_init(void)
diff --git a/arch/x86/boot/tty.c b/arch/x86/boot/tty.c
index d1d34c6ca153..b6aa9db65cda 100644
--- a/arch/x86/boot/tty.c
+++ b/arch/x86/boot/tty.c
@@ -15,6 +15,9 @@
#include "boot.h"
+unsigned int (*serial_in)(unsigned long addr, int offset);
+void (*serial_out)(unsigned long addr, int offset, int value);
+
unsigned long early_serial_base;
#define XMTRDY 0x20
@@ -31,10 +34,10 @@ static void __attribute__((section(".inittext"))) serial_putchar(int ch)
{
unsigned timeout = 0xffff;
- while ((inb(early_serial_base + LSR) & XMTRDY) == 0 && --timeout)
+ while ((serial_in(early_serial_base, LSR) & XMTRDY) == 0 && --timeout)
cpu_relax();
- outb(ch, early_serial_base + TXR);
+ serial_out(early_serial_base, TXR, ch);
}
static void __attribute__((section(".inittext"))) bios_putchar(int ch)
--
2.15.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 3/6] x86/boot: Add MMIO byte accessors
2018-01-14 14:32 [PATCH v1 1/6] x86/boot: Convert early_serial_base to unsigned long Andy Shevchenko
2018-01-14 14:32 ` [PATCH v1 2/6] x86/boot: Introduce helpers for serial I/O Andy Shevchenko
@ 2018-01-14 14:32 ` Andy Shevchenko
2018-01-14 14:32 ` [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk Andy Shevchenko
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-14 14:32 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
Cc: Andy Shevchenko
readb() and writeb() would help to access serial device
via MMIO address space.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/x86/boot/boot.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 7e6ac187275c..09f6829f2778 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -79,6 +79,18 @@ static inline void io_delay(void)
asm volatile("outb %%al,%0" : : "dN" (DELAY_PORT));
}
+static inline u8 readb(const volatile void __iomem *addr)
+{
+ u8 v;
+ asm volatile("movb %1,%0" : "=q" (v) : "m" (*(volatile u8 __force *)addr));
+ return v;
+}
+
+static inline void writeb(u8 v, volatile void __iomem *addr)
+{
+ asm volatile("movb %0,%1" : : "q" (v), "m" (*(volatile u8 __force *)addr));
+}
+
/* These functions are used to reference data in other segments. */
static inline u16 ds(void)
--
2.15.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk
2018-01-14 14:32 [PATCH v1 1/6] x86/boot: Convert early_serial_base to unsigned long Andy Shevchenko
2018-01-14 14:32 ` [PATCH v1 2/6] x86/boot: Introduce helpers for serial I/O Andy Shevchenko
2018-01-14 14:32 ` [PATCH v1 3/6] x86/boot: Add MMIO byte accessors Andy Shevchenko
@ 2018-01-14 14:32 ` Andy Shevchenko
2018-01-16 3:13 ` Ingo Molnar
2018-01-14 14:32 ` [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk Andy Shevchenko
2018-01-14 14:32 ` [PATCH v1 6/6] x86/boot: Support nocfg parameter " Andy Shevchenko
4 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-14 14:32 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
Cc: Andy Shevchenko
If user supplied serial base address via kernel command line and value
is higher than IO space limit (64k boundary), assume for now that MMIO
byte access is required.
Later we might expand or modify this if needed.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/x86/boot/early_serial_console.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 8b091919b3b9..8cb7b0e8fe98 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -33,6 +33,22 @@ static void io_serial_out(unsigned long addr, int offset, int value)
outb(value, addr + offset);
}
+#define IO_SPACE_LIMIT 0xffff
+
+static void mem8_serial_out(unsigned long addr, int offset, int value)
+{
+ u8 __iomem *vaddr = (u8 __iomem *)addr;
+ /* shift implied by pointer type */
+ writeb(value, vaddr + offset);
+}
+
+static unsigned int mem8_serial_in(unsigned long addr, int offset)
+{
+ u8 __iomem *vaddr = (u8 __iomem *)addr;
+ /* shift implied by pointer type */
+ return readb(vaddr + offset);
+}
+
static void early_serial_configure(unsigned long port, int baud)
{
unsigned char c;
@@ -54,7 +70,11 @@ static void early_serial_configure(unsigned long port, int baud)
static void early_serial_init(unsigned long port, int baud)
{
/* Assign serial I/O accessors */
- if (port) {
+ if (port > IO_SPACE_LIMIT) {
+ /* It is memory mapped - assume 8-bit alignment */
+ serial_in = mem8_serial_in;
+ serial_out = mem8_serial_out;
+ } else if (port) {
/* These will always be IO based ports */
serial_in = io_serial_in;
serial_out = io_serial_out;
--
2.15.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk
2018-01-14 14:32 ` [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk Andy Shevchenko
@ 2018-01-16 3:13 ` Ingo Molnar
2018-01-16 10:55 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2018-01-16 3:13 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> If user supplied serial base address via kernel command line and value
> is higher than IO space limit (64k boundary), assume for now that MMIO
> byte access is required.
>
> Later we might expand or modify this if needed.
Is this a standard pattern for serial code configuration values?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk
2018-01-16 3:13 ` Ingo Molnar
@ 2018-01-16 10:55 ` Andy Shevchenko
2018-01-16 15:55 ` Ingo Molnar
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-16 10:55 UTC (permalink / raw)
To: Ingo Molnar, Greg Kroah-Hartman
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
On Tue, 2018-01-16 at 04:13 +0100, Ingo Molnar wrote:
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > If user supplied serial base address via kernel command line and
> > value
> > is higher than IO space limit (64k boundary), assume for now that
> > MMIO
> > byte access is required.
> >
> > Later we might expand or modify this if needed.
>
> Is this a standard pattern for serial code configuration values?
I didn't get what you meant under "standard" here.
IO space limit comes from generic io.h header and AFAIU is a hardware
limitation (outN (%dx), ...; inX (%dx); dx is 16 bit register).
Using mmio8 out of the IO space is dictated by the (modern) x86
platforms with non-standard (okay, high speed) UART location in address
space.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk
2018-01-16 10:55 ` Andy Shevchenko
@ 2018-01-16 15:55 ` Ingo Molnar
2018-05-03 10:45 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2018-01-16 15:55 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
x86, linux-kernel
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2018-01-16 at 04:13 +0100, Ingo Molnar wrote:
> > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > If user supplied serial base address via kernel command line and
> > > value
> > > is higher than IO space limit (64k boundary), assume for now that
> > > MMIO
> > > byte access is required.
> > >
> > > Later we might expand or modify this if needed.
> >
> > Is this a standard pattern for serial code configuration values?
>
> I didn't get what you meant under "standard" here.
>
> IO space limit comes from generic io.h header and AFAIU is a hardware
> limitation (outN (%dx), ...; inX (%dx); dx is 16 bit register).
>
> Using mmio8 out of the IO space is dictated by the (modern) x86
> platforms with non-standard (okay, high speed) UART location in address
> space.
So I was wondering whether we should just make mmio configuration an explicit
parameter instead of a 'range hack'.
Since we are introducing something entirely new the choice is ours.
Doing it that way would technically be cleaner, as, at least theoretically,
there could be platforms with mmio addresses below 64k physical, right?
It's also more self-documenting if the new configuration/parameter says 'mmio'
explicitly.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk
2018-01-16 15:55 ` Ingo Molnar
@ 2018-05-03 10:45 ` Andy Shevchenko
0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2018-05-03 10:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: Greg Kroah-Hartman, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
x86, linux-kernel
On Tue, 2018-01-16 at 16:55 +0100, Ingo Molnar wrote:
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > On Tue, 2018-01-16 at 04:13 +0100, Ingo Molnar wrote:
> > > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > > If user supplied serial base address via kernel command line and
> > > > value
> > > > is higher than IO space limit (64k boundary), assume for now
> > > > that
> > > > MMIO
> > > > byte access is required.
> > > >
> > > > Later we might expand or modify this if needed.
> > >
> > > Is this a standard pattern for serial code configuration values?
> >
> > I didn't get what you meant under "standard" here.
> >
> > IO space limit comes from generic io.h header and AFAIU is a
> > hardware
> > limitation (outN (%dx), ...; inX (%dx); dx is 16 bit register).
> >
> > Using mmio8 out of the IO space is dictated by the (modern) x86
> > platforms with non-standard (okay, high speed) UART location in
> > address
> > space.
>
> So I was wondering whether we should just make mmio configuration an
> explicit
> parameter instead of a 'range hack'.
I _think_ it can be done later without breaking any existed
configurations.
> Since we are introducing something entirely new the choice is ours.
>
> Doing it that way would technically be cleaner, as, at least
> theoretically,
> there could be platforms with mmio addresses below 64k physical,
> right?
Correct, though we are talking about x86 world where I have no example
like this. Either we have LPC ports (I/O 0x3f8 and alike) or HS UARTS on
higher MMIO addresses. Note, that PCI code guarantees that range bump as
well when resources would be allocated for unassigned, by firmware, PCI
UART controllers.
> It's also more self-documenting if the new configuration/parameter
> says 'mmio'
> explicitly.
I would look at it, though it might make things much more complicated to
implement. And yes, it wouldn't reduce fragility of the parser anyway.
Perhaps we can unify earlycon parser with earlyprintk one (not sure if
it's possible since this nice trick with sharing same code between
compressed and boot code).
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk
2018-01-14 14:32 [PATCH v1 1/6] x86/boot: Convert early_serial_base to unsigned long Andy Shevchenko
` (2 preceding siblings ...)
2018-01-14 14:32 ` [PATCH v1 4/6] x86/boot: Assume MMIO if serial base address supplied via earlyprintk Andy Shevchenko
@ 2018-01-14 14:32 ` Andy Shevchenko
2018-01-16 3:11 ` Ingo Molnar
2018-01-14 14:32 ` [PATCH v1 6/6] x86/boot: Support nocfg parameter " Andy Shevchenko
4 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-14 14:32 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
Cc: Andy Shevchenko
Allow longer parameter list for earlyprintk to support new coming
parameters.
No functional change intended.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/x86/boot/early_serial_console.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 8cb7b0e8fe98..ef10926ab9e5 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -90,7 +90,7 @@ static void early_serial_init(unsigned long port, int baud)
static void parse_earlyprintk(void)
{
int baud = DEFAULT_BAUD;
- char arg[32];
+ char arg[64];
int pos = 0;
unsigned long port = 0;
--
2.15.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk
2018-01-14 14:32 ` [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk Andy Shevchenko
@ 2018-01-16 3:11 ` Ingo Molnar
2018-01-16 10:57 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2018-01-16 3:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> Allow longer parameter list for earlyprintk to support new coming
> parameters.
>
> No functional change intended.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> arch/x86/boot/early_serial_console.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
> index 8cb7b0e8fe98..ef10926ab9e5 100644
> --- a/arch/x86/boot/early_serial_console.c
> +++ b/arch/x86/boot/early_serial_console.c
> @@ -90,7 +90,7 @@ static void early_serial_init(unsigned long port, int baud)
> static void parse_earlyprintk(void)
> {
> int baud = DEFAULT_BAUD;
> - char arg[32];
> + char arg[64];
> int pos = 0;
> unsigned long port = 0;
BTW., what happens if the parsed string is longer than this buffer?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk
2018-01-16 3:11 ` Ingo Molnar
@ 2018-01-16 10:57 ` Andy Shevchenko
2018-01-16 15:51 ` Ingo Molnar
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-16 10:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
On Tue, 2018-01-16 at 04:11 +0100, Ingo Molnar wrote:
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > Allow longer parameter list for earlyprintk to support new coming
> > parameters.
> >
> > No functional change intended.
> BTW., what happens if the parsed string is longer than this buffer?
It will be cut up to 63 characters IIUC.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk
2018-01-16 10:57 ` Andy Shevchenko
@ 2018-01-16 15:51 ` Ingo Molnar
2018-05-03 11:48 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2018-01-16 15:51 UTC (permalink / raw)
To: Andy Shevchenko
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2018-01-16 at 04:11 +0100, Ingo Molnar wrote:
> > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > Allow longer parameter list for earlyprintk to support new coming
> > > parameters.
> > >
> > > No functional change intended.
>
> > BTW., what happens if the parsed string is longer than this buffer?
>
> It will be cut up to 63 characters IIUC.
Ok, that's fine!
Could you please add a comment that explains the limit and the (benign) effects if
we go past it?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk
2018-01-16 15:51 ` Ingo Molnar
@ 2018-05-03 11:48 ` Andy Shevchenko
0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2018-05-03 11:48 UTC (permalink / raw)
To: Ingo Molnar
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
On Tue, 2018-01-16 at 16:51 +0100, Ingo Molnar wrote:
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > On Tue, 2018-01-16 at 04:11 +0100, Ingo Molnar wrote:
> > > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > > Allow longer parameter list for earlyprintk to support new
> > > > coming
> > > > parameters.
> > > >
> > > > No functional change intended.
> > > BTW., what happens if the parsed string is longer than this
> > > buffer?
> >
> > It will be cut up to 63 characters IIUC.
>
> Ok, that's fine!
>
> Could you please add a comment that explains the limit and the
> (benign) effects if
> we go past it?
Will do.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1 6/6] x86/boot: Support nocfg parameter for earlyprintk
2018-01-14 14:32 [PATCH v1 1/6] x86/boot: Convert early_serial_base to unsigned long Andy Shevchenko
` (3 preceding siblings ...)
2018-01-14 14:32 ` [PATCH v1 5/6] x86/boot: Allow longer parameter list for earlyprintk Andy Shevchenko
@ 2018-01-14 14:32 ` Andy Shevchenko
2018-01-16 3:12 ` Ingo Molnar
4 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-14 14:32 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
Cc: Andy Shevchenko
If by BIOS or by other means serial port is configured user might want to
skip reconfiguration in the boot code.
Add support of 'nocfg' parameter to earlyprintk.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/x86/boot/early_serial_console.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index ef10926ab9e5..f4759f485d8e 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -67,7 +67,7 @@ static void early_serial_configure(unsigned long port, int baud)
serial_out(port, LCR, c & ~DLAB);
}
-static void early_serial_init(unsigned long port, int baud)
+static void early_serial_init(unsigned long port, int baud, bool configure)
{
/* Assign serial I/O accessors */
if (port > IO_SPACE_LIMIT) {
@@ -82,7 +82,8 @@ static void early_serial_init(unsigned long port, int baud)
return;
}
- early_serial_configure(port, baud);
+ if (configure)
+ early_serial_configure(port, baud);
early_serial_base = port;
}
@@ -93,6 +94,7 @@ static void parse_earlyprintk(void)
char arg[64];
int pos = 0;
unsigned long port = 0;
+ bool configure = true;
if (cmdline_find_option("earlyprintk", arg, sizeof arg) > 0) {
char *e;
@@ -133,12 +135,16 @@ static void parse_earlyprintk(void)
if (arg[pos] == ',')
pos++;
- baud = simple_strtoull(arg + pos, &e, 0);
- if (baud == 0 || arg + pos == e)
- baud = DEFAULT_BAUD;
+ if (strncmp(arg + pos, "nocfg", 5)) {
+ baud = simple_strtoull(arg + pos, &e, 0);
+ if (baud == 0 || arg + pos == e)
+ baud = DEFAULT_BAUD;
+ } else {
+ configure = false;
+ }
}
- early_serial_init(port, baud);
+ early_serial_init(port, baud, configure);
}
#define BASE_BAUD (1843200/16)
@@ -162,6 +168,7 @@ static void parse_console_uart8250(void)
char optstr[64], *options;
int baud = DEFAULT_BAUD;
unsigned long port = 0;
+ bool configure = true;
/*
* console=uart8250,io,0x3f8,115200n8
@@ -179,12 +186,16 @@ static void parse_console_uart8250(void)
else
return;
- if (options && (options[0] == ','))
- baud = simple_strtoull(options + 1, &options, 0);
- else
+ if (options[0] == ',') {
+ if (strncmp(options + 1, "nocfg", 5))
+ baud = simple_strtoull(options + 1, &options, 0);
+ else
+ configure = false;
+ } else {
baud = probe_baud(port);
+ }
- early_serial_init(port, baud);
+ early_serial_init(port, baud, configure);
}
void console_init(void)
--
2.15.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v1 6/6] x86/boot: Support nocfg parameter for earlyprintk
2018-01-14 14:32 ` [PATCH v1 6/6] x86/boot: Support nocfg parameter " Andy Shevchenko
@ 2018-01-16 3:12 ` Ingo Molnar
2018-01-16 11:00 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2018-01-16 3:12 UTC (permalink / raw)
To: Andy Shevchenko
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> @@ -133,12 +135,16 @@ static void parse_earlyprintk(void)
> if (arg[pos] == ',')
> pos++;
>
> - baud = simple_strtoull(arg + pos, &e, 0);
> - if (baud == 0 || arg + pos == e)
> - baud = DEFAULT_BAUD;
> + if (strncmp(arg + pos, "nocfg", 5)) {
> + baud = simple_strtoull(arg + pos, &e, 0);
> + if (baud == 0 || arg + pos == e)
> + baud = DEFAULT_BAUD;
> + } else {
> + configure = false;
> + }
> }
>
> - early_serial_init(port, baud);
> + early_serial_init(port, baud, configure);
> }
>
> #define BASE_BAUD (1843200/16)
> @@ -162,6 +168,7 @@ static void parse_console_uart8250(void)
> char optstr[64], *options;
> int baud = DEFAULT_BAUD;
> unsigned long port = 0;
> + bool configure = true;
>
> /*
> * console=uart8250,io,0x3f8,115200n8
> @@ -179,12 +186,16 @@ static void parse_console_uart8250(void)
> else
> return;
>
> - if (options && (options[0] == ','))
> - baud = simple_strtoull(options + 1, &options, 0);
> - else
> + if (options[0] == ',') {
> + if (strncmp(options + 1, "nocfg", 5))
> + baud = simple_strtoull(options + 1, &options, 0);
> + else
> + configure = false;
> + } else {
> baud = probe_baud(port);
These code patters seem very similar - could a common function be factored out, to
simplify future changes (such as the one done here)?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v1 6/6] x86/boot: Support nocfg parameter for earlyprintk
2018-01-16 3:12 ` Ingo Molnar
@ 2018-01-16 11:00 ` Andy Shevchenko
2018-01-16 15:53 ` Ingo Molnar
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2018-01-16 11:00 UTC (permalink / raw)
To: Ingo Molnar
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel
On Tue, 2018-01-16 at 04:12 +0100, Ingo Molnar wrote:
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > @@ -133,12 +135,16 @@ static void parse_earlyprintk(void)
> > if (arg[pos] == ',')
> > pos++;
> >
> > - baud = simple_strtoull(arg + pos, &e, 0);
> > - if (baud == 0 || arg + pos == e)
> > - baud = DEFAULT_BAUD;
> > + if (strncmp(arg + pos, "nocfg", 5)) {
> > + baud = simple_strtoull(arg + pos, &e, 0);
> > + if (baud == 0 || arg + pos == e)
> > + baud = DEFAULT_BAUD;
> > + } else {
> > + configure = false;
> > + }
> > }
> >
> > - early_serial_init(port, baud);
> > + early_serial_init(port, baud, configure);
> > }
> >
> > #define BASE_BAUD (1843200/16)
> > @@ -162,6 +168,7 @@ static void parse_console_uart8250(void)
> > char optstr[64], *options;
> > int baud = DEFAULT_BAUD;
> > unsigned long port = 0;
> > + bool configure = true;
> >
> > /*
> > * console=uart8250,io,0x3f8,115200n8
> > @@ -179,12 +186,16 @@ static void parse_console_uart8250(void)
> > else
> > return;
> >
> > - if (options && (options[0] == ','))
> > - baud = simple_strtoull(options + 1, &options, 0);
> > - else
> > + if (options[0] == ',') {
> > + if (strncmp(options + 1, "nocfg", 5))
> > + baud = simple_strtoull(options + 1,
> > &options, 0);
> > + else
> > + configure = false;
> > + } else {
> > baud = probe_baud(port);
>
> These code patters seem very similar - could a common function be
> factored out, to
> simplify future changes (such as the one done here)?
Need to think about. Moreoever, arch/x86/kernel/early_print.c contains
even more duplication, though I understand why it's split to different
folders.
And on top of that we have earlycon (which indeed would be more
preferable solution). Perhaps, instead of playing with earlyprintk at
boot stage we might parse earlycon option that more flexible?
P.S. In any choice at least patch 1 (and maybe patch 2) would be needed.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v1 6/6] x86/boot: Support nocfg parameter for earlyprintk
2018-01-16 11:00 ` Andy Shevchenko
@ 2018-01-16 15:53 ` Ingo Molnar
0 siblings, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2018-01-16 15:53 UTC (permalink / raw)
To: Andy Shevchenko
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
Peter Zijlstra, Greg Kroah-Hartman
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2018-01-16 at 04:12 +0100, Ingo Molnar wrote:
> > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > @@ -133,12 +135,16 @@ static void parse_earlyprintk(void)
> > > if (arg[pos] == ',')
> > > pos++;
> > >
> > > - baud = simple_strtoull(arg + pos, &e, 0);
> > > - if (baud == 0 || arg + pos == e)
> > > - baud = DEFAULT_BAUD;
> > > + if (strncmp(arg + pos, "nocfg", 5)) {
> > > + baud = simple_strtoull(arg + pos, &e, 0);
> > > + if (baud == 0 || arg + pos == e)
> > > + baud = DEFAULT_BAUD;
> > > + } else {
> > > + configure = false;
> > > + }
> > > }
> > >
> > > - early_serial_init(port, baud);
> > > + early_serial_init(port, baud, configure);
> > > }
> > >
> > > #define BASE_BAUD (1843200/16)
> > > @@ -162,6 +168,7 @@ static void parse_console_uart8250(void)
> > > char optstr[64], *options;
> > > int baud = DEFAULT_BAUD;
> > > unsigned long port = 0;
> > > + bool configure = true;
> > >
> > > /*
> > > * console=uart8250,io,0x3f8,115200n8
> > > @@ -179,12 +186,16 @@ static void parse_console_uart8250(void)
> > > else
> > > return;
> > >
> > > - if (options && (options[0] == ','))
> > > - baud = simple_strtoull(options + 1, &options, 0);
> > > - else
> > > + if (options[0] == ',') {
> > > + if (strncmp(options + 1, "nocfg", 5))
> > > + baud = simple_strtoull(options + 1,
> > > &options, 0);
> > > + else
> > > + configure = false;
> > > + } else {
> > > baud = probe_baud(port);
> >
> > These code patters seem very similar - could a common function be
> > factored out, to
> > simplify future changes (such as the one done here)?
>
> Need to think about. Moreoever, arch/x86/kernel/early_print.c contains
> even more duplication, though I understand why it's split to different
> folders.
>
> And on top of that we have earlycon (which indeed would be more
> preferable solution). Perhaps, instead of playing with earlyprintk at
> boot stage we might parse earlycon option that more flexible?
>
> P.S. In any choice at least patch 1 (and maybe patch 2) would be needed.
I'm fine with your current approach - and earlyprintk is preferred by many kernel
developers. Was just wondering how hard it would be to create a common parser -
and whether that's desirable at all (it might not be).
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread