Re: [PATCH v15 07/10] tb-stats: Adding info [tb-list|tb] commands to HMP (WIP)

[Richard Henderson <richard.henderson@linaro.org>]

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.


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.
>