Re: [Qemu-devel] [PATCH 11/19] target-i386: introduce apic-id property

[Igor Mammedov]

On Mon, 15 Apr 2013 17:49:20 -0300
Eduardo Habkost <address@hidden> wrote:

> On Mon, Apr 15, 2013 at 10:27:57PM +0200, Igor Mammedov wrote:
> > On Mon, 15 Apr 2013 11:49:40 -0300
> > Eduardo Habkost <address@hidden> wrote:
> > 
> > > On Mon, Apr 15, 2013 at 04:34:41PM +0200, Igor Mammedov wrote:
> > > > On Mon, 15 Apr 2013 10:45:24 -0300
> > > > Eduardo Habkost <address@hidden> wrote:
> > > > 
> > > > > On Mon, Apr 15, 2013 at 03:37:00PM +0200, Igor Mammedov wrote:
> > > > > [...]
> > > > > > > > > ID to be directly specified in the device_add/-device options.
> > > > > > > > > That's how real CPUs work: as the CPU manufacturer doesn't 
> > > > > > > > > know
> > > > > > > > > what will be the package ID of the CPU, the APIC IDs are not
> > > > > > > > > hardcoded in the CPU; they are calculated based on the CPU 
> > > > > > > > > topology
> > > > > > > > > and some socket identifier signal coming from the board.
> > > > > > > > that is why apic_id has been made a property, to be set from 
> > > > > > > > outside.
> > > > > > > 
> > > > > > > True. I believe the conflict here is: we want other objects to 
> > > > > > > set the
> > > > > > > APIC ID (be it the board, or socket/core objects), but at the 
> > > > > > > same time
> > > > > > > it would be interesting to not expose the APIC ID outside QEMU. 
> > > > > > > Being
> > > > > > > too flexible regarding the APIC ID is more likely to cause 
> > > > > > > problems
> > > > > > > later.
> > > > > > > 
> > > > > > > That said, I don't mind having a "apic-id" property because it is 
> > > > > > > easier
> > > > > > > to simply expose it directly. But do you agree that: 1) we don't 
> > > > > > > really
> > > > > > > need to expose it to be set from outside QEMU; 2) we shouldn't 
> > > > > > > require
> > > > > > > it to be set from outside QEMU; 3) we should recommend users to 
> > > > > > > not try
> > > > > > > to fiddle it with?
> > > > > > Due to nature of per thread CPU hotplug, management will have to 
> > > > > > specify
> > > > > > some kind of ID to specify which CPU is being plugged. Management 
> > > > > > really
> > > > > > doesn't/shouldn't care what this ID is.
> > > > > 
> > > > > As long as management really doesn't/shouldn't care what the ID is,
> > > > > exposing the APIC ID in the form of an opaque CPU identifier wouldn't 
> > > > > be
> > > > > a problem to me. I just wanted to make clarify if we agree that 
> > > > > messing
> > > > > with the APIC ID directly won't be recommended and that the "apic-id"
> > > > > property will be for QEMU internal use only.
> > > > On contrary, it's useful external feature, x86 guests see only APIC ID, 
> > > > since
> > > > it's the only ID they [should] know about. So guest aware mgmt could
> > > > definitely use apic_id propery to correlate CPU in guest with QEMU view 
> > > > of
> > > > them.
> > > 
> > > You're right, _reading_ the APIC ID is very useful. I am worried about
> > > _setting_ it from external code.
> > 
> > currently it's not possible since cpu-add doesn't allow to set any 
> > properties.
> > 
> > We will need setting it for device_add though.
> 
> Not necessarily. That's why I am insisting on an interface based on
> links/topology, not based on a raw "apic-id" property: instead of
> setting apic-id directly, we could just require that the CPU be attached
> to the right socket/core objects, and the APIC ID would be magically
> calculated correctly.
> 
> Or we could just let the right socket/core/thread IDs to be set as
> properties, and apic-id could be calculated based on that. There are
> many ways to expose an abstraction that's simpler to use and less likely
> to cause problems.

if we have tree like this:

/nodes/0../sockets/0../cores/0../threads/0..
user will have to parse all branches IDs to collect node/socket/core/thread IDs
before adding CPU.
If use APIC ID on thread level then user is required to get only it and no
magic would be required to calculate it, because board has already calculated
it in advance.

BTW:
it'd be nicer to refuse wrong id earlier at property setting time then
postponing it till realize when it would be possible to check that
node/socket/core/thread info provided earlier was correct.

> 
> 
> > By then, I guess some way to check that it's valid would be enough, 
> > otherwise
> > hot-plugged CPU will be out of scope of MADT and guest would ignore it or
> > through an error.
> > 
> 
> -- 
> Eduardo


-- 
Regards,
  Igor