From d8d665f0e7be278d50a4040e13db66b695f2dcf8 Mon Sep 17 00:00:00 2001 From: Adrian Lees Date: Fri, 5 Jan 2024 17:18:36 +0000 Subject: [PATCH] [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 --- hw/ip/mbx/data/mbx.hjson | 18 ++++++++++-------- sw/ip/mbx/dif/dif_mbx.c | 14 ++++++++------ sw/ip/mbx/dif/dif_mbx_unittest.cc | 7 ++++--- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/hw/ip/mbx/data/mbx.hjson b/hw/ip/mbx/data/mbx.hjson index 7618f75404a4ac..ef63060293eb70 100644 --- 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 index 5843bf13ec43f2..f1e1eab1a5c535 100644 --- 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 index a6b314ff5e6472..0245d63fa0e62a 100644 --- 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{{ {.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