From 2a385b52773a6a19f309f02760731ff2fa50ee18 Mon Sep 17 00:00:00 2001 From: DesperateProgrammer Date: Sun, 4 Feb 2024 12:31:57 +0100 Subject: [PATCH] Cleaned up some more magic numbers Fixed a bug causing overlapping protection regions priority not taken into account, when access permission or cachability bits were changed only on the least priority overlap --- src/ARM.h | 12 +-- src/CP15.cpp | 223 ++++++++++++++++++++++++++++++------------- src/CP15_Constants.h | 34 ++++--- 3 files changed, 184 insertions(+), 85 deletions(-) diff --git a/src/ARM.h b/src/ARM.h index 2f9f4507..61a087fd 100644 --- a/src/ARM.h +++ b/src/ARM.h @@ -555,14 +555,14 @@ public: u32 DCacheTags[DCACHE_LINESPERSET*DCACHE_SETS]; //! Data Cache Tags organized in @ref DCACHE_LINESPERSET times @ref DCACHE_SETS Tags u8 DCacheCount; //! Global data line fill counter. Used for round-robin replacement strategy with the instruction cache - u32 PU_CodeCacheable; - u32 PU_DataCacheable; - u32 PU_DataCacheWrite; + u32 PU_CodeCacheable; //! CP15 Register 2 Opcode 1: Code Cachable Bits + u32 PU_DataCacheable; //! CP15 Register 2 Opcode 0: Data Cachable Bits + u32 PU_DataCacheWrite; //! CP15 Register 3 Opcode 0: WriteBuffer Control Register - u32 PU_CodeRW; - u32 PU_DataRW; + u32 PU_CodeRW; //! CP15 Register 5 Opcode 3: Code Access Permission register + u32 PU_DataRW; //! CP15 Register 5 Opcode 2: Data Access Permission register - u32 PU_Region[8]; + u32 PU_Region[8]; //! CP15 Register 6 Opcode 0..7: Protection Region Base and Size Register // 0=dataR 1=dataW 2=codeR 4=datacache 5=datawrite 6=codecache u8 PU_PrivMap[0x100000]; diff --git a/src/CP15.cpp b/src/CP15.cpp index 5f84543b..fa0a426d 100644 --- a/src/CP15.cpp +++ b/src/CP15.cpp @@ -512,6 +512,7 @@ void ARMv5::ICacheInvalidateBySetAndWay(const u8 cacheSet, const u8 cacheLine) void ARMv5::ICacheInvalidateAll() { + #pragma GCC ivdep for (int i = 0; i < ICACHE_SIZE / ICACHE_LINELENGTH; i++) ICacheTags[i] &= ~CACHE_FLAG_VALID; ; } @@ -731,6 +732,7 @@ void ARMv5::DCacheInvalidateBySetAndWay(const u8 cacheSet, const u8 cacheLine) void ARMv5::DCacheInvalidateAll() { + #pragma GCC ivdep for (int i = 0; i < DCACHE_SIZE / DCACHE_LINELENGTH; i++) DCacheTags[i] &= ~CACHE_FLAG_VALID; ; } @@ -767,13 +769,11 @@ void ARMv5::CP15Write(u32 id, u32 val) case 0x100: { u32 old = CP15Control; - val &= 0x000FF085; - CP15Control &= ~0x000FF085; - CP15Control |= val; + CP15Control = (CP15Control & ~CP15_CR_CHANGEABLE_MASK) | (val & CP15_CR_CHANGEABLE_MASK); //Log(LogLevel::Debug, "CP15Control = %08X (%08X->%08X)\n", CP15Control, old, val); UpdateDTCMSetting(); UpdateITCMSetting(); - u32 changedBits = old^val; + u32 changedBits = old ^ CP15Control; if (changedBits & (CP15_CR_MPUENABLE | CP15_CACHE_CR_ICACHEENABLE| CP15_CACHE_CR_DCACHEENABLE)) { UpdatePURegions(changedBits & CP15_CR_MPUENABLE); @@ -789,10 +789,25 @@ void ARMv5::CP15Write(u32 id, u32 val) { u32 diff = PU_DataCacheable ^ val; PU_DataCacheable = val; - for (u32 i = 0; i < CP15_REGION_COUNT; i++) - { - if (diff & (1<> (i * 2) & 3) << (i * CP15_REGIONACCESS_BITS_PER_REGION); + + #if 0 + // This code just updates the PU_Map entries of the given region + // this works fine, if the regions do not overlap + // If overlapping and the least priority region access permission + // would change, this results in wrong map entries. On HW the changed + // access permissions would not be applied because of a higher priority + // region overwriting them. + // + // Writing to the data permission bits is sparse, so we + // should just take the long but correct update via all regions + // so the permission priority is correct + + u32 diff = old ^ PU_DataRW; + for (u32 i = 0; i < CP15_REGION_COUNT; i++) + { + if (diff & (CP15_REGIONACCESS_REGIONMASK<<(i*CP15_REGIONACCESS_BITS_PER_REGION))) UpdatePURegion(i); + } + #else + UpdatePURegions(true); + #endif } return; @@ -844,19 +901,30 @@ void ARMv5::CP15Write(u32 id, u32 val) { u32 old = PU_CodeRW; PU_CodeRW = 0; - PU_CodeRW |= (val & 0x0003); - PU_CodeRW |= ((val & 0x000C) << 2); - PU_CodeRW |= ((val & 0x0030) << 4); - PU_CodeRW |= ((val & 0x00C0) << 6); - PU_CodeRW |= ((val & 0x0300) << 8); - PU_CodeRW |= ((val & 0x0C00) << 10); - PU_CodeRW |= ((val & 0x3000) << 12); - PU_CodeRW |= ((val & 0xC000) << 14); - u32 diff = old ^ PU_CodeRW; - for (u32 i = 0; i < CP15_REGION_COUNT; i++) - { - if (diff & (CP15_REGIONACCESS_REGIONMASK<<(i*CP15_REGIONACCESS_BITS_PER_REGION))) UpdatePURegion(i); - } + #pragma GCC ivdep + #pragma GCC unroll 8 + for (int i=0;i> (i * 2) & 3) << (i * CP15_REGIONACCESS_BITS_PER_REGION); + + #if 0 + // This code just updates the PU_Map entries of the given region + // this works fine, if the regions do not overlap + // If overlapping and the least priority region access permission + // would change, this results in wrong map entries, because it + // would on HW be overridden by the higher priority region + // + // Writing to the data permission bits is sparse, so we + // should just take the long but correct update via all regions + // so the permission priority is correct + + u32 diff = old ^ PU_CodeRW; + for (u32 i = 0; i < CP15_REGION_COUNT; i++) + { + if (diff & (CP15_REGIONACCESS_REGIONMASK<<(i*CP15_REGIONACCESS_BITS_PER_REGION))) UpdatePURegion(i); + } + #else + UpdatePURegions(true); + #endif } return; @@ -864,10 +932,23 @@ void ARMv5::CP15Write(u32 id, u32 val) { u32 diff = PU_DataRW ^ val; PU_DataRW = val; - for (u32 i = 0; i < CP15_REGION_COUNT; i++) - { - if (diff & (CP15_REGIONACCESS_REGIONMASK<<(i*CP15_REGIONACCESS_BITS_PER_REGION))) UpdatePURegion(i); - } + #if 0 + // This code just updates the PU_Map entries of the given region + // this works fine, if the regions do not overlap + // If overlapping and the least priority region access permission + // would change, this results in wrong map entries, because it + // would on HW be overridden by the higher priority region + // + // Writing to the data permission bits is sparse, so we + // should just take the long but correct update via all regions + // so the permission priority is correct + for (u32 i = 0; i < CP15_REGION_COUNT; i++) + { + if (diff & (CP15_REGIONACCESS_REGIONMASK<<(i*CP15_REGIONACCESS_BITS_PER_REGION))) UpdatePURegion(i); + } + #else + UpdatePURegions(true); + #endif } return; @@ -875,10 +956,23 @@ void ARMv5::CP15Write(u32 id, u32 val) { u32 diff = PU_CodeRW ^ val; PU_CodeRW = val; - for (u32 i = 0; i < CP15_REGION_COUNT; i++) - { - if (diff & (CP15_REGIONACCESS_REGIONMASK<<(i*CP15_REGIONACCESS_BITS_PER_REGION))) UpdatePURegion(i); - } + #if 0 + // This code just updates the PU_Map entries of the given region + // this works fine, if the regions do not overlap + // If overlapping and the least priority region access permission + // would change, this results in wrong map entries, because it + // would on HW be overridden by the higher priority region + // + // Writing to the data permission bits is sparse, so we + // should just take the long but correct update via all regions + // so the permission priority is correct + for (u32 i = 0; i < CP15_REGION_COUNT; i++) + { + if (diff & (CP15_REGIONACCESS_REGIONMASK<<(i*CP15_REGIONACCESS_BITS_PER_REGION))) UpdatePURegion(i); + } + #else + UpdatePURegions(true); + #endif } return; @@ -1242,28 +1336,25 @@ u32 ARMv5::CP15Read(u32 id) const case 0x500: { + // this format has 2 bits per region, but we store 4 per region + // so we reduce and consoldate the bits + // 0x502 returns all 4 bits per region u32 ret = 0; - ret |= (PU_DataRW & 0x00000003); - ret |= ((PU_DataRW & 0x00000030) >> 2); - ret |= ((PU_DataRW & 0x00000300) >> 4); - ret |= ((PU_DataRW & 0x00003000) >> 6); - ret |= ((PU_DataRW & 0x00030000) >> 8); - ret |= ((PU_DataRW & 0x00300000) >> 10); - ret |= ((PU_DataRW & 0x03000000) >> 12); - ret |= ((PU_DataRW & 0x30000000) >> 14); + #pragma GCC ivdep + #pragma GCC unroll 8 + for (int i=0;i> (i * CP15_REGIONACCESS_BITS_PER_REGION) & CP15_REGIONACCESS_REGIONMASK) << (i*2); return ret; } case 0x501: { + // this format has 2 bits per region, but we store 4 per region + // so we reduce and consoldate the bits + // 0x503 returns all 4 bits per region u32 ret = 0; - ret |= (PU_CodeRW & 0x00000003); - ret |= ((PU_CodeRW & 0x00000030) >> 2); - ret |= ((PU_CodeRW & 0x00000300) >> 4); - ret |= ((PU_CodeRW & 0x00003000) >> 6); - ret |= ((PU_CodeRW & 0x00030000) >> 8); - ret |= ((PU_CodeRW & 0x00300000) >> 10); - ret |= ((PU_CodeRW & 0x03000000) >> 12); - ret |= ((PU_CodeRW & 0x30000000) >> 14); + #pragma GCC unroll 8 + for (int i=0;i> (i * CP15_REGIONACCESS_BITS_PER_REGION) & CP15_REGIONACCESS_REGIONMASK) << (i*2); return ret; } case 0x502: diff --git a/src/CP15_Constants.h b/src/CP15_Constants.h index b148372d..98ddf84d 100644 --- a/src/CP15_Constants.h +++ b/src/CP15_Constants.h @@ -76,31 +76,39 @@ constexpr u32 CP15_MAINID_IMPLEMENTATION_946 = (0x946 << 4); constexpr u32 CP15_MAINID_REVISION_0 = (0 << 0); constexpr u32 CP15_MAINID_REVISION_1 = (1 << 0); -/* CP15 Control Register */ -constexpr u32 CP15_CR_MPUENABLE = (1 << 0); -constexpr u32 CP15_CR_BIGENDIAN = (1 << 7); -constexpr u32 CP15_CR_HIGHEXCEPTIONBASE = (1 << 13); - -/* CP15 Internal Exception base value */ -constexpr u32 CP15_EXCEPTIONBASE_HIGH = 0xFFFF0000; -constexpr u32 CP15_EXCEPTIONBASE_LOW = 0x00000000; - /* CP15 Cache and Write Buffer Conrol Register */ constexpr u32 CP15_CACHE_CR_ROUNDROBIN = (1 << 14); constexpr u32 CP15_CACHE_CR_ICACHEENABLE = (1 << 12); constexpr u32 CP15_CACHE_CR_DCACHEENABLE = (1 << 2); constexpr u32 CP15_CACHE_CR_WRITEBUFFERENABLE = (1 << 3); +/* CP15 TCM Control Register */ +constexpr u32 CP15_TCM_CR_DTCM_ENABLE = (1 << 16); +constexpr u32 CP15_TCM_CR_ITCM_ENABLE = (1 << 18); +constexpr u32 CP15_TCM_CR_DTCM_LOADMODE = (1 << 17); +constexpr u32 CP15_TCM_CR_ITCM_LOADMODE = (1 << 19); + +/* CP15 Control Register */ +constexpr u32 CP15_CR_MPUENABLE = (1 << 0); +constexpr u32 CP15_CR_BIGENDIAN = (1 << 7); +constexpr u32 CP15_CR_HIGHEXCEPTIONBASE = (1 << 13); +constexpr u32 CP15_CR_DISABLE_THUMBBIT = (1 << 15); +constexpr u32 CP15_CR_CHANGEABLE_MASK = CP15_CR_MPUENABLE | CP15_CR_BIGENDIAN | CP15_CACHE_CR_DCACHEENABLE + | CP15_CACHE_CR_ICACHEENABLE | CP15_CR_HIGHEXCEPTIONBASE + | CP15_TCM_CR_DTCM_ENABLE | CP15_TCM_CR_ITCM_ENABLE + | CP15_TCM_CR_DTCM_LOADMODE | CP15_TCM_CR_ITCM_LOADMODE + | CP15_CACHE_CR_ROUNDROBIN | CP15_CR_DISABLE_THUMBBIT; + +/* CP15 Internal Exception base value */ +constexpr u32 CP15_EXCEPTIONBASE_HIGH = 0xFFFF0000; +constexpr u32 CP15_EXCEPTIONBASE_LOW = 0x00000000; + /* CP15 BIST Test State register */ constexpr u32 CP15_BIST_TR_DISABLE_ICACHE_STREAMING = (1 << 11); constexpr u32 CP15_BIST_TR_DISABLE_DCACHE_STREAMING = (1 << 12); constexpr u32 CP15_BIST_TR_DISABLE_ICACHE_LINEFILL = (1 << 9); constexpr u32 CP15_BIST_TR_DISABLE_DCACHE_LINEFILL = (1 << 10); -/* CP15 TCM Control Register */ -constexpr u32 CP15_TCM_CR_DTCM_ENABLE = (1 << 16); -constexpr u32 CP15_TCM_CR_ITCM_ENABLE = (1 << 18); - /* CP15 Region Base and Size Register */ constexpr u32 CP15_REGION_COUNT = 8; constexpr u32 CP15_REGION_ENABLE = (1 << 0);