[PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

Subsystems: hypervisor virtual console driver, the rest, tty layer and serial drivers

STALE4006d

9 messages, 4 authors, 2015-08-21 · open the first message on its own page

[PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Laurent Dufour <hidden>
Date: 2015-07-31 09:29:58

This patch fixes several endianness issues detected when running the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data exchanged in
memory between the kernel and the hypervisor has to be in big endian
format.

Signed-off-by: Laurent Dufour <redacted>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Jiri Slaby <redacted>
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-kernel@vger.kernel.org
---
 drivers/tty/hvc/hvsi.c | 46 ++++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index 41901997c0d6..a75146f600cb 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -240,9 +240,9 @@ static void hvsi_recv_control(struct hvsi_struct *hp, uint8_t *packet,
 {
 	struct hvsi_control *header = (struct hvsi_control *)packet;
 
-	switch (header->verb) {
+	switch (be16_to_cpu(header->verb)) {
 		case VSV_MODEM_CTL_UPDATE:
-			if ((header->word & HVSI_TSCD) == 0) {
+			if ((be32_to_cpu(header->word) & HVSI_TSCD) == 0) {
 				/* CD went away; no more connection */
 				pr_debug("hvsi%i: CD dropped\n", hp->index);
 				hp->mctrl &= TIOCM_CD;
@@ -267,6 +267,7 @@ static void hvsi_recv_control(struct hvsi_struct *hp, uint8_t *packet,
 static void hvsi_recv_response(struct hvsi_struct *hp, uint8_t *packet)
 {
 	struct hvsi_query_response *resp = (struct hvsi_query_response *)packet;
+	uint32_t mctrl_word;
 
 	switch (hp->state) {
 		case HVSI_WAIT_FOR_VER_RESPONSE:
@@ -274,9 +275,10 @@ static void hvsi_recv_response(struct hvsi_struct *hp, uint8_t *packet)
 			break;
 		case HVSI_WAIT_FOR_MCTRL_RESPONSE:
 			hp->mctrl = 0;
-			if (resp->u.mctrl_word & HVSI_TSDTR)
+			mctrl_word = be32_to_cpu(resp->u.mctrl_word);
+			if (mctrl_word & HVSI_TSDTR)
 				hp->mctrl |= TIOCM_DTR;
-			if (resp->u.mctrl_word & HVSI_TSCD)
+			if (mctrl_word & HVSI_TSCD)
 				hp->mctrl |= TIOCM_CD;
 			__set_state(hp, HVSI_OPEN);
 			break;
@@ -295,10 +297,10 @@ static int hvsi_version_respond(struct hvsi_struct *hp, uint16_t query_seqno)
 
 	packet.hdr.type = VS_QUERY_RESPONSE_PACKET_HEADER;
 	packet.hdr.len = sizeof(struct hvsi_query_response);
-	packet.hdr.seqno = atomic_inc_return(&hp->seqno);
-	packet.verb = VSV_SEND_VERSION_NUMBER;
+	packet.hdr.seqno = cpu_to_be16(atomic_inc_return(&hp->seqno));
+	packet.verb = cpu_to_be16(VSV_SEND_VERSION_NUMBER);
 	packet.u.version = HVSI_VERSION;
-	packet.query_seqno = query_seqno+1;
+	packet.query_seqno = cpu_to_be16(query_seqno+1);
 
 	pr_debug("%s: sending %i bytes\n", __func__, packet.hdr.len);
 	dbg_dump_hex((uint8_t*)&packet, packet.hdr.len);
@@ -319,7 +321,7 @@ static void hvsi_recv_query(struct hvsi_struct *hp, uint8_t *packet)
 
 	switch (hp->state) {
 		case HVSI_WAIT_FOR_VER_QUERY:
-			hvsi_version_respond(hp, query->hdr.seqno);
+			hvsi_version_respond(hp, be16_to_cpu(query->hdr.seqno));
 			__set_state(hp, HVSI_OPEN);
 			break;
 		default:
@@ -555,8 +557,8 @@ static int hvsi_query(struct hvsi_struct *hp, uint16_t verb)
 
 	packet.hdr.type = VS_QUERY_PACKET_HEADER;
 	packet.hdr.len = sizeof(struct hvsi_query);
-	packet.hdr.seqno = atomic_inc_return(&hp->seqno);
-	packet.verb = verb;
+	packet.hdr.seqno = cpu_to_be16(atomic_inc_return(&hp->seqno));
+	packet.verb = cpu_to_be16(verb);
 
 	pr_debug("%s: sending %i bytes\n", __func__, packet.hdr.len);
 	dbg_dump_hex((uint8_t*)&packet, packet.hdr.len);
@@ -596,14 +598,14 @@ static int hvsi_set_mctrl(struct hvsi_struct *hp, uint16_t mctrl)
 	struct hvsi_control packet __ALIGNED__;
 	int wrote;
 
-	packet.hdr.type = VS_CONTROL_PACKET_HEADER,
-	packet.hdr.seqno = atomic_inc_return(&hp->seqno);
+	packet.hdr.type = VS_CONTROL_PACKET_HEADER;
+	packet.hdr.seqno = cpu_to_be16(atomic_inc_return(&hp->seqno));
 	packet.hdr.len = sizeof(struct hvsi_control);
-	packet.verb = VSV_SET_MODEM_CTL;
-	packet.mask = HVSI_TSDTR;
+	packet.verb = cpu_to_be16(VSV_SET_MODEM_CTL);
+	packet.mask = cpu_to_be32(HVSI_TSDTR);
 
 	if (mctrl & TIOCM_DTR)
-		packet.word = HVSI_TSDTR;
+		packet.word = cpu_to_be32(HVSI_TSDTR);
 
 	pr_debug("%s: sending %i bytes\n", __func__, packet.hdr.len);
 	dbg_dump_hex((uint8_t*)&packet, packet.hdr.len);
@@ -680,7 +682,7 @@ static int hvsi_put_chars(struct hvsi_struct *hp, const char *buf, int count)
 	BUG_ON(count > HVSI_MAX_OUTGOING_DATA);
 
 	packet.hdr.type = VS_DATA_PACKET_HEADER;
-	packet.hdr.seqno = atomic_inc_return(&hp->seqno);
+	packet.hdr.seqno = cpu_to_be16(atomic_inc_return(&hp->seqno));
 	packet.hdr.len = count + sizeof(struct hvsi_header);
 	memcpy(&packet.data, buf, count);
 
@@ -697,9 +699,9 @@ static void hvsi_close_protocol(struct hvsi_struct *hp)
 	struct hvsi_control packet __ALIGNED__;
 
 	packet.hdr.type = VS_CONTROL_PACKET_HEADER;
-	packet.hdr.seqno = atomic_inc_return(&hp->seqno);
+	packet.hdr.seqno = cpu_to_be16(atomic_inc_return(&hp->seqno));
 	packet.hdr.len = 6;
-	packet.verb = VSV_CLOSE_PROTOCOL;
+	packet.verb = cpu_to_be16(VSV_CLOSE_PROTOCOL);
 
 	pr_debug("%s: sending %i bytes\n", __func__, packet.hdr.len);
 	dbg_dump_hex((uint8_t*)&packet, packet.hdr.len);
@@ -1180,7 +1182,7 @@ static int __init hvsi_console_init(void)
 	/* search device tree for vty nodes */
 	for_each_compatible_node(vty, "serial", "hvterm-protocol") {
 		struct hvsi_struct *hp;
-		const uint32_t *vtermno, *irq;
+		const __be32 *vtermno, *irq;
 
 		vtermno = of_get_property(vty, "reg", NULL);
 		irq = of_get_property(vty, "interrupts", NULL);
@@ -1202,11 +1204,11 @@ static int __init hvsi_console_init(void)
 		hp->index = hvsi_count;
 		hp->inbuf_end = hp->inbuf;
 		hp->state = HVSI_CLOSED;
-		hp->vtermno = *vtermno;
-		hp->virq = irq_create_mapping(NULL, irq[0]);
+		hp->vtermno = be32_to_cpup(vtermno);
+		hp->virq = irq_create_mapping(NULL, be32_to_cpup(irq));
 		if (hp->virq == 0) {
 			printk(KERN_ERR "%s: couldn't create irq mapping for 0x%x\n",
-				__func__, irq[0]);
+			       __func__, be32_to_cpup(irq));
 			tty_port_destroy(&hp->port);
 			continue;
 		}
-- 
1.9.1

RE: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: David Laight <hidden>
Date: 2015-08-03 11:03:13

RnJvbTogTGF1cmVudCBEdWZvdXINCj4gU2VudDogMzEgSnVseSAyMDE1IDEwOjMwDQo+IFRoaXMg
cGF0Y2ggZml4ZXMgc2V2ZXJhbCBlbmRpYW5uZXNzIGlzc3VlcyBkZXRlY3RlZCB3aGVuIHJ1bm5p
bmcgdGhlIEhWU0kNCj4gZHJpdmVyIGluIGxpdHRsZSBlbmRpYW4gbW9kZS4NCj4gDQo+IFRoZXNl
IGlzc3VlcyBhcmUgcmFpc2VkIGluIGxpdHRsZSBlbmRpYW4gbW9kZSBiZWNhdXNlIHRoZSBkYXRh
IGV4Y2hhbmdlZCBpbg0KPiBtZW1vcnkgYmV0d2VlbiB0aGUga2VybmVsIGFuZCB0aGUgaHlwZXJ2
aXNvciBoYXMgdG8gYmUgaW4gYmlnIGVuZGlhbg0KPiBmb3JtYXQuDQouLi4NCj4gZGlmZiAtLWdp
dCBhL2RyaXZlcnMvdHR5L2h2Yy9odnNpLmMgYi9kcml2ZXJzL3R0eS9odmMvaHZzaS5jDQo+IGlu
ZGV4IDQxOTAxOTk3YzBkNi4uYTc1MTQ2ZjYwMGNiIDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL3R0
eS9odmMvaHZzaS5jDQo+ICsrKyBiL2RyaXZlcnMvdHR5L2h2Yy9odnNpLmMNCj4gQEAgLTI0MCw5
ICsyNDAsOSBAQCBzdGF0aWMgdm9pZCBodnNpX3JlY3ZfY29udHJvbChzdHJ1Y3QgaHZzaV9zdHJ1
Y3QgKmhwLCB1aW50OF90ICpwYWNrZXQsDQo+ICB7DQo+ICAJc3RydWN0IGh2c2lfY29udHJvbCAq
aGVhZGVyID0gKHN0cnVjdCBodnNpX2NvbnRyb2wgKilwYWNrZXQ7DQo+IA0KPiAtCXN3aXRjaCAo
aGVhZGVyLT52ZXJiKSB7DQo+ICsJc3dpdGNoIChiZTE2X3RvX2NwdShoZWFkZXItPnZlcmIpKSB7
DQo+ICAJCWNhc2UgVlNWX01PREVNX0NUTF9VUERBVEU6DQo+IC0JCQlpZiAoKGhlYWRlci0+d29y
ZCAmIEhWU0lfVFNDRCkgPT0gMCkgew0KPiArCQkJaWYgKChiZTMyX3RvX2NwdShoZWFkZXItPndv
cmQpICYgSFZTSV9UU0NEKSA9PSAwKSB7DQoNCkl0IGlzIGdlbmVyYWxseSBiZXN0IHRvIGJ5dGVz
d2FwIGNvbnN0YW50cy4NCg0KCURhdmlkDQoNCg==

Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2015-08-04 00:51:38

On Fri, 2015-07-31 at 11:29 +0200, Laurent Dufour wrote:
This patch fixes several endianness issues detected when running the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data exchanged in
memory between the kernel and the hypervisor has to be in big endian
format.
Can you include the sparse output before and after?

cheers

Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Laurent Dufour <hidden>
Date: 2015-08-19 21:37:18

On 03/08/2015 13:00, David Laight wrote:
From: Laurent Dufour
quoted
Sent: 31 July 2015 10:30
This patch fixes several endianness issues detected when running the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data exchanged in
memory between the kernel and the hypervisor has to be in big endian
format.
...
quoted
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index 41901997c0d6..a75146f600cb 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -240,9 +240,9 @@ static void hvsi_recv_control(struct hvsi_struct *hp, uint8_t *packet,
 {
 	struct hvsi_control *header = (struct hvsi_control *)packet;

-	switch (header->verb) {
+	switch (be16_to_cpu(header->verb)) {
 		case VSV_MODEM_CTL_UPDATE:
-			if ((header->word & HVSI_TSCD) == 0) {
+			if ((be32_to_cpu(header->word) & HVSI_TSCD) == 0) {
It is generally best to byteswap constants.

	David
Thanks David for your review.
Regarding the byte swapping of the constants, I'm wondering if this  the
best way here.
For instance, Benjamin wrote a similar patch to fix another endianness
issue (99fc1d91b8fc) and he doesn't convert the constant neither.
It think that byte swapping the constant value will impact more code,
and may not ease code reading.

Cheers,
Laurent.

Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2015-08-19 21:51:12

On Wed, 2015-08-19 at 23:37 +0200, Laurent Dufour wrote:
On 03/08/2015 13:00, David Laight wrote:
From: Laurent Dufour
quoted
quoted
Sent: 31 July 2015 10:30
This patch fixes several endianness issues detected when running
the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data
exchanged in
memory between the kernel and the hypervisor has to be in big
endian
format.
...
quoted
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index 41901997c0d6..a75146f600cb 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -240,9 +240,9 @@ static void hvsi_recv_control(struct
hvsi_struct *hp, uint8_t *packet,
 {
 	struct hvsi_control *header = (struct hvsi_control
*)packet;

-	switch (header->verb) {
+	switch (be16_to_cpu(header->verb)) {
 		case VSV_MODEM_CTL_UPDATE:
-			if ((header->word & HVSI_TSCD) == 0) {
+			if ((be32_to_cpu(header->word) &
HVSI_TSCD) == 0) {
It is generally best to byteswap constants.

	David
Thanks David for your review.
Regarding the byte swapping of the constants, I'm wondering if this 
 the
best way here.
For instance, Benjamin wrote a similar patch to fix another
endianness
issue (99fc1d91b8fc) and he doesn't convert the constant neither.
It think that byte swapping the constant value will impact more code,
and may not ease code reading.
Right, I disagree with byteswapping the constants at their definition
point, however maybe he meant using cpu_to_be16(CONSTANT) ?

In any case, it's pretty moot as we have the lhbrx instruction which
will do the load and byteswap and for HVSI, even if it was a tad slower
than a normal load, it would not make a noticeable difference.

Ben.

Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Laurent Dufour <hidden>
Date: 2015-08-19 21:53:54

On 04/08/2015 02:51, Michael Ellerman wrote:
On Fri, 2015-07-31 at 11:29 +0200, Laurent Dufour wrote:
quoted
This patch fixes several endianness issues detected when running the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data exchanged in
memory between the kernel and the hypervisor has to be in big endian
format.
Can you include the sparse output before and after?
Hi Michael,

Here is the output message displayed on the console when the bug occurred:

[    0.000517] irq: (null) didn't like hwirq-0x1000a00 to VIRQ16 mapping
(rc=-22)
[    0.000578] hvsi_console_init: couldn't create irq mapping for 0x1000a00

With the patch is applied, the hvsi driver is initializing correctly and
no message is displayed, except the one saying the number of device the
hvsi driver has configured. For instance:
[    1.535783] HVSI: registered 1 devices

Cheers,
Laurent.

Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2015-08-20 01:40:14

On Wed, 2015-08-19 at 23:53 +0200, Laurent Dufour wrote:
On 04/08/2015 02:51, Michael Ellerman wrote:
quoted
On Fri, 2015-07-31 at 11:29 +0200, Laurent Dufour wrote:
quoted
This patch fixes several endianness issues detected when running the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data exchanged in
memory between the kernel and the hypervisor has to be in big endian
format.
Can you include the sparse output before and after?
Hi Michael,

Here is the output message displayed on the console when the bug occurred:

[    0.000517] irq: (null) didn't like hwirq-0x1000a00 to VIRQ16 mapping (rc=-22)
[    0.000578] hvsi_console_init: couldn't create irq mapping for 0x1000a00

With the patch is applied, the hvsi driver is initializing correctly and
no message is displayed, except the one saying the number of device the
hvsi driver has configured. For instance:
[    1.535783] HVSI: registered 1 devices

OK that's also good.

I was talking about the output from sparse endian checking, as the driver seems
to already be endian annotated.

ie, before:

$ make C=2 CF=-D__CHECK_ENDIAN__
...
drivers/tty/hvc/hvsi.c:245:36: warning: restricted __be32 degrades to integer
drivers/tty/hvc/hvsi.c:243:23: warning: restricted __be16 degrades to integer
drivers/tty/hvc/hvsi.c:243:23: warning: restricted __be16 degrades to integer
drivers/tty/hvc/hvsi.c:277:36: warning: restricted __be32 degrades to integer
drivers/tty/hvc/hvsi.c:279:36: warning: restricted __be32 degrades to integer
drivers/tty/hvc/hvsi.c:298:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:298:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:298:26:    got int
drivers/tty/hvc/hvsi.c:299:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:299:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:299:21:    got int
drivers/tty/hvc/hvsi.c:301:28: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:301:28:    expected restricted __be16 [assigned] [usertype] query_seqno
drivers/tty/hvc/hvsi.c:301:28:    got int
drivers/tty/hvc/hvsi.c:322:60: warning: incorrect type in argument 2 (different base types)
drivers/tty/hvc/hvsi.c:322:60:    expected unsigned short [unsigned] [usertype] query_seqno
drivers/tty/hvc/hvsi.c:322:60:    got restricted __be16 [usertype] seqno
drivers/tty/hvc/hvsi.c:558:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:558:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:558:26:    got int
drivers/tty/hvc/hvsi.c:559:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:559:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:559:21:    got unsigned short [unsigned] [usertype] verb
drivers/tty/hvc/hvsi.c:600:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:600:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:600:26:    got int
drivers/tty/hvc/hvsi.c:602:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:602:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:602:21:    got int
drivers/tty/hvc/hvsi.c:603:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:603:21:    expected restricted __be32 [assigned] [usertype] mask
drivers/tty/hvc/hvsi.c:603:21:    got int
drivers/tty/hvc/hvsi.c:606:29: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:606:29:    expected restricted __be32 [assigned] [usertype] word
drivers/tty/hvc/hvsi.c:606:29:    got int
drivers/tty/hvc/hvsi.c:683:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:683:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:683:26:    got int
drivers/tty/hvc/hvsi.c:700:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:700:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:700:26:    got int
drivers/tty/hvc/hvsi.c:702:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:702:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:702:21:    got int
...

And with your patch applied there are no warnings from hvsi.c!

So it seems you fixed all the issues, or at least all the issues we can detect
with sparse. So I'll merge this as-is with an updated changelog.

cheers

Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2015-08-20 04:20:07

On Thu, 2015-08-20 at 11:40 +1000, Michael Ellerman wrote:
On Wed, 2015-08-19 at 23:53 +0200, Laurent Dufour wrote:
quoted
On 04/08/2015 02:51, Michael Ellerman wrote:
quoted
On Fri, 2015-07-31 at 11:29 +0200, Laurent Dufour wrote:
quoted
This patch fixes several endianness issues detected when running the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data exchanged in
memory between the kernel and the hypervisor has to be in big endian
format.
...
So it seems you fixed all the issues, or at least all the issues we can detect
with sparse. So I'll merge this as-is with an updated changelog.
Unless Greg wants to merge it? But I'll assume he's not bothered unless he says
otherwise as it's a powerpc only driver.

cheers

Re: powerpc/hvsi: Fix endianness issues in the HVSI driver

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2015-08-21 07:43:37

On Fri, 2015-31-07 at 09:29:50 UTC, Laurent Dufour wrote:
This patch fixes several endianness issues detected when running the HVSI
driver in little endian mode.

These issues are raised in little endian mode because the data exchanged in
memory between the kernel and the hypervisor has to be in big endian
format.

Signed-off-by: Laurent Dufour <redacted>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Jiri Slaby <redacted>
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-kernel@vger.kernel.org
Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/480798044eb268a31f6b

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help