From 479f809dbce2c42f10b2615bb1f86eccb4f183c9 Mon Sep 17 00:00:00 2001 From: Wilko Nienhaus Date: Sun, 3 Oct 2021 10:35:21 +0300 Subject: [PATCH 1/4] fix off-by-one error for when working with ULP addresses binutils-esp32ulp allows peripheral registers to be specified either as a full address (e.g. 0x3ff48000) or as a direct ULP address (e.g. 0x120), i.e. the address as seen from the ULP. "direct" addresses are anything from 0x0 to 0x3ff (inclusive). This change ensures that 0x3ff is included in what is treated as a direct ULP address. (See https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/config/tc-esp32ulp_esp32.c#L145 for reference to how binutils treats anything larger than DR_REG_MAX_DIRECT (0x3ff) as a full address, so everything less *AND* equal to DR_REG_MAX_DIRECT is therefore a direct ULP address.) This commit contributes to being able to eventually assemble the esp32ulp_ranges.s test from binutils-esp32ulp. It addresses this line: https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/testsuite/gas/esp32ulp/esp32/esp32ulp_ranges.s#L136 --- esp32_ulp/opcodes.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esp32_ulp/opcodes.py b/esp32_ulp/opcodes.py index 98d7284..893c8d5 100644 --- a/esp32_ulp/opcodes.py +++ b/esp32_ulp/opcodes.py @@ -358,7 +358,7 @@ def get_cond(arg): def _soc_reg_to_ulp_periph_sel(reg): # Map SoC peripheral register to periph_sel field of RD_REG and WR_REG instructions. - if reg < DR_REG_MAX_DIRECT: + if reg <= DR_REG_MAX_DIRECT: ret = RD_REG_PERIPH_RTC_CNTL elif reg < DR_REG_RTCCNTL_BASE: raise ValueError("invalid register base") @@ -377,7 +377,7 @@ def _soc_reg_to_ulp_periph_sel(reg): def i_reg_wr(reg, high_bit, low_bit, val): reg = get_imm(reg) - if reg < DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c + if reg <= DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c _wr_reg.addr = reg else: _wr_reg.addr = (reg & 0xff) >> 2 @@ -391,7 +391,7 @@ def i_reg_wr(reg, high_bit, low_bit, val): def i_reg_rd(reg, high_bit, low_bit): reg = get_imm(reg) - if reg < DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c + if reg <= DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c _rd_reg.addr = reg else: _rd_reg.addr = (reg & 0xff) >> 2 From c967c1dfb567bd813eedd929382bf34e82b91376 Mon Sep 17 00:00:00 2001 From: Wilko Nienhaus Date: Tue, 5 Oct 2021 08:06:23 +0300 Subject: [PATCH 2/4] fix peripheral register read/write for larger ULP addresses When register addresses less than or equal to 0x3ff are given to the reg_rd and reg_wr opcodes, these addresses should be used unmodified as the address in the machine instruction. In our implementation we split the 10 bit address field of the machine instruction into two fields, namely "addr" for the lower 8 bits and "periph_sel" for the upper 2 bits. We already had a mechanism for determining the periph_sel part for "full" addresses (e.g. 0x3ff48000), but for direct (ULP) addresses, we always set periph_sel to 0 instead of using the upper 2 bits from the given address. This commit fixes that. Note 1: In binutils-esp32ulp, they don't split the address into these 2 fields but simply put the direct ULP address into a single combined field of 10 bits, which has the same effect. See: https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/config/tc-esp32ulp_esp32.c#L145 Note 2: In the "macro approach" in esp-idf for creating ULP code, they also use the split field approach (I assume our implementation is modelled after that) and they also don't handle direct (ULP) addresses correctly (or seemingly at all). See: https://github.com/espressif/esp-idf/blob/9d34a1c/components/ulp/include/esp32/ulp.h#L349 This commit contributes to being able to eventually assemble the esp32ulp_ranges.s test from binutils-esp32ulp. It addresses this line: https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/testsuite/gas/esp32ulp/esp32/esp32ulp_ranges.s#L136 --- esp32_ulp/opcodes.py | 6 ++-- tests/opcodes.py | 70 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/esp32_ulp/opcodes.py b/esp32_ulp/opcodes.py index 893c8d5..eb453f3 100644 --- a/esp32_ulp/opcodes.py +++ b/esp32_ulp/opcodes.py @@ -379,9 +379,10 @@ def i_reg_wr(reg, high_bit, low_bit, val): reg = get_imm(reg) if reg <= DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c _wr_reg.addr = reg + _wr_reg.periph_sel = reg >> 8 else: _wr_reg.addr = (reg & 0xff) >> 2 - _wr_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg) + _wr_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg) _wr_reg.data = get_imm(val) _wr_reg.low = get_imm(low_bit) _wr_reg.high = get_imm(high_bit) @@ -393,9 +394,10 @@ def i_reg_rd(reg, high_bit, low_bit): reg = get_imm(reg) if reg <= DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c _rd_reg.addr = reg + _rd_reg.periph_sel = reg >> 8 else: _rd_reg.addr = (reg & 0xff) >> 2 - _rd_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg) + _rd_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg) _rd_reg.unused = 0 _rd_reg.low = get_imm(low_bit) _rd_reg.high = get_imm(high_bit) diff --git a/tests/opcodes.py b/tests/opcodes.py index f14829a..85cd710 100644 --- a/tests/opcodes.py +++ b/tests/opcodes.py @@ -108,10 +108,78 @@ def assert_raises(exception, func, *args): assert raised +def test_reg_direct_ulp_addressing(): + """ + Test direct ULP addressing of peripheral registers + input must be <= 0x3ff (10 bits) + periph_sel == high 2 bits from input + addr == low 8 bits from input + """ + + ins = make_ins(""" + addr : 8 # Address within either RTC_CNTL, RTC_IO, or SARADC + periph_sel : 2 # Select peripheral: RTC_CNTL (0), RTC_IO(1), SARADC(2) + unused : 8 # Unused + low : 5 # Low bit + high : 5 # High bit + opcode : 4 # Opcode (OPCODE_RD_REG) + """) + + ins.all = opcodes.i_reg_rd("0x0", "0", "0") + assert ins.periph_sel == 0 + assert ins.addr == 0x0 + + ins.all = opcodes.i_reg_rd("0x012", "0", "0") + assert ins.periph_sel == 0 + assert ins.addr == 0x12 + + ins.all = opcodes.i_reg_rd("0x123", "0", "0") + assert ins.periph_sel == 1 + assert ins.addr == 0x23 + + ins.all = opcodes.i_reg_rd("0x2ee", "0", "0") + assert ins.periph_sel == 2 + assert ins.addr == 0xee + + ins.all = opcodes.i_reg_rd("0x3ff", "0", "0") + assert ins.periph_sel == 3 + assert ins.addr == 0xff + + # anything bigger than 0x3ff must be a valid full address + assert_raises(ValueError, opcodes.i_reg_rd, "0x400", "0", "0") + + +def test_reg_address_translations(): + """ + Test addressing of peripheral registers using full DPORT bus addresses + """ + + ins = make_ins(""" + addr : 8 # Address within either RTC_CNTL, RTC_IO, or SARADC + periph_sel : 2 # Select peripheral: RTC_CNTL (0), RTC_IO(1), SARADC(2) + unused : 8 # Unused + low : 5 # Low bit + high : 5 # High bit + opcode : 4 # Opcode (OPCODE_RD_REG) + """) + + # direct ULP address is derived from full address as follows: + # full:0x3ff484a8 == ulp:(0x3ff484a8-DR_REG_RTCCNTL_BASE) / 4 + # full:0x3ff484a8 == ulp:(0x3ff484a8-0x3ff48000) / 4 + # full:0x3ff484a8 == ulp:0x4a8 / 4 + # full:0x3ff484a8 == ulp:0x12a + # see: https://github.com/espressif/binutils-esp32ulp/blob/249ec34/gas/config/tc-esp32ulp_esp32.c#L149 + ins.all = opcodes.i_reg_rd("0x3ff484a8", "0", "0") + assert ins.periph_sel == 1 # high 2 bits of 0x12a + assert ins.addr == 0x2a # low 8 bits of 0x12a + + test_make_ins_struct_def() test_make_ins() test_arg_qualify() test_get_reg() test_get_imm() test_get_cond() -test_eval_arg() \ No newline at end of file +test_eval_arg() +test_reg_direct_ulp_addressing() +test_reg_address_translations() From e3597a702fc029f51e55edf32014cc5fea9e2ade Mon Sep 17 00:00:00 2001 From: Wilko Nienhaus Date: Tue, 5 Oct 2021 18:01:30 +0300 Subject: [PATCH 3/4] cleanup peripheral register addressing code There is no need to duplicate the "<= DR_REG_MAG_DIRECT" check in two functions. Now the _soc_reg_to_ulp_periph_sel() function is used only for "full addresses" (as it was some commits ago) and will return a ValueError if used with direct ULP addresses. This commit also masks values correctly when setting the addr and periph_sel fields for "direct ULP addresses", so only the bits we're interested in actually get used (rather than being implicitly trimmed because there aren't more bits available in a field). --- esp32_ulp/opcodes.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/esp32_ulp/opcodes.py b/esp32_ulp/opcodes.py index eb453f3..6910081 100644 --- a/esp32_ulp/opcodes.py +++ b/esp32_ulp/opcodes.py @@ -358,9 +358,7 @@ def get_cond(arg): def _soc_reg_to_ulp_periph_sel(reg): # Map SoC peripheral register to periph_sel field of RD_REG and WR_REG instructions. - if reg <= DR_REG_MAX_DIRECT: - ret = RD_REG_PERIPH_RTC_CNTL - elif reg < DR_REG_RTCCNTL_BASE: + if reg < DR_REG_RTCCNTL_BASE: raise ValueError("invalid register base") elif reg < DR_REG_RTCIO_BASE: ret = RD_REG_PERIPH_RTC_CNTL @@ -378,8 +376,8 @@ def _soc_reg_to_ulp_periph_sel(reg): def i_reg_wr(reg, high_bit, low_bit, val): reg = get_imm(reg) if reg <= DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c - _wr_reg.addr = reg - _wr_reg.periph_sel = reg >> 8 + _wr_reg.addr = reg & 0xff + _wr_reg.periph_sel = (reg & 0x300) >> 8 else: _wr_reg.addr = (reg & 0xff) >> 2 _wr_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg) @@ -393,8 +391,8 @@ def i_reg_wr(reg, high_bit, low_bit, val): def i_reg_rd(reg, high_bit, low_bit): reg = get_imm(reg) if reg <= DR_REG_MAX_DIRECT: # see https://github.com/espressif/binutils-esp32ulp/blob/master/gas/config/tc-esp32ulp_esp32.c - _rd_reg.addr = reg - _rd_reg.periph_sel = reg >> 8 + _rd_reg.addr = reg & 0xff + _rd_reg.periph_sel = (reg & 0x300) >> 8 else: _rd_reg.addr = (reg & 0xff) >> 2 _rd_reg.periph_sel = _soc_reg_to_ulp_periph_sel(reg) From e6ba353ea545f68ed41aa854501b0838138b57c6 Mon Sep 17 00:00:00 2001 From: Wilko Nienhaus Date: Wed, 6 Oct 2021 09:15:04 +0300 Subject: [PATCH 4/4] enable the last skipped tests from binutils-esp32ulp Everything necessary to pass previously skipped binutils-esp32ulp tests is now fixed. So we no longer need to skip them. (Tests using unsupported features, such as assembler macros, are still skipped.) Note 1: There is one test, esp32ulp_ranges.s, which requires symbols defined in esp32ulp_globals.s. binutils-esp32ulp joins these during the linking stage in its test scripts. Since we don't separate stages, we simply concatenate the two files before assembly. Note 2: binutils-esp32ulp has a bug related to how absolute symbols defined with .set are interpreted by the JUMP instruction. If a symbol is marked global, the value is taken as-is, but if a symbol is not global, it's value is divided by 4. Since py-esp32-ulp treats the symbol value the same, whether global or not (the believed correct behaviour), we work around the bug in our test script and patch the input files to make the relevant symbols global. --- tests/02_compat_rtc_tests.sh | 59 +++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/tests/02_compat_rtc_tests.sh b/tests/02_compat_rtc_tests.sh index 05a3ee2..b609bb6 100755 --- a/tests/02_compat_rtc_tests.sh +++ b/tests/02_compat_rtc_tests.sh @@ -51,6 +51,42 @@ build_defines_db() { esp-idf/components/esp_common/include/*.h 1>$log_file } +patch_test() { + local test_name=$1 + local out_file="${test_name}.tmp" + + if [ "${test_name}" = esp32ulp_jumpr ]; then + ( + cd binutils-esp32ulp/gas/testsuite/gas/esp32ulp/esp32 + cp ${test_name}.s ${out_file} + echo -e "\tPatching test to work around binutils-esp32ulp .global bug" + cat >> ${out_file} < ${out_file} + echo -e "\tPatching test to work around binutils-esp32ulp .global bug" + cat >> ${out_file} <$log_file # generates $ulp_file