Skip to content

Big endianness Support added #751

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

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ Supported RISC-V ISA features
- Svinval extension for fine-grained address-translation cache invalidation, v1.0
- Sv32, Sv39, Sv48 and Sv57 page-based virtual-memory systems
- Physical Memory Protection (PMP)
- Endianness control

**For a list of unsupported extensions and features, see the [Extension Roadmap](https://github.com/riscv/sail-riscv/wiki/Extension-Roadmap).**

Expand Down
3 changes: 3 additions & 0 deletions model/prelude.sail
Original file line number Diff line number Diff line change
Expand Up @@ -238,3 +238,6 @@ type max_mem_access : Int = 4096

// Type used for memory access widths. Zero byte accesses are not allowed.
type mem_access_width = range(1, max_mem_access)

// Function to reverse endianness.
val reverse_endianness = pure {c: "reverse_endianness"} : forall 'n . bits(8 * 'n) -> bits(8 * 'n)
29 changes: 20 additions & 9 deletions model/riscv_mem.sail
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,16 @@ function write_kind_of_flags (aq : bool, rl : bool, con : bool) -> write_kind =
(true, false, true) => throw(Error_not_implemented("sc.aq"))
}

function is_big_endian(typ : AccessType(ext_access_type)) -> bool = {
Copy link
Collaborator

@jordancarlin jordancarlin Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have this be something like get_endianness that returns a type from the Endianness enum and push the check for an Execute access into it? So it becomes something like:

function get_endianness(typ : AccessType(ext_access_type)) -> Endianness = {
     match (typ, effectivePrivilege(typ, mstatus, cur_privilege)) {
       (Execute(), _)   => LittleEndian
       (_, Machine)     => if bits_to_bool(mstatus[MBE]) then BigEndian else LittleEndian,
       (_, Supervisor)  => if bits_to_bool(mstatus[SBE]) then BigEndian else LittleEndian,
       (_, User)        => if bits_to_bool(mstatus[UBE]) then BigEndian else LittleEndian,
     }
 }

Then you can replace all of the uses of let current_endianness: Endianness = if is_big_endian(typ) & typ != Execute() then BigEndian else LittleEndian with just get_endianness(typ).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I will implement this

match effectivePrivilege(typ, mstatus, cur_privilege) {
Machine => bits_to_bool(mstatus[MBE]),
Supervisor => bits_to_bool(mstatus[SBE]),
User => bits_to_bool(mstatus[UBE]),
}
}

// only used for actual memory regions, to avoid MMIO effects
function phys_mem_read forall 'n, 0 < 'n <= max_mem_access . (t : AccessType(ext_access_type), paddr : physaddr, width : int('n), aq : bool, rl: bool, res : bool, meta : bool) -> MemoryOpResult((bits(8 * 'n), mem_meta)) = {
function phys_mem_read forall 'n, 0 < 'n <= max_mem_access . (t : AccessType(ext_access_type), paddr : physaddr, width : int('n), aq : bool, rl: bool, res : bool, meta : bool, to_be: bool) -> MemoryOpResult((bits(8 * 'n), mem_meta)) = {
let result = (match read_kind_of_flags(aq, rl, res) {
Some(rk) => Some(read_ram(rk, paddr, width, meta)),
None() => None()
Expand All @@ -73,9 +81,11 @@ function phys_mem_read forall 'n, 0 < 'n <= max_mem_access . (t : AccessType(ext
(Execute(), None()) => Err(E_Fetch_Access_Fault()),
(Read(Data), None()) => Err(E_Load_Access_Fault()),
(_, None()) => Err(E_SAMO_Access_Fault()),
(_, Some(v, m)) => { if get_config_print_mem()
then print_mem("mem[" ^ to_str(t) ^ "," ^ BitStr(physaddr_bits(paddr)) ^ "] -> " ^ BitStr(v));
Ok(v, m) }
(_, Some(v, m)) => {
let v = endianness_conversion(v, to_be);
if get_config_print_mem()
then print_mem("mem[" ^ to_str(t) ^ "," ^ BitStr(physaddr_bits(paddr)) ^ "] -> " ^ BitStr(v));
Ok(v, m) }
}
}

Expand All @@ -102,10 +112,10 @@ function checked_mem_read forall 'n, 0 < 'n <= max_mem_access . (
Some(e) => Err(e),
None() => {
if within_mmio_readable(paddr, width)
then MemoryOpResult_add_meta(mmio_read(t, paddr, width), default_meta)
then MemoryOpResult_add_meta(mmio_read(t, paddr, width, is_big_endian(t) & not(t == Execute())), default_meta)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it have been easier to just handle endianness in checked_mem_read/write as suggested here:
#751 (comment)

That way I think you wouldn't have needed to touch riscv_platform.sail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For write, yes, I have updated it. But, for read, I don't know how we can handle it here as we print the data in the phys_mem_read functions, so we can get the data from those functions and reverse them here but in the log we will have a different value....

Or am I mis-understanding this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better just to always print them little endian in the log. The code will be a lot simpler, it's debatable which way it should be printed, and I think virtually nobody will be using big endian anyway so it probably doesn't matter too much.

else if within_phys_mem(paddr, width)
then match ext_check_phys_mem_read(t, paddr, width, aq, rl, res, meta) {
Ext_PhysAddr_OK() => phys_mem_read(t, paddr, width, aq, rl, res, meta),
Ext_PhysAddr_OK() => phys_mem_read(t, paddr, width, aq, rl, res, meta, is_big_endian(t) & not(t == Execute())),
Ext_PhysAddr_Error(e) => Err(e)
} else match t {
Execute() => Err(E_Fetch_Access_Fault()),
Expand Down Expand Up @@ -195,7 +205,8 @@ function rvfi_write (Physaddr(addr), width, value, meta, result) = ()
$endif

// only used for actual memory regions, to avoid MMIO effects
function phys_mem_write forall 'n, 0 < 'n <= max_mem_access . (wk : write_kind, paddr : physaddr, width : int('n), data : bits(8 * 'n), meta : mem_meta) -> MemoryOpResult(bool) = {
function phys_mem_write forall 'n, 0 < 'n <= max_mem_access . (wk : write_kind, paddr : physaddr, width : int('n), data : bits(8 * 'n), meta : mem_meta, to_be: bool) -> MemoryOpResult(bool) = {
let data = endianness_conversion(data, to_be);
let result = write_ram(wk, paddr, width, data, meta);
if get_config_print_mem()
then print_mem("mem[" ^ BitStr(physaddr_bits(paddr)) ^ "] <- " ^ BitStr(data));
Expand All @@ -218,12 +229,12 @@ function checked_mem_write forall 'n, 0 < 'n <= max_mem_access . (
Some(e) => Err(e),
None() => {
if within_mmio_writable(paddr, width)
then mmio_write(paddr, width, data)
then mmio_write(paddr, width, data, is_big_endian(typ) & not(typ == Execute()))
else if within_phys_mem(paddr, width)
then {
let wk = write_kind_of_flags(aq, rl, con);
match ext_check_phys_mem_write (wk, paddr, width, data, meta) {
Ext_PhysAddr_OK() => phys_mem_write(wk, paddr, width, data, meta),
Ext_PhysAddr_OK() => phys_mem_write(wk, paddr, width, data, meta, is_big_endian(typ) & not(typ == Execute())),
Ext_PhysAddr_Error(e) => Err(e),
}
} else Err(E_SAMO_Access_Fault())
Expand Down
64 changes: 40 additions & 24 deletions model/riscv_platform.sail
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ val elf_entry = pure {
c: "elf_entry"
} : unit -> int

function endianness_conversion forall 'n, 0 < 'n <= max_mem_access . (data : bits(8 * 'n), to_be: bool) -> bits(8 * 'n) =
if to_be then reverse_endianness(data) else data

// Cache block size is 2^cache_block_size_exp. Max is `max_mem_access` (4096)
// because this model performs `cbo.zero` with a single write, and the behaviour
// with cache blocks larger than a page is not clearly defined.
Expand Down Expand Up @@ -137,8 +140,16 @@ let MTIMECMP_BASE_HI : physaddrbits = zero_extend(0x04004)
let MTIME_BASE : physaddrbits = zero_extend(0x0bff8)
let MTIME_BASE_HI : physaddrbits = zero_extend(0x0bffc)

val clint_load : forall 'n, 'n > 0. (AccessType(ext_access_type), physaddr, int('n)) -> MemoryOpResult(bits(8 * 'n))
function clint_load(t, Physaddr(addr), width) = {
val clint_load : forall 'n, 'n > 0. (AccessType(ext_access_type), physaddr, int('n), bool) -> MemoryOpResult(bits(8 * 'n))
function clint_load(t, Physaddr(addr), width, to_be) = {
//these two registers are 64 bits, so simple conversion is not accurate.
//consider 0x78563412 stored at mtime/cmp and when reading in big endian format,
//we change it to 0x1234567800000000. So position for mtime and mtime_hi are reversed. (not for ld)
var converted_mtime = endianness_conversion(mtime, to_be);
var converted_mtimecmp = endianness_conversion(mtimecmp, to_be);
if (to_be & 'n == 4) then { converted_mtime = converted_mtime[31..0] @ converted_mtime[63..32];
converted_mtimecmp = converted_mtimecmp[31..0] @ converted_mtimecmp[63..32];
};
let addr = addr - plat_clint_base ();
/* FIXME: For now, only allow exact aligned access. */
if addr == MSIP_BASE & ('n == 8 | 'n == 4)
Expand All @@ -150,41 +161,41 @@ function clint_load(t, Physaddr(addr), width) = {
else if addr == MTIMECMP_BASE & ('n == 4)
then {
if get_config_print_platform()
then print_platform("clint<4>[" ^ BitStr(addr) ^ "] -> " ^ BitStr(mtimecmp[31..0]));
then print_platform("clint<4>[" ^ BitStr(addr) ^ "] -> " ^ BitStr(converted_mtimecmp[31..0]));
/* FIXME: Redundant zero_extend currently required by Lem backend */
Ok(zero_extend(32, mtimecmp[31..0]))
Ok(zero_extend(32, converted_mtimecmp[31..0]))
}
else if addr == MTIMECMP_BASE & ('n == 8)
then {
if get_config_print_platform()
then print_platform("clint<8>[" ^ BitStr(addr) ^ "] -> " ^ BitStr(mtimecmp));
then print_platform("clint<8>[" ^ BitStr(addr) ^ "] -> " ^ BitStr(converted_mtimecmp));
/* FIXME: Redundant zero_extend currently required by Lem backend */
Ok(zero_extend(64, mtimecmp))
Ok(zero_extend(64, converted_mtimecmp))
}
else if addr == MTIMECMP_BASE_HI & ('n == 4)
then {
if get_config_print_platform()
then print_platform("clint-hi<4>[" ^ BitStr(addr) ^ "] -> " ^ BitStr(mtimecmp[63..32]));
then print_platform("clint-hi<4>[" ^ BitStr(addr) ^ "] -> " ^ BitStr(converted_mtimecmp[63..32]));
/* FIXME: Redundant zero_extend currently required by Lem backend */
Ok(zero_extend(32, mtimecmp[63..32]))
Ok(zero_extend(32, converted_mtimecmp[63..32]))
}
else if addr == MTIME_BASE & ('n == 4)
then {
if get_config_print_platform()
then print_platform("clint[" ^ BitStr(addr) ^ "] -> " ^ BitStr(mtime));
Ok(zero_extend(32, mtime[31..0]))
then print_platform("clint[" ^ BitStr(addr) ^ "] -> " ^ BitStr(converted_mtime));
Ok(zero_extend(32, converted_mtime[31..0]))
}
else if addr == MTIME_BASE & ('n == 8)
then {
if get_config_print_platform()
then print_platform("clint[" ^ BitStr(addr) ^ "] -> " ^ BitStr(mtime));
Ok(zero_extend(64, mtime))
then print_platform("clint[" ^ BitStr(addr) ^ "] -> " ^ BitStr(converted_mtime));
Ok(zero_extend(64, converted_mtime))
}
else if addr == MTIME_BASE_HI & ('n == 4)
then {
if get_config_print_platform()
then print_platform("clint[" ^ BitStr(addr) ^ "] -> " ^ BitStr(mtime));
Ok(zero_extend(32, mtime[63..32]))
then print_platform("clint[" ^ BitStr(addr) ^ "] -> " ^ BitStr(converted_mtime));
Ok(zero_extend(32, converted_mtime[63..32]))
}
else {
if get_config_print_platform()
Expand Down Expand Up @@ -308,17 +319,20 @@ function reset_htif () -> unit = {
* dispatched the address.
*/

val htif_load : forall 'n, 'n > 0. (AccessType(ext_access_type), physaddr, int('n)) -> MemoryOpResult(bits(8 * 'n))
function htif_load(t, Physaddr(paddr), width) = {
val htif_load : forall 'n, 'n > 0. (AccessType(ext_access_type), physaddr, int('n), bool) -> MemoryOpResult(bits(8 * 'n))
function htif_load(t, Physaddr(paddr), width, to_be) = {
var converted_htif_tohost = endianness_conversion(htif_tohost, to_be);
//htif_tohost is a 64 bit register, same logic as of mtime, mtimecmp for handling big endianness conversion.
if (to_be & width == 4) then converted_htif_tohost = converted_htif_tohost[31..0] @ converted_htif_tohost[63..32];
if get_config_print_platform()
then print_platform("htif[" ^ BitStr(paddr) ^ "] -> " ^ BitStr(htif_tohost));
then print_platform("htif[" ^ BitStr(paddr) ^ "] -> " ^ BitStr(converted_htif_tohost));
/* FIXME: For now, only allow the expected access widths. */
if width == 8 & (paddr == plat_htif_tohost())
then Ok(zero_extend(64, htif_tohost)) /* FIXME: Redundant zero_extend currently required by Lem backend */
then Ok(zero_extend(64, converted_htif_tohost)) /* FIXME: Redundant zero_extend currently required by Lem backend */
else if width == 4 & paddr == plat_htif_tohost()
then Ok(zero_extend(32, htif_tohost[31..0])) /* FIXME: Redundant zero_extend currently required by Lem backend */
then Ok(zero_extend(32, converted_htif_tohost[31..0])) /* FIXME: Redundant zero_extend currently required by Lem backend */
else if width == 4 & paddr == plat_htif_tohost() + 4
then Ok(zero_extend(32, htif_tohost[63..32])) /* FIXME: Redundant zero_extend currently required by Lem backend */
then Ok(zero_extend(32, converted_htif_tohost[63..32])) /* FIXME: Redundant zero_extend currently required by Lem backend */
else match t {
Execute() => Err(E_Fetch_Access_Fault()),
Read(Data) => Err(E_Load_Access_Fault()),
Expand Down Expand Up @@ -406,24 +420,26 @@ $else
function within_mmio_writable forall 'n, 0 < 'n <= max_mem_access . (addr : physaddr, width : int('n)) -> bool = false
$endif

function mmio_read forall 'n, 0 < 'n <= max_mem_access . (t : AccessType(ext_access_type), paddr : physaddr, width : int('n)) -> MemoryOpResult(bits(8 * 'n)) =
function mmio_read forall 'n, 0 < 'n <= max_mem_access . (t : AccessType(ext_access_type), paddr : physaddr, width : int('n), to_be : bool) -> MemoryOpResult(bits(8 * 'n)) =
if within_clint(paddr, width)
then clint_load(t, paddr, width)
then clint_load(t, paddr, width, to_be)
else if within_htif_readable(paddr, width) & (1 <= 'n)
then htif_load(t, paddr, width)
then htif_load(t, paddr, width, to_be)
else match t {
Execute() => Err(E_Fetch_Access_Fault()),
Read(Data) => Err(E_Load_Access_Fault()),
_ => Err(E_SAMO_Access_Fault())
}

function mmio_write forall 'n, 0 <'n <= max_mem_access . (paddr : physaddr, width : int('n), data: bits(8 * 'n)) -> MemoryOpResult(bool) =
function mmio_write forall 'n, 0 <'n <= max_mem_access . (paddr : physaddr, width : int('n), data: bits(8 * 'n), to_be : bool) -> MemoryOpResult(bool) = {
let data = endianness_conversion(data, to_be);
if within_clint(paddr, width)
then clint_store(paddr, width, data)
else if within_htif_writable(paddr, width) & 'n <= 8
then htif_store(paddr, width, data)
else Err(E_SAMO_Access_Fault())

}
/* Platform initialization and ticking. */

function init_platform() -> unit = {
Expand Down
5 changes: 1 addition & 4 deletions model/riscv_sys_control.sail
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,7 @@ function reset_sys() -> unit = {

// "If little-endian memory accesses are supported, the mstatus/mstatush field
// MBE is reset to 0."
// TODO: The handling of mstatush is a bit awkward currently, but the model
// currently only supports little endian so MBE is always 0.
// See https://github.com/riscv/sail-riscv/issues/639
// mstatus[MBE] = 0b0;
mstatus[MBE] = 0b0;

// "The misa register is reset to enable the maximal set of supported extensions"
reset_misa();
Expand Down
11 changes: 8 additions & 3 deletions model/riscv_sys_regs.sail
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ bitfield Mstatus : bits(64) = {
SPP : 8,

MPIE : 7,
UBE : 6,
SPIE : 5,

MIE : 3,
Expand Down Expand Up @@ -255,9 +256,8 @@ function legalize_mstatus(o : Mstatus, v : bits(64)) -> Mstatus = {
// MPELP = v[MPELP],
// MPV = v[MPV],
// GVA = v[GVA],
/* We don't currently support changing MBE and SBE. */
// MBE = v[MBE],
// SBE = v[SBE],
MBE = v[MBE],
SBE = if extensionEnabled(Ext_S) then v[SBE] else 0b0,
/* We don't support dynamic changes to SXL and UXL. */
// SXL = if xlen == 64 then v[SXL] else o[SXL],
// UXL = if xlen == 64 then v[UXL] else o[UXL],
Expand All @@ -281,6 +281,7 @@ function legalize_mstatus(o : Mstatus, v : bits(64)) -> Mstatus = {
SPP = if extensionEnabled(Ext_S) then v[SPP] else 0b0,
VS = v[VS],
MPIE = v[MPIE],
UBE = if extensionEnabled(Ext_U) then v[UBE] else 0b0,
SPIE = if extensionEnabled(Ext_S) then v[SPIE] else 0b0,
MIE = v[MIE],
SIE = if extensionEnabled(Ext_S) then v[SIE] else 0b0,
Expand All @@ -306,6 +307,7 @@ register mstatus : Mstatus = {
}

mapping clause csr_name_map = 0x300 <-> "mstatus"
mapping clause csr_name_map = 0x310 <-> "mstatush"

function clause is_CSR_defined(0x300) = true // mstatus
function clause is_CSR_defined(0x310) = xlen == 32 // mstatush
Expand Down Expand Up @@ -738,6 +740,7 @@ bitfield Sstatus : bits(64) = {
FS : 14 .. 13,
VS : 10 .. 9,
SPP : 8,
UBE : 6,
SPIE : 5,
SIE : 1,
}
Expand All @@ -757,6 +760,7 @@ function lower_mstatus(m : Mstatus) -> Sstatus = {
FS = m[FS],
VS = m[VS],
SPP = m[SPP],
UBE = m[UBE],
SPIE = m[SPIE],
SIE = m[SIE],
]
Expand All @@ -777,6 +781,7 @@ function lift_sstatus(m : Mstatus, s : Sstatus) -> Mstatus = {
FS = s[FS],
VS = s[VS],
SPP = s[SPP],
UBE = s[UBE],
SPIE = s[SPIE],
SIE = s[SIE],
]
Expand Down
1 change: 1 addition & 0 deletions test/first_party/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ set(tests
"test_hello_world.c"
"test_max_pmp.c"
"test_minstret.S"
"test_endianness.S"
)

foreach (xlen IN ITEMS 32 64)
Expand Down
Loading
Loading