lkml.org 
[lkml]   [2008]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] hvc - register all available consoles (was: Re: [PATCH] powerpc/lpar - defer prefered console setup)
    On Wed, Jul 30, 2008 at 04:13:47AM -0500, Milton Miller wrote:
    > On Wed Jul 30 at 17:34:38 EST in 2008, Bastian Blank wrote:
    > > On Wed, Jul 30, 2008 at 08:29:19AM +0200, Bastian Blank wrote:
    > >> Okay, so hvc_console is the culprit. It don't register a preferred
    > >> console if it knows it is not the first in the list.
    > >
    > > The patch registers all available hvc consoles. It adds one "struct
    > > console" for all possible hvc consoles.
    >
    > [ a 6 page patch adding forward declartions, arrays of console
    > structs, moving lots of code and creating N struct console in the
    > hvc_driver shell]
    >
    > After previously having written:
    > > On Wed, Jul 30, 2008 at 08:23:13AM +0200, Bastian Blank wrote:
    > >> On Wed, Jul 30, 2008 at 12:34:51PM +1000, Michael Ellerman wrote:
    > >>> On Mon, 2008-07-28 at 20:56 +0200, Bastian Blank wrote:
    > >>> * add_preferred_console - add a device to the list of preferred consoles.
    > >>> ...
    > >>> * The last preferred console added will be used for kernel messages
    > >>> * and stdin/out/err for init.
    > >>>
    > >>> The last console will be added by the console= parsing, and so that will
    > >>> be used. The console we add in the pseries setup is only used if nothing
    > >>> is specified on the command line.
    > >>
    > >> Okay, then there is a completely different problem. In the case of
    > >> "console=hvc0 console=hvc1" it uses the _first_ add stdin/out bla and
    > >> ignores the second one completely.
    > >
    > > Okay, so hvc_console is the culprit. It don't register a preferred
    > > console if it knows it is not the first in the list.
    > >
    > > Bastian
    >
    >
    > [ back to the patch ]
    > > -/*
    > > - * Early console initialization. Precedes driver initialization.
    > > - *
    > > - * (1) we are first, and the user specified another driver
    > > - * -- index will remain -1
    > > - * (2) we are first and the user specified no driver
    > > - * -- index will be set to 0, then we will fail setup.
    > > - * (3) we are first and the user specified our driver
    > > - * -- index will be set to user specified driver, and we will fail
    > > - * (4) we are after driver, and this initcall will register us
    > > - * -- if the user didn't specify a driver then the console will match
    > > - *
    > > - * Note that for cases 2 and 3, we will match later when the io driver
    > > - * calls hvc_instantiate() and call register again.
    > > - */
    > > -static int __init hvc_console_init(void)
    > > -{
    > > - register_console(&hvc_con_driver);
    > > - return 0;
    > >
    >
    >
    > Please explain how the reasoning you removed breaks down.
    > What is your usage case?

    I have several hvc consoles on a Power hypervisor.

    > More importantly , how is this different than, say, drivers/serial/8250.c,
    > which also registers just one struct console?
    > would not console=ttyS0 console=ttyS1 have the same problem?

    Yes, it have the same problem. Only one of the two (I think the first)
    will get enabled as console.

    > Please instrument the calls to register_console and add_preferred_console
    > and give a detailed description of the problem you are encountering.

    | add_preferred_console("hvc", 4, NULL)

    This call was added recently by the Power LPAR code.

    | add_preferred_console("hvc", 1, NULL)

    This comes from the command line ("console=hvc1").

    | hvc_config_driver.index == -1
    | register_console(&hvc_con_driver)
    | hvc_config_driver.index == 4

    This call is used to detect the id of the to be enabled hvc device. See
    the comment of hvc_console_init. register_console sets it to the
    _first_ id it finds, in this case 4. There is no other call to
    register_console because there is no hvc console with id 4 registered
    and hvc_instantiate checks this.

    The list of consoles looks like:
    - hvc, 4
    - hvc, 1

    The boot console (udbg0) is destructed later without a real console
    remaining.

    > Perhaps the real fix should be scan the command line for console= at
    > console_init time? How does that compare to __setup that is called now?

    This was removed because it break different things. See
    5faae2e5d1f53df9dce482032c8486bc3a1feffc.

    > > for (i = 0; i < n; ++i) {
    > > #ifdef CONFIG_MAGIC_SYSRQ
    > > - if (hp->index == hvc_con_driver.index) {
    > > + if (consoles[hp->index].console.flags & CON_CONSDEV) {
    > > /* Handle the SysRq Hack */
    > > /* XXX should support a sequence */
    > > if (buf[i] == '\x0f') { /* ^O */
    > > @@ -775,8 +764,8 @@ struct hvc_struct __devinit *hvc_alloc(uint32_t vtermno, int irq,
    > > * see if this vterm id matches one registered for console.
    > > */
    > > for (i=0; i < MAX_NR_HVC_CONSOLES; i++)
    > > - if (vtermnos[i] == hp->vtermno &&
    > > - cons_ops[i] == hp->ops)
    > > + if (consoles[i].vtermno == hp->vtermno &&
    > > + consoles[i].ops == hp->ops)
    > > break;
    > >
    >
    >
    > NACK
    > you broke this assertion:
    > > /*
    > > * Initial console vtermnos for console API usage prior to full console
    > > * initialization. Any vty adapter outside this range will not have usable
    > > * console interfaces but can still be used as a tty device. This has to be
    > > * static because kmalloc will not work during early console init.
    > > */

    Well. It speaks about "range", but in fact it was exactly one.

    > The idea is you might want serial port to 250 other partitions, but only
    > need to have your choice of console be on the first 8 or so.

    hvc is limited to 16 devices.

    Bastian

    --
    No one wants war.
    -- Kirk, "Errand of Mercy", stardate 3201.7


    \
     
     \ /
      Last update: 2008-07-30 12:09    [W:0.046 / U:1.032 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site