| Message ID | 20210206232058.4209-1-elias-djossou.git@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | Expose i386 model specific registers via HMP | expand |
Patchew URL: https://patchew.org/QEMU/20210206232058.4209-1-elias-djossou.git@gmx.de/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210206232058.4209-1-elias-djossou.git@gmx.de Subject: [PATCH] Expose i386 model specific registers via HMP === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20210206232058.4209-1-elias-djossou.git@gmx.de -> patchew/20210206232058.4209-1-elias-djossou.git@gmx.de Switched to a new branch 'test' 110d6be Expose i386 model specific registers via HMP === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 82 lines checked Commit 110d6be24005 (Expose i386 model specific registers via HMP) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210206232058.4209-1-elias-djossou.git@gmx.de/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index 117ba25f91..d78c3e799d 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -893,3 +893,18 @@ SRST ``info replay`` Display the record/replay information: mode and the current icount. ERST + +#if defined(TARGET_I386) + { + .name = "msr-registers", + .args_type = "cpustate_all:-a", + .params = "[-a]", + .help = "show the cpu msr registers (-a: all - show msr register info for all cpus)", + .cmd = hmp_info_msr_registers, + }, +#endif + +SRST + ``info msr-registers`` + Show the cpu msr registers. +ERST diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h index 60fc92722a..756102a773 100644 --- a/include/monitor/hmp-target.h +++ b/include/monitor/hmp-target.h @@ -49,5 +49,6 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict); void hmp_mce(Monitor *mon, const QDict *qdict); void hmp_info_local_apic(Monitor *mon, const QDict *qdict); void hmp_info_io_apic(Monitor *mon, const QDict *qdict); +void hmp_info_msr_registers(Monitor *mon, const QDict *qdict); #endif /* MONITOR_HMP_TARGET_H */ diff --git a/target/i386/monitor.c b/target/i386/monitor.c index 1bc91442b1..b274c42bf3 100644 --- a/target/i386/monitor.c +++ b/target/i386/monitor.c @@ -736,3 +736,58 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr, { sev_inject_launch_secret(packet_hdr, secret, gpa, errp); } + +static void x86_print_msr_registers(Monitor *mon, CPUState *cs) +{ + X86CPU *cpu = X86_CPU(cs); + CPUX86State *env = &cpu->env; + + monitor_printf(mon, + "MSR_IA32_SYSENTER_CS=%08x\n" + "MSR_IA32_SYSENTER_ESP=%016lx\n" + "MSR_IA32_SYSENTER_EIP=%016lx\n" + "MSR_STAR=%016lx\n", + env->sysenter_cs, + env->sysenter_esp, + env->sysenter_eip, + env->star); + +#ifdef TARGET_X86_64 + monitor_printf(mon, + "MSR_LSTAR=%016lx\n" + "MSR_CSTAR=%016lx\n" + "MSR_FMASK=%016lx\n" + "MSR_FSBASE=%016lx\n" + "MSR_GSBASE=%016lx\n" + "MSR_KERNELGSBASE=%016lx\n", + env->lstar, + env->cstar, + env->fmask, + env->segs[R_FS].base, + env->segs[R_GS].base, + env->kernelgsbase); +#endif + +} + +void hmp_info_msr_registers(Monitor *mon, const QDict *qdict) +{ + bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false); + CPUState *cs; + + if (all_cpus) { + CPU_FOREACH(cs) { + monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index); + x86_print_msr_registers(mon, cs); + } + } else { + cs = mon_get_cpu(mon); + + if (!cs) { + monitor_printf(mon, "No CPU available\n"); + return; + } + + x86_print_msr_registers(mon, cs); + } +}
This patch aims to expose certain model specific registers on i386 to the HMP. To achieve this a new HMP command "info msr-registers" is added. Since this is only relevant for i386 an "#if defined(TARGET_I386)" ensures it stays there. The corrsponding function "hmp_info_msr_registers()" is then implemented in target/i386/monitor.c which uses the helper function "x86_print_msr_registers()" in the same file where the print itself happens. Signed-of-by: Elias Djossou <elias-djossou.git@gmx.de> --- This feature can be crucial in certain Debug scenarios since the MSR registers maintain acceas to highly relevant data structures on the i386 architecture. (e.g. on x86_64 LSTAR stores the OS's system call entrance point) Being able to access these can immensly enhance introspection capabilities. Also providing this functionality via the HMP makes the feature available to both the QEMU Monitor itself as well as utilities like for example GDB (via the "monitor info msr-registers" command). --- This patch follows up on an idea of Mar 2017 (see https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg01559.html) but is intended to be more light-weight. While working on this I came across a problem i would like to get your opinion on. I would really much like to expose more of the MSRs in struct CPUX86State (ideally all of them) at sometime in a future patch. I decided to limit myself to these ones for this one for the following reason: While these registers would in principle be easily available. Problem is that there is no function available that allows to read MSRs. The only related function seems to be "helper_rdmsr()" from target/i386/tcg which doesn't return the value but writes it directly into R_EAX / R_EDX instead. So my only options to realize this seem to be: 1. copy the content of the individual cases from helper_rdmsr() to my helper x86_print_msr_registers() 2. backup R_EAX and R_EDX - call helper_rdmsr() - copy R_EAX, R_EDX - apply backup (seems much too hacky to me) 3. write a new interface similar to helper_rdmsr that really returns the value - maybe rewrite helper_rdmsr() to use that helper function as well (such a function existed back in the times of the above referenced patch as "x86_cpu_rdmsr()" in target/i386/misc_helper.c) I would really appreciate your opinions on this --- hmp-commands-info.hx | 15 ++++++++++ include/monitor/hmp-target.h | 1 + target/i386/monitor.c | 55 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) -- 2.25.1