[mbx] Limit addresses are inclusive

Modify documentation to specify that the mailbox limit addresses are inclusive and indicate the final usable DWORD location. Update DIF address checks accordingly. Signed-off-by: Adrian Lees <[email protected]>
lowRISC · Jan 9, 2024 · d4cadf5 · d4cadf5
1 parent 8ef974c
commit d4cadf5
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 29 deletions.
diff --git a/hw/ip/mbx/data/mbx.hjson b/hw/ip/mbx/data/mbx.hjson
@@ -187,7 +187,7 @@
       }
       { name: "INBOUND_BASE_ADDRESS"
         desc: '''Base address of SRAM region, which is used to back up the inbound mailbox data.
-              This address is 4-byte aligned, the lower 2-bits are ignored.
+              This address is 4-byte aligned, the lower 2 bits are ignored.
               '''
         regwen: "ADDRESS_RANGE_REGWEN"
         swaccess: "rw"
@@ -202,8 +202,9 @@
         tags: ["excl:CsrAllTests:CsrExclAll"] // TODO(#477)
       }
       { name: "INBOUND_LIMIT_ADDRESS"
-        desc: '''Limit Address to mark the end of the inbound mailbox memory range in the private SRAM..
-              This address is 4-byte aligned, the lower 2-bits are ignored.
+        desc: '''Inclusive end address of the inbound mailbox memory range in the private SRAM.
+              This address is 4-byte aligned and it specifies the start address of the final valid
+              DWORD location. The lower 2 bits are ignored.
               '''
         regwen: "ADDRESS_RANGE_REGWEN"
         swaccess: "rw"
@@ -219,7 +220,7 @@
       }
       { name: "INBOUND_WRITE_PTR"
         desc: '''Write pointer for the next inbound data write.
-              This pointer is 4-byte aligned, the lower 2-bits are always zero.
+              This pointer is 4-byte aligned, the lower 2 bits are always zero.
               '''
         hwext: "true"
         fields: [
@@ -235,7 +236,7 @@
       }
       { name: "OUTBOUND_BASE_ADDRESS"
         desc: '''Base address of SRAM region, which is used to buffer the outbound mailbox data.
-              This address is 4-byte aligned, the lower 2-bits are ignored.
+              This address is 4-byte aligned, the lower 2 bits are ignored.
               '''
         regwen: "ADDRESS_RANGE_REGWEN"
         swaccess: "rw"
@@ -250,8 +251,9 @@
         tags: ["excl:CsrAllTests:CsrExclAll"] // TODO(#477)
       }
       { name: "OUTBOUND_LIMIT_ADDRESS"
-        desc: '''Limit Address to mark the end of the outbound mailbox memory range in the private SRAM.
-              This address is 4-byte aligned, the lower 2-bits are ignored.
+        desc: '''Inclusive end address of the outbound mailbox memory range in the private SRAM.
+              This address is 4-byte aligned and it specifies the start address of the final valid
+              DWORD location. The lower 2 bits are ignored.
               '''
         regwen: "ADDRESS_RANGE_REGWEN"
         swaccess: "rw"
@@ -267,7 +269,7 @@
       }
       { name: "OUTBOUND_READ_PTR"
         desc: '''Read pointer for the next outbound data read.
-              This pointer is 4-byte aligned, the lower 2-bits are always zero.
+              This pointer is 4-byte aligned, the lower 2 bits are always zero.
               '''
         hwext: "true"
         fields: [

diff --git a/sw/ip/mbx/dif/dif_mbx.c b/sw/ip/mbx/dif/dif_mbx.c
@@ -15,16 +15,18 @@ dif_result_t dif_mbx_range_set(const dif_mbx_t *mbx,
   if (mbx == NULL) {
     return kDifBadArg;
   }
-  if (config.imbx_base_addr >= config.imbx_limit_addr) {
+  // Note: the limit addresses are _inclusive_, specifying the start address of
+  // the final valid DWORD.
+  if (config.imbx_base_addr > config.imbx_limit_addr) {
     return kDifBadArg;
   }
-  if (config.ombx_base_addr >= config.ombx_limit_addr) {
+  if (config.ombx_base_addr > config.ombx_limit_addr) {
     return kDifBadArg;
   }
-  // Check that the inbound mailbox and outbound mailbox memory ranges collide
-  // with each other.
-  if ((config.imbx_base_addr < config.ombx_limit_addr) &&
-      (config.ombx_base_addr < config.imbx_limit_addr)) {
+  // Check that the inbound mailbox and outbound mailbox memory ranges do not
+  // collide with each other.
+  if ((config.imbx_base_addr <= config.ombx_limit_addr) &&
+      (config.ombx_base_addr <= config.imbx_limit_addr)) {
     return kDifBadArg;
   }
 

diff --git a/sw/ip/mbx/dif/dif_mbx_unittest.cc b/sw/ip/mbx/dif/dif_mbx_unittest.cc
@@ -48,21 +48,23 @@ TEST_P(MemoryRangeSuccessTests, SetSuccess) {
   EXPECT_DIF_OK(dif_mbx_range_set(&mbx_, range));
 }
 
-INSTANTIATE_TEST_SUITE_P(MemoryRangeSuccessTests, MemoryRangeSuccessTests,
-                         testing::ValuesIn(std::vector<dif_mbx_range_config_t>{{
-                             {.imbx_base_addr = 0xD0CF2C50,
-                              .imbx_limit_addr = 0xD1CF2C0F,
-                              .ombx_base_addr = 0xD1CF3C0F,
-                              .ombx_limit_addr = 0xD1CF3C10},
-                             {.imbx_base_addr = 0x1000,
-                              .imbx_limit_addr = 0x2000,
-                              .ombx_base_addr = 0x3000,
-                              .ombx_limit_addr = 0x4000},
-                             {.imbx_base_addr = 0x1000,
-                              .imbx_limit_addr = 0x1001,
-                              .ombx_base_addr = 0x1001,
-                              .ombx_limit_addr = 0x1002},
-                         }}));
+// 'Limit' addresses are _inclusive_.
+INSTANTIATE_TEST_SUITE_P(
+    MemoryRangeSuccessTests, MemoryRangeSuccessTests,
+    testing::ValuesIn(std::vector<dif_mbx_range_config_t>{{
+        {.imbx_base_addr = 0xD0CF2C50,
+         .imbx_limit_addr = 0xD1CF2C0F,
+         .ombx_base_addr = 0xD1CF3C0F,
+         .ombx_limit_addr = 0xD1CF3C10},
+        {.imbx_base_addr = 0x1000,
+         .imbx_limit_addr = 0x2000,
+         .ombx_base_addr = 0x3000,
+         .ombx_limit_addr = 0x4000},
+        {.imbx_base_addr = 0x1000,
+         .imbx_limit_addr = 0x1003,  // Inbound mailbox is a single DWORD.
+         .ombx_base_addr = 0x1004,
+         .ombx_limit_addr = 0x1007},  // Single DWORD
+    }}));
 
 class MemoryRangeBadArgTests
     : public MbxTestInitialized,