[PATCH i-g-t v3 3/6] tools/intel-gfx-fw-info: Embed fw decoding

[Gustavo Sousa]

Quoting Lucas De Marchi (2024-08-26 12:26:53-03:00)
>Use a factory class method to decide what class to use depending on the
>magic and embed the calls to cstruct in each of them. Main motivation is
>that in future the decode method will need to seek the binary as it
>can't really know the version just checking the beginning of the file.
>
>For now, keep the open/close of the file in the main function, but
>eventually can be migrated as well.
>
>v2 (Gustavo):
>- Fix s/kw/**kw/
>- Remove unnecessary seek()
>- Use a constructor variable in create()
>- Squash call to sys.exit() that was in the wrong patch
>
>Acked-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>---
> tools/intel-gfx-fw-info | 74 +++++++++++++++++++++++++++++------------
> 1 file changed, 52 insertions(+), 22 deletions(-)
>
>diff --git a/tools/intel-gfx-fw-info b/tools/intel-gfx-fw-info
>index 3105144b7..a4b9414f2 100755
>--- a/tools/intel-gfx-fw-info
>+++ b/tools/intel-gfx-fw-info
>@@ -105,8 +105,38 @@ def FIELD_GET(mask: int, value: int) -> int:
> 
> 
> class Fw(abc.ABC):
>-    def __init__(self, fw):
>-        self.fw = fw
>+    def __init__(self, cparser, f):
>+        self.f = f
>+        self.cparser = cparser
>+        self.fw = None
>+
>+    def checksum(self):
>+        self.f.seek(0, 0)
>+        return hashlib.sha256(self.f.read()).hexdigest()
>+
>+    def dump(self, **kw):
>+        cstruct.dumpstruct(self.fw, **kw)
>+
>+    @classmethod
>+    def create(cls, f):
>+        cparser = cstruct.cstruct()
>+        cparser.load(CDEF)
>+
>+        magic = cparser.magic(f).data
>+
>+        constructor = None
>+
>+        for s in cls.__subclasses__():
>+            if s.MAGIC is None:
>+                constructor = s
>+            elif s.MAGIC == magic:
>+                constructor = s
>+                break
>+
>+        if constructor:
>+            return constructor(cparser, f)
>+
>+        return None
> 
>     @abc.abstractmethod
>     def decode(self):
>@@ -114,8 +144,13 @@ class Fw(abc.ABC):
> 
> 
> class FwCss(Fw):
>+    MAGIC = None
>+
>     def decode(self):
>-        data = []
>+        self.f.seek(0, 0)
>+        self.fw = self.cparser.uc_css_header(self.f)
>+
>+        data = ["header-type: CSS"]
> 
>         CSS_SW_VERSION_UC_MAJOR = 0xFF << 16
>         CSS_SW_VERSION_UC_MINOR = 0xFF << 8
>@@ -137,8 +172,13 @@ class FwCss(Fw):
> 
> 
> class FwGsc(Fw):
>+    MAGIC = b"$CPD"
>+
>     def decode(self):
>-        data = []
>+        self.f.seek(0, 0)
>+        self.fw = self.cparser.uc_huc_gsc_header(self.f)
>+
>+        data = ["header-type: GSC"]
> 
>         HUC_GSC_MINOR_VER_HI_MASK = 0xFF << 16
>         HUC_GSC_MAJOR_VER_HI_MASK = 0xFF
>@@ -167,40 +207,30 @@ def parse_args(argv: typing.List[str]) -> argparse.Namespace:
>     return parser.parse_args(argv)
> 
> 
>-def calculate_checksum(f: typing.BinaryIO) -> str:
>-    return hashlib.sha256(f.read()).hexdigest()
>-
>-
> def main(argv: typing.List[str]) -> int:
>     args = parse_args(argv)
> 
>-    cparser = cstruct.cstruct()
>-    cparser.load(CDEF)
>-
>     checksum = None
> 
>     try:
>         with open(args.filename, mode="rb") as f:
>-            magic = cparser.magic(f)
>-            f.seek(0, 0)
>-            if magic.data == b"$CPD":
>-                fw = FwGsc(cparser.uc_huc_gsc_header(f))
>-            else:
>-                fw = FwCss(cparser.uc_css_header(f))
>+            fw = Fw.create(f)
>+            if not fw:
>+                logging.fatal("Unknown firmware type in '{args.filename}'")

Sorry I didn't spot this earlier: we are missing the f prefix here.

With that,

    Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>

>+                sys.exit(1)
> 
>+            decoded_data = fw.decode()
>             if args.checksum:
>-                f.seek(0, 0)
>-                checksum = calculate_checksum(f)
>-
>+                checksum = fw.checksum()
>     except FileNotFoundError as e:
>         logging.fatal(e)
>         return 1
> 
>-    print(*fw.decode(), sep="\n")
>+    print(*decoded_data, sep="\n")
> 
>     if args.raw:
>         print("raw dump:", end="")
>-        cstruct.dumpstruct(fw.fw, color=sys.stdout.isatty())
>+        fw.dump(color=sys.stdout.isatty())
> 
>     if checksum:
>         print(f"checksum: {checksum}")
>-- 
>2.43.0
>