diff --git a/RELEASENOTES-1.4.docu b/RELEASENOTES-1.4.docu index 592e86d75..11573211d 100644 --- a/RELEASENOTES-1.4.docu +++ b/RELEASENOTES-1.4.docu @@ -521,4 +521,12 @@ This fix is only enabled by default with Simics API version 7 or above. With version 6 or below it must be explicitly enabled by passing --no-compat=shared_logs_on_device to DMLC. + Bank instrumentation callbacks are now + issued from the read and write methods instead of the + transaction_access/io_memory_access method of + banks. This means that breakpoints on banks will trigger also on + reads or writes initiated within the device. This also means + that if read or write is overridden in a bank, + then instrumentation callbacks will only be called if they + call default(). diff --git a/lib/1.4/dml-builtins.dml b/lib/1.4/dml-builtins.dml index 86c3b8031..dd0cf4053 100644 --- a/lib/1.4/dml-builtins.dml +++ b/lib/1.4/dml-builtins.dml @@ -568,6 +568,8 @@ template device { // this carried semantics in DML 1.2; deprecated in 1.4 param _confidentiality = undefined; + session conf_object_t *_current_initiator; + method _init() { _rec_init(); } method _post_init() { _rec_post_init(); } method init() default {} @@ -2007,52 +2009,78 @@ template bank is (object, shown_desc) { return 0; } - local register hits[8]; - local int num_hits; - local uint64 unmapped_bytes; + local uint64 readval = 0; + local bool ok = true; local uint64 size = _enabled_bytes_to_size(enabled_bytes); - (num_hits, unmapped_bytes) = this._dispatch(offset, size, hits, false); + local bool inquiry_override = false; + _callback_before_read(this._bank_obj(), dev._current_initiator, + &inquiry_override, &offset, + size, &_connections, &_before_read_callbacks); + if (inquiry_override) { + try { + readval = this.get(offset, size); + } catch { + ok = false; + } + } else { + try { + local register hits[8]; + local int num_hits; + local uint64 unmapped_bytes; - local uint64 readval = 0; - if ((unmapped_bytes & enabled_bytes) != 0) { - readval[size * 8 - 1 : 0] = this.unmapped_read( - offset, unmapped_bytes & enabled_bytes, aux); - } + (num_hits, unmapped_bytes) = this._dispatch( + offset, size, hits, false); - for (local int r = 0; r < num_hits; ++r) { - local int r_start; - local int r_end; - (r_start, r_end) = this._intersect( - hits[r].offset, hits[r]._size(), offset, size, true); + if ((unmapped_bytes & enabled_bytes) != 0) { + readval[size * 8 - 1 : 0] = this.unmapped_read( + offset, unmapped_bytes & enabled_bytes, aux); + } - local int start; - local int end; - (start, end) = this._intersect( - offset, size, hits[r].offset, hits[r]._size(), true); + for (local int r = 0; r < num_hits; ++r) { + local int r_start; + local int r_end; + (r_start, r_end) = this._intersect( + hits[r].offset, hits[r]._size(), offset, size, true); + + local int start; + local int end; + (start, end) = this._intersect( + offset, size, hits[r].offset, hits[r]._size(), true); + + local uint64 r_enabled_bytes = 0; + r_enabled_bytes[r_end * 8 - 1 : r_start * 8] = + enabled_bytes[end * 8 - 1 : start * 8]; + if (r_enabled_bytes == 0) { + continue; + } - local uint64 r_enabled_bytes = 0; - r_enabled_bytes[r_end * 8 - 1 : r_start * 8] = - enabled_bytes[end * 8 - 1 : start * 8]; - if (r_enabled_bytes == 0) { - continue; - } + local uint64 r_val = hits[r].read_register(r_enabled_bytes, aux); + if (r_end - r_start == hits[r]._size()) { + log info, 4, Register_Read: + "Read from register %s -> 0x%0*x", + hits[r]._qname(), cast(size*2, int), r_val; + } else { + log info, 4, Register_Read: + "Partial read from register %s: bytes %d-%d -> 0x%0*x", + hits[r]._qname(), r_start, r_end-1, + cast(r_end - r_start, int), + r_val[r_end * 8 - 1 : r_start * 8]; + } - local uint64 r_val = hits[r].read_register(r_enabled_bytes, aux); - if (r_end - r_start == hits[r]._size()) { - log info, 4, Register_Read: - "Read from register %s -> 0x%0*x", - hits[r]._qname(), cast(size*2, int), r_val; - } else { - log info, 4, Register_Read: - "Partial read from register %s: bytes %d-%d -> 0x%0*x", - hits[r]._qname(), r_start, r_end-1, - cast(r_end - r_start, int), - r_val[r_end * 8 - 1 : r_start * 8]; + readval[end * 8 - 1 : start * 8] = + (r_val & r_enabled_bytes)[r_end * 8 - 1 : r_start * 8]; + } + } catch { + ok = false; } - - readval[end * 8 - 1 : start * 8] = - (r_val & r_enabled_bytes)[r_end * 8 - 1 : r_start * 8]; + } + _callback_after_read(this._bank_obj(), dev._current_initiator, + &offset, size, + &readval, &ok, &_connections, + &_after_read_callbacks); + if (!ok) { + throw; } return readval; @@ -2100,52 +2128,72 @@ template bank is (object, shown_desc) { return; } - local register hits[8]; - local int num_hits; - local uint64 unmapped_bytes; - + local bool suppress = false; local uint64 size = _enabled_bytes_to_size(enabled_bytes); - (num_hits, unmapped_bytes) = this._dispatch(offset, size, hits, false); - if ((unmapped_bytes & enabled_bytes) != 0) { - this.unmapped_write( - offset, value, unmapped_bytes & enabled_bytes, aux); - } - - for (local int r = 0; r < num_hits; ++r) { - local int r_start; - local int r_end; - (r_start, r_end) = this._intersect( - hits[r].offset, hits[r]._size(), offset, size, true); + _callback_before_write( + this._bank_obj(), dev._current_initiator, &offset, size, + &value, + &suppress, &_connections, &_before_write_callbacks); + local bool ok = true; + if (!suppress) { + try { + local register hits[8]; + local int num_hits; + local uint64 unmapped_bytes; + + (num_hits, unmapped_bytes) = this._dispatch( + offset, size, hits, false); + if ((unmapped_bytes & enabled_bytes) != 0) { + this.unmapped_write( + offset, value, unmapped_bytes & enabled_bytes, aux); + } - local int start; - local int end; - (start, end) = this._intersect( - offset, size, hits[r].offset, hits[r]._size(), true); + for (local int r = 0; r < num_hits; ++r) { + local int r_start; + local int r_end; + (r_start, r_end) = this._intersect( + hits[r].offset, hits[r]._size(), offset, size, true); + + local int start; + local int end; + (start, end) = this._intersect( + offset, size, hits[r].offset, hits[r]._size(), true); + + local uint64 r_enabled_bytes = 0; + r_enabled_bytes[r_end * 8 - 1 : r_start * 8] = + enabled_bytes[end * 8 - 1 : start * 8]; + if (r_enabled_bytes == 0) { + continue; + } - local uint64 r_enabled_bytes = 0; - r_enabled_bytes[r_end * 8 - 1 : r_start * 8] = - enabled_bytes[end * 8 - 1 : start * 8]; - if (r_enabled_bytes == 0) { - continue; - } + local uint64 r_value = 0; + r_value[r_end * 8 - 1 : r_start * 8] + = value[end * 8 - 1 : start * 8]; + if (r_end - r_start == hits[r]._size()) { + log info, 4, Register_Write: + "Write to register %s <- 0x%0*x", + hits[r]._qname(), cast(size*2, int), r_value; + } else { + log info, 4, Register_Write: + "Partial write to register %s: bytes %d-%d <- 0x%0*x", + hits[r]._qname(), r_start, r_end-1, + cast(r_end - r_start, int), + r_value[r_end * 8 - 1 : r_start * 8]; + } - local uint64 r_value = 0; - r_value[r_end * 8 - 1 : r_start * 8] - = value[end * 8 - 1 : start * 8]; - if (r_end - r_start == hits[r]._size()) { - log info, 4, Register_Write: - "Write to register %s <- 0x%0*x", - hits[r]._qname(), cast(size*2, int), r_value; - } else { - log info, 4, Register_Write: - "Partial write to register %s: bytes %d-%d <- 0x%0*x", - hits[r]._qname(), r_start, r_end-1, - cast(r_end - r_start, int), - r_value[r_end * 8 - 1 : r_start * 8]; + hits[r].write_register( + r_value & r_enabled_bytes, r_enabled_bytes, aux); + } + } catch { + ok = false; } + } + _callback_after_write( + this._bank_obj(), dev._current_initiator, &offset, size, + &ok, &_connections, &_after_write_callbacks); - hits[r].write_register(r_value & r_enabled_bytes, r_enabled_bytes, - aux); + if (!ok) { + throw; } } @@ -2240,7 +2288,8 @@ template bank is (object, shown_desc) { default { local uint64 size = SIM_get_mem_op_size(memop); local bool inquiry = SIM_get_mem_op_inquiry(memop); - local conf_object_t *ini = SIM_get_mem_op_initiator(memop); + local conf_object_t *prev_initiator = dev._current_initiator; + dev._current_initiator = SIM_get_mem_op_initiator(memop); local bool success = true; if (inquiry) { @@ -2262,46 +2311,26 @@ template bank is (object, shown_desc) { } else { if (SIM_mem_op_is_read(memop)) { local uint64 value = 0; - local bool inquiry_override = false; - _callback_before_read( - this._bank_obj(), ini, &inquiry_override, - &offset, size, &_connections, &_before_read_callbacks); try { - if (inquiry_override) { - value = this.get(offset, size); - } else { - value = this.read(offset, _mask(size), aux); - } + value = this.read(offset, _mask(size), aux); } catch { success = false; } - _callback_after_read(this._bank_obj(), ini, &offset, size, - &value, &success, &_connections, - &_after_read_callbacks); if (success) { this._memop_set_read_value(memop, value); } } else { local uint64 writevalue = this._memop_write_value(memop); - - local bool suppress = false; - _callback_before_write( - this._bank_obj(), ini, &offset, size, &writevalue, - &suppress, &_connections, &_before_write_callbacks); - if (!suppress) { - try { - this.write(offset, writevalue, _mask(size), aux); - } catch { - success = false; - } + try { + this.write(offset, writevalue, _mask(size), aux); + } catch { + success = false; } - _callback_after_write(this._bank_obj(), ini, &offset, size, - &success, &_connections, - &_after_write_callbacks); } } + dev._current_initiator = prev_initiator; return success; } @@ -2346,7 +2375,8 @@ template bank is (object, shown_desc) { shared method _transaction_access( conf_object_t *ini, bool is_read, bool inquiry, uint64 offset, uint64 size, uint8 *buf, void *aux) -> (bool) { - + local conf_object_t *prev_ini = dev._current_initiator; + dev._current_initiator = ini; local bool success = true; if (inquiry) { if (is_read) { @@ -2366,47 +2396,23 @@ template bank is (object, shown_desc) { } } else { if (is_read) { - local bool inquiry_override = false; - _callback_before_read(this._bank_obj(), ini, - &inquiry_override, &offset, - size, &_connections, &_before_read_callbacks); - local uint64 value = 0; try { - if (inquiry_override) { - value = this.get(offset, size); - } else { - value = this.read(offset, _mask(size), aux); - } - } catch { - success = false; - } - - _callback_after_read(this._bank_obj(), ini, &offset, size, - &value, &success, &_connections, - &_after_read_callbacks); - - if (success) { + local uint64 value = this.read(offset, _mask(size), aux); this._set_read_value(size, buf, value); + return true; + } catch { + return false; } } else { local uint64 writevalue = this._write_value(size, buf); - - local bool suppress = false; - _callback_before_write( - this._bank_obj(), ini, &offset, size, &writevalue, - &suppress, &_connections, &_before_write_callbacks); - if (!suppress) { - try { - this.write(offset, writevalue, _mask(size), aux); - } catch { - success = false; - } + try { + this.write(offset, writevalue, _mask(size), aux); + } catch { + success = false; } - _callback_after_write(this._bank_obj(), ini, &offset, size, - &success, &_connections, - &_after_write_callbacks); } } + dev._current_initiator = prev_ini; return success; } diff --git a/test/1.2/registers/T_instrumentation.py b/test/1.2/registers/T_instrumentation.py index d2c56795b..0a287adcd 100644 --- a/test/1.2/registers/T_instrumentation.py +++ b/test/1.2/registers/T_instrumentation.py @@ -1,6 +1,7 @@ # © 2021-2023 Intel Corporation # SPDX-License-Identifier: MPL-2.0 +import instrumentation_access_initiator import instrumentation_access_inquire import instrumentation_access_set_missed import instrumentation_access_set_offset @@ -23,10 +24,8 @@ import instrumentation_remove_connection_callbacks import instrumentation_subscribe_multiple -subscribe_b1 = SIM_get_port_interface( - obj, 'bank_instrumentation_subscribe', 'b1') -subscribe_b2 = SIM_get_port_interface( - obj, 'bank_instrumentation_subscribe', 'b2') +subscribe_b1 = obj.bank.b1.iface.bank_instrumentation_subscribe +subscribe_b2 = obj.bank.b2.iface.bank_instrumentation_subscribe order_b1 = SIM_get_port_interface(obj, 'instrumentation_order', 'b1') subscribe_ba = [ @@ -37,6 +36,7 @@ # order test first instrumentation_connection_order.test(obj, subscribe_b1, order_b1) +instrumentation_access_initiator.test(obj, subscribe_b2) instrumentation_access_inquire.test(obj, subscribe_b2) instrumentation_access_set_missed.test(obj, subscribe_b1) instrumentation_access_set_offset.test(obj, subscribe_b1) diff --git a/test/1.4/registers/instrumentation_test.py b/test/1.4/registers/instrumentation_test.py index 1f35fd92c..18a123a17 100644 --- a/test/1.4/registers/instrumentation_test.py +++ b/test/1.4/registers/instrumentation_test.py @@ -1,6 +1,7 @@ # © 2021-2023 Intel Corporation # SPDX-License-Identifier: MPL-2.0 +import instrumentation_access_initiator import instrumentation_access_inquire import instrumentation_access_set_missed import instrumentation_access_set_offset @@ -38,8 +39,7 @@ def test(obj): # order test first instrumentation_connection_order.test(obj, subscribe_b1, order_b1) - # Disabled tests below broke at some point around 84eff65716f while - # the instrumentation tests were disabled + instrumentation_access_initiator.test(obj, subscribe_b2) instrumentation_access_inquire.test(obj, subscribe_b2) with stest.allow_log_mgr(None, 'spec-viol'): instrumentation_access_set_missed.test(obj, subscribe_b1) diff --git a/test/common/instrumentation_access_initiator.py b/test/common/instrumentation_access_initiator.py new file mode 100644 index 000000000..aba53d07a --- /dev/null +++ b/test/common/instrumentation_access_initiator.py @@ -0,0 +1,44 @@ +# © 2023 Intel Corporation +# SPDX-License-Identifier: MPL-2.0 + +import conf +import dev_util +import stest + +def test(obj, provider): + calls = [] + handles = [ + provider.register_before_read( + None, 0, 4, + lambda _, access, handle, _2: calls.append( + ('br', access.initiator(handle))), + None), + provider.register_after_read( + None, 0, 4, + lambda _, access, handle, _2: calls.append( + ('ar', access.initiator(handle))), + None), + provider.register_before_write( + None, 0, 4, + lambda _, access, handle, _2: calls.append( + ('bw', access.initiator(handle))), + None), + provider.register_after_write( + None, 0, 4, + lambda _, access, handle, _2: calls.append( + ('aw', access.initiator(handle))), + None)] + + for (reg, ini) in [ + (dev_util.Register_LE(obj.bank.b2, 0, 4), None), + (dev_util.Register_LE(obj.bank.b2, 0, 4, initiator=conf.sim), + conf.sim)]: + stest.expect_equal(reg.read(), 4) + stest.expect_equal(calls, [('br', ini), ('ar', ini)]) + del calls[:] + reg.write(4) + stest.expect_equal(calls, [('bw', ini), ('aw', ini)]) + del calls[:] + + for handle in handles: + provider.remove_callback(handle)