Em Thu, 27 Feb 2025 14:30:28 +0100 Igor Mammedov <[email protected]> escreveu:
> On Thu, 27 Feb 2025 12:03:30 +0100 > Mauro Carvalho Chehab <[email protected]> wrote: > > > Now that the ghes preparation patches were merged, let's add support > > for error injection. > > > > On this version, HEST table got added to ACPI tables testing for aarch64 > > virt. > > > > There are also some patch reorder to help reviewers to check the changes. > > > > The code itself is almost identical to v4, with just a few minor nits > > addressed. > > series still has checkpatch errors 'line over 80' which are not false > positive, > it needs to be fixed The long line warnings are at the patch adding the Python script. IMO, all but one are false positives: 1. Long lines at patch description because of the tool output example added inside the commit description: ERROR: line over 90 characters #148: FILE: scripts/arm_processor_error.py:83: +[Hardware Error]: bus error, operation type: Generic read (type of instruction or data request cannot be determined) ERROR: line over 90 characters #153: FILE: scripts/arm_processor_error.py:88: +[Hardware Error]: Program execution can be restarted reliably at the PC associated with the error. WARNING: line over 80 characters #170: FILE: scripts/arm_processor_error.py:105: +[Hardware Error]: 00000000: 13 7b 04 05 01 .{... WARNING: line over 80 characters #174: FILE: scripts/arm_processor_error.py:109: +[Firmware Warn]: GHES: Unhandled processor error type 0x10: micro-architectural error ERROR: line over 90 characters #175: FILE: scripts/arm_processor_error.py:110: +[Firmware Warn]: GHES: Unhandled processor error type 0x14: TLB error|micro-architectural error IMO, breaking command output at the description is a bad practice. 2. Big strings at help message: WARNING: line over 80 characters #261: FILE: scripts/arm_processor_error.py:196: + help="Power State Coordination Interface - PSCI state") ERROR: line over 90 characters #276: FILE: scripts/arm_processor_error.py:211: + help="Number of errors: 0: Single error, 1: Multiple errors, 2-65535: Error count if known") WARNING: line over 80 characters #278: FILE: scripts/arm_processor_error.py:213: + help="Error information (UEFI 2.10 tables N.18 to N.20)") ERROR: line over 90 characters #287: FILE: scripts/arm_processor_error.py:222: + help="Type of the context (0=ARM32 GPR, 5=ARM64 EL1, other values supported)") WARNING: line over 80 characters #1046: FILE: scripts/qmp_helper.py:442: + help="Marks the timestamp as precise if --timestamp is used") WARNING: line over 80 characters #1048: FILE: scripts/qmp_helper.py:444: + help=f"General Error Data Block flags: {gedb_flags_bits}") Those might be changed if we add one variable per string to store the help lines, at the expense of doing some code obfuscation. I don't think doing it is a good idea. 3. Long class function names that are part of Python's standard library: ERROR: line over 90 characters #576: FILE: scripts/ghes_inject.py:29: + parser = argparse.ArgumentParser(formatter_class=argparse.ArgumentDefaultsHelpFormatter, We can't change the big name of the argparse formatter. The only possible fix would be to obfuscate it by doing: format = argparse.ArgumentDefaultsHelpFormatter, parser = argparse.ArgumentParser(formatter_class=format, IMO this is a bad practice. 4. False-positive warning disable for pylint coding style tool: ERROR: line over 90 characters #805: FILE: scripts/qmp_helper.py:201: + data.extend(value.to_bytes(num_bytes, byteorder="little")) # pylint: disable=E1101 WARNING: line over 80 characters #1028: FILE: scripts/qmp_helper.py:424: + g_gen = parser.add_argument_group("Generic Error Data") # pylint: disable=E1101 AFAIKT, those need to be at the same line for pylint to process them properly. 5. A long name inside an indented block: WARNING: line over 80 characters #1109: FILE: scripts/qmp_helper.py:505: + value=args.gen_err_valid_bits, Again the only solution would be to obfuscate the argument, like: a = args.gen_err_valid_bits value=a, Not nice, IMHO. Now, there is one warning that I is not a false positive, which I ended missing: WARNING: line over 80 characters #1227: FILE: scripts/qmp_helper.py:623: + ret = self.send_cmd("qom-get", args, may_open=True, return_error=False) I'll fix it at the next respin. Regards, Mauro
