RISC-V: Print SBI info at startup
ClosedPublic
Actions

Authored by mhorne on Nov 12 2019, 4:53 AM.

Details

Reviewers

jhb
kp

Group Reviewers

riscv

Commits

rS354722: RISC-V: Print SBI info at startup

Summary

SBI version 0.2 introduces functions for obtaining the details of the
SBI implementation, such as version and implemntation ID. Print this
info at startup when it is available.

Test Plan

Snippet when booting with OpenSBI (which supports v0.2):

WARNING: WITNESS option enabled, expect reduced performance.
Preloaded elf64 kernel "kernel" at 0xffffffc0007cfb20.
SBI: OpenSBI v0.5
SBI Specification Version: 0.2
CPU(0): Unknown Implementer Unknown Processor

Booting with bbl:

WARNING: WITNESS option enabled, expect reduced performance.
Preloaded elf64 kernel "kernel" at 0xffffffc0007cfb20.
SBI: Unknown (Legacy) Implementation
SBI Specification Version: 0.1
CPU(0): Unknown Implementer Unknown Processor

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27488
Build 25720: arc lint + arc unit

Event Timeline

mhorne created this revision.Nov 12 2019, 4:53 AM

Harbormaster completed remote builds in B27466: Diff 64223.Nov 12 2019, 4:53 AM

mhorne edited the test plan for this revision. (Show Details)Nov 12 2019, 4:57 AM

kp added a subscriber: kp.Nov 12 2019, 11:54 AM

kp added inline comments.

sys/riscv/riscv/sbi.c
36	Why have OPENSBI_VERSION_MINOR_MASK here and SBI_SPEC_VERS_MINOR_MASK in sbi.h?
107	Maybe `#define SBI_SPEC_VERS_MAJOR(x) ((x) SBI_SPEC_VERS_MAJOR_MASK) >> SBI_SPEC_VERS_MAJOR_OFFSET)` That'd let you spell this as `major = SBI_SPEC_VERS_MAJOR(sbi_spec_version);`

jhb accepted this revision.Nov 13 2019, 12:36 AM

jhb added a subscriber: jhb.

jhb added inline comments.

sys/riscv/include/sbi.h
44	It doesn't hurt to leave this even if it's unused.
sys/riscv/riscv/sbi.c
36	This looks to be OpenSBI specific as OpenSBI has its own version separate from the versions of the SBI spec it implements? Perhaps these constants should be in sbi.h if we think they will be used anywhere else, but they are fine to be here for now.
107	We mostly open-code these sorts of things in the tree. The Chelsio driver does go full-bore with 4 sets of macros for each field in a register (S_* is the shift count (like _OFFSET), M_* is the mask, G_* does the shift and and and to extract the value, V_* does the shift so you can or them together to build a register value, and F_ is V_*(1) for single-bit fields), but we haven't really adopted anything like that in other parts of the tree.

This revision is now accepted and ready to land.Nov 13 2019, 12:36 AM

Address comments.

This revision now requires review to proceed.Nov 13 2019, 2:48 AM

Harbormaster completed remote builds in B27488: Diff 64266.Nov 13 2019, 2:48 AM

mhorne marked an inline comment as done.Nov 13 2019, 2:51 AM

mhorne added inline comments.

sys/riscv/riscv/sbi.c
36	Exactly this. They are implementation specific and seem unlikely to be used elsewhere.
107	I think what you suggested is cleaner kp, but if it's not what is widely used I'll keep these as they are.