Re: [Qemu-devel] [PATCH v2] hmp: allow cpu index for "info lapic"

[Igor Mammedov]

On Tue, 18 Jul 2017 20:26:50 -0300
Eduardo Habkost <address@hidden> wrote:

> On Tue, Jul 18, 2017 at 04:54:17PM +0200, Igor Mammedov wrote:
> > On Mon, 17 Jul 2017 21:49:37 -0400
> > Yi Wang <address@hidden> wrote:
> > 
> > > Add [vcpu] index support for hmp command "info lapic", which is
> > > useful when debugging ipi and so on. Current behavior is not
> > > changed when the parameter isn't specified.
> > we shouldn't expose cpu_index to users anymore,
> > 
> > I would suggest using to use real APIC ID here but we don't
> > have monitor command that returns APIC IDs for present cpus.
> > 
> > "info hotpluggable-cpus" gives you a list of available CPUs
> > it also gives you qom_path to cpu so potentially you could
> > read apic-id property of cpu.
> > 
> > But we have only QMP variant of qom-get so monitor needs
> > addition of qom-get command that will be a wrapper around
> > QMP command.
> > 
> > It could be solved in 2 ways:
> >  * use socket-id/core-id/thread-id to specify desired cpu
> >    /possible values in 'info hotpluggable-cpus'/
> > 
> >  * use apic-id value to specify interrupt controller
> >    - apic-id could be retrieved with new qom-get
> >      (qom-get would also be useful to read other properties)
> >    - extend 'info registers' with apic id value
> >     for example instead of current:
> >     
> >      CPU#1
> >      EAX=00000c06 EBX=00000000 ECX=000002ff EDX=00000000
> >      ....
> > 
> >     it would look like:
> > 
> >      CPU#1 (socket-id: a, core-id: b, thread-id: c, apic-id: d)
> >      ...
> 
> We already print "CPU #<n>" on "info cpus", so <n> is already a
> perfectly good identifier for a human interface.  I think HMP
> should not require any identifier that isn't a simple number that
> is shown very prominently on "info cpus".
> 
> If we don't want to use cpu_index as an identifier anymore, we
> can start printing arch ID instead of cpu_index on commands that
> print "CPU #<n>", and change mon_get_cpu() and monitor_set_cpu()
> accordingly.
Looks like a good plan, but it probably should touch all commands
that use cpu_index, also I'd leave cpu_index in 'info cpus' where
it is till we faze it out from CLI.