Re: [Qemu-devel] [PATCH] X86/HMP: Expose x86 model specific registers via human monitor

[Eduardo Habkost]

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