)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"ce658b6a30f9c6c2013d825c23dcf7cb83138e54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"807643b3_cacd1d5d","updated":"2022-09-05 15:01:21.000000000","message":"hi Antonio, this is the a separate patch for the vdebug driver, which, per your direction, implements the target-dependent functionality, like backdoor memory access and polling. ","commit_id":"430a3a6f33273cf0b3a71c86c62f72fb61c56189"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"f724803d46291b146092e590fc6282603b1da9e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"72b9680f_a9ca8a9e","updated":"2023-05-01 15:39:51.000000000","message":"Jacek, sorry for not reviewing this patch earlier.\nThis patch implements several thinks that should be better put in separate patches:\n- endianess fixes,\n- indentation fixes,\n- type change from uint8_t to tap_state_t,\n- new target write,\n- polling (command \"vdebug polling\" was there, but no implementation),\n- new xtensa implementation.\nI don\u0027t know if a patch for polling alone has sense, or it has to be together with the \"new target write\". Probably you already explained it, I don\u0027t remember.\n\nRegarding the \"new target write\", I still have some concern. I already mentioned it in the past.\nA similar problem (bypass target write_memory) is now required for other situations, like loading a non-secure application in a system that has secured the memory text segment. I\u0027m thinking about intercepting the memory_write and call a TCL function, but I want to investigate further...\n\nDo you think that in mean time you could push separate patches for the other part of this 7149?","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"b6bf1415a33359d0c73e942289f0a2cb13c8e137","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4289f868_1b5398cd","updated":"2023-05-29 21:42:51.000000000","message":"Thanks for the new version.\n\nIt\u0027s not only vdebug that needs this write-memory backdoor.\nCortex-A is not efficient in mem write through CPU and debug port, having a direct bus access from DAP is way faster.\nMemory protections can block handling SW breakpoints. A backdoor can fix it.\n\nMy plan, still cooking...\n1) extend the current OpenOCD \"events\" framework with \"callbacks\" that accept arguments and return values. Practically existing \"events\" could be seen as \"callbacks\" with 0 arguments and \"void\" return.\n2) add generic target code that calls the callbacks \"read-memory\" and \"write-memory\", if present. The callback will return e.g. ERROR_NOT_IMPLEMENTED if it cannot handle the request (e.g. address out of range); in this case the old code will be used, as of today.\n3) In your patch you will have to declare the callback \"write-memory\" for the targets. The callback will call a new command of vdebug, e.g. \"vdebug write_backdoor\", to implement the fast write.\n\nDo you think it could work for vdebug?\n\nWhat I still don\u0027t know, is the overhead of this method.\nIt should not be a problem as host code is way fast than the host-target communication. We will see!","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"efc6d9b5d78d16d85bb0aec5bf6ad6781cb3d41a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"40714d3c_f7e372fe","updated":"2023-05-30 10:15:52.000000000","message":"hi Antonio,\n\nThank you for the very quick feedback.\n\nI think your plan will works well for vdebug and other drivers that need to re-implement some of the target functionality.\n\nAs for the details I am not sure what do you mean to extend the generic OpenOCD events framework.  And the generic target code would simply check for existence of the driver-write-memory, driver-read-memory and call them, if defined ? \nReturning ERROR_NOT_IMPLEMENTED from these callbacks would work well, I think.\n\nWhat about the idea of adding the needed events to the existing target structure ? There is already many events defined there, just memory-related events are missing. I am not sure if that covers all use models and what the time overhead would be, but as you said, host execution is much faster than the communication with the target.\n\nVdebug use yet another event - target idle, when there is no activity. The existing mechanism is to use the target_timer_callback to implement \u0027polling\u0027 as suggested by you earlier. But it is not perfect, as this does not get called timely when target is really busy and the timeout still occurs. Ideally I\u0027d like to throttle the hardware, only when any of the targets is not busy...\nOr perhaps there is a better way to handle this ?","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"5f8950f0d4f59bf3a6cc5f4aa5a3467efcc3cc92","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9598b4d2_8d2e2562","updated":"2023-05-24 12:09:44.000000000","message":"hi Antonio, I am looking at the change 7149 now. I understand you are saying that the target-related functionality may not be merged soon, but everything else what you mention - could.\nSo perhaps it\u0027s more convenient to remove the target-related functionality from this old change, remove the empty line and get it reviewed and merged and submit a new patch with target functions and config ? Rather than reverting all other changes here and filing separate changes.","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"18b3570605c25d6ca2d558acae301cecaefa5d5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7970a05b_8b324982","in_reply_to":"40714d3c_f7e372fe","updated":"2023-05-30 22:33:05.000000000","message":"\u003e what do you mean to extend the generic OpenOCD events framework\n\nWhat we call \"events\" in OpenOCD are simply block of TCL codes that are executed by OpenOCD when a specific condition (event) occurs.\nThese blocks of TCL code have no input/output values/arguments.\nThey work fine for simple cases like \"gdb-attach\" (run these TCL lines when GDB attaches).\nBut to handle something more complex, like \"mem-write-backdoor\", we need to pass e.g. \"address\", \"size\", \"values\" and maybe \"len\". Add the backdoor should report that it cannot write if e.g. the address is not handled.","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"d87c103bff59081f90887abb6f22c1312c263b0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3d9632e1_6613a859","in_reply_to":"7970a05b_8b324982","updated":"2023-05-31 10:51:35.000000000","message":"hi Antonio,\nI see now what you meant. \nI do not think invoking an interpreter will work well for the envisioned cases then. It\u0027s fine for a single event per session, or perhaps something firing once per second. But not for events that happen more frequently. \nThis is just a thought, I actually have not done any profiling or do not know how well the JTCK performs.\n \nI was envisioning some mechanism within C. When obtaining a target handle by the driver is problematic, perhaps we could enhance the target to define default hooks returning ERROR_NOT_IMPLEMENTED and link them as a weak symbol ? That way they can be overridden by any driver.","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c5ffb770219146ccc9d8624d6e008b9d3e5d3646","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dd38299a_a3ad7ad9","in_reply_to":"9598b4d2_8d2e2562","updated":"2023-05-24 22:40:52.000000000","message":"Hi Jacek,\nit\u0027s better having one patch addressing a single topic.\nEach small change in this 7149 can be quickly merged.\nIt\u0027s up to your convenience how to make the split.","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"25c8a46d61216f4903e06ad5fe23fe74ea66eaf9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8e0870cc_2f6cfe59","in_reply_to":"dd38299a_a3ad7ad9","updated":"2023-05-29 17:10:53.000000000","message":"hi Antonio,\nI have created 3 new changes that address\n- the endianness fix: 7721\n- the tap_state: 7722\n- added xtensa configuration, resolving your comment: 7723\nI am going to add you as a reviewer to these changes. Hope that can be merged quickly leaving only this change, with target-dependency, outstanding","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"}],"tcl/target/xtensa-core-xt8.cfg":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"f724803d46291b146092e590fc6282603b1da9e7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"650a5406_9668ed5a","line":168,"updated":"2023-05-01 15:39:51.000000000","message":"please remove the extra empty line","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"},{"author":{"_account_id":1001872,"name":"Jacek Wuwer","email":"jacekmw8@gmail.com","username":"jacekmw8"},"change_message_id":"efc6d9b5d78d16d85bb0aec5bf6ad6781cb3d41a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"10366110_72b4b62b","line":168,"in_reply_to":"650a5406_9668ed5a","updated":"2023-05-30 10:15:52.000000000","message":"addressed in the change 7723","commit_id":"76d0fdab5e1de1225d531e4255982776564ce3d5"}]}
