failing on 54945_PPR_Family_17h_Models_00h-0Fh.pdf

[orangecms]

$ ./configure 54945_PPR_Family_17h_Models_00h-0Fh.pdf
rm -rf result.txt resultvol1.txt resultvol2.txt resultvol3.txt resultvol4.txt resultvol5.txt resultvol6.txt resultvol7.txt resultvol8.txt resultvol9.txt phase2_result.py phase3_*.svd phase4_*.svd partsvol1 partsvol2 partsvol3 partsvol4 partsvol5 partsvol6 partsvol7 partsvol8 partsvol9
$ make
mkdir -p partsvol1
pdftohtml -xml 54945_PPR_Family_17h_Models_00h-0Fh.pdf partsvol1/a >/dev/null
./extract.py partsvol1/a.xml > "resultvol1.txt".new && mv "resultvol1.txt".new "resultvol1.txt"
Traceback (most recent call last):
  File "/home/dan/firmware/amd-register-interface-extractor/./extract.py", line 427, in <module>
    traverse(state, root)
  File "/home/dan/firmware/amd-register-interface-extractor/./extract.py", line 422, in traverse
    traverse(state, node, indent + 4, fontspecs)
  File "/home/dan/firmware/amd-register-interface-extractor/./extract.py", line 421, in traverse
    state.process_text(text, attrib, xx)
  File "/home/dan/firmware/amd-register-interface-extractor/./extract.py", line 333, in process_text
    assert False
AssertionError
make: *** [compiler.mk:40: resultvol1.txt] Error 1

[daym]

The reason was that the headline 1.2 was misaligned in the PDF (is at left=76--should have been at most at left=59 (!)). Now I'm also accepting left=76.

Unfortunately, the formatting in general is very different from the usual PDFs I see:

They have MCA::CS::MCA_ADDR_CS_instCS1_alias[MSR,MSRLEGACY]; MSR[C0002152,00000456] which amd-register-interface-extractor currently does not support (the shorthand to the right MSR[C0002152,00000456] is handled, but the shorthand to the left is unsupported because I've never seen it before).

But for example this entry should still work MCA::UMC::MCA_DESTAT_UMC_instUMC_umc[1:0]_aliasMSR; MSRC000_2[10:0F]8. Investigating.

[daym]

The reason is that the table formatting of the table headers ["Bits", "Description"] was not detected.

[orangecms]

It works now, thank you! \o/

[orangecms]

For reference, this is the working commit:
e65cfd64767fb7a9ee1a96c1f274a054b1e7821d

[daym]

Yeah, but there's no result in the svd files.

Looking through that PDF, I don't think this can be parsed in an automated way by the usual phase2 parser, even. The logical file format is not compatible.

See partsvol1/a.xml (the result of pdftohtml), which has:

<text top="427" left="80" width="70" height="18" font="4"><b>IOx0CFC</b></text>
<text top="427" left="150" width="10" height="18" font="0"> [</text>
<text top="427" left="159" width="243" height="18" font="4"><b>IO-Space Configuration Data Port</b></text>
<text top="427" left="403" width="5" height="18" font="0">]</text>
<text top="427" left="408" width="89" height="18" font="4"><b> (IoCfgData)</b></text>
<text top="451" left="80" width="218" height="18" font="0">Read-write. Reset: 0000_0000h. </text>
<text top="472" left="80" width="250" height="13" font="7">IO::IoCfgData_aliasIO; IOx0CFC; IO=0000_0000h</text>
<text top="487" left="95" width="27" height="18" font="4"><b>Bits</b></text>
<text top="487" left="147" width="81" height="18" font="4"><b>Description</b></text>

That's easier to read in resultvol1.txt (the result of extract.py--which is phase 1):

// TABLE STARTS (in_table_header=0, in_table_prefix=1)
//      TEXT IO-Space Configuration Data Port {'meaning': 'h1', 'left': 159, 'top': 427, 'width': 243, 'height': 18}
//          b  {}
//      TEXT ] {'meaning': 'register-cell-description', 'left': 403, 'top': 427, 'width': 5, 'height': 18}
//      TEXT  (IoCfgData) {'meaning': 'h1', 'left': 408, 'top': 427, 'width': 89, 'height': 18}
//          b  {}
//      TEXT Read-write. Reset: 0000_0000h.  {'meaning': 'register-cell-description', 'left': 80, 'top': 451, 'width': 218, 'height': 18}
//      TEXT IO::IoCfgData_aliasIO; IOx0CFC; IO=0000_0000h {'meaning': 'itemization-register-cell-_inst', 'left': 80, 'top': 472, 'width': 250, 'height': 13}
//      TEXT Bits {'meaning': 'h1', 'left': 95, 'top': 487, 'width': 27, 'height': 18}
// TABLE PREFIX ENDS

The IO::IoCfgData_aliasIO mingles two different things together without any space in-between. The IO::IoCfgData is a logical variable name (because of the ::), and the _aliasIO is an access method.
And the (IoCfgData), no idea what that is. Redundant?

The variable name is supposed to be in parens like this (IO::IoCfgData) (and have double colons), and the _aliasIO should be seperated by whitespace from it.

amd-register-interface-extractor needs the :: since the table header (or what I can "table prefix") usually contains human-readable text, which does contain (...) things that are NOT variables as well.

Hence, after processing, IoCfgData is represented like this (see phase2_result.py--the result of phase2):

IOx0CFC = ' [ IO-Space Configuration Data Port ]  (IoCfgData) Read-write. Reset: 0000_0000h.  IO::IoCfgData_aliasIO; IOx0CFC; IO=0000_0000h', [
['Bits Description', ''],
['31:0', 'Data . Read-write. Reset: 0. See IO::IoCfgAddr.'],
[],
]

while it should be this:

IOx0CFC = ' [ IO-Space Configuration Data Port ]  (IO::IoCfgData)\nRead-write. Reset: 0000_0000h. _aliasIO; IOx0CFC; IO=0000_0000h', [
['Bits', 'Description'],
['31:0', 'Data . Read-write. Reset: 0000_0000h. See IO::IoCfgAddr.'],
[],
]

The tuple is prefix, table.

Note that the latter says (IO::IoCfgData).

Also, the space before _aliasIO at the latter makes it possible for amd-register-interface-extractor to disambiguate an actual token (_aliasIO) from something that is just a suffix.

What's worse, phase2_result with that file doesn't have seperation between different registers. It mingles together many different registers into one table.

That looks as if someone wrote it manually--not even copy&pasting...

Long story short, I don't think that that file is going to work. That file has a different logical representation to what amd-register-interface-extractor expects (and to what both the NDA and public docs I had seen so far have!).

[daym]

Is the data in that PDF file important to you? If so, I can suggest this patch

diff --git a/phase2.py b/phase2.py
index cb4ba87..84957de 100755
--- a/phase2.py
+++ b/phase2.py
@@ -145,6 +145,9 @@ for line in open("result.txt", "r"):
                        current_table = table_reference
                        if len(cells) > 1:
                                prefix_metadata = ""
+                               if len(cells) >= 2 and cells[0].endswith("Bits") and cells[1].startswith("Description"): # doc 54945
+                                       cells[0] = cells[0] + " Description"
+                                       cells[1] = cells[1][len("Description"):].strip()
                                if len(cells) >= 1 and cells[0].find("\n") and cells[0].find("Bits Description") != -1:
                                        assert re_table_prefix.match(cells[0]), repr(cells[0])
                                if len(cells) >= 1 and re_table_prefix.match(cells[0]):
@@ -159,7 +162,7 @@ for line in open("result.txt", "r"):
                                        nice_names = re_table_prefix_nice_names.match(prefix) or re_table_prefix_nice_name.match(prefix)
                                        # Add nice name to CURRENT_TABLE.
                                        if not current_table.strip().endswith(")") or current_table.find("::") == -1:
-                                               assert nice_names, (table_reference, prefix)
+                                               #assert nice_names, (table_reference, prefix)
                                                if nice_names:
                                                        extra = nice_names.group(1)
                                                        nice_name = nice_names.group(2)

(which I don't merge it that form because it's an ugly workaround) on top of commit e65cfd6 and then fix up phase2_result.py manually somehow. We could even add some automated fix to this project--but not in the regular program (because that file is just too weird).

I'm thinking of even reverting commit e65cfd6 because the issue here actually had detected an incompatible file format very early. So it had been helpful. That said, it doesn't hurt for the time being. Once it does, I'll revert it.

[orangecms]

I just tried a few PPRs I have floating around, none of which are special.
The idea was just to see how far this tool here can get and report issues, known goods, etc.. I guess it's a bit too messy with a bunch of hand-written docs. 😅

[daym]

Yeah--what was AMD thinking? Seriously.

In any case, it's nice to have these issue reports so we can document why things don't work and what exactly is going on so we don't have to analyze the same thing again later.