Skip to contentSkip to conversation

interrupt driven hvc_console as vio device

PATCH In review Ryan Arnold 3 participants 6 messages Aug 25, 2004 Char and misc drivers

lore
Share & tools
lore

Ryan Arnold

author
#0 lore raw

PATCH 1/1 interrupt driven hvc_console as vio device

Andrew, Paul,

This is an hvc_console patch which provides driver and ppc64 architecture fixes to enable the hvc_console driver to register itself as a vio device with the vio bus, provide hotplug add/remove for vty adapters, and act as an interrupt driven driver on Power-5 hardware or remain as a polling driver on Power-4 hardware.

Paul MacKerras should probably approve this patch since it includes arch/ppc64/ changes.

Ben Herrenschmidt wrote the original patch for the Ameslab tree, which provided the int-driven or poll driven console that this patch is extends upon. He had me add his 'signed-off-by' line.

Changelog:

arch/ppc64/kernel/hvconsole.c
-----------------------------------------------------------------------
-Changed hvc_get_chars() and hvc_put_chars() api to take vtermno rather than index number.

-Added hvc_find_vtys() function which walks the bus looking for vterm/vty devices to callback to the hvc_console driver. This provides console output functionality prior to early console init (pre mem init and pre device probe).

include/asm-ppc64/hvconsole.h
-----------------------------------------------------------------------

-Changed hvc_get_chars() and hvc_put_chars() api to take vtermno rather than index number.

-Added hvc_find_vtys() function.

-Added hvc_instantiate() function which is implemented by a console driver wanting to receive a callback of and early console init.

drivers/char/hvc_console.c
-----------------------------------------------------------------------

-Switch khvcd from kernel_threads to kthreads which got rid of deprecated daemonize().

-Added module exit clause to be thorough (not terribly necessary with a console driver of course)

-Added early discovery of vterm/vty adapters by doing a bus walk on early console init which results in hvc_instantiate() callback and addition of the vtermno into a static array of vtermnos supported as console adapters (meaning the console api's work against these vtermnos prior to full console initialization).

-This driver is now registered as a vio driver which means that vty adapters are now managed via probe/remove. This means hvc_console supports hotplug vty adapters.

-Driver now requests more device nodes than what was found on the initial bus walk when registered as a tty driver to make room for hotplug vty adapters. These secondary vty adapters provide a tty tunnel between partitions.

-Removed static hvc_struct array and replaced with a linux list that has elements (hvc_struct instances) added/removed on probe/remove AFTER early console init. This is important because kmalloc can't be done at early console init.

-Driver now either runs in interrupt driven mode or in polling mode on older hardware. The khvcd is smart enough to not 'schedule()' when there are no interrupts.

-kobjects are now used for ref counting on the hvc_struct instances.

This patch was tested on both Power-4 and Power-5 hardware. Thanks,

Ryan S. Arnold
IBM Linux Technology Center

Signed-off-by Benjamin Herrenschmidt <[email protected]>
Signed-off-by Ryan S. Arnold <[email protected]>
arch/ppc64/kernel/hvconsole.c +50 -22
@@ -1,6 +1,10 @@
 /*
  * hvconsole.c
  * Copyright (C) 2004 Hollis Blanchard, IBM Corporation
+ * Copyright (C) 2004 IBM Corporation
+ *
+ * Additional Author(s):
+ *  Ryan S. Arnold <[email protected]>
  *
  * LPAR console support.
  * 

@@ -22,14 +26,22 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <asm/hvcall.h>
-#include <asm/prom.h>
 #include <asm/hvconsole.h>
+#include <asm/prom.h>
 
-int hvc_get_chars(int index, char *buf, int count)
+/**
+ * hvc_get_chars - retrieve characters from firmware for denoted vterm adatper
+ * @vtermno: The vtermno or unit_address of the adapter from which to fetch the
+ *	data.
+ * @buf: The character buffer into which to put the character data fetched from
+ *	firmware.
+ * @count: not used?
+ */
+int hvc_get_chars(uint32_t vtermno, char *buf, int count)
 {
 	unsigned long got;
 
-	if (plpar_hcall(H_GET_TERM_CHAR, index, 0, 0, 0, &got,
+	if (plpar_hcall(H_GET_TERM_CHAR, vtermno, 0, 0, 0, &got,
 		(unsigned long *)buf, (unsigned long *)buf+1) == H_Success) {
 		/*
 		 * Work around a HV bug where it gives us a null

@@ -53,41 +65,57 @@
 
 EXPORT_SYMBOL(hvc_get_chars);
 
-int hvc_put_chars(int index, const char *buf, int count)
+/**
+ * hvc_put_chars: send characters to firmware for denoted vterm adapter
+ * @vtermno: The vtermno or unit_address of the adapter from which the data
+ *	originated.
+ * @buf: The character buffer that contains the character data to send to
+ *	firmware.
+ * @count: Send this number of characters.
+ */
+int hvc_put_chars(uint32_t vtermno, const char *buf, int count)
 {
 	unsigned long *lbuf = (unsigned long *) buf;
 	long ret;
 
-	ret = plpar_hcall_norets(H_PUT_TERM_CHAR, index, count, lbuf[0],
+	ret = plpar_hcall_norets(H_PUT_TERM_CHAR, vtermno, count, lbuf[0],
 				 lbuf[1]);
 	if (ret == H_Success)
 		return count;
 	if (ret == H_Busy)
 		return 0;
-	return -1;
+	return -EIO;
 }
 
 EXPORT_SYMBOL(hvc_put_chars);
 
-/* return the number of client vterms present */
-/* XXX this requires an interface change to handle multiple discontiguous
- * vterms */
-int hvc_count(int *start_termno)
+/*
+ * We hope/assume that the first vty found corresponds to the first console
+ * device.
+ */
+int hvc_find_vtys(void)
 {
 	struct device_node *vty;
-	int num_found = 0;
+	int count = 0;
 
-	/* consider only the first vty node.
-	 * we should _always_ be able to find one. */
-	vty = of_find_node_by_name(NULL, "vty");
-	if (vty && device_is_compatible(vty, "hvterm1")) {
-		u32 *termno = (u32 *)get_property(vty, "reg", NULL);
-
-		if (termno && start_termno)
-			*start_termno = *termno;
-		num_found = 1;
-		of_node_put(vty);
+	for (vty = of_find_node_by_name(NULL, "vty"); vty != NULL;
+			vty = of_find_node_by_name(vty, "vty")) {
+		uint32_t *vtermno;
+
+		vtermno = (uint32_t *)get_property(vty, "reg", NULL);
+		if (!vtermno)
+			continue;
+
+		/* We have statically defined space for only a certain number of
+		 * console adapters. */
+		if (count >= MAX_NR_HVC_CONSOLES)
+			break;
+
+		if (device_is_compatible(vty, "hvterm1")) {
+			hvc_instantiate(*vtermno, count);
+			count++;
+		}
 	}
 
-	return num_found;
+	return count;
 }
include/asm-ppc64/hvconsole.h +14 -4
@@ -22,9 +22,19 @@
 #ifndef _PPC64_HVCONSOLE_H
 #define _PPC64_HVCONSOLE_H
 
-extern int hvc_get_chars(int index, char *buf, int count);
-extern int hvc_put_chars(int index, const char *buf, int count);
-extern int hvc_count(int *start_termno);
+/*
+ * This is the max number of console adapters that can/will be found as
+ * console devices on first stage console init.  Any number beyond this range
+ * can't be used as a console device but is still a valid tty device.
+ */
+#define MAX_NR_HVC_CONSOLES	16
 
-#endif /* _PPC64_HVCONSOLE_H */
+extern int hvc_get_chars(uint32_t vtermno, char *buf, int count);
+extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count);
+
+/* Early discovery of console adapters. */
+extern int hvc_find_vtys(void);
 
+/* Implemented by a console driver */
+extern int hvc_instantiate(uint32_t vtermno, int index);
+#endif /* _PPC64_HVCONSOLE_H */
drivers/char/hvc_console.c +608 -138

941 lines not on this page · raw message

#1 lore raw
Quoted 9 lines · Ryan Arnold

static void hvc_close(struct tty_struct *tty, struct file * filp)
{
...
+ while (hp->n_outbuf) {
+ spin_unlock_irqrestore(&hp->lock, flags);
+ yield();
+ spin_lock_irqsave(&hp->lock, flags);
+ }

ick.

I suspect that if the caller of hvc_close() has realtime scheduling policy, this locks up. Unless it's waiting for interrupt activity.

Really, a real sleep/wakeup would be tons better.

author
#2 lore raw
Quoted 9 lines · Andrew Morton
Quoted 8 lines

static void hvc_close(struct tty_struct *tty, struct file * filp)
{
...
+ while (hp->n_outbuf) {
+ spin_unlock_irqrestore(&hp->lock, flags);
+ yield();
+ spin_lock_irqsave(&hp->lock, flags);
+ }

Quoted 6 lines

ick.

I suspect that if the caller of hvc_close() has realtime scheduling policy, this locks up. Unless it's waiting for interrupt activity.

Really, a real sleep/wakeup would be tons better.

Paulus suggested that tty_wait_until_sent() would be most appropriate since it actually does a sleep/wakeup. I think I'll rearrange this function a bit so that I won't have to drop the spin_lock and then grab it again as well. Thanks for the suggestion.

Ryan S. Arnold
IBM Linux Technology Center

#3 lore raw
In reply to Ryan Arnold
Quoted 20 lines · Ryan Arnold

static int hvc_write_room(struct tty_struct *tty)
{
struct hvc_struct *hp = tty->driver_data;
+ unsigned long flags;
+ int retval;

- return N_OUTBUF - hp->n_outbuf;
+ spin_lock_irqsave(&hp->lock, flags);
+ retval = N_OUTBUF - hp->n_outbuf;
+ spin_unlock_irqrestore(&hp->lock, flags);
+ return retval;
}

static int hvc_chars_in_buffer(struct tty_struct *tty) {
struct hvc_struct *hp = tty->driver_data;
+ unsigned long flags;
+ int retval;

Quoted 6 lines

- return hp->n_outbuf;
+ spin_lock_irqsave(&hp->lock, flags);
+ retval = hp->n_outbuf;
+ spin_unlock_irqrestore(&hp->lock, flags);
+ return retval;
}

The new locking in these functions doesn't really do anything, apart from adding memory barriers. If that's what you really want, I suggest you simply add (commented) memory barriers.

author
#4 lore raw
Quoted 7 lines · Andrew Morton
Quoted 3 lines

+ spin_lock_irqsave(&hp->lock, flags);
+ retval = N_OUTBUF - hp->n_outbuf;
+ spin_unlock_irqrestore(&hp->lock, flags);

The new locking in these functions doesn't really do anything, apart from adding memory barriers. If that's what you really want, I suggest you simply add (commented) memory barriers.

Since chars_in_buffer() can be called from the tty write task and the n_outbuf value is changed from the hvc_console task I didn't want there to be any confusion as to the real value of the variable when chars_in_buffer() was reading it. Is this the proper scenario for a memory barrier?

Ryan S. Arnold
IBM Linux Technology Center