On 03/07/2017 06:16 PM, Julian Kirsch wrote: > Provide read/write access to x86 model specific registers (MSRs) by means of > two new HMP commands "msr-list" and "msr-set". The rationale behind this > is to improve introspection capabilities for system virtualization mode. > For instance, many modern x86-64 operating systems maintain access to internal > data structures via the MSR_GSBASE/MSR_KERNELGSBASE MSRs. Giving > introspection utilities (such as a remotely attached gdb) a way of > accessing these registers improves analysis results drastically. > > Signed-off-by: Julian Kirsch <g...@kirschju.re> > ---
I'm just focusing on the QMP interface portion of this.
Is any of this information...
> This patch moves the logic of the rdmsr and wrmsr functions to helper.c and
> replaces their current versions in misc_helper.c with stubs calling the new
> functions. The ordering of MSRs now loosely follows the ordering used in the
> KVM
> module. As qemu operates on cached values in the CPUX86State struct, the
> msr-set
> command is implemented in a hackish way: In order to force qemu to flush the
> new
> values to KVM a call to cpu_synchronize_post_init is triggered, eventually
> ending up in calling kvm_put_msrs at KVM_PUT_FULL_STATE level. As MSR writes
> could *still* be caught by the code logic in this function, the msr-set
> function
> reads back the values written to determine whether the write was successful.
> This way, we don't have to duplicate the logic used in kvm_put_msrs
> (has_msr_XXX)
> to x86_cpu_wrmsr.
> There are several things I would like to pooint out about this patch:
> - The "msr-list" command currently displays MSR values for all virtual cpus.
> This is somewhat inconsistent with "info registers" just displaying the
> value of the current default cpu. One might think about just displaying
> the
> current value and offer access to other CPU's MSRs by means of switching
> between CPUs using the "cpu" monitor command.
> - The new version of x86_cpu_rdmsr exposes MSRs that are arguably of
> questionable help for any human / tool using the monitor. However I merely
> felt a deep urge to reflect the full MSR list from kvm.c when writing the
> code.
> - While the need for msr-list is evident (see commit msg), msr-set could be
> used in more obscure cases. For instance, one might offer a way to access
> and configure performance counter MSRs of the guest via the hmp. If this
> feels too much like an inacceptable hack, I'll happily drop the msr-set
> part.
...useful above the --- as part of the commit message?
> +++ b/qapi-schema.json
> @@ -2365,6 +2365,55 @@
> 'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>
> ##
> +# @MsrInfo:
> +#
> +# Information about a MSR
> +#
> +# @cpu_idx: CPU index
> +#
> +# @msr_idx: MSR index
> +#
> +# @value: MSR value
> +#
> +# Since: 2.8.1
You've missed 2.8 by a long shot; you've even missed soft freeze for
2.9. This should be 2.10.
> +##
> +{ 'struct': 'MsrInfo',
> + 'data': {'cpu_idx': 'int', 'msr_idx': 'uint32', 'value': 'uint64'} }
Please spell new members with '-' rather than '_', as in 'cpu-idx' (or
even spell it out as 'cpu-index') and 'msr-idx'.
> +
> +##
> +# @msr-list:
> +#
> +# Retrieve model specific registers (MSRs) on x86
> +#
> +# @msr_idx: MSR index to read
> +#
> +# Returns: A list of one MSR value per CPU, or nothing
> +#
> +# Since: 2.8.1
2.10
> +##
> +{ 'command': 'msr-list', 'returns': ['MsrInfo'],
> + 'data': {'msr_idx': 'uint32'} }
'msr-idx' (or even 'msr-index')
> +
> +##
> +# @msr-set:
> +#
> +# Set model specific registers (MSRs) on x86
> +#
> +# @cpu_idx: CPU holding the MSR that should be written
> +#
> +# @msr_idx: MSR index to write
> +#
> +# @value: Value to write
> +#
> +# Returns: Nothing on success
Useless Returns: line.
> +#
> +# Since: 2.8.1
2.10
> +##
> +{ 'command': 'msr-set',
> + 'data': {'cpu_idx': 'uint32', 'msr_idx': 'uint32', 'value': 'uint64'} }
again, dash instead of underscore
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
