On 6/7/2023 8:49 PM, Wu, Fei wrote: > On 6/1/2023 10:40 AM, Richard Henderson wrote: >>> +static int >>> +__attribute__((format(printf, 2, 3))) >>> +fprintf_log(FILE *a, const char *b, ...) >>> +{ >>> + va_list ap; >>> + va_start(ap, b); >>> + >>> + if (!to_string) { >>> + vfprintf(a, b, ap); >>> + } else { >>> + qemu_vlog(b, ap); >>> + } >>> + >>> + va_end(ap); >>> + >>> + return 1; >>> +} >>> + >> >> Not need on this either. Global variable being checked on each >> callback, instead of selecting the proper callback earlier -- preferably >> without the global variable. >> >> Did you really need something different than monitor_disas? You almost >> certainly want to read physical memory and not virtual anyway. >> > Yes, it's usually okay for kernel address, but cannot re-gen the code > for userspace virtual address (guest kernel panic on riscv guest). I > tried monitor_disas() but it failed to disas too: > monitor_disas(mon, mon_get_cpu(mon), tbs->phys_pc, num_inst, true); > > How to use this function correctly? > 'phys_pc' in tbs is returned by get_page_addr_code_hostp(), which is not guest phys address actually, but ram_addr_t instead, so it's always wrong for monitor_disas. After some dirty changes, tbs can record the guest pa. Now we can disas both kernel and user space code. But still, no code is regenerated, disas in 'info tb' is just a convenient way to 'xp'.
Is there any existing function to convert ram_addr_t to guest pa? (qemu) info tb-list 2 TB id:0 | phys:0x11b97762e virt:0x00aaaaaab76c262e flags:0x00024010 0 inv/1 | exec:4447539979/0 guest inst cov:69.06% | trans:1 ints: g:8 op:28 op_opt:23 spills:0 | h/g (host bytes / guest insts): 37.000000 TB id:1 | phys:0x8063474e virt:0xffffffff8043474e flags:0x01024001 0 inv/1 | exec:131719290/0 guest inst cov:2.38% | trans:1 ints: g:9 op:37 op_opt:35 spills:0 | h/g (host bytes / guest insts): 51.555557 (qemu) info tb 0 ------------------------------ TB id:0 | phys:0x11b97762e virt:0x00aaaaaab76c262e flags:0x00024010 0 inv/1 | exec:5841751800/0 guest inst cov:69.06% | trans:1 ints: g:8 op:28 op_opt:23 spills:0 | h/g (host bytes / guest insts): 37.000000 0x11b97762e: 00002797 auipc a5,8192 # 0x11b97962e 0x11b977632: a2278793 addi a5,a5,-1502 0x11b977636: 639c ld a5,0(a5) 0x11b977638: 00178713 addi a4,a5,1 0x11b97763c: 00002797 auipc a5,8192 # 0x11b97963c 0x11b977640: a1478793 addi a5,a5,-1516 0x11b977644: e398 sd a4,0(a5) 0x11b977646: b7e5 j -24 # 0x11b97762e ------------------------------ (qemu) xp/8i 0x11b97762e 0x11b97762e: 00002797 auipc a5,8192 # 0x11b97962e 0x11b977632: a2278793 addi a5,a5,-1502 0x11b977636: 639c ld a5,0(a5) 0x11b977638: 00178713 addi a4,a5,1 0x11b97763c: 00002797 auipc a5,8192 # 0x11b97963c 0x11b977640: a1478793 addi a5,a5,-1516 0x11b977644: e398 sd a4,0(a5) 0x11b977646: b7e5 j -24 # 0x11b97762e (qemu) (qemu) info tb 1 ------------------------------ TB id:1 | phys:0x8063474e virt:0xffffffff8043474e flags:0x01024001 0 inv/1 | exec:131719290/0 guest inst cov:2.38% | trans:1 ints: g:9 op:37 op_opt:35 spills:0 | h/g (host bytes / guest insts): 51.555557 0x8063474e: 00194a83 lbu s5,1(s2) 0x80634752: 00094803 lbu a6,0(s2) 0x80634756: 0b09 addi s6,s6,2 0x80634758: 008a9a9b slliw s5,s5,8 0x8063475c: 01586833 or a6,a6,s5 0x80634760: ff0b1f23 sh a6,-2(s6) 0x80634764: 1c7d addi s8,s8,-1 0x80634766: 0909 addi s2,s2,2 0x80634768: fe0c13e3 bnez s8,-26 # 0x8063474e ------------------------------ (qemu) xp/9i 0x8063474e 0x8063474e: 00194a83 lbu s5,1(s2) 0x80634752: 00094803 lbu a6,0(s2) 0x80634756: 0b09 addi s6,s6,2 0x80634758: 008a9a9b slliw s5,s5,8 0x8063475c: 01586833 or a6,a6,s5 0x80634760: ff0b1f23 sh a6,-2(s6) 0x80634764: 1c7d addi s8,s8,-1 0x80634766: 0909 addi s2,s2,2 0x80634768: fe0c13e3 bnez s8,-26 # 0x8063474e Thanks, Fei. > Thanks, > Fei. > >>> +void qemu_log_to_monitor(bool enable) >>> +{ >>> + to_monitor = enable; >>> +} >>> + >>> +void qemu_log_to_string(bool enable, GString *s) >>> +{ >>> + to_string = enable; >>> + string = s; >>> +} >> >> What are these for, and why do you need them? >> Why would to_string ever be anything other than (string != NULL)? >> Why are you using such very generic names for global variables? >> >> >> r~ >