| Message ID | 20081024035434.GF4267@yookeroo.seuss (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Paul Mackerras |
| Headers | show |
David Gibson writes: > This patch adds a new backend for the hvc console based on the > low-level udbg callbacks. This effectively implements a working > runtime console in terms of the simple udbg primitives. This is kind > of a hack - since udbg isn't something you really want to be using > routinely - but it's really useful during bringup. > > This can be used to quickly implement a userspace usable console while > you're working on a proper driver for whatever console I/O device the > hardware has. Or, it can be used to avoid writing a full blown > tty/console driver entirely for quick-and-dirty I/O hardware that will > later be replaced by something else. Since only powerpc has udbg, shouldn't the Kconfig option depend on PPC? Paul.
On Thu, Oct 23, 2008 at 9:54 PM, David Gibson <david@gibson.dropbear.id.au> wrote: > This patch adds a new backend for the hvc console based on the > low-level udbg callbacks. This effectively implements a working > runtime console in terms of the simple udbg primitives. This is kind > of a hack - since udbg isn't something you really want to be using > routinely - but it's really useful during bringup. David, I have a stupid question. I already have an HVC console driver that works. Do I need to do anything, other than enable HVC_UDBG, to get my hvc console drive to use udbg?
On Thu, Oct 23, 2008 at 9:54 PM, David Gibson <david@gibson.dropbear.id.au> wrote: > This can be used to quickly implement a userspace usable console while > you're working on a proper driver for whatever console I/O device the > hardware has. Or, it can be used to avoid writing a full blown > tty/console driver entirely for quick-and-dirty I/O hardware that will > later be replaced by something else. Ok, I think I understand this better now. Your approach seems backwards to me. HVC console client drivers already have simple put/get functions. You've written an hvc driver that makes udbg calls. Wouldn't it have been better to make a udbg driver that makes hvc calls? That way, you effectively give udbg support to all hvc drivers in one shot. In order to support udbg in my hvc driver, I have to add udbg calls. In fact, now that I've thought about it, I don't understand what your driver does. You take hvc callbacks and route them through udbg, but this only works on drivers that have udbg callbacks in the first place. In that case, why would these drivers need an hvc middle-man?
On Thu, Oct 23, 2008 at 9:54 PM, David Gibson <david@gibson.dropbear.id.au> wrote: One other thing ... > --- working-2.6.orig/drivers/char/Makefile 2008-10-22 15:50:59.000000000 +1100 > +++ working-2.6/drivers/char/Makefile 2008-10-24 14:22:59.000000000 +1100 > @@ -50,6 +50,7 @@ obj-$(CONFIG_HVC_BEAT) += hvc_beat.o > obj-$(CONFIG_HVC_DRIVER) += hvc_console.o > obj-$(CONFIG_HVC_IRQ) += hvc_irq.o > obj-$(CONFIG_HVC_XEN) += hvc_xen.o > +obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o HVC console drivers must be compiled before hvc_console.o, so the above should look like this: obj-$(CONFIG_HVC_XEN) += hvc_xen.o obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o obj-$(CONFIG_HVC_DRIVER) += hvc_console.o obj-$(CONFIG_HVC_IRQ) += hvc_irq.o see http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git;a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389
On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote: > On Thu, Oct 23, 2008 at 9:54 PM, David Gibson > <david@gibson.dropbear.id.au> wrote: > > This patch adds a new backend for the hvc console based on the > > low-level udbg callbacks. This effectively implements a working > > runtime console in terms of the simple udbg primitives. This is kind > > of a hack - since udbg isn't something you really want to be using > > routinely - but it's really useful during bringup. > > David, I have a stupid question. I already have an HVC console driver > that works. Do I need to do anything, other than enable HVC_UDBG, to > get my hvc console drive to use udbg? Um.. well.. if you have both activated, I think you can select which HVC console backend will be used by using console=/dev/hvcNN on the commandline, where values of NN correspond to different backends, in order depending on link order in some complex fashion. But if you already have a working HVC console driver, I don't see why you'd want to use HVC_UDBG - it's essentially a bringup hack.
On Mon, Nov 17, 2008 at 02:04:38PM -0600, Timur Tabi wrote: > On Thu, Oct 23, 2008 at 9:54 PM, David Gibson > <david@gibson.dropbear.id.au> wrote: > > > This can be used to quickly implement a userspace usable console while > > you're working on a proper driver for whatever console I/O device the > > hardware has. Or, it can be used to avoid writing a full blown > > tty/console driver entirely for quick-and-dirty I/O hardware that will > > later be replaced by something else. > > Ok, I think I understand this better now. > > Your approach seems backwards to me. HVC console client drivers > already have simple put/get functions. You've written an hvc driver > that makes udbg calls. Wouldn't it have been better to make a udbg > driver that makes hvc calls? That way, you effectively give udbg > support to all hvc drivers in one shot. Well, except that hvc calls aren't necessarily safe in the context that udbg calls are made. The idea here is that you want to implement udbg during bringup anyway, to get really early debug info. This means that you can then use that udbg support to get a full console. > In order to support udbg in my hvc driver, I have to add udbg calls. > In fact, now that I've thought about it, I don't understand what your > driver does. You take hvc callbacks and route them through udbg, but > this only works on drivers that have udbg callbacks in the first > place. In that case, why would these drivers need an hvc middle-man? Because the udbg console works for kernel messages, but doesn't support a full tty interface, so you can't run userspace with only a udbg console.
On Mon, Nov 17, 2008 at 6:40 PM, David Gibson <david@gibson.dropbear.id.au> wrote: > Because the udbg console works for kernel messages, but doesn't > support a full tty interface, so you can't run userspace with only a > udbg console. I still don't understand the point of your driver. You're just routing hcalls to a udbg back-end. So if you have a driver that supports udbg but not standard console or TTY, then you can use your driver to get console/tty I/O. Is there a platform where such a setup exists? All HVC drivers support console and TTY, as do all serial drivers. So what platform would use your driver?
On Mon, Nov 17, 2008 at 10:42:46PM -0600, Timur Tabi wrote: > On Mon, Nov 17, 2008 at 6:40 PM, David Gibson > <david@gibson.dropbear.id.au> wrote: > > > Because the udbg console works for kernel messages, but doesn't > > support a full tty interface, so you can't run userspace with only a > > udbg console. > > I still don't understand the point of your driver. You're just > routing hcalls to a udbg back-end. So if you have a driver that > supports udbg but not standard console or TTY, then you can use your > driver to get console/tty I/O. Is there a platform where such a setup > exists? All HVC drivers support console and TTY, as do all serial > drivers. So what platform would use your driver? Like I said, it's a bringup hack. If you have hardware that's supported by an existing console driver, there's no reason you'd want it. For example, I've used it on a bringup prototype platform whose only I/O is a weird FPGA-implemented UART (not 8250 compatible, or even close). I implemented udbg callbacks as one of the first things I did, to get the early debugging. But eventually I got userspace up, and discovered that it wouldn't work on the udbg consoleuserspace won't work with the udbg console. I didn't really want to write another, more complex, driver for this strange UART, so I glued hvconsole to my existing udbg callbacks, giving me a userspace-capable console. Given the variety of strange I/O configurations in prototype and embedded platforms, I can't imagine this was a unique situation. So I've pushed my patch out, so anyone else in a similar situation can immediately turn their little udbg methods for whatever strange I/O they have into a fully-functional console. Maybe it's not something you'd want to go to release with, but it certainly simplifies life during bringup.
David Gibson wrote: > Given the variety of strange I/O configurations in prototype and > embedded platforms, I can't imagine this was a unique situation. So > I've pushed my patch out, so anyone else in a similar situation can > immediately turn their little udbg methods for whatever strange I/O > they have into a fully-functional console. Maybe it's not something > you'd want to go to release with, but it certainly simplifies life > during bringup. Ok, I understand now. However, I would like to see two changes: 1) Re-arrange the Makefile as I pointed out in another post. 2) Update the Kconfig help file to be very clear that this feature is only meaningful if the platform has a udbg back-end but no other console or TTY driver.
On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote: > David Gibson wrote: > > > Given the variety of strange I/O configurations in prototype and > > embedded platforms, I can't imagine this was a unique situation. So > > I've pushed my patch out, so anyone else in a similar situation can > > immediately turn their little udbg methods for whatever strange I/O > > they have into a fully-functional console. Maybe it's not something > > you'd want to go to release with, but it certainly simplifies life > > during bringup. > > Ok, I understand now. However, I would like to see two changes: > > 1) Re-arrange the Makefile as I pointed out in another post. Um.. yeah.. I'm a bit baffled by this.. all the existing backends are listed after hvc_console, I just added hvc_udbg to the end. I didn't really understand the rationale in that commit, but then I haven't had time to look at it very much yet. > 2) Update the Kconfig help file to be very clear that this feature > is only meaningful if the platform has a udbg back-end but no other > console or TTY driver. Alrighty...
David Gibson wrote: > Um.. yeah.. I'm a bit baffled by this.. all the existing backends > are listed after hvc_console, I just added hvc_udbg to the end. I > didn't really understand the rationale in that commit, but then I > haven't had time to look at it very much yet. No, some are before: obj-$(CONFIG_HVC_RTAS) += hvc_rtas.o <--- obj-$(CONFIG_HVC_BEAT) += hvc_beat.o <--- obj-$(CONFIG_HVC_DRIVER) += hvc_console.o obj-$(CONFIG_HVC_IRQ) += hvc_irq.o obj-$(CONFIG_HVC_XEN) += hvc_xen.o obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o Until your patch, only Xen was wrong.
[I'm going to reply to several points in this thread in one reply. I have restored context that was trimmed in later replys when I wanted to speak to.] David Gibson wrote at 2008-11-18 00:28:28: > On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote: >> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson >> <david@gibson.dropbear.id.au> wrote: >>> This patch adds a new backend for the hvc console based on the >>> low-level udbg callbacks. This effectively implements a working >>> runtime console in terms of the simple udbg primitives. This is kind >>> of a hack - since udbg isn't something you really want to be using >>> routinely - but it's really useful during bringup. Why is udbg hvc something you want to not use routinely? Stated differently, if your routine (1) fundamently works one character at a time and (2) is not interrupt driven, and (3) only supports one channel, what avantage is there to an explicit hvc driver? >>> This can be used to quickly implement a userspace usable console >>> while >>> you're working on a proper driver for whatever console I/O device the >>> hardware has. Or, it can be used to avoid writing a full blown >>> tty/console driver entirely for quick-and-dirty I/O hardware that >>> will >>> later be replaced by something else. Actually it looks remarkably similar to a cleaned up version of a patch i've been using since hvc_console was split to be a hookable shell. Or was it the motivation for adding the hooks? The code is different in how it structures the error checks and that it is implemented in drivers/char where it belongs (hence the cleaned up comment) but the fact that David recreated what I did does speak to its general utility. >> David, I have a stupid question. I already have an HVC console driver >> that works. Do I need to do anything, other than enable HVC_UDBG, to >> get my hvc console drive to use udbg? As was pointed out elsewhere, this is hvc calling udbg not udbg hooks calling hvc_cosole methods, which might need a different context. But I wanted to comment on David's reply: > Um.. well.. if you have both activated, I think you can select which > HVC console backend will be used by using console=/dev/hvcNN on the > commandline, where values of NN correspond to different backends, in > order depending on link order in some complex fashion. It doesn't just work that way because of the way the hvc_console shell implements the console hooks. If the drivers request different hvc channel numbers, then yes, you can select which hvc backend is used by choosing console=hvcNN. If all request 0, then which ever one registers with hvc_console first wins the slot, and the others get a busy error return. They can still register their tty later, at which point they get a dynamically determined /dev/hvcN slot, above all registered console slots. There are multiple reasons for this design. The original user, pseries vterm, wants to assign the channel number based on what the hypervisor tells it the channel should be. There are two loops though the device tree, searching for different protocols (and consequently different backends), so they need to be able to specify the number and not just use the order they are found. The platform describes not only the protocol, but which instance that is, and the backend currently registers with that number. Another reason is its a bad idea to have console=/dev/hvc0 depend on link order instead of knowing which driver is selected. If a user is specifing what the console is, it should not depend on other linked drivers. Until this point, all mainline drivers have been exclusive in that only one will actually register with hvc_console midlayer on any given platform. This is the first backend that is not exclusive, and therefore its coexistence needs special attention. In my internal tree, I register (my version of) this not as hvc0 but as hvc4. hvc0 is rtas, hvc1-3 are an internal backend, and this one is hvc4. The order I chose is arbitrary, but the main point is it does't compete with hvc_rtas for slot 0. (All of these drivers coexist, and I can choose which one I want to use based on my needs for that boot.) We could make it configurable via Kconfig, or just choose a slot. (I think vterm can have 2 channels on some boxes where they drive real serial ports on the box.) > But if you already have a working HVC console driver, I don't see why > you'd want to use HVC_UDBG - it's essentially a bringup hack. As I said, why should we need a fancier hvc_console backend if you are polling and gain no efficency processing multiple characters at once. The udbg drivers know if they are sharing the interface with a hvc_console some other driver. If we trigger the registration of this backend on the udbg hook saying its useful (eg by setting a flag word in the function where we assign the udbg putc and getc pointers) then this driver will not need to be compiled out for "production" kernels. David Gibson wrote at 2008-11-19 00:42:20: > On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote: >> David Gibson wrote: [at 2008-11-18 05:14:08] >>> Given the variety of strange I/O configurations in prototype and >>> embedded platforms, I can't imagine this was a unique situation. So >>> I've pushed my patch out, so anyone else in a similar situation can >>> immediately turn their little udbg methods for whatever strange I/O >>> they have into a fully-functional console. Maybe it's not something >>> you'd want to go to release with, but it certainly simplifies life >>> during bringup. >> >> Ok, I understand now. However, I would like to see two changes: >> >> 1) Re-arrange the Makefile as I pointed out in another post. [the other post] Timur Tabi - 2008-11-17 20:18:31 >> One other thing ... >> HVC console drivers must be compiled before hvc_console.o >> >> see >> http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git; >> a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389 [end other post] > > Um.. yeah.. I'm a bit baffled by this.. all the existing backends > are listed after hvc_console, I just added hvc_udbg to the end. I > didn't really understand the rationale in that commit, but then I > haven't had time to look at it very much yet. I too challanged the necessity of that change. I carefully designed the hvc_console layer to find the selected console weither it was registered before or after the console_initcall of hvc_console. I am very disapointed that the changelog is so sparse. However, I think the bottom line was that add_preferred_console is suppsoed to be called by architecture setup code. Doing it the console_initcall is almost abuse. But if the console_initcall is going to call add_preferred_console, then it must link before the hvc_console driver. I would have to go back and find the discussion to remember the exact details. (maybe it only needed to call add_preferreed_console before registering itself if its the only hvc backend). Which does bring up the point of avoinding calling add_preferred_console to udbg just because it is linked in. The udbg getc routines might work for xmon, but one would probably not like the result of the real driver and udbg trying to read the same device at the same time. Perhaps the same flag that says "consider me or udbg_hvc" should apply, or we do add_preferred_console in arch code before console_initcall. (its ok to preferr a console that never registers as long as another preferred console does register). >> 2) Update the Kconfig help file to be very clear that this feature >> is only meaningful if the platform has a udbg back-end but no other >> console or TTY driver. > > Alrighty... That does bring up the point, I don't see any kconfig help for this user-selectable option. And if we don't have the udbg hooks say to enable it, then we really need to have text discouraging its use as it might break their console. milton
Milton Miller wrote: > Stated differently, if your routine (1) fundamently works one character > at a time and (2) is not interrupt driven, and (3) only supports one > channel, what avantage is there to an explicit hvc driver? I think it's because HVC has the ability to use HVC client drivers for console and TTY, but udbg does not. So if you have a udbg-only driver, it will cease to function when the kernel switches to the normal console. Perhaps it would make more sense to have the kernel continue to use udbg if there is no normal console driver, instead of relying on David's driver. > I too challanged the necessity of that change. I carefully designed > the hvc_console layer to find the selected console weither it was > registered before or after the console_initcall of hvc_console. I'm just pointing out the patch that was committed in 2006 to address this issue. I discovered the same bug that the patch mentioned, and only after a long time searching did I find this patch. And sure enough, re-arranging the Makefile made the problem I was seeing go away. Now, it's possible that this bug in hvc_console no longer exists. However, I have seen nothing to indicate it. Unfortunately, I don't remember enough of the details to reproduce the problem, so I can't verify it. > However, I think the bottom line was that add_preferred_console is > suppsoed to be called by architecture setup code. Doing it the > console_initcall is almost abuse. But if the console_initcall is going > to call add_preferred_console, then it must link before the hvc_console > driver. I would have to go back and find the discussion to remember > the exact details. (maybe it only needed to call > add_preferreed_console before registering itself if its the only hvc > backend). Well, I can't comment on your theory, but I do know that the add_preferred_console() call should be made from the HVC client driver, or at least HVC itself. It should not be in the platform code, because then we have a dependency between the driver and the platform code. I prefer my drivers to initialize everything on their own, without the help of the platform code.
On Thu, Nov 20, 2008 at 01:16:27PM -0600, Timur Tabi wrote: > Milton Miller wrote: > > > Stated differently, if your routine (1) fundamently works one character > > at a time and (2) is not interrupt driven, and (3) only supports one > > channel, what avantage is there to an explicit hvc driver? > > I think it's because HVC has the ability to use HVC client drivers > for console and TTY, but udbg does not. So if you have a udbg-only > driver, it will cease to function when the kernel switches to the > normal console. > > Perhaps it would make more sense to have the kernel continue to use udbg if > there is no normal console driver, instead of relying on David's > driver. This doesn't work - the udbg console can't support a full tty driver which is needed by userspace. Originally, I was going to extend the udbg console so that it supported a tty backended onto the udbg methods. Then I realised that the code I'd need to do this would be essentially identical to hvconsole, so why duplicate it.
On Thu, Nov 20, 2008 at 01:07:11PM -0600, Milton Miller wrote: > [I'm going to reply to several points in this thread in one reply. I > have restored context that was trimmed in later replys when I wanted to > speak to.] > > David Gibson wrote at 2008-11-18 00:28:28: >> On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote: >>> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson >>> <david@gibson.dropbear.id.au> wrote: >>>> This patch adds a new backend for the hvc console based on the >>>> low-level udbg callbacks. This effectively implements a working >>>> runtime console in terms of the simple udbg primitives. This is kind >>>> of a hack - since udbg isn't something you really want to be using >>>> routinely - but it's really useful during bringup. > > Why is udbg hvc something you want to not use routinely? Well.. because the udbg methods themselves are not sometimes (not always, but reasonably often), something you don't really want to use except during bringup. A number of them use various hacks that lets them work very early, and very reliably even when much of the kernel is borked, but aren't the sort of thing you want to do except during bringup. e.g. being mapped by special bolted TLB entries established very early, rather than using ioremap(); polled access for interrupt capable devices; hard-coded device addresses, meaning the kernel will only work on one precise type of system. > Stated differently, if your routine (1) fundamently works one character > at a time and (2) is not interrupt driven, and (3) only supports one > channel, what avantage is there to an explicit hvc driver? Using standard kernel interfaces, instead of random early debug hacks. >>>> This can be used to quickly implement a userspace usable console >>>> while >>>> you're working on a proper driver for whatever console I/O device the >>>> hardware has. Or, it can be used to avoid writing a full blown >>>> tty/console driver entirely for quick-and-dirty I/O hardware that >>>> will >>>> later be replaced by something else. > > Actually it looks remarkably similar to a cleaned up version of a patch > i've been using since hvc_console was split to be a hookable shell. Or > was it the motivation for adding the hooks? The code is different in > how it structures the error checks and that it is implemented in > drivers/char where it belongs (hence the cleaned up comment) but the > fact that David recreated what I did does speak to its general > utility. Heh, ok. >>> David, I have a stupid question. I already have an HVC console driver >>> that works. Do I need to do anything, other than enable HVC_UDBG, to >>> get my hvc console drive to use udbg? > > As was pointed out elsewhere, this is hvc calling udbg not udbg hooks > calling hvc_cosole methods, which might need a different context. But I > wanted to comment on David's reply: > >> Um.. well.. if you have both activated, I think you can select which >> HVC console backend will be used by using console=/dev/hvcNN on the >> commandline, where values of NN correspond to different backends, in >> order depending on link order in some complex fashion. > > It doesn't just work that way because of the way the hvc_console shell > implements the console hooks. > > If the drivers request different hvc channel numbers, then yes, you can > select which hvc backend is used by choosing console=hvcNN. If all > request 0, then which ever one registers with hvc_console first wins the > slot, and the others get a busy error return. They can still register > their tty later, at which point they get a dynamically determined > /dev/hvcN slot, above all registered console slots. Ah, ok, my mistake. > There are multiple reasons for this design. The original user, pseries > vterm, wants to assign the channel number based on what the hypervisor > tells it the channel should be. There are two loops though the device > tree, searching for different protocols (and consequently different > backends), so they need to be able to specify the number and not just > use the order they are found. The platform describes not only the > protocol, but which instance that is, and the backend currently > registers with that number. Another reason is its a bad idea to have > console=/dev/hvc0 depend on link order instead of knowing which driver > is selected. If a user is specifing what the console is, it should not > depend on other linked drivers. > > Until this point, all mainline drivers have been exclusive in that only > one will actually register with hvc_console midlayer on any given > platform. This is the first backend that is not exclusive, and > therefore its coexistence needs special attention. > > In my internal tree, I register (my version of) this not as hvc0 but as > hvc4. hvc0 is rtas, hvc1-3 are an internal backend, and this one is > hvc4. The order I chose is arbitrary, but the main point is it does't > compete with hvc_rtas for slot 0. (All of these drivers coexist, and I > can choose which one I want to use based on my needs for that boot.) We > could make it configurable via Kconfig, or just choose a slot. (I think > vterm can have 2 channels on some boxes where they drive real serial > ports on the box.) Hrm, ok. Not sure what the sensible way to do this for my case would be. >> But if you already have a working HVC console driver, I don't see why >> you'd want to use HVC_UDBG - it's essentially a bringup hack. > > As I said, why should we need a fancier hvc_console backend if you are > polling and gain no efficency processing multiple characters at once. > > The udbg drivers know if they are sharing the interface with a > hvc_console some other driver. If we trigger the registration of this > backend on the udbg hook saying its useful (eg by setting a flag word in > the function where we assign the udbg putc and getc pointers) then this > driver will not need to be compiled out for "production" kernels. > > David Gibson wrote at 2008-11-19 00:42:20: >> On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote: >>> David Gibson wrote: [at 2008-11-18 05:14:08] >>>> Given the variety of strange I/O configurations in prototype and >>>> embedded platforms, I can't imagine this was a unique situation. So >>>> I've pushed my patch out, so anyone else in a similar situation can >>>> immediately turn their little udbg methods for whatever strange I/O >>>> they have into a fully-functional console. Maybe it's not something >>>> you'd want to go to release with, but it certainly simplifies life >>>> during bringup. >>> >>> Ok, I understand now. However, I would like to see two changes: >>> >>> 1) Re-arrange the Makefile as I pointed out in another post. > [the other post] > Timur Tabi - 2008-11-17 20:18:31 >>> One other thing ... >>> HVC console drivers must be compiled before hvc_console.o >>> >>> see http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git; >>> a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389 > [end other post] >> >> Um.. yeah.. I'm a bit baffled by this.. all the existing backends >> are listed after hvc_console, I just added hvc_udbg to the end. I >> didn't really understand the rationale in that commit, but then I >> haven't had time to look at it very much yet. > > I too challanged the necessity of that change. I carefully designed the > hvc_console layer to find the selected console weither it was registered > before or after the console_initcall of hvc_console. > > I am very disapointed that the changelog is so sparse. Uh... but you're the author of that commit...? > However, I think the bottom line was that add_preferred_console is > suppsoed to be called by architecture setup code. Doing it the > console_initcall is almost abuse. But if the console_initcall is going > to call add_preferred_console, then it must link before the hvc_console > driver. I would have to go back and find the discussion to remember > the exact details. (maybe it only needed to call add_preferreed_console > before registering itself if its the only hvc backend). > > Which does bring up the point of avoinding calling add_preferred_console > to udbg just because it is linked in. The udbg getc routines might work > for xmon, but one would probably not like the result of the real driver > and udbg trying to read the same device at the same time. Perhaps the > same flag that says "consider me or udbg_hvc" should apply, or we do > add_preferred_console in arch code before console_initcall. (its ok to > preferr a console that never registers as long as another preferred > console does register). Ah, yeah. I'm not sure if my hvc_udbg really wants an add_preferred_console. It's there because it was in hvc_rtas or whichever other hvc backend I based this on (forgotten which, now). And the semantics of console selection makes my brain hurt, so I'm not sure if it's actually needed for this case or not. >>> 2) Update the Kconfig help file to be very clear that this feature >>> is only meaningful if the platform has a udbg back-end but no other >>> console or TTY driver. >> >> Alrighty... > > That does bring up the point, I don't see any kconfig help for this > user-selectable option. Yeah, that's a good point. > And if we don't have the udbg hooks say to enable it, then we really > need to have text discouraging its use as it might break their console. Possibly it should just be dependent on CONFIG_PPC_EARLY_DEBUG.
On Nov 20, 2008, at 1:16 PM, Timur Tabi wrote: > Milton Miller wrote: > >> Stated differently, if your routine (1) fundamently works one >> character >> at a time and (2) is not interrupt driven, and (3) only supports one >> channel, what avantage is there to an explicit hvc driver? > > I think it's because HVC has the ability to use HVC client drivers for > console > and TTY, but udbg does not. So if you have a udbg-only driver, it > will cease to > function when the kernel switches to the normal console. > > Perhaps it would make more sense to have the kernel continue to use > udbg if > there is no normal console driver, instead of relying on David's > driver. First, we have that option: just specify udbg-imortal on the command line, and the udbg console driver removes CON_BOOT from its flags. Second, as David said in his reply, that doesn't let your userspace have a tty, which means no job control. >> I too challanged the necessity of that change. I carefully designed >> the hvc_console layer to find the selected console weither it was >> registered before or after the console_initcall of hvc_console. > > I'm just pointing out the patch that was committed in 2006 to address > this > issue. I discovered the same bug that the patch mentioned, and only > after a > long time searching did I find this patch. And sure enough, > re-arranging the > Makefile made the problem I was seeing go away. > > Now, it's possible that this bug in hvc_console no longer exists. > However, I > have seen nothing to indicate it. Unfortunately, I don't remember > enough of the > details to reproduce the problem, so I can't verify it. As I said, I conceeded then the patch was required the way things worked. There was another propsed patch a few months ago (3-9) that I pointed out they needed to be fixing the common console code; I don't remember seeing anything related after that but I could have missed it. >> However, I think the bottom line was that add_preferred_console is >> suppsoed to be called by architecture setup code. Doing it the >> console_initcall is almost abuse. But if the console_initcall is >> going >> to call add_preferred_console, then it must link before the >> hvc_console >> driver. I would have to go back and find the discussion to remember >> the exact details. (maybe it only needed to call >> add_preferreed_console before registering itself if its the only hvc >> backend). > > Well, I can't comment on your theory, but I do know that the > add_preferred_console() call should be made from the HVC client > driver, or at > least HVC itself. It should not be in the platform code, because then > we have a > dependency between the driver and the platform code. I prefer my > drivers to > initialize everything on their own, without the help of the platform > code. Think about this for a second. If every console driver said "prefer me", what point is there in having add_preferred_console? The second reason is its too late. We have already started scanning for drivers. We are asking the console layer to adjust its choices after it has started scanning. We want the last console= parameter on the command line to win. So if that implys the last call to add_preferred_console wins, then you have code overriding the command line. We have code that searches the device tree for the stdout that firmware used. That code then tells the console layer which driver to choose when it recognises it. The pSeries platform also has code that says when it finds vterm channels under lpar, then it should choose it. I think we should have the code that sets the udbg method indicate that the console should be preferred. While its not really arch code, it is architecture code in that its hooked from early discovrey based not cosole_initcall based (which runs before arch_initcall). This is also the point where we know if the udbg method is sharing hardware with a full driver and therefore udbg_hvc should not be used. milton
Milton Miller wrote: > We want the last console= parameter on the command line to win. So if > that implys the last call to add_preferred_console wins, then you have > code overriding the command line. Hmm, good point. However, how likely is it that we'll have more than one console driver? Also, without calling add_preferred_console(), the kernel needs to have a console= on the command line. In my case, my driver only calls add_preferred_console() if the device tree contains a specific property that it looks for. In effect, this property override the console= line. However, the console= line goes to the HVC subsystem, and not my driver, and I can't use it to send the configuration data the driver needs. Unfortunately, my driver hasn't been published yet, so it's hard to explain the details. I guess I need to think about this more.
On Nov 21, 2008, at 10:13 AM, Timur Tabi wrote: > Milton Miller wrote: >> We want the last console= parameter on the command line to win. So if >> that implys the last call to add_preferred_console wins, then you have >> code overriding the command line. > > Hmm, good point. However, how likely is it that we'll have more than > one > console driver? For anything than custom embedded configs, quite likely. As I said, I have 3 hvc clients, but that is unusual. But frame buffer, vterm, rtas, and serial console would be a typical mix for a distribution. > Also, without calling add_preferred_console(), the kernel needs > to have a console= on the command line. I'm not arguing against the call, I'm arguing when/from where it should be made. > In my case, my driver only calls add_preferred_console() if the device > tree > contains a specific property that it looks for. In effect, this > property > override the console= line. However, the console= line goes to the HVC > subsystem, and not my driver, and I can't use it to send the > configuration data > the driver needs. Discovering the hardware from the device tree and triggering your regitration is the right approach. I think if you discover from an early setup then you can go before the command line. As far as getting parameters, you are talking like ttyS0,9600,8,n,1 ? If you go after hvc_console then you could add a patch for hvc_console to remember the setup and return it to possible clients. > Unfortunately, my driver hasn't been published yet, so it's hard to > explain the > details. I guess I need to think about this more. I dont' know what details you would want on the console= command line that you should not have in the device tree. If its which hypervisor channel, then I would think just choosing your hvc number accordingly would work. But I can only make wild guesses without details. milton
On Nov 20, 2008, at 6:35 PM, David Gibson wrote: > On Thu, Nov 20, 2008 at 01:07:11PM -0600, Milton Miller wrote: >> David Gibson wrote at 2008-11-18 00:28:28: >>> On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote: >>>> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson >>>> <david@gibson.dropbear.id.au> wrote: >>>>> This patch adds a new backend for the hvc console based on the >>>>> low-level udbg callbacks. This effectively implements a working >>>>> runtime console in terms of the simple udbg primitives. This is >>>>> kind >>>>> of a hack - since udbg isn't something you really want to be using >>>>> routinely - but it's really useful during bringup. >> >> Why is udbg hvc something you want to not use routinely? > > Well.. because the udbg methods themselves are not sometimes (not > always, but reasonably often), something you don't really want to use > except during bringup. A number of them use various hacks that lets > them work very early, and very reliably even when much of the kernel > is borked, but aren't the sort of thing you want to do except during > bringup. e.g. being mapped by special bolted TLB entries established > very early, rather than using ioremap(); polled access for interrupt > capable devices; hard-coded device addresses, meaning the kernel will > only work on one precise type of system. You are describing the CONFIG_PPC_DEBUG_EARLY udbg methods, but they are far from all of the available methods. Several methods use the standard kernel apis, or do not need them (rtas, vterm). If we had this, would we need hvc_rtas? >> Stated differently, if your routine (1) fundamently works one >> character >> at a time and (2) is not interrupt driven, and (3) only supports one >> channel, what avantage is there to an explicit hvc driver? > > Using standard kernel interfaces, instead of random early debug hacks. I was thinking of the less hackish udbg hooks that registsered in the platform setup timeframe. Not requiring a pinned tlb that you would not want for io performance anyways I can see. >> >> Actually it looks remarkably similar to a cleaned up version of a >> patch >> i've been using since hvc_console was split to be a hookable shell. >> Or >> was it the motivation for adding the hooks? > > Heh, ok. ... >>> Um.. well.. if you have both activated, I think you can select which >>> HVC console backend will be used by using console=/dev/hvcNN on the >>> commandline, where values of NN correspond to different backends, in >>> order depending on link order in some complex fashion. >> >> It doesn't just work that way because of the way the hvc_console shell >> implements the console hooks. >> >> If the drivers request different hvc channel numbers, then yes, you >> can >> select which hvc backend is used by choosing console=hvcNN. If all >> request 0, then which ever one registers with hvc_console first wins >> the >> slot, and the others get a busy error return. They can still >> register >> their tty later, at which point they get a dynamically determined >> /dev/hvcN slot, above all registered console slots. > > Ah, ok, my mistake. > >> There are multiple reasons for this design. The original user, >> pseries >> vterm, wants to assign the channel number based on what the hypervisor >> tells it the channel should be. There are two loops though the device >> tree, searching for different protocols (and consequently different >> backends), so they need to be able to specify the number and not just >> use the order they are found. The platform describes not only the >> protocol, but which instance that is, and the backend currently >> registers with that number. Another reason is its a bad idea to have >> console=/dev/hvc0 depend on link order instead of knowing which driver >> is selected. If a user is specifing what the console is, it should >> not >> depend on other linked drivers. >> >> Until this point, all mainline drivers have been exclusive in that >> only >> one will actually register with hvc_console midlayer on any given >> platform. This is the first backend that is not exclusive, and >> therefore its coexistence needs special attention. >> >> In my internal tree, I register (my version of) this not as hvc0 but >> as >> hvc4. hvc0 is rtas, hvc1-3 are an internal backend, and this one is >> hvc4. The order I chose is arbitrary, but the main point is it does't >> compete with hvc_rtas for slot 0. (All of these drivers coexist, and >> I >> can choose which one I want to use based on my needs for that boot.) >> We >> could make it configurable via Kconfig, or just choose a slot. (I >> think >> vterm can have 2 channels on some boxes where they drive real serial >> ports on the box.) > > Hrm, ok. Not sure what the sensible way to do this for my case would > be. > >>> But if you already have a working HVC console driver, I don't see why >>> you'd want to use HVC_UDBG - it's essentially a bringup hack. >> >> As I said, why should we need a fancier hvc_console backend if you are >> polling and gain no efficency processing multiple characters at once. >> >> The udbg drivers know if they are sharing the interface with a >> hvc_console some other driver. If we trigger the registration of this >> backend on the udbg hook saying its useful (eg by setting a flag word >> in >> the function where we assign the udbg putc and getc pointers) then >> this >> driver will not need to be compiled out for "production" kernels. >> >> David Gibson wrote at 2008-11-19 00:42:20: >>> On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote: >>>> David Gibson wrote: [at 2008-11-18 05:14:08] >>>>> Given the variety of strange I/O configurations in prototype and >>>>> embedded platforms, I can't imagine this was a unique situation. >>>>> So >>>>> I've pushed my patch out, so anyone else in a similar situation can >>>>> immediately turn their little udbg methods for whatever strange I/O >>>>> they have into a fully-functional console. Maybe it's not >>>>> something >>>>> you'd want to go to release with, but it certainly simplifies life >>>>> during bringup. >>>> >>>> Ok, I understand now. However, I would like to see two changes: >>>> >>>> 1) Re-arrange the Makefile as I pointed out in another post. >> [the other post] >> Timur Tabi - 2008-11-17 20:18:31 >>>> One other thing ... >>>> HVC console drivers must be compiled before hvc_console.o >>>> >>>> see http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git; >>>> a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389 >> [end other post] >>> >>> Um.. yeah.. I'm a bit baffled by this.. all the existing backends >>> are listed after hvc_console, I just added hvc_udbg to the end. I >>> didn't really understand the rationale in that commit, but then I >>> haven't had time to look at it very much yet. >> >> I too challanged the necessity of that change. I carefully designed >> the >> hvc_console layer to find the selected console weither it was >> registered >> before or after the console_initcall of hvc_console. >> >> I am very disapointed that the changelog is so sparse. > > Uh... but you're the author of that commit...? Hmm, it would appear I was the author of the patch. I'm still disapointed, maybe my standards are higher after a few years. I was thinking that was externally triggered. >> However, I think the bottom line was that add_preferred_console is >> suppsoed to be called by architecture setup code. Doing it the >> console_initcall is almost abuse. But if the console_initcall is >> going >> to call add_preferred_console, then it must link before the >> hvc_console >> driver. I would have to go back and find the discussion to remember >> the exact details. (maybe it only needed to call >> add_preferreed_console >> before registering itself if its the only hvc backend). >> >> Which does bring up the point of avoinding calling >> add_preferred_console >> to udbg just because it is linked in. The udbg getc routines might >> work >> for xmon, but one would probably not like the result of the real >> driver >> and udbg trying to read the same device at the same time. Perhaps the >> same flag that says "consider me or udbg_hvc" should apply, or we do >> add_preferred_console in arch code before console_initcall. (its ok >> to >> preferr a console that never registers as long as another preferred >> console does register). > > Ah, yeah. I'm not sure if my hvc_udbg really wants an > add_preferred_console. It's there because it was in hvc_rtas or > whichever other hvc backend I based this on (forgotten which, now). > And the semantics of console selection makes my brain hurt, so I'm not > sure if it's actually needed for this case or not. I think part of the problem is dummy_console ends up as a default too often. I'm not quite sure what its purpose really is. I'm not actually sure it is the problem either. >>>> 2) Update the Kconfig help file to be very clear that this feature >>>> is only meaningful if the platform has a udbg back-end but no other >>>> console or TTY driver. >>> >>> Alrighty... >> >> That does bring up the point, I don't see any kconfig help for this >> user-selectable option. > > Yeah, that's a good point. > >> And if we don't have the udbg hooks say to enable it, then we really >> need to have text discouraging its use as it might break their >> console. > > Possibly it should just be dependent on CONFIG_PPC_EARLY_DEBUG. If you do that then I will have to patch it back out. I choose which hvc based on I think that is too restrictive. I use udbg as one of my hvc channels, but I don't use PPC_EARLY_DEBUG, that requires too much platform knowledge. A simple flag that says 0 = only udbg-console, 1 = allow hvc_udbg, 2 = prefer hvc_udbg that is set with the udbg hooks would suffice for me. That's all I have time to write at the moment, I might reply to more later. milton
Index: working-2.6/drivers/char/Kconfig =================================================================== --- working-2.6.orig/drivers/char/Kconfig 2008-10-22 16:01:39.000000000 +1100 +++ working-2.6/drivers/char/Kconfig 2008-10-24 14:22:59.000000000 +1100 @@ -631,6 +631,12 @@ config HVC_XEN help Xen virtual console device driver +config HVC_UDBG + bool "udbg based fake hypervisor console" + depends on EXPERIMENTAL + select HVC_DRIVER + default n + config VIRTIO_CONSOLE tristate "Virtio console" depends on VIRTIO Index: working-2.6/drivers/char/Makefile =================================================================== --- working-2.6.orig/drivers/char/Makefile 2008-10-22 15:50:59.000000000 +1100 +++ working-2.6/drivers/char/Makefile 2008-10-24 14:22:59.000000000 +1100 @@ -50,6 +50,7 @@ obj-$(CONFIG_HVC_BEAT) += hvc_beat.o obj-$(CONFIG_HVC_DRIVER) += hvc_console.o obj-$(CONFIG_HVC_IRQ) += hvc_irq.o obj-$(CONFIG_HVC_XEN) += hvc_xen.o +obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o obj-$(CONFIG_VIRTIO_CONSOLE) += virtio_console.o obj-$(CONFIG_RAW_DRIVER) += raw.o obj-$(CONFIG_SGI_SNSC) += snsc.o snsc_event.o Index: working-2.6/drivers/char/hvc_udbg.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ working-2.6/drivers/char/hvc_udbg.c 2008-10-24 14:38:55.000000000 +1100 @@ -0,0 +1,96 @@ +/* + * udbg interface to hvc_console.c + * + * (C) Copyright David Gibson, IBM Corporation 2008. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <linux/console.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/moduleparam.h> +#include <linux/types.h> +#include <linux/irq.h> + +#include <asm/udbg.h> + +#include "hvc_console.h" + +struct hvc_struct *hvc_udbg_dev; + +static int hvc_udbg_put(uint32_t vtermno, const char *buf, int count) +{ + int i; + + for (i = 0; i < count; i++) + udbg_putc(buf[i]); + + return i; +} + +static int hvc_udbg_get(uint32_t vtermno, char *buf, int count) +{ + int i, c; + + if (!udbg_getc_poll) + return 0; + + for (i = 0; i < count; i++) { + if ((c = udbg_getc_poll()) == -1) + break; + buf[i] = c; + } + + return i; +} + +static struct hv_ops hvc_udbg_ops = { + .get_chars = hvc_udbg_get, + .put_chars = hvc_udbg_put, +}; + +static int __init hvc_udbg_init(void) +{ + struct hvc_struct *hp; + + BUG_ON(hvc_udbg_dev); + + hp = hvc_alloc(0, NO_IRQ, &hvc_udbg_ops, 16); + if (IS_ERR(hp)) + return PTR_ERR(hp); + + hvc_udbg_dev = hp; + + return 0; +} +module_init(hvc_udbg_init); + +static void __exit hvc_udbg_exit(void) +{ + if (hvc_udbg_dev) + hvc_remove(hvc_udbg_dev); +} +module_exit(hvc_udbg_exit); + +static int __init hvc_udbg_console_init(void) +{ + hvc_instantiate(0, 0, &hvc_udbg_ops); + add_preferred_console("hvc", 0, NULL); + + return 0; +} +console_initcall(hvc_udbg_console_init);
This patch adds a new backend for the hvc console based on the low-level udbg callbacks. This effectively implements a working runtime console in terms of the simple udbg primitives. This is kind of a hack - since udbg isn't something you really want to be using routinely - but it's really useful during bringup. This can be used to quickly implement a userspace usable console while you're working on a proper driver for whatever console I/O device the hardware has. Or, it can be used to avoid writing a full blown tty/console driver entirely for quick-and-dirty I/O hardware that will later be replaced by something else. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>