Re: [PATCH v5 7/9] contrib/plugins/uftrace: implement x64 support

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Fri, 08 Aug 2025 05:07, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>It's trivial to implement x64 support, as it's the same stack layout
>than aarch64.

s/than/as

>
>Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>---
> contrib/plugins/uftrace.c | 85 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
>diff --git a/contrib/plugins/uftrace.c b/contrib/plugins/uftrace.c
>index 6628b4256fd..f10172eed10 100644
>--- a/contrib/plugins/uftrace.c
>+++ b/contrib/plugins/uftrace.c
>@@ -79,6 +79,20 @@ typedef struct {
>     struct qemu_plugin_register *reg_scr_el3;
> } Aarch64Cpu;
> 
>+typedef enum {
>+    X64_RING0,
>+    X64_RING1,
>+    X64_RING2,
>+    X64_RING3,
>+    X64_REAL_MODE,
>+} X64PrivilegeLevel;
>+
>+typedef struct {
>+    struct qemu_plugin_register *reg_rbp;
>+    struct qemu_plugin_register *reg_cs;
>+    struct qemu_plugin_register *reg_cr0;
>+} X64Cpu;
>+
> typedef struct {
>     uint64_t timestamp;
>     uint64_t data;
>@@ -565,6 +579,75 @@ static CpuOps aarch64_ops = {
>     .does_insn_modify_frame_pointer = aarch64_does_insn_modify_frame_pointer,
> };
> 
>+static uint8_t x64_num_privilege_levels(void)
>+{
>+    return X64_REAL_MODE + 1;
>+}
>+
>+static const char *x64_get_privilege_level_name(uint8_t pl)
>+{
>+    switch (pl) {
>+    case X64_RING0: return "Ring0";
>+    case X64_RING1: return "Ring1";
>+    case X64_RING2: return "Ring2";
>+    case X64_RING3: return "Ring3";
>+    case X64_REAL_MODE: return "RealMode";
>+    default:
>+        g_assert_not_reached();
>+    }
>+}
>+
>+static uint8_t x64_get_privilege_level(Cpu *cpu_)
>+{
>+    X64Cpu *cpu = cpu_->arch;
>+    uint64_t cr0 = cpu_read_register64(cpu_, cpu->reg_cr0);
>+    uint64_t protected_mode = (cr0 >> 0) & 0b1;
>+    if (!protected_mode) {
>+        return X64_REAL_MODE;
>+    }
>+    uint32_t cs = cpu_read_register32(cpu_, cpu->reg_cs);
>+    uint32_t ring_level = (cs >> 0) & 0b11;
>+    return ring_level;
>+}
>+
>+static uint64_t x64_get_frame_pointer(Cpu *cpu_)
>+{
>+    X64Cpu *cpu = cpu_->arch;
>+    return cpu_read_register64(cpu_, cpu->reg_rbp);
>+}
>+
>+static void x64_init(Cpu *cpu_)
>+{
>+    X64Cpu *cpu = g_new0(X64Cpu, 1);
>+    cpu_->arch = cpu;
>+    cpu->reg_rbp = plugin_find_register("rbp");
>+    g_assert(cpu->reg_rbp);
>+    cpu->reg_cs = plugin_find_register("cs");
>+    g_assert(cpu->reg_cs);
>+    cpu->reg_cr0 = plugin_find_register("cr0");
>+    g_assert(cpu->reg_cr0);
>+}
>+
>+static void x64_end(Cpu *cpu)
>+{
>+    g_free(cpu->arch);
>+}
>+
>+static bool x64_does_insn_modify_frame_pointer(const char *disas)
>+{
>+    return strstr(disas, "rbp");
>+}
>+
>+static CpuOps x64_ops = {
>+    .init = x64_init,
>+    .end = x64_end,
>+    .get_frame_pointer = x64_get_frame_pointer,
>+    .get_privilege_level = x64_get_privilege_level,
>+    .num_privilege_levels = x64_num_privilege_levels,
>+    .get_privilege_level_name = x64_get_privilege_level_name,
>+    .does_insn_modify_frame_pointer = x64_does_insn_modify_frame_pointer,
>+};
>+
> static void track_privilege_change(unsigned int cpu_index, void *udata)
> {
>     Cpu *cpu = qemu_plugin_scoreboard_find(score, cpu_index);
>@@ -771,6 +854,8 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> 
>     if (!strcmp(info->target_name, "aarch64")) {
>         arch_ops = aarch64_ops;
>+    } else if (!strcmp(info->target_name, "x86_64")) {
>+        arch_ops = x64_ops;
>     } else {
>         fprintf(stderr, "plugin uftrace: %s target is not supported\n",
>                 info->target_name);
>-- 
>2.47.2
>

No idea about x86 assembly tbh but this looks correct to me.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>