On 6/12/2023 3:17 PM, Richard Henderson wrote: > On 6/12/23 03:44, Wu, Fei wrote: >> On 6/7/2023 8:24 PM, Fei Wu wrote: >>> +void hmp_info_tb(Monitor *mon, const QDict *qdict) >>> +{ >>> + const int id = qdict_get_int(qdict, "id"); >>> + g_autoptr(GString) buf = g_string_new(""); >>> + >>> + if (!tcg_enabled()) { >>> + monitor_printf(mon, "Only available with accel=tcg\n"); >>> + return; >>> + } >>> + >>> + TBStatistics *tbs = get_tbstats_by_id(id); >>> + if (tbs == NULL) { >>> + monitor_printf(mon, "TB %d information is not recorded\n", id); >>> + return; >>> + } >>> + >>> + monitor_printf(mon, "\n------------------------------\n\n"); >>> + >>> + int valid_tb_num = dump_tb_info(buf, tbs, id); >>> + monitor_printf(mon, "%s", buf->str); >>> + >>> + if (valid_tb_num > 0) { >>> + unsigned num_inst = tbs->code.num_guest_inst / >>> tbs->translations.total; >>> + >>> + monitor_printf(mon, "\n----------------n\n"); >>> + // FIXME: cannot disas >>> + monitor_disas(mon, mon_get_cpu(mon), tbs->phys_pc, num_inst, >>> true); >>> + monitor_printf(mon, "\n------------------------------\n\n"); >>> + } >>> +} >>> + >> So far the following methods are candidates for monitor_disas: >> >> 1. still use ram_addr_t for tbs->phys_pc, and extend monitor_disas to >> support disassemble ram_addr_t by using qemu_map_ram_ptr(NULL, ram_addr) >> to convert it to hva first >> >> 2. use gpa for tbs->phys_pc, there is no need to change monitor_disas, >> but add another parameter for get_page_addr_code_hostp() to return extra >> gpa, probe_access_internal() has already returned CPUTLBEntryFull, so >> it's plain to get gpa here. > > No, we need the ram_addr_t for dirty-page handling in order to detect > self-modifying code. Leave tb->phys_pc alone. > I mean return both ram_addr_t and gpa from get_page_addr_code_hostp(), tb->phys_pc will not be changed, I'm not going to change tbs->phys_pc either (like #3), the extra gpa will be saved in tbs for 'info tb' purpose.
In short, no change on phys_pc, but add gpa to tbs, sounds good? Thanks, Fei. > > r~ > >> >> 3. record gpa in another field of tbs, and keep tbs->phys_pc as it is, >> this is just a variation of #2. >> >> I'm inclined to use method #2. I think gpa carries more information for >> debugging than ram_addr_t, guest can map gpa to the executable file >> etc., but it has little knowledge of ram_addr_t. >> >> What do you suggest? >> >> Thanks, >> Fei. >> >