All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/4] umr: Print errors to stderr
@ 2022-03-12  0:52 Luben Tuikov
  2022-03-12  0:52 ` [PATCH v1 2/4] umr: Fix ring-stream segmentation fault Luben Tuikov
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Luben Tuikov @ 2022-03-12  0:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Tom StDenis, Luben Tuikov

Print the following error message,
   Invalid gca config data header
to stderr, since printing it to stdout,
confuses parser scripts.

Also modify this message to be clearer. For instance,
   Invalid or unknown GCA config data header version:4

Cc: Alex Deucher <Alexander.Deucher@amd.com>
Cc: Tom StDenis <tom.stdenis@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
Reviewed-by: Tom StDenis <tom.stdenis@amd.com>
---
 src/lib/scan_config.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/lib/scan_config.c b/src/lib/scan_config.c
index c7f4e9d3c69faa..6dd752a1149754 100644
--- a/src/lib/scan_config.c
+++ b/src/lib/scan_config.c
@@ -122,7 +122,7 @@ static uint64_t read_int_drm(int cardno, char *fname)
 }
 
 /**
- * umr_scan_config - Scan the debugfs confiruration data
+ * umr_scan_config - Scan the debugfs configuration data
  */
 int umr_scan_config(struct umr_asic *asic, int xgmi_scan)
 {
@@ -246,7 +246,8 @@ gca_config:
 		case 4: parse_rev4(asic, data, &r);
 			break;
 		default:
-			printf("Invalid gca config data header\n");
+			asic->err_msg("Invalid or unknown GCA config data header version:%d\n",
+				      data[0]);
 			return -1;
 	}
 

base-commit: 818bb1e8f1b5a26fda0e3d7abfc1e68605ebad28
-- 
2.35.1.291.gdab1b7905d


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH v1 2/4] umr: Fix ring-stream segmentation fault
  2022-03-12  0:52 [PATCH v1 1/4] umr: Print errors to stderr Luben Tuikov
@ 2022-03-12  0:52 ` Luben Tuikov
  2022-03-12  0:52 ` [PATCH v1 3/4] umr: Consistent indentation in switch Luben Tuikov
  2022-03-12  0:52 ` [PATCH v1 4/4] umr: Fix unhandled enumeration value " Luben Tuikov
  2 siblings, 0 replies; 4+ messages in thread
From: Luben Tuikov @ 2022-03-12  0:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Tom StDenis, Luben Tuikov

Fix a segmentation fault when running --ring-stream for a ring and no
bounds are specified. For instance "umr --ring-stream sdma0" on Sienna
Cichlid, generates the following segmentation fault:

Core was generated by `umr --ring-stream sdma0'.
Program terminated with signal SIGSEGV, Segmentation fault.
0  umr_sdma_decode_ring (asic=0x86cff0, ringname=0x7ffe92844ae0 "sdma0", start=1484, stop=10000) at /home/ltuikov/proj/open/umr/src/lib/read_sdma_stream.c:68
68				lineardata[linearsize++] = ringdata[3 + start];  // first 3 words are rptr/wptr/dwptr
Missing separate debuginfos, use: dnf debuginfo-install SDL2-2.0.14-1.fc33.x86_64 glibc-2.32-10.fc33.x86_64 libedit-3.1-38.20210714cvs.fc33.x86_64 libffi-3.1-26.fc33.x86_64 libgcc-10.3.1-1.fc33.x86_64 libpciaccess-0.16-3.fc33.x86_64 libstdc++-10.3.1-1.fc33.x86_64 llvm-libs-11.0.0-1.fc33.x86_64 nanomsg-1.1.5-6.fc33.x86_64 ncurses-libs-6.2-3.20200222.fc33.x86_64 zlib-1.2.11-23.fc33.x86_64
(gdb) bt
0  umr_sdma_decode_ring (asic=0x86cff0, ringname=0x7ffe92844ae0 "sdma0", start=1484, stop=10000) at /home/ltuikov/proj/open/umr/src/lib/read_sdma_stream.c:68
1  0x0000000000473b71 in present_sdma (asic=0x86cff0, ringname=0x7ffe92844ae0 "sdma0", start=0, end=10000, vmid=4294967295, addr=139867074238864, nwords=0)
    at /home/ltuikov/proj/open/umr/src/app/ring_stream_read.c:1214
2  0x00000000004740c9 in umr_read_ring_stream (asic=0x86cff0, ringpath=0x7ffe92847190 "sdma0") at /home/ltuikov/proj/open/umr/src/app/ring_stream_read.c:1325
3  0x0000000000457567 in main (argc=3, argv=0x7ffe92845268) at /home/ltuikov/proj/open/umr/src/app/main.c:473
(gdb) l
63
64			// copy ring data into linear array
65			lineardata = calloc(ringsize, sizeof(*lineardata));
66			linearsize = 0;
67			while (start != stop) {
68				lineardata[linearsize++] = ringdata[3 + start];  // first 3 words are rptr/wptr/dwptr
69				start = (start + 1) % ringsize;
70			}
71
72			ps = umr_sdma_decode_stream(asic, -1, 0, 0, lineardata, linearsize);
(gdb) p ringsize
$1 = 2048
(gdb) p linearsize
$2 = 30157
(gdb)

Where "linearsize" of 30157 is clearly out of bounds of "lineardata."

Cc: Alex Deucher <Alexander.Deucher@amd.com>
Cc: Tom StDenis <tom.stdenis@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
Reviewed-by: Tom StDenis <tom.stdenis@amd.com>
---
 src/lib/read_sdma_stream.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/lib/read_sdma_stream.c b/src/lib/read_sdma_stream.c
index 63c4fc284afa17..863d251ef64a63 100644
--- a/src/lib/read_sdma_stream.c
+++ b/src/lib/read_sdma_stream.c
@@ -63,11 +63,10 @@ struct umr_sdma_stream *umr_sdma_decode_ring(struct umr_asic *asic, char *ringna
 
 		// copy ring data into linear array
 		lineardata = calloc(ringsize, sizeof(*lineardata));
-		linearsize = 0;
-		while (start != stop) {
-			lineardata[linearsize++] = ringdata[3 + start];  // first 3 words are rptr/wptr/dwptr
-			start = (start + 1) % ringsize;
-		}
+		for (linearsize = 0;
+		     start != stop && linearsize < ringsize;
+		     linearsize++, start = (start + 1) % ringsize)
+			lineardata[linearsize] = ringdata[3 + start];  // first 3 words are rptr/wptr/dwptr
 
 		ps = umr_sdma_decode_stream(asic, -1, 0, 0, lineardata, linearsize);
 		free(lineardata);
-- 
2.35.1.291.gdab1b7905d


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH v1 3/4] umr: Consistent indentation in switch
  2022-03-12  0:52 [PATCH v1 1/4] umr: Print errors to stderr Luben Tuikov
  2022-03-12  0:52 ` [PATCH v1 2/4] umr: Fix ring-stream segmentation fault Luben Tuikov
@ 2022-03-12  0:52 ` Luben Tuikov
  2022-03-12  0:52 ` [PATCH v1 4/4] umr: Fix unhandled enumeration value " Luben Tuikov
  2 siblings, 0 replies; 4+ messages in thread
From: Luben Tuikov @ 2022-03-12  0:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Tom StDenis, Luben Tuikov

A mix of multiple spaces and TAB char was being used in this switch. This
commit makes the usage of indent chars consistent, i.e. leading-TAB only.

Cc: Alex Deucher <Alexander.Deucher@amd.com>
Cc: Tom StDenis <tom.stdenis@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
Reviewed-by: Tom StDenis <tom.stdenis@amd.com>
---
 src/lib/ih_decode_vectors.c | 80 ++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/src/lib/ih_decode_vectors.c b/src/lib/ih_decode_vectors.c
index bb9764bb01783d..ed5705059c542f 100644
--- a/src/lib/ih_decode_vectors.c
+++ b/src/lib/ih_decode_vectors.c
@@ -41,48 +41,48 @@ int umr_ih_decode_vectors(struct umr_asic *asic, struct umr_ih_decode_ui *ui, ui
 	uint32_t off = 0;
 
 	switch (asic->family) {
-		case FAMILY_VI: // oss30
-			while (length) {
-				ui->start_vector(ui, off);
-				ui->add_field(ui, off + 0, "SourceID",   BITS(ih_data[off + 0], 0, 8), NULL, 10); // TODO: add ID to name translation
-				ui->add_field(ui, off + 1, "SourceData", BITS(ih_data[off + 1], 0, 24), NULL, 16);
-				ui->add_field(ui, off + 2, "VMID",       BITS(ih_data[off + 2], 8, 16), NULL, 10);
-				ui->add_field(ui, off + 2, "PASID",      BITS(ih_data[off + 2], 16, 32), NULL, 10);
-				ui->add_field(ui, off + 3, "ContextID0", ih_data[off + 3], NULL, 16);
-				ui->done(ui);
-				length -= 16;
-				off += 4;
-			}
-			return off / 4;
+	case FAMILY_VI: // oss30
+		while (length) {
+			ui->start_vector(ui, off);
+			ui->add_field(ui, off + 0, "SourceID",   BITS(ih_data[off + 0], 0, 8), NULL, 10); // TODO: add ID to name translation
+			ui->add_field(ui, off + 1, "SourceData", BITS(ih_data[off + 1], 0, 24), NULL, 16);
+			ui->add_field(ui, off + 2, "VMID",       BITS(ih_data[off + 2], 8, 16), NULL, 10);
+			ui->add_field(ui, off + 2, "PASID",      BITS(ih_data[off + 2], 16, 32), NULL, 10);
+			ui->add_field(ui, off + 3, "ContextID0", ih_data[off + 3], NULL, 16);
+			ui->done(ui);
+			length -= 16;
+			off += 4;
+		}
+		return off / 4;
 
-		case FAMILY_NV: // oss40/50
-		case FAMILY_AI:
-			while (length) {
-				ui->start_vector(ui, off);
-				ui->add_field(ui, off + 0, "ClientID", BITS(ih_data[off + 0], 0, 8), NULL, 10); // TODO: add ID to name translation
-				ui->add_field(ui, off + 0, "SourceID", BITS(ih_data[off + 0], 8, 16), NULL, 10); // TODO: add ID to name translation
-				ui->add_field(ui, off + 0, "RingID",   BITS(ih_data[off + 0], 16, 24), NULL, 10);
-				ui->add_field(ui, off + 0, "VMID",     BITS(ih_data[off + 0], 24, 28), NULL, 10);
-				ui->add_field(ui, off + 0, "VMID_TYPE", BITS(ih_data[off + 0], 31, 32), NULL, 10);
-				ui->add_field(ui, off + 1, "Timestamp", ih_data[off + 1] + ((uint64_t)BITS(ih_data[off+2], 0, 16) << 32), NULL, 10);
-				ui->add_field(ui, off + 2, "Timestamp_SRC", BITS(ih_data[off + 2], 31, 32), NULL, 10);
-				ui->add_field(ui, off + 3, "PASID", BITS(ih_data[off + 3], 0, 16), NULL, 16);
-				ui->add_field(ui, off + 4, "ContextID0", ih_data[off + 4], NULL, 16);
-				ui->add_field(ui, off + 5, "ContextID1", ih_data[off + 5], NULL, 16);
-				ui->add_field(ui, off + 6, "ContextID2", ih_data[off + 6], NULL, 16);
-				ui->add_field(ui, off + 7, "ContextID3", ih_data[off + 7], NULL, 16);
-				ui->done(ui);
-				length -= 32;
-				off += 8;
-			}
-			return off / 8;
+	case FAMILY_NV: // oss40/50
+	case FAMILY_AI:
+		while (length) {
+			ui->start_vector(ui, off);
+			ui->add_field(ui, off + 0, "ClientID", BITS(ih_data[off + 0], 0, 8), NULL, 10); // TODO: add ID to name translation
+			ui->add_field(ui, off + 0, "SourceID", BITS(ih_data[off + 0], 8, 16), NULL, 10); // TODO: add ID to name translation
+			ui->add_field(ui, off + 0, "RingID",   BITS(ih_data[off + 0], 16, 24), NULL, 10);
+			ui->add_field(ui, off + 0, "VMID",     BITS(ih_data[off + 0], 24, 28), NULL, 10);
+			ui->add_field(ui, off + 0, "VMID_TYPE", BITS(ih_data[off + 0], 31, 32), NULL, 10);
+			ui->add_field(ui, off + 1, "Timestamp", ih_data[off + 1] + ((uint64_t)BITS(ih_data[off+2], 0, 16) << 32), NULL, 10);
+			ui->add_field(ui, off + 2, "Timestamp_SRC", BITS(ih_data[off + 2], 31, 32), NULL, 10);
+			ui->add_field(ui, off + 3, "PASID", BITS(ih_data[off + 3], 0, 16), NULL, 16);
+			ui->add_field(ui, off + 4, "ContextID0", ih_data[off + 4], NULL, 16);
+			ui->add_field(ui, off + 5, "ContextID1", ih_data[off + 5], NULL, 16);
+			ui->add_field(ui, off + 6, "ContextID2", ih_data[off + 6], NULL, 16);
+			ui->add_field(ui, off + 7, "ContextID3", ih_data[off + 7], NULL, 16);
+			ui->done(ui);
+			length -= 32;
+			off += 8;
+		}
+		return off / 8;
 
-			case FAMILY_CONFIGURE:
-			case FAMILY_SI:
-			case FAMILY_CIK:
-			case FAMILY_NPI:
-				asic->err_msg("[BUG]: unhandled family case in umr_ih_decode_vectors()\n");
-				return -1;
+	case FAMILY_CONFIGURE:
+	case FAMILY_SI:
+	case FAMILY_CIK:
+	case FAMILY_NPI:
+		asic->err_msg("[BUG]: unhandled family case in umr_ih_decode_vectors()\n");
+		return -1;
 	}
 	return 0;
 }
-- 
2.35.1.291.gdab1b7905d


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH v1 4/4] umr: Fix unhandled enumeration value in switch
  2022-03-12  0:52 [PATCH v1 1/4] umr: Print errors to stderr Luben Tuikov
  2022-03-12  0:52 ` [PATCH v1 2/4] umr: Fix ring-stream segmentation fault Luben Tuikov
  2022-03-12  0:52 ` [PATCH v1 3/4] umr: Consistent indentation in switch Luben Tuikov
@ 2022-03-12  0:52 ` Luben Tuikov
  2 siblings, 0 replies; 4+ messages in thread
From: Luben Tuikov @ 2022-03-12  0:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Tom StDenis, Luben Tuikov

Add a default case in the switch, instead of the last unhandled value,
FAMILY_CONFIGURE. This solves the case when in the future other families
are not handled--they'll all fall into the default case.

Also, in the diagnostic print, print the value of the unhandled
enumeration--this could help debug easily, as opposed to having to
reproduce the issue locally.

Cc: Alex Deucher <Alexander.Deucher@amd.com>
Cc: Tom StDenis <tom.stdenis@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
Reviewed-by: Tom StDenis <tom.stdenis@amd.com>
---
 src/lib/ih_decode_vectors.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/lib/ih_decode_vectors.c b/src/lib/ih_decode_vectors.c
index ed5705059c542f..f2057df825f5c0 100644
--- a/src/lib/ih_decode_vectors.c
+++ b/src/lib/ih_decode_vectors.c
@@ -77,11 +77,11 @@ int umr_ih_decode_vectors(struct umr_asic *asic, struct umr_ih_decode_ui *ui, ui
 		}
 		return off / 8;
 
-	case FAMILY_CONFIGURE:
 	case FAMILY_SI:
 	case FAMILY_CIK:
 	case FAMILY_NPI:
-		asic->err_msg("[BUG]: unhandled family case in umr_ih_decode_vectors()\n");
+	default:
+		asic->err_msg("[BUG]: unhandled family case:%d in umr_ih_decode_vectors()\n", asic->family);
 		return -1;
 	}
 	return 0;
-- 
2.35.1.291.gdab1b7905d


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-12  0:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-12  0:52 [PATCH v1 1/4] umr: Print errors to stderr Luben Tuikov
2022-03-12  0:52 ` [PATCH v1 2/4] umr: Fix ring-stream segmentation fault Luben Tuikov
2022-03-12  0:52 ` [PATCH v1 3/4] umr: Consistent indentation in switch Luben Tuikov
2022-03-12  0:52 ` [PATCH v1 4/4] umr: Fix unhandled enumeration value " Luben Tuikov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.