On Wed, Mar 08, 2017 at 11:59:11AM +0000, Dr. David Alan Gilbert wrote:
[...]
> > +MsrInfoList *qmp_msr_list(uint32_t msr_idx, Error **errp)
> > +{
> > +#ifdef TARGET_I386
> > + bool valid;
> > + X86CPU *cpu;
> > + CPUX86State *env;
> > + CPUState *cpu_state;
> > + MsrInfoList *head = NULL, *cur_item = NULL;
> > +
> > + CPU_FOREACH(cpu_state) {
> > + cpu = X86_CPU(cpu_state);
> > + env = &cpu->env;
> > +
> > + cpu_synchronize_state(cpu_state);
> > +
> > + MsrInfoList *info;
> > + info = g_malloc0(sizeof(*info));
> > + info->value = g_malloc0(sizeof(*info->value));
> > +
> > + info->value->cpu_idx = cpu_state->cpu_index;
> > + info->value->msr_idx = msr_idx;
> > + info->value->value = x86_cpu_rdmsr(env, msr_idx, &valid);
> > +
> > + if (!valid) {
> > + error_setg(errp, "Failed to retreive MSR 0x%08x on CPU %u",
> > + msr_idx, cpu_state->cpu_index);
> > + return head;
> > + }
> > +
> > + /* XXX: waiting for the qapi to support GSList */
> > + if (!cur_item) {
> > + head = cur_item = info;
> > + } else {
> > + cur_item->next = info;
> > + cur_item = info;
> > + }
> > + }
> > +
> > + return head;
> > +#else
> > + error_setg(errp, "MSRs are not supported on this architecture");
> > + return NULL;
> > +#endif
> > +}
>
> This should be abstracted some how so that we don't need
> x86 specifics in cpus.c; perhaps just an architecture call
> back on the CPU.
If it's only supported by x86, I would just move the
implementation to a x86-specific file, and add a stub for the
other architectures. See qmp_query_gic_capabilities() for an
example.
Also, the command should be added to
qmp_unregister_commands_hack() so we don't even report it as
available on other architectures.
--
Eduardo
