riscv / riscv-cheri

This repository contains the CHERI extension specification, adding hardware capabilities to RISC-V ISA to enable fine-grained memory protection and scalable compartmentalization.
https://jira.riscv.org/browse/RVG-148
Creative Commons Attribution 4.0 International
47 stars 28 forks source link

fix invalid address handling definition to cover all bytes in the range #327

Closed tariqkurd-repo closed 2 months ago

tariqkurd-repo commented 2 months ago

fixes https://github.com/riscv/riscv-cheri/issues/326

Timmmm commented 2 months ago

One slightly awkward thing is that you now have to check for invalid addresses of the whole range before alignment checks, because of the priorities:

image

And for something like lr.c, which must be aligned that means you're doing slightly extra work. At least for Codasip's Sv39-based invalid address scheme, if an access is naturally aligned then you only need to check the lowest address is valid, but because the alignment check comes after the invalid address range check you now have to check the lowest and highest.

I dunno if that makes much difference (I guess one extra add?). Just thought I'd point it out...

tariqkurd-repo commented 2 months ago

In all cases a core which supports misaligned load/store must detect whether an access crosses the edge of the valid region. There's no getting away from checking this case. The hardware will detect all the cases in parallel and then prioritise according to the table. So lr.c which crosses the invalid space will take a CHERI invalid address exception before a misaligned exception. This check needs to be done in the hardware anyway as it must detect normal loads which cross the boundary.

The real point here is for all of the CHERI checks to come first, before the RISC-V checks are done.

Timmmm commented 2 months ago

In all cases a core which supports misaligned load/store must detect whether an access crosses the edge of the valid region.

Right but for accesses that must be aligned you can skip that check. I guess it doesn't make much difference for us since it's only AMOs so we need the hardware in place anyway. But doesn't this mean that cores that don't support unaligned access in general will now have to always check that the range crosses the edge of the valid region?

I mean instead of doing this:

if (not isValid(addr)) { raise invalid_address_exception; }
if (not isValid(addr + width - 1)) { raise invalid_address_exception; }
if (misaligned) { raise misaligned; }
// ok

they could do this, eliminating some hardware:

if (misaligned) { raise misaligned; }
if (not isValid(addr)) { raise invalid_address_exception; }
// impossible for it to be aligned, addr to be valid, and addr+width-1 not to be valid.
// ok

The real point here is for all of the CHERI checks to come first, before the RISC-V checks are done.

Ah I didn't know that was a requirement - why does it matter?

tariqkurd-repo commented 2 months ago

CHERI is a checking layer above normal RISC-V, pass the CHERI checks and then it's a normal RISC-V core underneath. It's all designed this way.

jrtc27 commented 2 months ago

For a more concrete justification in this specific instance, it’s so that your unaligned access handler doesn’t have to perfectly replicate all the same checks in software, which is messy and error-prone

Timmmm commented 2 months ago

Ah that makes sense, thanks!