Skip to content
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.

Sign up for GitHub

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

Drive protection user bits on TL #2383

Merged
merged 2 commits into from
Mar 31, 2020
Merged

Drive protection user bits on TL #2383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d90652a
Drive protection user bits on TL
jsmithsf Mar 30, 2020
6962cac
Add new dprv bits to RoccExample
jsmithsf Mar 31, 2020
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
2 changes: 2 additions & 0 deletions src/main/resources/vsrc/RoccBlackBox.v
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ module RoccBlackBox
output rocc_mem_req_bits_phys,
output rocc_mem_req_bits_no_alloc,
output rocc_mem_req_bits_no_xcpt,
output [1:0] rocc_mem_req_bits_dprv,
output [coreDataBits-1:0] rocc_mem_req_bits_data,
output [coreDataBytes-1:0] rocc_mem_req_bits_mask,
output rocc_mem_s1_kill,
Expand All @@ -95,6 +96,7 @@ module RoccBlackBox
input [coreDataBits-1:0] rocc_mem_resp_bits_data_word_bypass,
input [coreDataBits-1:0] rocc_mem_resp_bits_data_raw,
input [coreDataBits-1:0] rocc_mem_resp_bits_store_data,
input [1:0] rocc_mem_resp_bits_dprv,
input rocc_mem_replay_next,
input rocc_mem_s2_xcpt_ma_ld,
input rocc_mem_s2_xcpt_ma_st,
Expand Down
15 changes: 15 additions & 0 deletions src/main/scala/rocket/DCache.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package freechips.rocketchip.rocket

import Chisel._
import Chisel.ImplicitConversions._
import freechips.rocketchip.amba._
import freechips.rocketchip.config.Parameters
import freechips.rocketchip.diplomacy._
import freechips.rocketchip.diplomaticobjectmodel.model.OMSRAM
Expand Down Expand Up @@ -551,6 +552,20 @@ class DCacheModule(outer: DCache) extends HellaCacheModule(outer) {
Mux(s2_req.cmd === M_PWR, putpartial,
Mux(!s2_read, put, atomics))))

// Drive APROT Bits
tl_out_a.bits.user.lift(AMBAProt).foreach { x =>
val user_bit_cacheable = edge.manager.supportsAcquireTFast(access_address, a_size)

x.privileged := s2_req.dprv === PRV.M || user_bit_cacheable
Copy link
Contributor

Choose a reason for hiding this comment

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

so all cacheable accesses are also by definition privileged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The relevant point from the AMBA spec is that if a master cannot generate accurate protection information, it is recommended that we mark the transaction as privileged. Because caches like the L2 can merge requests from multiple different privilege modes, we can't accurately provide the protection information.

x.cacheable := user_bit_cacheable

// Following are always tied off
x.fetch := false.B
x.secure := true.B
x.bufferable := false.B
x.modifiable := false.B
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether bufferable/modifiable should be !user_bit_cacheable ?

}

// Set pending bits for outstanding TileLink transaction
val a_sel = UIntToOH(a_source, maxUncachedInFlight+mmioOffset) >> mmioOffset
when (tl_out_a.fire()) {
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/rocket/Frontend.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ class FrontendModule(outer: Frontend) extends LazyModuleImp(outer)
val s2_can_speculatively_refill = s2_tlb_resp.cacheable && !io.ptw.customCSRs.asInstanceOf[RocketCustomCSRs].disableSpeculativeICacheRefill
icache.io.s2_kill := s2_speculative && !s2_can_speculatively_refill || s2_xcpt
icache.io.s2_prefetch := s2_tlb_resp.prefetchable && !io.ptw.customCSRs.asInstanceOf[RocketCustomCSRs].disableICachePrefetch
icache.io.privileged := io.ptw.status.prv === PRV.M

fq.io.enq.valid := RegNext(s1_valid) && s2_valid && (icache.io.resp.valid || !s2_tlb_resp.miss && icache.io.s2_kill)
fq.io.enq.bits.pc := s2_pc
Expand Down
7 changes: 5 additions & 2 deletions src/main/scala/rocket/HellaCache.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package freechips.rocketchip.rocket

import Chisel._
import chisel3.dontTouch
import freechips.rocketchip.amba._
import freechips.rocketchip.config.{Parameters, Field}
import freechips.rocketchip.subsystem._
import freechips.rocketchip.diplomacy._
Expand Down Expand Up @@ -98,6 +99,7 @@ trait HasCoreMemOp extends HasCoreParameters {
val cmd = Bits(width = M_SZ)
val size = Bits(width = log2Ceil(coreDataBytes.log2 + 1))
val signed = Bool()
val dprv = UInt(width = PRV.SZ)
}

trait HasCoreData extends HasCoreParameters {
Expand Down Expand Up @@ -190,8 +192,9 @@ abstract class HellaCache(hartid: Int)(implicit p: Parameters) extends LazyModul
def firstMMIO = (cacheClientParameters.map(_.sourceId.end) :+ 0).max

val node = TLClientNode(Seq(TLMasterPortParameters.v1(
cacheClientParameters ++ mmioClientParameters,
minLatency = 1)))
clients = cacheClientParameters ++ mmioClientParameters,
minLatency = 1,
requestFields = tileParams.core.useVM.option(Seq()).getOrElse(Seq(AMBAProtField())))))

val module: HellaCacheModule

Expand Down
26 changes: 23 additions & 3 deletions src/main/scala/rocket/ICache.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package freechips.rocketchip.rocket

import Chisel._
import Chisel.ImplicitConversions._
import freechips.rocketchip.amba._
import freechips.rocketchip.config.Parameters
import freechips.rocketchip.diplomacy._
import freechips.rocketchip.tile._
Expand Down Expand Up @@ -52,9 +53,12 @@ class ICacheErrors(implicit p: Parameters) extends CoreBundle()(p)

class ICache(val icacheParams: ICacheParams, val hartId: Int)(implicit p: Parameters) extends LazyModule {
lazy val module = new ICacheModule(this)
val masterNode = TLClientNode(Seq(TLMasterPortParameters.v1(Seq(TLMasterParameters.v1(
sourceId = IdRange(0, 1 + icacheParams.prefetch.toInt), // 0=refill, 1=hint
name = s"Core ${hartId} ICache")))))
val useVM = p(TileKey).core.useVM
val masterNode = TLClientNode(Seq(TLMasterPortParameters.v1(
clients = Seq(TLMasterParameters.v1(
sourceId = IdRange(0, 1 + icacheParams.prefetch.toInt), // 0=refill, 1=hint
name = s"Core ${hartId} ICache")),
requestFields = useVM.option(Seq()).getOrElse(Seq(AMBAProtField())))))

val size = icacheParams.nSets * icacheParams.nWays * icacheParams.blockBytes
val itim_control_offset = size - icacheParams.nSets * icacheParams.blockBytes
Expand Down Expand Up @@ -120,6 +124,8 @@ class ICacheBundle(val outer: ICache) extends CoreBundle()(outer.p) {

val clock_enabled = Bool(INPUT)
val keep_clock_enabled = Bool(OUTPUT)

val privileged = Bool(INPUT)
}

class ICacheModule(outer: ICache) extends LazyModuleImp(outer)
Expand Down Expand Up @@ -406,6 +412,7 @@ class ICacheModule(outer: ICache) extends LazyModuleImp(outer)
fromSource = UInt(0),
toAddress = (refill_paddr >> blockOffBits) << blockOffBits,
lgSize = lgCacheBlockBytes)._2

if (cacheParams.prefetch) {
val (crosses_page, next_block) = Split(refill_paddr(pgIdxBits-1, blockOffBits) +& 1, pgIdxBits-blockOffBits)
when (tl_out.a.fire()) {
Expand Down Expand Up @@ -436,6 +443,19 @@ class ICacheModule(outer: ICache) extends LazyModuleImp(outer)
ccover(!refill_valid && (tl_out.d.fire() && !refill_one_beat), "PREFETCH_D_AFTER_MISS_D", "I$ prefetch resolves after miss")
ccover(tl_out.a.fire() && hint_outstanding, "PREFETCH_D_AFTER_MISS_A", "I$ prefetch resolves after second miss")
}
// Drive APROT information
tl_out.a.bits.user.lift(AMBAProt).foreach { x =>
val user_bit_cacheable = edge_out.manager.supportsAcquireTFast(refill_paddr, lgCacheBlockBytes.U)

x.privileged := io.privileged || user_bit_cacheable // privileged if machine mode or memory port
Copy link
Contributor

Choose a reason for hiding this comment

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

oh i see the comment here. so we are assuming everythign cacheable is going to memory port? If not this comment is a bit misleading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, currently we are assuming any cacheable thing is going to the memory port. During a short discussion that I had with @hcook he mentioned that it's possible this may change in the future if we add other types of ports.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems it should be the opposite for safety, i.e. if you cannot figure out the exact privilege level, you should assume the lowest level of privilege.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand ARM's reasoning behind their decision, but we are making this change to be more compatible with ARM behavior. So I think for that reason we should follow the same thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused. Since we cache everything on fetch, why is privileged not just tied to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@terpstra, @aswaterman posed a similar question but was unsure.

x.cacheable := user_bit_cacheable

// Following are always tied off
x.fetch := true.B
x.secure := true.B
x.bufferable := false.B
x.modifiable := false.B
}
tl_out.b.ready := Bool(true)
tl_out.c.valid := Bool(false)
tl_out.e.valid := Bool(false)
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/rocket/PTW.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ class PTW(n: Int)(implicit edge: TLEdgeOut, p: Parameters) extends CoreModule()(
io.mem.req.bits.size := log2Ceil(xLen/8)
io.mem.req.bits.signed := false
io.mem.req.bits.addr := pte_addr
io.mem.req.bits.dprv := PRV.S.U // PTW accesses are S-mode by definition
io.mem.s1_kill := l2_hit || state =/= s_wait1
io.mem.s2_kill := Bool(false)

Expand Down
1 change: 1 addition & 0 deletions src/main/scala/rocket/RocketCore.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ class Rocket(tile: RocketTile)(implicit p: Parameters) extends CoreModule()(p)
io.dmem.req.bits.signed := !ex_reg_inst(14)
io.dmem.req.bits.phys := Bool(false)
io.dmem.req.bits.addr := encodeVirtualAddress(ex_rs(0), alu.io.adder_out)
io.dmem.req.bits.dprv := csr.io.status.dprv
io.dmem.s1_data.data := (if (fLen == 0) mem_reg_rs2 else Mux(mem_ctrl.fp, Fill((xLen max fLen) / fLen, io.fpu.store_data), mem_reg_rs2))
io.dmem.s1_kill := killm_common || mem_ldst_xcpt || fpu_kill_mem
io.dmem.s2_kill := false
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/tile/LazyRoCC.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ class AccumulatorExampleModuleImp(outer: AccumulatorExample)(implicit p: Paramet
io.mem.req.bits.signed := Bool(false)
io.mem.req.bits.data := Bits(0) // we're not performing any stores...
io.mem.req.bits.phys := Bool(false)
io.mem.req.bits.dprv := cmd.bits.status.dprv
}

class TranslatorExample(opcodes: OpcodeSet)(implicit p: Parameters) extends LazyRoCC(opcodes, nPTWPorts = 1) {
Expand Down