-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Smbios cleanup #90
Smbios cleanup #90
Conversation
Only the Type 44 content is added over, as the Type 4 content is already part of the SMBIOS spec. Further commits clean up the Type 44 structure. Signed-off-by: Andrei Warkentin <[email protected]>
This field is unnecessary and redundant, because: a) type 44 header itself has a length field b) processor-specific block structure (7.5.4) contains another redundant length field An SMBIOS structure is limited by the width of the length field (byte), so we ought to make judicious use of the space, avoiding unnecessary redundancy. Signed-off-by: Andrei Warkentin <[email protected]>
It is really expensive to pad every CSR-like field to 16 bytes, especially since every structure is limited to 249 bytes (4 byte TLV header + 2 byte processor block header). - RV128 is not ratified. * It's not clear if the affected CSR values would really need to be padded. * Further changes may be needed to accommodate RV128 once ratified. * No one in the PRS/BRS groups is working on 128-bit designs, thus there is no need to complicate things. - SMBIOS itself is not 128-bit clean. * E.g. memory (type 16, type type 17) entries cannot encode a 128-bit memory size. * _SM3_ locator structure is not 128-bit clean * A lot more work is needed on 128-bit support, so bail on it for the time being. - We should err on the side of caution and attempt to minimize overall structure length. Technically the defined structure is sound for RV32, but the BRS document only focuses on RV64. Signed-off-by: Andrei Warkentin <[email protected]>
It's obvious at some point there may be incompatible changes. Allow for this by only treating minor rev increments as being backwards compatible. Major changes are treated as breaking changes. Also add a separate table to list revision changes. Since this patch set changes the PSD definitions, increment the major revision and clear the minor. This puts us at v1.0 (the original version for definitions at https://github.com/riscv/riscv-smbios/blob/main/riscv-smbios.adoc is v0.10). Signed-off-by: Andrei Warkentin <[email protected]>
SMBIOS is used to describe physical assets of a system. For a processor this may include uArch/SoC/vendor versioning, but the XLEN fields themselves aren't useful - and if they are, they belong in other specs that are critical to OS startup. As mentioned previously, SMBIOS struct len is a bit of a premium so there needs to be a great reason to include particular info. Signed-off-by: Andrei Warkentin <[email protected]>
Similarly to the XLEN fields, the priv. level is not at all interesting from a physical asset standpoint. If an OS needs to know this info, it needs to get it via DT/ACPI. Signed-off-by: Andrei Warkentin <[email protected]>
Similarly to XLEN and priv-level, this info is not interesting from an asset/physical perspective. If an OS needs it, it needs to be provided via DT/ACPI. Signed-off-by: Andrei Warkentin <[email protected]>
Also clarify the mimpid-derived value is not per-hart, it is per-processor (matches the ISA spec, but ought to be clarified that a processor may be a SoC?) Signed-off-by: Andrei Warkentin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like one little nit.
smbios.adoc
Outdated
| Offset | Version | Name | Length | Value | Description | ||
| 00h| 0100h|Revision|WORD|Varies|See <<smbios-psd-ver>>. | ||
| 02h| 0100h| Hart ID| QWORD| Varies| The ID of this RISC-V hart | ||
| 0Ah| 0100h| Boot hart| BYTE| Boolean| 1: This is the boot hart. + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this information required in SMBIOS? How will it be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree with you that it isn't. I'll take it out.
smbios.adoc
Outdated
microarchitecture of the hart. Value of 0 is possible to indicate the field is | ||
not implemented. The combination of Machine Architecture ID and Machine Vendor | ||
ID should uniquely identify the type of hart microarchitecture that is implemented. | ||
| 1Bh| 0100h| Machine Implementation ID| QWORD| Varies| Unique encoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I understand this is a spec, I have an implementation question for these fields. These vendor ID/impl ID for each hart can be fetched using SBI call from S-mode FW only from the hart which executes it right? Typically, the SMBIOS creator FW like EDK2 runs with single CPU. How is it supposed to create these fields each hart?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On some SoCs, the values for all harts are going to the same as the BSP. On other SoCs, the values could be populated in the DT blob passed from the SBI implementation, or hardcoded in the FW implementation and looked up by Hart ID. On other implementations, you could use EFI MP services + SBI call.
Dups info already available to boot software (via UEFI) and not useful from an asset tracking perspective. Signed-off-by: Andrei Warkentin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks a lot for taking care of this.
Now, this spec being moved to BRS, an ECR to SMBIOS spec required to update the link it is pointing for table 44?
As discussed in the PRS call, here's my rework on the SMBIOS bits that were left orphaned over at riscv-smbios.