Skip to content

Commit 1a5fa06

Browse files
authored
Format output xml and align GUID format for fpdt_parser (#1006)
Format output xml and align GUID format for fpdt_parser. Apply `minidom` to format the output xml of the `fpdt_parser.py` to improve readability. The script was not adding entries to FBDT table so the output is not a proper tree structure. Before it was: ``` |- fbpt_header |- fbpt_record |- fbpt_record ``` Now it is: ``` |- fbpt_header | |- fbpt_record | |- fbpt_record ``` it can be properly consumed by `perf_report_generator.py` to generate perf report. Before this change there will be 0 entries in the report because the input structure doesn't match what the script needs. Update the GUID format for different entries to make sure the output GUID format stay the same and can be recognized by the perf_report_generator.py. Add xml formatter to make sure the output xml more readable. Examples for GUID format change: ``` Before this fix: For DynamicStringEvent: `<GUID Value="23C9322F-2AF2-476A-BC4C-26BC-8826-6C71"/>` For DualGuidStringEvent: `<GUID1 Value="6D33944A-EC75-4855-A54D-809C75241F6C"/>` `<GUID2 Value="7B94C75C-36A4-4AA4-A1DF-14BC9A049AE4"/>` After this fix: All event guids will be in the same format of `<GUID1 Value="6D33944A-EC75-4855-A54D-809C75241F6C"/>` ``` With this change the `perf_report_generator.py` can successfully parse these GUIDs and map to the right module name. - [x] Breaking change? Detail of the changes can be seen above. The structure and content of the output xml for `fpdt_parser.py` changed. - [x] Integration instruction: Use `fpdt_parser.py` first and use the output xml file as the input for the `perf_report_generator.py` to get perf report.
1 parent 0dc2700 commit 1a5fa06

File tree

2 files changed

+29
-15
lines changed

2 files changed

+29
-15
lines changed

edk2toolext/perf/fpdt_parser.py

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
)
7878
from io import TextIOWrapper
7979
from typing import BinaryIO
80+
from xml.dom import minidom
8081

8182
FPDT_PARSER_VER = "3.00"
8283

@@ -576,7 +577,7 @@ def to_xml(self) -> ET.Element:
576577
guid_xml = ET.SubElement(xml_repr, "GUID")
577578
guid_xml.set(
578579
"Value",
579-
"%08X-%04X-%04X-%02X%02X-%02X%02X-%02X%02X-%02X%02X"
580+
"%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X"
580581
% (
581582
self.guid_uint32,
582583
self.guid_uint16_0,
@@ -705,7 +706,7 @@ def to_xml(self) -> ET.Element:
705706
guid_xml = ET.SubElement(xml_repr, "GUID")
706707
guid_xml.set(
707708
"Value",
708-
"%08X-%04X-%04X-%02X%02X-%02X%02X-%02X%02X-%02X%02X"
709+
"%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X"
709710
% (
710711
self.guid_uint32,
711712
self.guid_uint16_0,
@@ -873,15 +874,15 @@ def to_xml(self) -> ET.Element:
873874
guid1_xml.set(
874875
"Value",
875876
f"{self.guid1_uint32:08X}-{self.guid1_uint16_0:04X}-{self.guid1_uint16_1:04X}-"
876-
f"{self.guid1_uint8_0:02X}{self.guid1_uint8_1:02X}{self.guid1_uint8_2:02X}{self.guid1_uint8_3:02X}"
877+
f"{self.guid1_uint8_0:02X}{self.guid1_uint8_1:02X}-{self.guid1_uint8_2:02X}{self.guid1_uint8_3:02X}"
877878
f"{self.guid1_uint8_4:02X}{self.guid1_uint8_5:02X}{self.guid1_uint8_6:02X}{self.guid1_uint8_7:02X}",
878879
)
879880

880881
guid2_xml = ET.SubElement(xml_repr, "GUID2")
881882
guid2_xml.set(
882883
"Value",
883884
f"{self.guid2_uint32:08X}-{self.guid2_uint16_0:04X}-{self.guid2_uint16_1:04X}-"
884-
f"{self.guid2_uint8_0:02X}{self.guid2_uint8_1:02X}{self.guid2_uint8_2:02X}{self.guid2_uint8_3:02X}"
885+
f"{self.guid2_uint8_0:02X}{self.guid2_uint8_1:02X}-{self.guid2_uint8_2:02X}{self.guid2_uint8_3:02X}"
885886
f"{self.guid2_uint8_4:02X}{self.guid2_uint8_5:02X}{self.guid2_uint8_6:02X}{self.guid2_uint8_7:02X}",
886887
)
887888

@@ -1473,6 +1474,7 @@ def __init__(self) -> None:
14731474
self.text_log = self.handle_output_file()
14741475
self.handle_input_file()
14751476
self.uefi_version, self.model = self.get_uefi_version_model()
1477+
self.fbpt_tree = None # Initialize FBPT container element
14761478

14771479
self.write_text_header()
14781480
self.xml_tree = self.write_xml_header()
@@ -1612,9 +1614,9 @@ def write_fbpt(self, fbpt_file: BinaryIO) -> None:
16121614
if self.options.output_text_file:
16131615
self.text_log.write(str(fbpt_header))
16141616
if self.options.output_xml_file:
1615-
# Store FBPT header and records under a separate element under FPDT
1616-
fbpt_tree = fbpt_header.to_xml()
1617-
self.xml_tree.append(fbpt_tree)
1617+
# Store FBPT header as a container element that will hold all records
1618+
self.fbpt_tree = fbpt_header.to_xml()
1619+
# Don't append to xml_tree yet - we'll do that after adding all records
16181620

16191621
def gather_fbpt_records(self, fbpt_file: BinaryIO) -> list:
16201622
"""Collects FBPT records from an input file."""
@@ -1638,11 +1640,20 @@ def gather_fbpt_records(self, fbpt_file: BinaryIO) -> list:
16381640
def write_records(self, fbpt_records_list: list) -> int:
16391641
"""Writes FBPT records to an output file."""
16401642
if self.options.output_xml_file:
1643+
# Add all records to the FBPT container element
16411644
for record in fbpt_records_list:
1642-
self.xml_tree.append(record.to_xml())
1645+
self.fbpt_tree.append(record.to_xml())
16431646

1644-
with open(self.options.output_xml_file, "wb") as xml_file:
1645-
xml_file.write(ET.tostring(self.xml_tree))
1647+
# Now append the complete FBPT container to the main XML tree
1648+
self.xml_tree.append(self.fbpt_tree)
1649+
1650+
# Format XML properly with indentation
1651+
rough_string = ET.tostring(self.xml_tree, encoding="unicode")
1652+
reparsed = minidom.parseString(rough_string)
1653+
formatted_xml = reparsed.toprettyxml(indent=" ")
1654+
1655+
with open(self.options.output_xml_file, "w", encoding="utf-8") as xml_file:
1656+
xml_file.write(formatted_xml)
16461657

16471658
if self.options.output_text_file:
16481659
for record in fbpt_records_list:

tests.unit/test_fpdt_parser.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ def test_to_xml(self, mock_header: MockFbptRecordHeader, sample_data: bytes) ->
342342
assert xml_elem.find("ApicID").get("Value") == "0x56789ABC"
343343
assert xml_elem.find("Timestamp").get("RawValue") == "0xABCDEF0123456789"
344344
assert xml_elem.find("Timestamp").get("ValueInMilliseconds") == "12379813738877.119141"
345-
assert xml_elem.find("GUID").get("Value") == "DEADBEEF-BEEF-FEED-DEAD-BEEF-0123-4567"
345+
assert xml_elem.find("GUID").get("Value") == "DEADBEEF-BEEF-FEED-DEAD-BEEF01234567"
346346

347347

348348
class TestDynamicStringEventRecord:
@@ -614,8 +614,8 @@ def test_to_xml(self, mock_header: MockFbptRecordHeader) -> None:
614614
assert xml.find("ApicID").attrib["Value"] == "0x2"
615615
assert xml.find("Timestamp").attrib["RawValue"] == "0x3"
616616
assert xml.find("Timestamp").attrib["ValueInMilliseconds"] == "0.000003"
617-
assert xml.find("GUID1").attrib["Value"] == "00000004-0005-0006-0708090A0B0C0D0E"
618-
assert xml.find("GUID2").attrib["Value"] == "0000000F-0010-0011-1213141516171819"
617+
assert xml.find("GUID1").attrib["Value"] == "00000004-0005-0006-0708-090A0B0C0D0E"
618+
assert xml.find("GUID2").attrib["Value"] == "0000000F-0010-0011-1213-141516171819"
619619
assert xml.find("String").attrib["Value"] == "BootMsg"
620620

621621

@@ -1115,6 +1115,7 @@ def mock_parser_app(
11151115

11161116
app.text_log = mock.Mock(spec=["write", "close"])
11171117
app.xml_tree = mock.Mock(spec=["append"])
1118+
app.fbpt_tree = mock.Mock(spec=["append"])
11181119

11191120
return app
11201121

@@ -1268,7 +1269,7 @@ def test_write_fbpt(self) -> None:
12681269
with mock.patch.object(FwBasicBootPerformanceTableHeader, "__new__", return_value=mock_header):
12691270
mock_app.write_fbpt(mock_fbpt_file)
12701271
mock_app.text_log.write.assert_called_once_with("Fake FBPT Header String")
1271-
mock_app.xml_tree.append.assert_called_once_with("<fbpt_header_xml>fake_xml</fbpt_header_xml>")
1272+
assert mock_app.fbpt_tree == "<fbpt_header_xml>fake_xml</fbpt_header_xml>"
12721273

12731274
def test_gather_fbpt_records_parse_failure(self) -> None:
12741275
"""Tests error handling when a FBPT record has an invalid format."""
@@ -1297,10 +1298,12 @@ def test_write_records(self) -> None:
12971298
with (
12981299
mock.patch("builtins.open", mock.mock_open()),
12991300
mock.patch("xml.etree.ElementTree.tostring"),
1301+
mock.patch("xml.dom.minidom.parseString"),
13001302
):
13011303
result = mock_app.write_records(fbpt_records_list)
13021304
assert result == 1
1303-
mock_app.xml_tree.append.assert_any_call("<Record1></Record1>")
1305+
mock_app.fbpt_tree.append.assert_any_call("<Record1></Record1>")
1306+
mock_app.xml_tree.append.assert_any_call(mock_app.fbpt_tree)
13041307
mock_app.text_log.write.assert_any_call("Record1")
13051308

13061309

0 commit comments

Comments
 (0)