W dniu 20.04.2023 o 14:43, Leif Lindholm pisze:
On Fri, Apr 07, 2023 at 17:29:57 +0200, Marcin Juszkiewicz wrote:
+#
+# This flag specifies whether HII resource section is generated into PE image.
+#
+ UEFI_HII_RESOURCE_SECTION = TRUE
The above stanza, and its comment, can be dropped.
This relates to native language support, which there isn't any in this app.
done
+VOID
+PrintText (
+ CONST CHAR8 *field,
For coding style, name should be "*Field".
done, in all places
+ CONST CHAR8 *Bits,
+ CONST UINT8 Value,
+ CONST CHAR8 *Description
+ )
+{
+ STATIC CONST CHAR8 binaries[][5] = {
Could I propose renaming "binaries" to "Nibbles", since this is an
array holding string values for all possible nibbles?
done
+ "0000", "0001", "0010", "0011", "0100", "0101", "0110", "0111",
+ "1000", "1001", "1010", "1011", "1100", "1101", "1110", "1111"
+ };
+ switch (Value) {
+ case 0b0000:
I agree with Pedro's concern w.r.t. binary literals being a GCC
extension. But I also think this is the most appropriate
representation since this is how it's documented in the ARM ARM.
A bit hacky, but could we do
enum {
b0000,
b0001,
...
b1111
};
and then use those instead?
(you can build with -pedantic to verify you catch them all)
done, passed with -pedantic
+ // only valid for BigEnd != 0b0000
Could we possibly reword as
// If mixed-endian support is present, check whether supported at EL0
done
+ case 0b0001: // add FEAT_LPA2 check
That's sounds like a TODO. And we don't want TODOs in code.
Can we either drop the comment, add the check, or skip the test?
(That is my order of preference - we shouldn't need to be verifying
architectural compliance.)
dropped
+ case 0b0010: // add FEAT_LPA2 check
Same comment as for 4k.
dropped as well
+ Bits = "59:56";
+ Value = (Aa64Pfr0 >> 56) & 0xf;
+ switch (Value) {
+ case 0b0000:
+ Description = "no info is FEAT_CSV2 implemented.";
Consider rewording.
Is the text from the ARM ARM too long?
"The implementation does not disclose whether FEAT_CSV2 is
implemented."
if so, maybe
"Not disclosed whether FEAT_CSV2 is implemented."?
thx, done
+ PrintValues (RegName, Bits, Value, Description);
+
+ // 35:32 is CSV2_frac
That's a TODO.
Is it more info why those bits are not handled here. Same with 15:12 for
RAS_frac and 19:16 for MPAM_frac. They are not reserved like 63:40 are.
And CSV2_frac bits are used earlier, in PRF0 handling. If we have CSV2
implemented then CSV2_frac says do we have CSV2_1p1 or CSV2_1p2 implemented.
MPAM_frac and RAS_frac are used in PRF0 handling as well.
+EFI_STATUS
+EFIAPI
+UefiMain (
Stray space before '(' here too. Is it an editor setting?
fixed
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103306): https://edk2.groups.io/g/devel/message/103306
Mute This Topic: https://groups.io/mt/98126605/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
