diff --git a/bittide/src/Bittide/Calendar.hs b/bittide/src/Bittide/Calendar.hs index 95a55191a..187ed9597 100644 --- a/bittide/src/Bittide/Calendar.hs +++ b/bittide/src/Bittide/Calendar.hs @@ -38,7 +38,8 @@ import Protocols import Protocols.MemoryMap import Protocols.MemoryMap.FieldType (FieldType, ToFieldType (..), Var) import Protocols.MemoryMap.Registers.WishboneStandard ( - RegisterConfig (access, description), + Lock (NoLock), + RegisterConfig (access, description, lock), busActivityWrite, deviceWb, registerConfig, @@ -393,11 +394,13 @@ mkCalendarC (registerConfig "shadowEntry") { access = WriteOnly , description = "Stores the shadow entry before writing it to the shadow calendar." + , lock = NoLock } readEntryCfg = (registerConfig "shadowEntry") { access = ReadOnly , description = "Contains the shadow entry at the read address in the shadow calendar." + , lock = NoLock } writeAddrCfg = (registerConfig "writeAddr") diff --git a/bittide/src/Bittide/ClockControl/Freeze.hs b/bittide/src/Bittide/ClockControl/Freeze.hs index 64c09c80e..0128096e5 100644 --- a/bittide/src/Bittide/ClockControl/Freeze.hs +++ b/bittide/src/Bittide/ClockControl/Freeze.hs @@ -13,7 +13,8 @@ import Protocols import Protocols.MemoryMap (Access (ReadOnly, WriteOnly), ConstBwd, MM) import Protocols.MemoryMap.Registers.WishboneStandard ( BusActivity (BusWrite), - RegisterConfig (access, description), + Lock (NoLock), + RegisterConfig (access, description, lock), deviceWb, registerConfig, registerWb, @@ -95,24 +96,28 @@ freeze clk rst = { access = ReadOnly , description = "Elastic buffer counters (at last freeze). Note that this is coming from domain difference counters, not actual elastic buffers." + , lock = NoLock } lastSyncPulseConfig = (registerConfig "cycles_since_sync_pulse") { access = ReadOnly , description = "Number of clock cycles since last synchronization pulse (at last freeze)" + , lock = NoLock } syncCounterConfig = (registerConfig "number_of_sync_pulses_seen") { access = ReadOnly , description = "Number of synchronization pulses seen (at last freeze)" + , lock = NoLock } localClockConfig = (registerConfig "local_clock_counter") { access = ReadOnly , description = "Clock counter of local clock domain (at last freeze)" + , lock = NoLock } freezeCounterConfig = diff --git a/clash-bitpackc/src/Clash/Class/BitPackC.hs b/clash-bitpackc/src/Clash/Class/BitPackC.hs index a627d2a32..2fbad53de 100644 --- a/clash-bitpackc/src/Clash/Class/BitPackC.hs +++ b/clash-bitpackc/src/Clash/Class/BitPackC.hs @@ -249,6 +249,24 @@ class -- Strip padding introduced by 'MultipleOf' size requirements msbResize val + -- | Returns 'True' if the type is atomic or a newtype wrapper around an atomic + -- type (single constructor with single field). An atomic type is a type that + -- can be read or written in a single bus access. Or rather, a type that + -- *will* be read or written in a single bus access in practice. Examples of + -- atomic types are 'BitVector', 'Unsigned', 'Signed', 'Float', 'Double', and + -- 'Bool'. + -- + -- Note that atomic types may still get written in multiple bus accesses if the + -- bus width is smaller than the type's packed size. + isAtomicC :: Proxy a -> Bool + default isAtomicC :: + ( Generic a + , GIsAtomicC (Rep a) + ) => + Proxy a -> + Bool + isAtomicC _ = gIsAtomicC (Proxy @(Rep a)) + {- | 'resize', but biased towards the most significant bit (MSB). That is, if the bit size of the source type is larger than the destination type, it will discard the least significant bits (LSBs) instead of the MSBs. If the bit size @@ -286,6 +304,8 @@ instance (KnownNat n) => BitPackC (BitVector n) where packC# byteOrder = toEndianBV byteOrder . resize maybeUnpackC# byteOrder = checkFits . fromEndianBV byteOrder + isAtomicC _ = True + instance (KnownNat n) => BitPackC (Unsigned n) where type ConstructorSizeC (Unsigned n) = 0 type AlignmentBoundaryC (Unsigned n) = AlignmentBoundaryC (BitVector n) @@ -294,6 +314,8 @@ instance (KnownNat n) => BitPackC (Unsigned n) where packC# byteOrder = packC# byteOrder . pack maybeUnpackC# byteOrder = fmap unpack . maybeUnpackC# byteOrder + isAtomicC _ = True + instance (KnownNat n) => BitPackC (Signed n) where type ConstructorSizeC (Signed n) = 0 type AlignmentBoundaryC (Signed n) = AlignmentBoundaryC (BitVector n) @@ -302,6 +324,8 @@ instance (KnownNat n) => BitPackC (Signed n) where packC# byteOrder = unpack . toEndianBV byteOrder . pack . resize maybeUnpackC# byteOrder = checkFits . unpack . fromEndianBV byteOrder + isAtomicC _ = True + {- | Checks whether the argument fits within the bounds of the result type. Only works when @BitSize (f a) <= @BitSize (f b)@. -} @@ -333,6 +357,8 @@ instance (KnownNat n, 1 <= n) => BitPackC (Index n) where then Nothing else Just (unpack val1) + isAtomicC _ = True + instance BitPackC Float where type ConstructorSizeC Float = 0 type AlignmentBoundaryC Float = 4 @@ -341,6 +367,8 @@ instance BitPackC Float where packC# byteOrder = toEndianBV byteOrder . pack maybeUnpackC# byteOrder = Just . unpack . toEndianBV byteOrder + isAtomicC _ = True + instance BitPackC Double where type ConstructorSizeC Double = 0 type AlignmentBoundaryC Double = 8 @@ -349,6 +377,8 @@ instance BitPackC Double where packC# byteOrder = toEndianBV byteOrder . pack maybeUnpackC# byteOrder = Just . unpack . toEndianBV byteOrder + isAtomicC _ = True + instance BitPackC Bool where type ConstructorSizeC Bool = 0 type AlignmentBoundaryC Bool = 1 @@ -359,6 +389,8 @@ instance BitPackC Bool where False -> 0 maybeUnpackC# _byteOrder v = Just (testBit v 0) + isAtomicC _ = True + instance BitPackC Bit where type ConstructorSizeC Bit = 0 type AlignmentBoundaryC Bit = 1 @@ -367,6 +399,8 @@ instance BitPackC Bit where packC# _byteOrder = resize . pack maybeUnpackC# _byteOrder v = Just (boolToBit (testBit v 0)) + isAtomicC _ = True + instance BitPackC () where type ConstructorSizeC () = 0 type AlignmentBoundaryC () = 1 @@ -375,6 +409,8 @@ instance BitPackC () where packC# _byteOrder _val = 0 maybeUnpackC# _byteOrder _val = Just () + isAtomicC _ = True + instance (BitPackC a, KnownNat n) => BitPackC (Vec n a) where type ConstructorSizeC (Vec n a) = 0 type AlignmentBoundaryC (Vec n a) = AlignmentBoundaryC a @@ -383,6 +419,11 @@ instance (BitPackC a, KnownNat n) => BitPackC (Vec n a) where packC# byteOrder = pack . map (packC# byteOrder) maybeUnpackC# byteOrder = sequence . map (maybeUnpackC# byteOrder) . unpack + isAtomicC _ = + -- TODO: I guess a unary vector is atomic if its element is? I'm not sure how + -- Rust handles this. + False + instance (BitPackC a) => BitPackC (Maybe a) instance (BitPackC a, BitPackC b) => BitPackC (Either a b) instance (BitPackC a, BitPackC b) => BitPackC (a, b) @@ -395,7 +436,8 @@ instance BitPackC T where { \ type AlignmentBoundaryC T = AlignmentBoundaryC (TT); \ type ByteSizeC T = ByteSizeC (TT); \ packC# byteOrder = packC# byteOrder . numConvert @_ @(TT); \ - maybeUnpackC# byteOrder = fmap numConvert . maybeUnpackC# @(TT) byteOrder \ + maybeUnpackC# byteOrder = fmap numConvert . maybeUnpackC# @(TT) byteOrder; \ + isAtomicC _ = isAtomicC (Proxy @(TT)) \ } THROUGH_INST (Word8, Unsigned 8) @@ -541,3 +583,25 @@ instance GBitPackC boundary U1 where gPackFieldsC _byteOrder _start cc _u = (cc, 0) gUnpackFieldsC _byteOrder _start _selectedConstr _currentConstr _packed = Just U1 + +-- | Generic class to determine if a type is atomic +class GIsAtomicC f where + gIsAtomicC :: Proxy f -> Bool + +instance (GIsAtomicC a) => GIsAtomicC (M1 m d a) where + gIsAtomicC _ = gIsAtomicC (Proxy @a) + +instance (GIsAtomicC f, GIsAtomicC g) => GIsAtomicC (f :+: g) where + gIsAtomicC _ = + -- TODO: Pure sum types (i.e., not sum-of-product types) are atomic, but it is + -- not immediately clear to me how to implement that. + False + +instance (GIsAtomicC f, GIsAtomicC g) => GIsAtomicC (f :*: g) where + gIsAtomicC _ = False + +instance (BitPackC c) => GIsAtomicC (K1 i c) where + gIsAtomicC _ = isAtomicC (Proxy @c) + +instance GIsAtomicC U1 where + gIsAtomicC _ = True diff --git a/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard.hs b/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard.hs index f280aea92..50c25b7c7 100644 --- a/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard.hs +++ b/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard.hs @@ -31,6 +31,7 @@ module Protocols.MemoryMap.Registers.WishboneStandard ( BusActivity (..), DeviceConfig (..), RegisterConfig (..), + Lock (..), ) where import Clash.Explicit.Prelude diff --git a/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard/Internal.hs b/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard/Internal.hs index 2c5edf2de..e0e208f76 100644 --- a/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard/Internal.hs +++ b/clash-protocols-memmap/src/Protocols/MemoryMap/Registers/WishboneStandard/Internal.hs @@ -1,6 +1,7 @@ -- SPDX-FileCopyrightText: 2025 Google LLC -- -- SPDX-License-Identifier: Apache-2.0 +{-# LANGUAGE MultiWayIf #-} module Protocols.MemoryMap.Registers.WishboneStandard.Internal where @@ -15,8 +16,9 @@ import Data.Constraint (Dict (Dict)) import Data.Constraint.Nat.Lemmas (divWithRemainder) import Data.Data (Proxy (Proxy)) import Data.Kind (Type) -import Data.Maybe (fromMaybe, isJust) +import Data.Maybe (fromMaybe, maybeToList) import GHC.Stack (HasCallStack, SrcLoc) +import Protocols.Idle (idleSource) import Protocols.MemoryMap ( Access (ReadOnly, ReadWrite, WriteOnly), ConstBwd, @@ -39,12 +41,13 @@ import Protocols.Wishbone ( WishboneM2S (..), WishboneMode (Standard), WishboneS2M (..), - emptyWishboneS2M, + emptyWishboneM2S, ) import qualified Data.List as L import qualified Data.Map as Map import qualified Data.String.Interpolate as I +import qualified Protocols.Wishbone as Wishbone data BusActivity a = BusRead a | BusWrite a deriving (Show, Eq, Functor, Generic, NFDataX) @@ -54,6 +57,18 @@ busActivityWrite :: Maybe (BusActivity a) -> Maybe a busActivityWrite (Just (BusWrite a)) = Just a busActivityWrite _ = Nothing +-- | Like 'emptyWishboneS2M', but easier for type inference in this module. +emptyWishboneS2M :: forall n. (KnownNat n) => WishboneS2M (Bytes n) +emptyWishboneS2M = (Wishbone.emptyWishboneS2M @(Bytes n)) + +-- | 'WishboneS2M' with its acknowledge set to 'True' +ackWishboneS2M :: forall n. (KnownNat n) => WishboneS2M (Bytes n) +ackWishboneS2M = emptyWishboneS2M{acknowledge = True} + +-- | Whether the Wishbone manager is active (i.e., 'strobe' and 'busCycle' are both 'True') +managerActive :: WishboneM2S aw wordSize (Bytes wordSize) -> Bool +managerActive m2s = m2s.strobe && m2s.busCycle + {- | Type synonym for all the information needed to create a Wishbone register with auto-assigned offsets. -} @@ -109,11 +124,59 @@ data RegisterMeta aw = RegisterMeta { name :: SimOnly Name , srcLoc :: SimOnly SrcLoc , register :: SimOnly Register + , lockRegister :: SimOnly (Maybe Register) + , hasLockRegister :: Bool , nWords :: BitVector (aw + 1) -- ^ Number of words occupied by this register. Note that it is (+1) because we need - -- to be able to represent registers that occupy the full address space. + -- to be able to represent registers that occupy the full address space. Also note + -- that this includes any lock register that may be present. } +data LockRequest + = -- | Lock the register to its current value. Writing any value to the register will + -- have no effect until a 'Commit' is written. + Lock + | -- | Unlock the register and update its value to the value written while locked. + Commit + | -- | Unlock the register and discard any writes that happened while locked. + Clear + deriving (Show, Eq, Generic, BitPackC, ToFieldType, NFDataX) + +-- | Type synonym to make type signatures clearer. +type InLock = Bool + +-- | State of the lock register, stores the data when locked. +data LockState nWords wordSize + = Unlocked + | Locked (Vec nWords (Bytes wordSize)) + deriving (Show, Eq, Generic, NFDataX) + +-- | Map a function over the data stored in a 'LockState' (if any). +mapLockState :: + (Vec nWords (Bytes wordSize) -> Vec nWords (Bytes wordSize)) -> + LockState nWords wordSize -> + LockState nWords wordSize +mapLockState _ Unlocked = Unlocked +mapLockState f (Locked d) = Locked (f d) + +-- | Convert a 'LockState' to a 'Maybe' value, discarding the 'Unlocked' state. +lockStateToMaybe :: LockState nWords wordSize -> Maybe (Vec nWords (Bytes wordSize)) +lockStateToMaybe Unlocked = Nothing +lockStateToMaybe (Locked d) = Just d + +{- | Whether to add a lock register for a given register. A lock register allows +atomic updates to registers that needs to be updated or read from in multiple +bus cycles. +-} +data Lock + = -- | Never add a lock register. + NoLock + | -- | Add a lock based on the size of the register and the bus word size. If the + -- register fits within a single bus word, is not zero-width, and an atomic + -- structure (see 'BitPackC') then no lock is added. + Auto + deriving (Show, Eq) + {- | Meta information for a zero-width register. Zero-width registers do not take up any flip-flops, but do need to be mapped on the bus to be able to observe bus activity. @@ -141,6 +204,8 @@ zeroWidthRegisterMeta Proxy conf = , tags = "zero-width" : conf.tags , reset = Nothing } + , lockRegister = SimOnly Nothing + , hasLockRegister = False , -- BitPackC would report a size of 0, but we want to be able to observe -- the bus activity, so an address needs to be reserved anyway. For this, -- the number of words gets overwritten to 1 here. @@ -171,6 +236,8 @@ data RegisterConfig = RegisterConfig , busRead :: BusReadBehavior -- ^ Behavior when a bus read occurs at the same time as a circuit write. Default: -- 'PreferRegister'. + , lock :: Lock + -- ^ Whether to add a lock register for this register. Default: 'Auto'. } deriving (Show) @@ -196,6 +263,7 @@ registerConfig name = , tags = [] , access = ReadWrite , busRead = PreferRegister + , lock = Auto } -- These have no business being in this module :) @@ -250,16 +318,36 @@ deviceWithOffsetsWb deviceName = unSimOnly (SimOnly a) = a -- Note that we filter zero-width registers out here - metaToRegister :: Offset aw -> RegisterMeta aw -> NamedLoc Register - metaToRegister o m = - NamedLoc - { name = unSimOnly m.name - , loc = unSimOnly m.srcLoc - , value = - (unSimOnly m.register) - { address = fromIntegral o * natToNum @wordSize - } - } + metaToRegisters :: Offset aw -> RegisterMeta aw -> [NamedLoc Register] + metaToRegisters o m = + baseRegister : maybeToList (makeLockRegister <$> unSimOnly m.lockRegister) + where + -- XXX: Quasiquoter doesn't support record dot access + baseName = baseRegister.name.name + + makeLockRegister r = + NamedLoc + { name = + Name + { name = baseRegister.name.name <> "_lock" + , description = [I.i|Lock register for '#{baseName}'|] + } + , loc = unSimOnly m.srcLoc + , value = + r + { address = (fromIntegral o + fromIntegral m.nWords - 1) * natToNum @wordSize + } + } + + baseRegister = + NamedLoc + { name = unSimOnly m.name + , loc = unSimOnly m.srcLoc + , value = + (unSimOnly m.register) + { address = fromIntegral o * natToNum @wordSize + } + } mm = MemoryMap @@ -267,7 +355,7 @@ deviceWithOffsetsWb deviceName = Map.singleton deviceName $ DeviceDefinition { deviceName = Name{name = deviceName, description = ""} - , registers = L.zipWith metaToRegister (toList offsets) (toList metas) + , registers = L.concat $ L.zipWith metaToRegisters (toList offsets) (toList metas) , definitionLoc = locN 0 , tags = [] } @@ -337,8 +425,466 @@ deviceWithOffsetsWb deviceName = | clashSimulation = foldl (\b a -> a `seqX` b) () v `seqX` v | otherwise = v -{- | Circuit writes always take priority over bus writes. Bus writes rejected with -an error if access rights are set to 'ReadOnly'. Similarly, bus reads are rejected +{- | Circuit internal to 'registerWbDf' that rejects illegal accesses on the bus. +If an illegal access takes place, an error is returned on the wishbone bus and +the request isn't propagated to the RHS. +-} +accessC :: + forall dom aw wordSize. + ( KnownNat wordSize + , KnownNat aw + , wordSize ~ Div ((wordSize * 8) + 7) 8 + ) => + Access -> + Circuit + (Wishbone dom 'Standard aw (Bytes wordSize)) + (Wishbone dom 'Standard aw (Bytes wordSize)) +accessC access = + Circuit $ \(wbM2SIn, wbS2MOut) -> + unbundle $ checkAccess <$> wbM2SIn <*> wbS2MOut + where + checkAccess :: + WishboneM2S aw wordSize (Bytes wordSize) -> + WishboneS2M (Bytes wordSize) -> + ( WishboneS2M (Bytes wordSize) + , WishboneM2S aw wordSize (Bytes wordSize) + ) + checkAccess m2s s2m + | managerActive m2s && accessFault = + ( (emptyWishboneS2M @wordSize){err = True, readData = 0} + , emptyWishboneM2S + ) + | otherwise = (s2m, m2s) + where + readOnlyFault = access == ReadOnly && m2s.writeEnable + writeOnlyFault = access == WriteOnly && not m2s.writeEnable + accessFault = readOnlyFault || writeOnlyFault + +{- | Circuit internal to 'registerWbDf' that reorders bytes on the bus if they +do not correspond to the register's internal byte order. The LHS is expected +to be in bus byte order, the RHS in register byte order. +-} +orderC :: + forall dom aw wordSize. + ( KnownNat wordSize + , ?busByteOrder :: ByteOrder + , ?regByteOrder :: ByteOrder + ) => + Circuit + (Wishbone dom 'Standard aw (Bytes wordSize)) + (Wishbone dom 'Standard aw (Bytes wordSize)) +orderC = Circuit go + where + needReverse = ?busByteOrder /= ?regByteOrder + + go (wbM2SIn, wbS2MOut) = (wbS2MIn, wbM2SOut) + where + wbM2SOut = if needReverse then reorderM2S <$> wbM2SIn else wbM2SIn + wbS2MIn = if needReverse then reorderS2M <$> wbS2MOut else wbS2MOut + + reorderM2S m2s = + m2s + { writeData = reverseBytes m2s.writeData + , busSelect = reverseBits m2s.busSelect + } + + reorderS2M s2m = s2m{readData = reverseBytes s2m.readData} + +{- | Circuit internal to 'registerWbDf' that computes the relative offset on the +bus, i.e., the offset within the register and replaces the address field with it. +-} +offsetC :: + forall a dom wordSize aw nWords. + ( RegisterWbConstraints a dom wordSize aw + , wordSize ~ Div ((wordSize * 8) + 7) 8 + , nWords ~ SizeInWordsC wordSize a + , KnownNat nWords + , 1 <= nWords + ) => + Proxy a -> + Offset aw -> + -- | Whether the register has a lock register + Bool -> + Circuit + (Wishbone dom 'Standard aw (Bytes wordSize)) + (Wishbone dom 'Standard aw (Bytes wordSize)) +offsetC Proxy offset hasLockRegister = + Circuit $ \(wbM2SIn, wbS2MOut) -> + ( wbS2MOut + , if hasLockRegister + then computeRelativeOffset (SNat @(nWords + 1)) offset <$> wbM2SIn + else computeRelativeOffset (SNat @nWords) offset <$> wbM2SIn + ) + +{- | Compute the relative offset on the bus and replace the address field with +it. A relative offset is the offset within the register. E.g., for a register +occupying 4 words, the possible relative offsets are 0, 1, 2, and 3. + +Note: If a lock register is present, the number of words passed in should +include the lock register. +-} +computeRelativeOffset :: + forall nWords wordSize aw. + (1 <= nWords, KnownNat aw) => + SNat nWords -> + Offset aw -> + WishboneM2S aw wordSize (Bytes wordSize) -> + WishboneM2S aw wordSize (Bytes wordSize) +computeRelativeOffset SNat offset m2s = m2s{addr = resize (pack relativeOffset)} + where + relativeOffset :: Index nWords + relativeOffset = unpack $ addrLsbs - offsetLsbs + + offsetLsbs = resize offset :: BitVector (BitSize (Index nWords)) + addrLsbs = resize m2s.addr :: BitVector (BitSize (Index nWords)) + +{- | Circuit internal to 'registerWbDf' that produces register meta information. +The address field is left as zero and will be set by 'deviceWithOffsetsWb'. +-} +metaC :: + forall a aw wordSize. + ( ToFieldType a + , BitPackC a + , NFDataX a + , KnownNat aw + , KnownNat wordSize + , 1 <= wordSize + , HasCallStack + , ?regByteOrder :: ByteOrder + ) => + SNat wordSize -> + Proxy a -> + RegisterConfig -> + a -> + Circuit (ConstBwd (RegisterMeta aw)) (ConstFwd Bool) +metaC SNat Proxy conf resetValue = Circuit $ \((), ()) -> (regMeta, hasLockRegister) + where + regMeta = + RegisterMeta + { name = SimOnly $ Name{name = conf.name, description = conf.description} + , srcLoc = SimOnly locCaller + , nWords = nWords + if hasLockRegister then 1 else 0 + , hasLockRegister + , lockRegister = + SimOnly + $ if hasLockRegister + then + Just + $ Register + { fieldType = regType @LockRequest + , address = 0x0 -- Note: will be set by 'deviceWithOffsetsWb' + , access = WriteOnly + , tags = ["is-lock"] + , reset = Nothing + } + else Nothing + , register = + SimOnly + $ Register + { fieldType = regType @a + , address = 0x0 -- Note: will be set by 'deviceWithOffsetsWb' + , access = conf.access + , tags = conf.tags <> ["has-lock" | hasLockRegister] + , reset = Just simOnlyResetValue + } + } + + nWords = natToNum @(SizeInWordsC wordSize a) + + hasLockRegister = + case conf.lock of + NoLock -> False + Auto -> nWords > 1 || not (isAtomicC (Proxy @a)) + + -- The fact that this goes into a 'SimOnly' construct should be enough for Clash to + -- deduce that it can throw the expression below away, but it doesn't. As a result, + -- it sees the constructor of BitVector and freaks out. Spelling it out like this + -- (i.e., putting it behind a 'clashSimulation' flag) makes Clash okay with it. + simOnlyResetValue + | clashSimulation = (pack (packWordC @wordSize ?regByteOrder resetValue)).unsafeToNatural + | otherwise = 0 + +{- | Circuit internal to 'registerWbDf' that implements the lock register +functionality. When no lock register is present, this circuit is a passthrough. +When there is, it acts as a passthrough until a lock is requested, after which +writes are stored until a 'Commit' or 'Clear' is received. During a lock, it +also indicates that the register is locked on the 'InLock' output. The normal +register (in 'registerC') must use this signal to refuse any writes from the +circuit side. + +TODO: We currently duplicate 'maskWriteData' for the shadow register and the + shadow register, even though in practice only one of them will be doing + useful work in one specific clock cycle. I've played around with sharing + the logic, but it just makes the code more complex for very little gain (?). +-} +lockC :: + forall a dom wordSize aw nWords. + ( RegisterWbConstraints a dom wordSize aw + , wordSize ~ Div ((wordSize * 8) + 7) 8 + , nWords ~ SizeInWordsC wordSize a + , KnownNat nWords + , 1 <= nWords + ) => + Proxy a -> + Clock dom -> + Reset dom -> + -- | Whether the register has a lock register. If not, this circuit is a + -- passthrough. + Bool -> + Circuit + ( Wishbone dom Standard aw (Bytes wordSize) + , CSignal dom (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) -- Packed current register value + ) + ( Wishbone dom Standard aw (Bytes wordSize) + , Df dom (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) + , CSignal dom InLock + ) +lockC Proxy _clk _rst False = circuit $ \(wb, _currentValue) -> do + df <- idleSource + idC -< (wb, df, Fwd (pure False)) +lockC Proxy clk rst True = Circuit go0 + where + commitAddress = natToNum @nWords @(BitVector aw) + + go0 :: + ( ( Signal dom (WishboneM2S aw wordSize (Bytes wordSize)) + , Signal dom (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) -- Packed current register value + ) + , ( Signal dom (WishboneS2M (Bytes wordSize)) + , Signal dom Ack + , Signal dom () + ) + ) -> + ( ( Signal dom (WishboneS2M (Bytes wordSize)) + , Signal dom () + ) + , ( Signal dom (WishboneM2S aw wordSize (Bytes wordSize)) + , Signal dom (Maybe (Vec (SizeInWordsC wordSize a) (Bytes wordSize))) + , Signal dom InLock + ) + ) + go0 ((m2sIn, currentValueIn), (s2mIn, lockWriteAck, _inLockAck)) = ((s2mOut, pure ()), (m2sOut, lockWrite, inLock)) + where + (s2mOut, m2sOut, lockWrite, inLock) = + mealyB clk rst enableGen go1 Unlocked (m2sIn, s2mIn, lockWriteAck, currentValueIn) + + go1 :: + LockState nWords wordSize -> + ( WishboneM2S aw wordSize (Bytes wordSize) + , WishboneS2M (Bytes wordSize) + , Ack + , Vec (SizeInWordsC wordSize a) (Bytes wordSize) -- Packed current register value + ) -> + ( LockState nWords wordSize + , ( WishboneS2M (Bytes wordSize) + , WishboneM2S aw wordSize (Bytes wordSize) + , Maybe (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) + , InLock + ) + ) + go1 s (m2s, s2mIn, _, currentValue) = + if + -- Just passthrough if.... + -- + -- 1. The bus is not active + | not (managerActive m2s) + -- 2. If the bus is *not* accessing the lock register and we're not locked + -- Note that if we *are* locked, we need to intercept reads and writes. + || (not isCommitAddress && not locked) + -- 3. If the bus is *not* accessing the lock register, we're locked, and + -- the bus is reading. Note that we expect data to be stored in the + -- normal register still. (Maybe we shouldn't?) + || (not isCommitAddress && not m2s.writeEnable) -> + passthrough + -- If the bus is *not* accessing the lock register, we're locked, and the + -- bus is writing, we need to update the lock register. + | not isCommitAddress -> + ( mapLockState (maskWriteData offset m2s.busSelect m2s.writeData) s + , (ackWishboneS2M, emptyWishboneM2S, Nothing, locked) + ) + -- Bus is writing to the lock register + | m2s.writeEnable -> + case (lockRequest, s) of + (Lock, _) -> + -- Lock the register - copy current value to shadow. We copy the + -- current value to support partial updates while locked, for example + -- vectors. + (Locked currentValue, (ackWishboneS2M, emptyWishboneM2S, Nothing, locked)) + (Commit, Locked d) -> + -- Commit the shadow writes + (Unlocked, (ackWishboneS2M, emptyWishboneM2S, Just d, locked)) + (Commit, Unlocked) -> + -- Error: trying to commit when not locked + (Unlocked, (emptyWishboneS2M{err = True}, emptyWishboneM2S, Nothing, locked)) + (Clear, _) -> + -- Clear the lock (works whether locked or not) + (Unlocked, (ackWishboneS2M, emptyWishboneM2S, Nothing, locked)) + -- Bus is reading from the lock register (write-only), return error + | otherwise -> + (s, (emptyWishboneS2M{err = True}, emptyWishboneM2S, Nothing, locked)) + where + passthrough = (s, (s2mIn, m2s, Nothing, locked)) + + offset = unpack $ resize m2s.addr + isCommitAddress = m2s.addr == commitAddress + locked = + case s of + Unlocked -> False + Locked _ -> True + + lockRequest :: LockRequest + lockRequest = + fromMaybe err + . maybeUnpackWordC ?regByteOrder + $ maskWriteData 0 m2s.busSelect m2s.writeData (repeat m2s.writeData) + + -- XXX: Quasiquoter doesn't work with implicit parameters nor record dot selectors + regByteOrder = ?regByteOrder + writeData = m2s.writeData + busSelect = m2s.busSelect + err = + deepErrorX + [I.i| + Unpack failed for LockRequest in registerWbDf: + wordSize: #{natToInteger @wordSize} + regByteOrder: #{regByteOrder} + m2s.writeData: #{writeData} + m2s.busSelect: #{busSelect} + |] + +{- | Circuit internal to 'registerWbDf' that holds the actual register value. It +assumes that access control, byte ordering, and offset calculation have already +been taken care of. See 'accessC', 'orderC', and 'offsetC'. +-} +registerC :: + forall a dom wordSize aw nWords. + ( RegisterWbConstraints a dom wordSize aw + , nWords ~ SizeInWordsC wordSize a + , KnownNat nWords + , 1 <= nWords + ) => + Clock dom -> + Reset dom -> + -- | Configuration values + RegisterConfig -> + -- | Reset value + a -> + Circuit + ( Wishbone dom Standard aw (Bytes wordSize) + , -- \| Write from circuit + CSignal dom (Maybe a) + , -- \| Data from lock register + Df dom (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) + , -- \| In lock? + CSignal dom InLock + ) + ( CSignal dom a -- Current value + , CSignal dom (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) -- Current value (packed) + , Df dom (BusActivity a) + ) +registerC clk rst regConfig resetValue = circuit $ \(wb, aIn, lockWrite, inLock) -> do + let packedOut = regMaybe clk rst enableGen (packC resetValue) packedWrite + (Fwd packedWrite, busActivity) <- goC -< (wb, aIn, lockWrite, inLock, Fwd packedOut) + idC -< (Fwd (unpackC <$> packedOut), Fwd packedOut, busActivity) + where + goC :: + Circuit + ( Wishbone dom Standard aw (Bytes wordSize) + , CSignal dom (Maybe a) -- Write from circuit + , Df dom (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) + , CSignal dom InLock + , CSignal dom (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) + ) + ( CSignal dom (Maybe (Vec (SizeInWordsC wordSize a) (Bytes wordSize))) + , Df dom (BusActivity a) + ) + goC = Circuit $ \((m2s, circuitWrite, lockWrite, inLock, registerValue), (_, busActivityAck)) -> do + let + (s2m, registerWrite, busActivity) = + unbundle + $ case divWithRemainder @wordSize @8 @7 of + Dict -> + go + <$> m2s + <*> (fmap packC <$> circuitWrite) + <*> lockWrite + <*> inLock + <*> registerValue + <*> busActivityAck + + ((s2m, pure (), busActivityAck, pure (), pure ()), (registerWrite, busActivity)) + + go :: + WishboneM2S aw wordSize (Bytes wordSize) -> + -- \| Circuit write + Maybe (Vec nWords (Bytes wordSize)) -> + -- \| Lock write + Maybe (Vec nWords (Bytes wordSize)) -> + InLock -> + Vec nWords (Bytes wordSize) -> + Ack -> + ( WishboneS2M (Bytes wordSize) + , Maybe (Vec nWords (Bytes wordSize)) + , Maybe (BusActivity a) + ) + go m2s circuitWrite lockWrite inLock registerValue ~(Ack acknowledge) + | Just d <- lockWrite = + ( emptyWishboneS2M + , if acknowledge then lockWrite else Nothing + , Just (BusWrite (unpackC d)) + ) + | m2s.strobe && m2s.busCycle = (s2m, registerWrite, busActivity) + | inLock = (emptyWishboneS2M, Nothing, Nothing) + | otherwise = (emptyWishboneS2M, circuitWrite, Nothing) + where + -- Note that every definition below this comment is only relevant when the + -- bus is **active** (and therefore not handling a lock write, as these two + -- events are mutually exclusive). + busActivity + | Just v <- registerValueAfterBusWrite = Just (BusWrite (unpackC v)) + | otherwise = (Just (BusRead (unpackC registerValue))) + + registerValueAfterBusWrite + | m2s.writeEnable = + Just (maskWriteData addressAsIndex m2s.busSelect m2s.writeData registerValue) + | otherwise = Nothing + + registerWrite + | acknowledge = circuitWrite <|> registerValueAfterBusWrite + | otherwise = Nothing + + addressAsIndex = unpack @(Index nWords) (resize m2s.addr) + s2m = (emptyWishboneS2M @nWords){acknowledge, readData} + readData + -- Though we could return a deepErrorX here, doing so would leak register data + | m2s.writeEnable = 0 + | PreferRegister <- regConfig.busRead = registerValue !! addressAsIndex + | PreferCircuit <- regConfig.busRead = + (fromMaybe registerValue circuitWrite) !! addressAsIndex + + -- Alias for packing with the correct word size and byte order + packC :: a -> Vec (SizeInWordsC wordSize a) (Bytes wordSize) + packC = packWordC ?regByteOrder + + -- Alias for unpacking with the correct word size and byte order + unpackC :: Vec (SizeInWordsC wordSize a) (Bytes wordSize) -> a + unpackC packed = fromMaybe err . maybeUnpackWordC ?regByteOrder $ packed + where + -- XXX: Quasiquoter doesn't work with implicit parameters + regByteOrder = ?regByteOrder + + err = + deepErrorX + [I.i| + Unpack failed in registerWbDf: + wordSize: #{natToInteger @wordSize} + regByteOrder: #{regByteOrder} + packedOut: #{packed} + |] + +{- | Circuit writes always take priority over bus writes, though circuit writes +may get ignored while a register is locked. Bus writes rejected with an error if +access rights are set to 'ReadOnly'. Similarly, bus reads are rejected if access rights are set to 'WriteOnly'. You can tie registers created using this function together with 'deviceWb'. If @@ -374,8 +920,15 @@ registerWbDf clk rst regConfig resetValue = SNatLE -> -- "Normal" register that actually holds data case divWithRemainder @wordSize @8 @7 of - Dict -> - Circuit go + Dict -> circuit $ \((Fwd offset, meta, wb0), circuitWrite) -> do + wb1 <- accessC regConfig.access -< wb0 + wb2 <- orderC -< wb1 + wb3 <- offsetC (Proxy @a) offset hasLock -< wb2 + (wb4, lockWrite, inLock) <- lockC (Proxy @a) clk rst hasLock -< (wb3, regValuePacked) + Fwd hasLock <- metaC (SNat @wordSize) (Proxy @a) regConfig resetValue -< meta + (regValue, regValuePacked, busActivity) <- + registerC clk rst regConfig resetValue -< (wb4, circuitWrite, lockWrite, inLock) + idC -< (regValue, busActivity) SNatGT -> -- Zero-width register that only provides bus activity information Circuit $ \(((_, _, m2s0), _), (_, ack)) -> @@ -385,7 +938,7 @@ registerWbDf clk rst regConfig resetValue = | m2s1.writeEnable = (Just (BusWrite resetValue), emptyWishboneS2M{acknowledge}) | otherwise = ( Just (BusRead resetValue) - , (emptyWishboneS2M @(BitVector (wordSize * 8))){acknowledge, readData = 0} + , (emptyWishboneS2M @wordSize){acknowledge} ) (unbundle -> (busActivity, s2m)) = update <$> m2s0 <*> coerce ack @@ -393,206 +946,6 @@ registerWbDf clk rst regConfig resetValue = ( (((), zeroWidthRegisterMeta @a Proxy regConfig, s2m), pure ()) , (pure resetValue, busActivity) ) - where - needReverse = ?busByteOrder /= ?regByteOrder - - unpackC :: Vec (SizeInWordsC wordSize a) (Bytes wordSize) -> a - unpackC packed = fromMaybe err . maybeUnpackWordC ?regByteOrder $ packed - where - -- XXX: Quasiquoter doesn't work with implicit parameters - regByteOrder = ?regByteOrder - - err = - deepErrorX - [I.i| - Unpack failed in registerWbDf: - wordSize: #{natToInteger @wordSize} - regByteOrder: #{regByteOrder} - packedOut: #{packed} - |] - - -- This behemoth of a type signature because the inferred type signature is - -- too general, confusing the type checker. - go :: - forall nWords. - ( nWords ~ SizeInWordsC wordSize a - , KnownNat nWords - , 1 <= nWords - ) => - ( ( (Offset aw, (), Signal dom (WishboneM2S aw wordSize (Bytes wordSize))) - , Signal dom (Maybe a) - ) - , (Signal dom (), Signal dom Ack) - ) -> - ( ( ((), RegisterMeta aw, Signal dom (WishboneS2M (Bytes wordSize))) - , Signal dom () - ) - , (Signal dom a, Signal dom (Maybe (BusActivity a))) - ) - go (((offset, _, wbM2S), aIn0), (_, coerce -> dfAck)) = - ((((), reg, wbS2M2), pure ()), (aOut, busActivity)) - where - relativeOffset = goRelativeOffset . addr <$> wbM2S - aOut = unpackC <$> packedOut - packedIn0 = fmap (packWordC @wordSize ?regByteOrder) <$> aIn0 - - -- Construct register meta data. This information is only used for simulation, - -- i.e., used to build the register map. - reg = - RegisterMeta - { name = SimOnly $ Name{name = regConfig.name, description = regConfig.description} - , srcLoc = SimOnly locCaller - , nWords = natToNum @nWords - , register = - SimOnly - $ Register - { fieldType = regType @a - , address = 0x0 -- Note: will be set by 'deviceWithOffsetsWb' - , access = regConfig.access - , tags = regConfig.tags - , reset = Just simOnlyResetValue - } - } - - -- The fact that this goes into a 'SimOnly' construct should be enough for Clash to - -- deduce that it can throw the expression below away, but it doesn't. As a result, - -- it sees the constructor of BitVector and freaks out. Spelling it out like this - -- (i.e., putting it behind a 'clashSimulation' flag) makes Clash okay with it. - simOnlyResetValue - | clashSimulation = (pack (packWordC @wordSize ?regByteOrder resetValue)).unsafeToNatural - | otherwise = 0 - - -- Construct output to the bus and the user logic. Note that the bus activity will - -- get fed to the user directly, but the output to the bus will only go to the bus - -- once the user acknowledges the bus activity. - busActivity = getBusActivity <$> wbS2M0 <*> aOut <*> aInFromBus0 - aInFromBus0 = fmap unpackC <$> packedInFromBus0 - (wbS2M0, packedInFromBus0) = - unbundle - $ goBus regConfig.access - <$> relativeOffset - <*> wbM2S - <*> packedOut - - -- Drop any circuit writes while we're waiting for the bus activity to get - -- acknowledged. This makes sure that 'packedOut' remains stable in turn making - -- 'busActivity' stable which is a requirement for Df. Note that if the user *does* - -- acknowledge we *do* allow circuit writes. This allows users to intercept bus - -- activity and change any incoming values atomically. - packedIn1 = mux (fmap isJust busActivity .&&. fmap not dfAck) (pure Nothing) packedIn0 - - -- Only write to the register from the bus if the bus activity gets acknowledged - ackBusActivity = fmap isJust busActivity .&&. dfAck - packedInFromBus1 = mux ackBusActivity packedInFromBus0 (pure Nothing) - packedOut = - regMaybe - clk - rst - enableGen - (packWordC ?regByteOrder resetValue) - (liftA2 (<|>) packedIn1 packedInFromBus1) - - -- Only acknowledge bus transactions if the bus activity gets acknowledged. Note that - -- 'ackBusActivity' is defined in such a way that is always 'False' if there is no - -- transaction to ack in the first place. I.e., we don't accidentally acknowledge in - -- case of errors. - wbS2M1 = setAck <$> wbS2M0 <*> ackBusActivity - - setAck :: WishboneS2M c -> Bool -> WishboneS2M c - setAck s2m acknowledge = s2m{acknowledge} - - -- Override the read data based on 'BusReadBehavior' - -- - -- XXX: We use this to allow use to have a register backed by a FIFO. Ideally we'd - -- just have a 'pipeDf' that is *not* backed by registers, but merely passes - -- on a 'Df' stream. - wbS2M2 = - case regConfig.busRead of - PreferCircuit -> setReadData <$> wbS2M1 <*> aIn0 <*> relativeOffset - PreferRegister -> wbS2M1 - - setReadData :: - WishboneS2M (Bytes wordSize) -> - Maybe a -> - Index nWords -> - WishboneS2M (Bytes wordSize) - setReadData s2m ma relOffset = - case ma of - Just a -> - let - packed = packWordC @wordSize ?regByteOrder a !! relOffset - readData = if needReverse then reverseBytes packed else packed - in - s2m{readData} - Nothing -> s2m - - goRelativeOffset :: BitVector aw -> Index nWords - goRelativeOffset addr = fromMaybe 0 (elemIndex addrLsbs expectedOffsets) - where - expectedOffsets = iterateI succ offsetLsbs - offsetLsbs = resize offset :: BitVector (BitSize (Index nWords)) - addrLsbs = resize addr :: BitVector (BitSize (Index nWords)) - - -- Handle bus transactions. Note that this function assumes that the bus transaction - -- is actually targeting this register. I.e., the address has already been checked. - goBus :: - forall nWords. - ( nWords ~ SizeInWordsC wordSize a - , KnownNat nWords - ) => - Access -> - Index nWords -> - WishboneM2S aw wordSize (Bytes wordSize) -> - Vec (SizeInWordsC wordSize a) (Bytes wordSize) -> - ( WishboneS2M (Bytes wordSize) - , Maybe (Vec (SizeInWordsC wordSize a) (Bytes wordSize)) - ) - goBus busAccess offset wbM2S packedFromReg = - () - `seqX` offset - `seqX` wbM2S - `seqX` packedFromReg - `seqX` readData - `seqX` acknowledge - `seqX` err - `seqX` wbWrite - `seqX` ( WishboneS2M - { readData - , acknowledge - , err - , retry = False - , stall = False - } - , wbWrite - ) - where - managerActive = wbM2S.strobe && wbM2S.busCycle - err = managerActive && accessFault - acknowledge = managerActive && not err - - -- Note that these "faults" are only considered in context of an active bus - readOnlyFault = busAccess == ReadOnly && wbM2S.writeEnable - writeOnlyFault = busAccess == WriteOnly && not wbM2S.writeEnable - accessFault = readOnlyFault || writeOnlyFault - - readData - | not wbM2S.writeEnable - , acknowledge = - (if needReverse then reverseBytes else id) (packedFromReg !! offset) - | otherwise = 0 - - maskedWriteData = - maskWriteData - offset - (if needReverse then reverseBits wbM2S.busSelect else wbM2S.busSelect) - (if needReverse then reverseBytes wbM2S.writeData else wbM2S.writeData) - packedFromReg - - wbWrite - | wbM2S.writeEnable - , acknowledge = - Just maskedWriteData - | otherwise = Nothing reverseBytes :: forall wordSize. diff --git a/clash-protocols-memmap/tests/Tests/Protocols/MemoryMap/Registers/WishboneStandard.hs b/clash-protocols-memmap/tests/Tests/Protocols/MemoryMap/Registers/WishboneStandard.hs index 0b03165df..2976c41c1 100644 --- a/clash-protocols-memmap/tests/Tests/Protocols/MemoryMap/Registers/WishboneStandard.hs +++ b/clash-protocols-memmap/tests/Tests/Protocols/MemoryMap/Registers/WishboneStandard.hs @@ -22,7 +22,10 @@ import Protocols import Protocols.Hedgehog (defExpectOptions, eoSampleMax) import Protocols.MemoryMap import Protocols.MemoryMap.Registers.WishboneStandard -import Protocols.MemoryMap.Registers.WishboneStandard.Internal +import Protocols.MemoryMap.Registers.WishboneStandard.Internal ( + LockRequest (..), + maskWriteData, + ) import Protocols.Wishbone import Protocols.Wishbone.Standard.Hedgehog ( WishboneMasterRequest (..), @@ -40,6 +43,22 @@ import qualified Prelude as P type Bytes n = BitVector (n * 8) +-- | State for tracking locks on multi-word registers +data LockInfo = LockInfo + { isLocked :: Bool + , shadowWrites :: Map.Map (BitVector AddressWidth) (BitVector 32) + -- ^ Shadow writes accumulated while locked, keyed by word address + } + deriving (Show) + +-- | Model state including both register values and lock state +data ModelState = ModelState + { registers :: Map.Map (BitVector AddressWidth) (BitVector 32) + , lockState :: Map.Map (BitVector AddressWidth) LockInfo + -- ^ Lock state keyed by the base address of the locked register + } + deriving (Show) + initFloat :: Float initFloat = 3.0 @@ -49,6 +68,9 @@ initDouble = 6.0 initU32 :: Unsigned 32 initU32 = 122222 +initBv64 :: BitVector 64 +initBv64 = 0x123456789abcdef0 + type AddressWidth = 4 wordSize :: SNat 4 @@ -71,23 +93,30 @@ myPaddedUnpackC = . maybeUnpackWordC regByteOrder {- | Initial state of 'deviceExample', represented as a map from address to bit -size and value. +size and value, plus lock state for multi-word registers. -} -initState :: Map.Map (BitVector AddressWidth) (BitVector 32) +initState :: ModelState initState = - Map.fromList @(BitVector AddressWidth) - -- TODO: zero-width registers - [ (0, myPaddedPackC initFloat !! nil) - , (1, myPaddedPackC initDouble !! nil) - , (2, myPaddedPackC initDouble !! succ nil) - , (3, myPaddedPackC initU32 !! nil) - , (4, myPaddedPackC initFloat !! nil) - , (5, myPaddedPackC initFloat !! nil) - , (6, myPaddedPackC initU32 !! nil) - , (7, myPaddedPackC initU32 !! nil) - , (8, myPaddedPackC initU32 !! nil) - , (9, myPaddedPackC False !! nil) - ] + ModelState + { registers = + Map.fromList @(BitVector AddressWidth) + -- TODO: zero-width registers + [ (0, myPaddedPackC initFloat !! nil) + , (1, myPaddedPackC initDouble !! nil) + , (2, myPaddedPackC initDouble !! succ nil) + , (3, myPaddedPackC initU32 !! nil) + , (4, myPaddedPackC initFloat !! nil) + , (5, myPaddedPackC initFloat !! nil) + , (6, myPaddedPackC initU32 !! nil) + , (7, myPaddedPackC initU32 !! nil) + , (8, myPaddedPackC initU32 !! nil) + , (9, myPaddedPackC False !! nil) + , (10, myPaddedPackC initBv64 !! nil) + , (11, myPaddedPackC initBv64 !! succ nil) + -- Note: address 12 will be the lock register for 'lockable' + ] + , lockState = Map.empty + } where nil = 0 :: Int @@ -111,11 +140,22 @@ deviceExample :: (ConstBwd MM, Wishbone dom 'Standard aw (Bytes wordSize)) () deviceExample clk rst = circuit $ \(mm, wb) -> do - [float, double, u32, readOnly, writeOnly, prio, prioPreferCircuit, delayed, delayedError] <- + [ float + , double + , u32 + , readOnly + , writeOnly + , prio + , prioPreferCircuit + , delayed + , delayedError + , lockable + ] <- deviceWb "example" -< (mm, wb) registerWb_ clk rst (registerConfig "f") initFloat -< (float, Fwd noWrite) - registerWb_ clk rst (registerConfig "d") initDouble -< (double, Fwd noWrite) + registerWb_ clk rst (registerConfig "d"){lock = NoLock} initDouble + -< (double, Fwd noWrite) registerWb_ clk rst (registerConfig "u") initU32 -< (u32, Fwd noWrite) registerWb_ clk rst (registerConfig "ro"){access = ReadOnly} initFloat @@ -165,6 +205,10 @@ deviceExample clk rst = circuit $ \(mm, wb) -> do registerWb_ clk rst (registerConfig "delayed_error"){access = ReadOnly} False -< (delayedError, Fwd (Just <$> unexpectedDelayedValue)) + -- A lockable 64-bit register that spans 2 words, so it will automatically get a lock register + registerWb_ clk rst (registerConfig "lockable") initBv64 + -< (lockable, Fwd noWrite) + idC where ena = enableGen @@ -250,8 +294,8 @@ prop_wb = model :: WishboneMasterRequest AddressWidth (BitVector 32) -> WishboneS2M (BitVector 32) -> - Map.Map (BitVector AddressWidth) (BitVector 32) -> - Either String (Map.Map (BitVector AddressWidth) (BitVector 32)) + ModelState -> + Either String ModelState model instr WishboneS2M{err = True} s = -- Errors should only happen when we use an unmapped address (in the future -- we may want to to test other errors too). @@ -265,15 +309,35 @@ prop_wb = isWrite (Write{}) = True isWrite _ = False + + isLockableRegisterLocked = case Map.lookup lockableAddress s.lockState of + Just lockInfo -> lockInfo.isLocked + Nothing -> False in - case Map.lookup errorAddress s of + case Map.lookup errorAddress s.registers of Nothing -> - -- Whenever an error occurs, the state should be unchanged. - Right s + -- Unmapped address or lock register (which isn't in the registers map) + if errorAddress == lockableAddress + 2 && isRead instr + then Right s -- Lock register is write-only, reads error as expected + else + if errorAddress == lockableAddress + 2 && isWrite instr + then + -- Writing to lock register - check if it's a Commit-when-unlocked error + case instr of + Write _ mask dat -> + let maskedData = maskWriteData @4 @1 0 mask dat (dat :> Nil) + in case maybeUnpackWordC regByteOrder maskedData of + Just Commit | not isLockableRegisterLocked -> Right s -- Commit when unlocked, error expected + _ -> Left $ "Unexpected error when writing to lock register: " <> show instr + _ -> Right s + else Right s -- Unmapped address Just v | isRead instr && errorAddress == woAddress -> -- Expect an error when trying to read from the WriteOnly register Right s + | isRead instr && errorAddress == lockableAddress + 2 -> + -- Expect an error when trying to read from the lock register (write-only) + Right s | isWrite instr && errorAddress `elem` roAddresses -> -- Expect an error when trying to write to a ReadOnly register Right s @@ -284,40 +348,117 @@ prop_wb = model (Read a _) WishboneS2M{readData} s = -- XXX: Note that we IGNORE the byte enable mask when reading. The circuit -- does too. - case Map.lookup a s of + case Map.lookup a s.registers of Nothing -> Left $ "Read from unmapped address: " <> show a Just v | v /= 0 && a == delayedErrorAddress -> Left $ "delayed error! v: " <> show v <> ", readData: " <> show readData | v == readData && a == prioAddress -> - Right $ Map.insert prioAddress (head $ myPaddedPackC initU32) s + Right $ s{registers = Map.insert prioAddress (head $ myPaddedPackC initU32) s.registers} | readData == pack initU32 && a == prioPreferCircuitAddress -> - Right $ Map.insert prioPreferCircuitAddress (head $ myPaddedPackC initU32) s + Right + $ s + { registers = Map.insert prioPreferCircuitAddress (head $ myPaddedPackC initU32) s.registers + } | v == readData -> Right s | otherwise -> Left $ "a: " <> show a <> ", v: " <> show v <> ", readData: " <> show readData model (Write a m newDat) _ s + -- Handle lock register writes + | a == lockableAddress + 2 = + handleLockWrite a m newDat s + -- Handle writes to locked registers + | Just lockInfo <- Map.lookup lockableAddress s.lockState + , lockInfo.isLocked + , a `elem` [lockableAddress, lockableAddress + 1] = + -- Write goes to shadow register, but only if mask != 0 + if m == 0 + then Right s -- mask=0 on locked register is a no-op (hardware ACKs but doesn't update shadow) + else + let oldVal = Map.findWithDefault 0 a lockInfo.shadowWrites + newVal = update oldVal + in Right + $ s + { lockState = + Map.adjust + (\li -> li{shadowWrites = Map.insert a newVal li.shadowWrites}) + lockableAddress + s.lockState + } + -- Normal register writes | a == delayedAddress = let double :: BitVector 32 -> BitVector 32 double = head . myPaddedPackC . (* (2 :: (Unsigned 32))) . myPaddedUnpackC . pure in - Right (Map.adjust (double . update) a s) + Right $ s{registers = Map.adjust (double . update) a s.registers} | a == prioAddress || a == prioPreferCircuitAddress = let inc :: BitVector 32 -> BitVector 32 inc = head . myPaddedPackC . (+ (1 :: (Unsigned 32))) . myPaddedUnpackC . pure in - Right (Map.adjust (inc . update) a s) + Right $ s{registers = Map.adjust (inc . update) a s.registers} | otherwise = - Right (Map.adjust update a s) + Right $ s{registers = Map.adjust update a s.registers} where update :: BitVector 32 -> BitVector 32 update oldDat = head $ maskWriteData @4 @1 0 m newDat (oldDat :> Nil) + handleLockWrite :: + BitVector AddressWidth -> + BitVector 4 -> + BitVector 32 -> + ModelState -> + Either String ModelState + handleLockWrite _addr mask dat state = do + -- Apply byte enables before unpacking, just like the hardware does + let maskedData = maskWriteData @4 @1 0 mask dat (dat :> Nil) + case maybeUnpackWordC regByteOrder maskedData of + Just Lock -> + -- Initialize shadow register with current register values (matching hardware's copy behavior) + let currentVal0 = Map.findWithDefault 0 lockableAddress state.registers + currentVal1 = Map.findWithDefault 0 (lockableAddress + 1) state.registers + shadowInit = Map.fromList [(lockableAddress, currentVal0), (lockableAddress + 1, currentVal1)] + in Right + $ state{lockState = Map.insert lockableAddress (LockInfo True shadowInit) state.lockState} + Just Commit -> + case Map.lookup lockableAddress state.lockState of + Nothing -> + -- Not locked - this should cause a bus error (handled in error path above) + -- But we shouldn't reach here since errors are handled before model updates + Left $ "Commit when not locked (should have been caught as error)" + Just lockInfo -> + let + -- Apply all shadow writes to the actual registers + -- Shadow register contains accumulated writes, or zeros if nothing was written + newRegs = Map.union lockInfo.shadowWrites state.registers + in + Right $ state{registers = newRegs, lockState = Map.delete lockableAddress state.lockState} + Just Clear -> + Right $ state{lockState = Map.delete lockableAddress state.lockState} + Nothing -> + -- This should never happen now that we generate valid requests and apply masks properly + Left $ "Invalid lock request after masking: " <> show maskedData + genInputs :: Gen [WishboneMasterRequest AddressWidth (Bytes 4)] - genInputs = Gen.list (Range.linear 0 300) (genWishboneTransfer genAddr genMask genData) + genInputs = Gen.list (Range.linear 0 300) genWbRequest + where + genWbRequest = do + addr <- genAddr + mask <- genMask + if addr == lockRegisterAddress + then genLockWrite addr mask + else genWishboneTransfer (pure addr) (pure mask) genData + + genLockWrite addr mask = do + -- Generate a valid LockRequest value (0, 1, or 2) + lockReq <- Gen.element [Lock, Commit, Clear] + -- Pack it as a 32-bit value using the register byte order + let lockData = myPaddedPackC lockReq !! nil + pure $ Write addr mask lockData + where + nil = 0 :: Int genMask :: Gen (BitVector 4) genMask = Gen.integral (Range.linear 0 maxBound) @@ -328,7 +469,7 @@ prop_wb = genAddr :: Gen (BitVector AddressWidth) genAddr = -- We do plus one to test unmapped addresses - Gen.integral (Range.constant 0 (1 + P.maximum (Map.keys initState))) + Gen.integral (Range.constant 0 (1 + P.maximum (Map.keys initState.registers))) dut :: Circuit (Wishbone XilinxSystem Standard AddressWidth (BitVector 32)) () dut = @@ -350,6 +491,8 @@ prop_wb = , regPrioPreferCircuit , regDelayed , regDelayedError + , regLockable + , regLockableLock -- The lock register for 'lockable' ] = example.registers woAddress = fromIntegral $ regWO.value.address `div` 4 roAddress = fromIntegral $ regRO.value.address `div` 4 @@ -357,6 +500,8 @@ prop_wb = prioPreferCircuitAddress = fromIntegral $ regPrioPreferCircuit.value.address `div` 4 delayedAddress = fromIntegral $ regDelayed.value.address `div` 4 delayedErrorAddress = fromIntegral $ regDelayedError.value.address `div` 4 + lockableAddress = fromIntegral $ regLockable.value.address `div` 4 + lockRegisterAddress = fromIntegral $ regLockableLock.value.address `div` 4 {- FOURMOLU_DISABLE -} case_maskWriteData :: Assertion @@ -402,6 +547,8 @@ case_memoryMap = do , regPrioPreferCircuit , regDelayed , regDelayedError + , regLockable + , regLockableLock ] = example.registers regF.name.name @?= "f" @@ -413,6 +560,8 @@ case_memoryMap = do regPrioPreferCircuit.name.name @?= "prio_prefer_circuit" regDelayed.name.name @?= "delayed" regDelayedError.name.name @?= "delayed_error" + regLockable.name.name @?= "lockable" + regLockableLock.name.name @?= "lockable_lock" regF.value.address @?= 0 regD.value.address @?= 4 @@ -423,6 +572,11 @@ case_memoryMap = do regPrioPreferCircuit.value.address @?= 28 regDelayed.value.address @?= 32 regDelayedError.value.address @?= 36 + regLockable.value.address @?= 40 + regLockableLock.value.address @?= 48 + + "has-lock" `elem` regLockable.value.tags @?= True + "is-lock" `elem` regLockableLock.value.tags @?= True tests :: TestTree tests = diff --git a/firmware-binaries/test-cases/registerwb_test/src/main.rs b/firmware-binaries/test-cases/registerwb_test/src/main.rs index 20b3f71a6..ca8128685 100644 --- a/firmware-binaries/test-cases/registerwb_test/src/main.rs +++ b/firmware-binaries/test-cases/registerwb_test/src/main.rs @@ -289,6 +289,56 @@ fn main() -> ! { hal::Either::Right(hal::Abc::B) ); + // Test lock/commit/clear functionality on maybe_u96 + { + let many_types = &mut INSTANCES.many_types; + + // First, set a known initial value + let test_initial = hal::Maybe::Just(0xABCDEF0123456789FEDCBA98); + many_types.set_maybe_u96(test_initial); + + // Test 1: Lock, write, confirm read returns old value, then commit + let initial_value = many_types.maybe_u96(); + expect("lock_test.initial", test_initial, initial_value); + + many_types.set_maybe_u96_lock(hal::LockRequest::Lock); + many_types.set_maybe_u96_non_atomic(hal::Maybe::Just(0x1234567890ABCDEF0FEDCBA9)); + let value_while_locked = many_types.maybe_u96_non_atomic(); + expect("lock_test.still_old", initial_value, value_while_locked); + + many_types.set_maybe_u96_lock(hal::LockRequest::Commit); + let value_after_commit = many_types.maybe_u96(); + expect( + "lock_test.after_commit", + hal::Maybe::Just(0x1234567890ABCDEF0FEDCBA9), + value_after_commit, + ); + + // Test 2: Lock, write, clear without committing + many_types.set_maybe_u96_lock(hal::LockRequest::Lock); + many_types.set_maybe_u96_non_atomic(hal::Maybe::Just(0xAAAAAAAAAAAAAAAAAAAAAAAA)); + many_types.set_maybe_u96_lock(hal::LockRequest::Clear); + let value_after_clear = many_types.maybe_u96(); + expect( + "lock_test.after_clear", + hal::Maybe::Just(0x1234567890ABCDEF0FEDCBA9), + value_after_clear, + ); + + // Write multiple values within a lock session. We should see the last + // write after commit. + many_types.set_maybe_u96_lock(hal::LockRequest::Lock); + many_types.set_maybe_u96_non_atomic(hal::Maybe::Just(0x1111111111111111111111111)); + many_types.set_maybe_u96_non_atomic(hal::Maybe::Just(0x2222222222222222222222222)); + many_types.set_maybe_u96_lock(hal::LockRequest::Commit); + let value_multiple_writes = many_types.maybe_u96(); + expect( + "lock_test.multiple_writes", + hal::Maybe::Just(0x2222222222222222222222222), + value_multiple_writes, + ); + } + test_ok(); } diff --git a/firmware-support/memmap-generate/src/generators/device_types.rs b/firmware-support/memmap-generate/src/generators/device_types.rs index 589b29c52..d1bb9efca 100644 --- a/firmware-support/memmap-generate/src/generators/device_types.rs +++ b/firmware-support/memmap-generate/src/generators/device_types.rs @@ -50,28 +50,80 @@ impl DeviceGenerator { let scalar_ty = ty_gen.generate_type_ref(inner); let size = *len as usize; - quote! { - #[doc = #reg_description] - pub fn #name(&self, idx: usize) -> Option<#scalar_ty> { - if idx >= #size { - None - } else { - Some(unsafe { self.#unchecked_name(idx)}) + // Check if this vector register has a lock + if let Some((_, lock_setter_name)) = find_lock_register(&desc.registers, reg) { + let non_atomic_name = ident(IdentType::Method, format!("{}_non_atomic", ®.name)); + let non_atomic_unchecked = ident(IdentType::Method, format!("{}_unchecked_non_atomic", ®.name)); + + quote! { + #[doc = #reg_description] + #[doc = "Atomically reads a single element using the lock mechanism."] + pub fn #name(&self, idx: usize) -> Option<#scalar_ty> { + if idx >= #size { + None + } else { + Some(unsafe { self.#unchecked_name(idx)}) + } } - } - #[doc = #reg_description] - pub unsafe fn #unchecked_name(&self, idx: usize) -> #scalar_ty { - let ptr = self.0.add(#offset).cast::<#scalar_ty>(); + #[doc = #reg_description] + pub unsafe fn #unchecked_name(&self, idx: usize) -> #scalar_ty { + self.#lock_setter_name(LockRequest::Lock); + let ptr = self.0.add(#offset).cast::<#scalar_ty>(); + let val = ptr.add(idx).read_volatile(); + self.#lock_setter_name(LockRequest::Clear); + val + } + + #[doc = #reg_description] + #[doc = "Iterator that atomically reads each element."] + pub fn #iter_name(&self) -> impl DoubleEndedIterator + '_ { + (0..#size).map(|i| unsafe { + self.#unchecked_name(i) + }) + } - ptr.add(idx).read_volatile() + #[doc = #reg_description] + #[doc = "Non-atomic read without lock protection."] + pub fn #non_atomic_name(&self, idx: usize) -> Option<#scalar_ty> { + if idx >= #size { + None + } else { + Some(unsafe { self.#non_atomic_unchecked(idx)}) + } + } + + #[doc = #reg_description] + pub unsafe fn #non_atomic_unchecked(&self, idx: usize) -> #scalar_ty { + let ptr = self.0.add(#offset).cast::<#scalar_ty>(); + ptr.add(idx).read_volatile() + } } + } else { + // Regular non-lockable vector register + quote! { + #[doc = #reg_description] + pub fn #name(&self, idx: usize) -> Option<#scalar_ty> { + if idx >= #size { + None + } else { + Some(unsafe { self.#unchecked_name(idx)}) + } + } - #[doc = #reg_description] - pub fn #iter_name(&self) -> impl DoubleEndedIterator + '_ { - (0..#size).map(|i| unsafe { - self.#unchecked_name(i) - }) + #[doc = #reg_description] + pub unsafe fn #unchecked_name(&self, idx: usize) -> #scalar_ty { + let ptr = self.0.add(#offset).cast::<#scalar_ty>(); + + ptr.add(idx).read_volatile() + } + + #[doc = #reg_description] + pub fn #iter_name(&self) -> impl DoubleEndedIterator + '_ { + (0..#size).map(|i| unsafe { + self.#unchecked_name(i) + }) + } } } } else if reg.tags.iter().any(|tag| tag == "zero-width") { @@ -88,11 +140,43 @@ impl DeviceGenerator { } } else { let ty = ty_gen.generate_type_ref(®.reg_type); - quote! { - #[doc = #reg_description] - pub fn #name(&self) -> #ty { - unsafe { - self.0.add(#offset).cast::<#ty>().read_volatile() + + // Check if this register has a lock + if let Some((_, lock_setter_name)) = find_lock_register(&desc.registers, reg) { + let non_atomic_name = ident(IdentType::Method, format!("{}_non_atomic", ®.name)); + + quote! { + #[doc = #reg_description] + #[doc = ""] + #[doc = "This method performs an atomic read using the lock mechanism:"] + #[doc = "1. Writes `Lock` to the lock register"] + #[doc = "2. Reads the register value"] + #[doc = "3. Writes `Clear` to the lock register"] + pub fn #name(&self) -> #ty { + unsafe { + self.#lock_setter_name(LockRequest::Lock); + let val = self.0.add(#offset).cast::<#ty>().read_volatile(); + self.#lock_setter_name(LockRequest::Clear); + val + } + } + + #[doc = #reg_description] + #[doc = ""] + #[doc = "Non-atomic read without lock protection."] + #[doc = "Use only when you need manual lock control or know atomicity is not required."] + pub fn #non_atomic_name(&self) -> #ty { + unsafe { self.0.add(#offset).cast::<#ty>().read_volatile() } + } + } + } else { + // Regular non-lockable register + quote! { + #[doc = #reg_description] + pub fn #name(&self) -> #ty { + unsafe { + self.0.add(#offset).cast::<#ty>().read_volatile() + } } } } @@ -103,6 +187,10 @@ impl DeviceGenerator { let set_funcs = desc .registers .iter() + .filter(|reg| { + // Filter out lock registers - they're handled separately + !reg.tags.iter().any(|tag| tag == "is-lock") + }) .map(|reg| { let can_write = reg.access == RegisterAccess::WriteOnly || reg.access == RegisterAccess::ReadWrite; @@ -121,22 +209,67 @@ impl DeviceGenerator { let scalar_ty = ty_gen.generate_type_ref(inner); let size = *len as usize; - quote! { - #[doc = #reg_description] - pub fn #name(&self, idx: usize, val: #scalar_ty) -> Option<()> { - if idx >= #size { - None - } else { - unsafe { self.#unchecked_name(idx, val) }; - Some(()) + // Check if this vector register has a lock + if let Some((_, lock_setter_name)) = find_lock_register(&desc.registers, reg) { + let non_atomic_name = ident(IdentType::Method, format!("set_{}_non_atomic", ®.name)); + let non_atomic_unchecked = ident(IdentType::Method, format!("set_{}_unchecked_non_atomic", ®.name)); + + quote! { + #[doc = #reg_description] + #[doc = "Atomically writes a single element using the lock mechanism."] + pub fn #name(&self, idx: usize, val: #scalar_ty) -> Option<()> { + if idx >= #size { + None + } else { + unsafe { self.#unchecked_name(idx, val) }; + Some(()) + } + } + + #[doc = #reg_description] + pub unsafe fn #unchecked_name(&self, idx: usize, val: #scalar_ty) { + self.#lock_setter_name(LockRequest::Lock); + let ptr = self.0.add(#offset).cast::<#scalar_ty>(); + ptr.add(idx).write_volatile(val); + self.#lock_setter_name(LockRequest::Commit); + } + + #[doc = #reg_description] + #[doc = "Non-atomic write without lock protection."] + pub fn #non_atomic_name(&self, idx: usize, val: #scalar_ty) -> Option<()> { + if idx >= #size { + None + } else { + unsafe { self.#non_atomic_unchecked(idx, val) }; + Some(()) + } + } + + #[doc = #reg_description] + pub unsafe fn #non_atomic_unchecked(&self, idx: usize, val: #scalar_ty) { + let ptr = self.0.add(#offset).cast::<#scalar_ty>(); + ptr.add(idx).write_volatile(val); } } + } else { + // Regular non-lockable vector register + quote! { + #[doc = #reg_description] + pub fn #name(&self, idx: usize, val: #scalar_ty) -> Option<()> { + if idx >= #size { + None + } else { + unsafe { self.#unchecked_name(idx, val) }; + Some(()) + } + } - #[doc = #reg_description] - pub unsafe fn #unchecked_name(&self, idx: usize, val: #scalar_ty) { - let ptr = self.0.add(#offset).cast::<#scalar_ty>(); + #[doc = #reg_description] + pub unsafe fn #unchecked_name(&self, idx: usize, val: #scalar_ty) { + let ptr = self.0.add(#offset).cast::<#scalar_ty>(); - ptr.add(idx).write_volatile(val); + ptr.add(idx).write_volatile(val); + } } } } else if reg.tags.iter().any(|tag| tag == "zero-width") { @@ -151,13 +284,66 @@ impl DeviceGenerator { } } else { let ty = ty_gen.generate_type_ref(®.reg_type); - quote! { - #[doc = #reg_description] - pub fn #name(&self, val: #ty) { - unsafe { - self.0.add(#offset).cast::<#ty>().write_volatile(val) + + // Check if this register has a lock + if let Some((_, lock_setter_name)) = find_lock_register(&desc.registers, reg) { + let non_atomic_name = ident(IdentType::Method, format!("set_{}_non_atomic", ®.name)); + + quote! { + #[doc = #reg_description] + #[doc = ""] + #[doc = "This method performs an atomic write using the lock mechanism:"] + #[doc = "1. Writes `Lock` to the lock register"] + #[doc = "2. Writes the value to the register (stored in shadow)"] + #[doc = "3. Writes `Commit` to apply the shadow value"] + pub fn #name(&self, val: #ty) { + unsafe { + self.#lock_setter_name(LockRequest::Lock); + self.0.add(#offset).cast::<#ty>().write_volatile(val); + self.#lock_setter_name(LockRequest::Commit); + } + } + + #[doc = #reg_description] + #[doc = ""] + #[doc = "Non-atomic write without lock protection."] + #[doc = "Use only when you need manual lock control or know atomicity is not required."] + pub fn #non_atomic_name(&self, val: #ty) { + unsafe { self.0.add(#offset).cast::<#ty>().write_volatile(val) } } } + } else { + // Regular non-lockable register + quote! { + #[doc = #reg_description] + pub fn #name(&self, val: #ty) { + unsafe { + self.0.add(#offset).cast::<#ty>().write_volatile(val) + } + } + } + } + } + }) + .collect::>(); + + // Generate setters for lock registers (write-only, so normally filtered out) + let lock_funcs = desc + .registers + .iter() + .filter(|reg| reg.tags.iter().any(|tag| tag == "is-lock")) + .map(|reg| { + let name = ident(IdentType::Method, format!("set_{}", ®.name)); + let offset = reg.address as usize; + let reg_description = ®.description; + + quote! { + #[doc = #reg_description] + pub fn #name(&self, val: LockRequest) { + unsafe { + self.0.add(#offset).cast::() + .write_volatile(val as u8) + } } } }) @@ -192,6 +378,8 @@ impl DeviceGenerator { #(#get_funcs)* #(#set_funcs)* + + #(#lock_funcs)* } } } @@ -203,6 +391,25 @@ impl Default for DeviceGenerator { } } +/// Find the lock register for a given register if it exists. +/// Returns the lock register and its setter method name. +fn find_lock_register<'a>( + registers: &'a [RegisterDesc], + reg: &RegisterDesc, +) -> Option<(&'a RegisterDesc, proc_macro2::Ident)> { + if !reg.tags.iter().any(|tag| tag == "has-lock") { + return None; + } + + let lock_name = format!("{}_lock", reg.name); + let lock_reg = registers + .iter() + .find(|r| r.name == lock_name && r.tags.iter().any(|tag| tag == "is-lock"))?; + + let lock_setter_name = ident(IdentType::Method, format!("set_{}", lock_name)); + Some((lock_reg, lock_setter_name)) +} + /// Generates a constant for the given register description if applicable. /// Returns `None` if the register type does not have a constant value. fn generate_const(desc: &RegisterDesc) -> Option {