[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 8, 2024 · d8d665f · d8d665f
1 parent e435ee1
commit d8d665f
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 17 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,6 +48,7 @@ TEST_P(MemoryRangeSuccessTests, SetSuccess) {
   EXPECT_DIF_OK(dif_mbx_range_set(&mbx_, range));
 }
 
+// 'Limit' addresses are _inclusive_.
 INSTANTIATE_TEST_SUITE_P(MemoryRangeSuccessTests, MemoryRangeSuccessTests,
                          testing::ValuesIn(std::vector<dif_mbx_range_config_t>{{
                              {.imbx_base_addr = 0xD0CF2C50,
@@ -59,9 +60,9 @@ INSTANTIATE_TEST_SUITE_P(MemoryRangeSuccessTests, MemoryRangeSuccessTests,
                               .ombx_base_addr = 0x3000,
                               .ombx_limit_addr = 0x4000},
                              {.imbx_base_addr = 0x1000,
-                              .imbx_limit_addr = 0x1001,
-                              .ombx_base_addr = 0x1001,
-                              .ombx_limit_addr = 0x1002},
+                              .imbx_limit_addr = 0x1003,  // Inbound mailbox is a single DWORD.
+                              .ombx_base_addr = 0x1004,
+                              .ombx_limit_addr = 0x1007}, // Single DWORD
                          }}));
 
 class MemoryRangeBadArgTests